FTP support

resam

Hi,

I noticed that FTP support is currently only available with bare bones commands. I'm having problems with a lot of "more advanced" tools to access and transfer files from/to a Duet Wifi with 1.19.2. I'm aware of https://duet3d.com/wiki/FTP_commands_supported, but can't really pinpoint what's missing.

I got a working connection with the plain CLI tool 'ftp', but other tools, such as 'ncftp', 'curlftpfs', or GUI tools like Cyberduck always give me an error (mostly during login / auth).

Is it possible to emulate / debug the code in (and around) https://github.com/dc42/RepRapFirmware/blob/dev/src/DuetNG/FtpResponder.cpp? I couldn't really find a suitable way to setup a debugging environment without re-flashing a FW every 2mins…

Thanks!

dc42

FileZilla works with the Duet.

In theory you can solder a JTAG connector to the Duet for debugging, but I've never resorted to that.

resam

Yes, FileZilla does work.
However, I'm still surprised that 5 out of 7 tested tools so far didn't work for me. Thats why I wanted to try and debug this, and maybe help fix it somehow.

Do you have any way of debugging RRF in software via emulation/simulation, or do you always flash it onto real hardware and connect via serial?

dc42

I debug via serial. To work out why FTP isn't working with your other clients, I suggest you start by getting a Wireshark trace to see if they are using a command that isn't supported by RRF.

resam

tcpdump revealed the following:

wget and Cyberduck try to use [c]LIST -a[/c], which seems to be non-standard behaviour and results in a [c]500 Unknown command.[/c]

curl tries to send [c]TYPE I[/c], which should be valid and supported but results in a [c]500 Unknown command.[/c] (strangely this command works with ncftp)

ncftp tries [c]MLST, MDTM, NLST, SIZE, STAT[/c] and all fail, are defined in an RFC, but not supported by the Duet yet.

resam

Ok, so first debugging results for curl:

curl sends PASV: RRF and curl properly open & conenct passive connection.

Now RRF is in state [c]ResponderState::pasvPortOpened[/c], where it only accepts a subset of commands.
https://github.com/dc42/RepRapFirmware/blob/871c22c0c5b1c5479488014f7d428efe423c38c7/src/DuetNG/FtpResponder.cpp#L672-L745

curl sends a [c]TYPE I[/c], which should be valid, but since RRF is in the wrong state, it falls through to this error case:
https://github.com/dc42/RepRapFirmware/blob/871c22c0c5b1c5479488014f7d428efe423c38c7/src/DuetNG/FtpResponder.cpp#L749

According to RFC959: PASV and TYPE are "transfer parameter commands", which "may be in any order except that they must precede the FTP service request".

So I guess RRF should handle a TYPE command even after a PASV?

@dc42: do you agree with my understanding of the RFC, or did I miss one of the many extensions and updates to the FTP protocol definition?

dc42

I'll change the next beta to accept (but ignore) parameters after LIST, and to allow TYPE after the passive port has been opened. This should get you a little further.

resam

Thanks! I'll keep you updated

resam

I can confirm that wget and curl are now able to transfer files with 1.20beta6!
Thanks!

Tested with [c]wget -r ftp://voron:voron@10.0.0.42/sys/[/c], which downloads all files in [c]/sys/[/c].
curl also seems to work now: [c]curl ftp://voron:voron@10.0.0.42/sys/config.g[/c] prints the content of config.g.
I'm still having problems with curlftpfs, I can list everything, but neither up- nor download files. But I think this could be unrelated to the Duet FTP stack - curlftpfs is just full of old bugs

The [c]SIZE[/c] command results in a [c]500 Unknown command.[/c] reply. Would be nice to implement this, since [c]RETR[/c] already replies with the file size in bytes.

Cyberduck seems to also try the [c]STAT[/c] command with a parameter (e.g., [c]/[/c]), which should be the same as [c]LIST[/c].

Not sure why, but ncftp sends a [c]CWD .[/c], which fails currently with [c]550 Failed to change directory.[/c]. [c]CWD ..[/c] is already supported.

Support for [c]EPSV[/c] is probably not needed at this time, unless you plan on supporting IPv6?

Thanks for the great work!

dc42

I'll look at supporting SIZE and CWD . in the next beta.

resam

I happy to report that [c]CWD .[/c] works in 1.20beta7! Thanks!
Now only [c]SIZE[/c] and [c]STAT /[/c] are missing to really call this a proper FTP support!

One minor caveat: only one FTP connection is supported at a time, most GUI FTP browsers open a second connection for file transfers. Mentioning this on the Wiki page should be enough for users to avoid this issue.

Great work so far - thanks a million!