Opened 6 years ago

Last modified 3 years ago

#4819 enhancement new

Add FTP support for REST and APPE

Reported by: eternicode Owned by:
Priority: highest Milestone:
Component: ftp Keywords:
Cc: eternicode, sirkonst Branch:


Currently, the FTP server doesn't support the FTP REST command. Each incoming command is read, and pauseProducing is called before the command is processed. No other commands are read until the current command is finished.

Clients (I tested with filezilla) send a REST command after a PASV command and then wait for a response before making the DTP connection. FTP.ftp_PASV, however, waits for a DTP connection before resumeProducing is called (effectively). This would result in a deadlock between the two, if the current implementation didn't return "Not Implemented" for REST.

I've posted my workaround for this on StackOverflow: Implementing REST in tiwsted.protocols.ftp.FTP?. This sets the reactor to call resumeProducing in the near future with callLater, canceling that call if the DTP connection is made in the interim. This allows a REST command to come in and be processed if necessary, or normal processing to go on if not.

Attachments (1)

test-ftp-server-rest-appe.patch (5.7 KB) - added by eternicode 6 years ago.

Download all attachments as: .zip

Change History (5)

comment:1 Changed 6 years ago by eternicode

More info: The instability I mention in the stackoverflow post is possibly due to the fact (?) that the callLater on resumeProducing results in two simultaneous flows of line processing. Adding a "peek" flag to prevent resuming (line 716) on a "peeking pass" seems to fix the issue and make REST stable.

Changed 6 years ago by eternicode

comment:2 Changed 6 years ago by eternicode

  • Cc eternicode added
  • Keywords ftp rest pasv removed
  • Summary changed from Add support for REST in t.p.ftp.FTP to Add FTP support for REST and APPE

In the interest of getting this started, I've attached a patch that adds unit tests for the server portion of APPE and REST (both marked todo), as well as one for STOR. I tried my darndest to write tests for the FTP client, too, but I'm simply not familiar enough with how it works. FTPShell may also need some tests.

Also, feedback on this patch is appreciated, since I'm still getting the hang of contributing twisted code.

comment:3 Changed 6 years ago by <automation>

  • Owner itamarst deleted

comment:4 Changed 3 years ago by sirkonst

  • Cc sirkonst added
  • Priority changed from normal to highest


Note: See TracTickets for help on using tickets.