Ticket #4819 enhancement new

Opened 3 years ago

Last modified 5 months ago

Add FTP support for REST and APPE

Reported by: eternicode Owned by:
Priority: highest Milestone:
Component: ftp Keywords:
Cc: andrew@…, sirkonst@… Branch:
Author: Launchpad Bug:

Description

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

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

Change History

1

Changed 3 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 3 years ago by eternicode

2

Changed 3 years ago by eternicode

  • keywords ftp rest pasv removed
  • cc andrew@… added
  • 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.

3

Changed 3 years ago by <automation>

  • owner itamarst deleted

4

Changed 5 months ago by sirkonst

  • priority changed from normal to highest
  • cc sirkonst@… added

ping

Note: See TracTickets for help on using tickets.