Site commands implementation.

[martin-cowie]

Enclosed is the implementation of site commands for libunftp.

The ServerBuilder structure gets a new site_command method to pass a subcommand and a SiteCommandHandler implementor. As a side-effect enum Reply and ReplyCode have become part of the SiteCommandHandler API, and are now fully documented.

The existing site md5 subcommand remains untouched.

All changes are tested with 100% coverage.

I developed this for an ongoing project: https://github.com/martin-cowie/ftp-kafka

[martin-cowie]

Hi @hannesdejager - happy to discuss any concerns the team may have to progress this PR 🙂

[hannesdejager]

Thanks, Martin! This is a really useful addition, and the implementation is nicely extensible. Also your project looks cool, integrating with Kafka.

I may make a couple of small adjustments post merge to align the new API with the existing builder conventions. In particular, I’d like configuration problems—such as duplicate or invalid SITE command names and incompatible builder ordering—to be reported through ServerBuilder::build() rather than by panicking. I may also tighten command-name validation and clarify that custom SITE handlers run after authentication.

These are API-hardening refinements; the overall design and implementation of your contribution will remain unchanged. Thanks again for contributing this!

Lastly, do you really need storage_features in SiteCommandContext. It is already available inside the storage field. Trying to limit public API exposure.

[hannesdejager]

Actually I was wondering if registration of sub-commands should perhaps just override.

[martin-cowie]

do you really need storage_features in SiteCommandContext

Good catch. I don't need it.

I was wondering if registration of sub-commands should perhaps just override

Yes. I think there are more circumstances where re-registering SITE FOO be useful, than treated as a configuration error.

This throws light on the existing special case of SITE MD5: should devs be able to re-register, or un-register this? Should the existing MD5 implementation be folded in, using the new site command machinery? Hmm 🤔

[hannesdejager]

@martin-cowie we probably need a crate of core site command implementations that libunftp can optionally use that uses your new machinery so that we dogfood amongst other things. I want to give it some thought.

perhaps you also want to share some of yours in a public crate? we have conventions going like “unftp-sbe-” names for storage backends, maybe we can have “unftp-sbe-site-” for such crates.

[martin-cowie]

we probably need a crate of core site command implementations that libunftp can optionally use

Thinking this over, re-usable SITE commands fall into a few categories

1. those relating to file content, e.g. MD5 but include SHA1 and SHA256 → unftp-site-hash
2. those relating to file metadata, i.e SITE commands to surface modern Unix filesystems features via system calls such as chmod, chown, even extended attributes via setxattr and getxattr → unftp-site-unix-fs*
3. those relating to specific applications, such as, but not limited to, Kafka.

A non-exhaustive survey of two popular FTP clients and one popular FTP server reveals this set of SITE commands

	lftp	filezilla	vsftpd	Other
HELP				✅
UTIME	✅
GROUP	✅
LINK	✅
SYMLINK	✅
IDLE	✅
MKDIR	✅
CHMOD		✅	✅
UMASK			✅

UTIME, GROUP, LINK, SYMLINK and CHMOD fall into category 1. MKDIR seems redundant, UMASK I dislike because it's not threadsafe and HELP, and IDLE fall into a new category 4 "quality of life"

*: Other OS filesystems may have useful metadata features that can be usefully surfaced. When the storage backend is not a filesystem, such SITE commands are inapplicable.