Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add a test case for QUIT command with a transfer in progress #410

Merged
merged 2 commits into from
Jul 19, 2023

Conversation

drakkan
Copy link
Collaborator

@drakkan drakkan commented Jul 2, 2023

specs are not completly clear

If file transfer is in progress, the connection will remain open for result response and the server will then close it.

it is not specified whether we have to send the QUIT response and then the transfer close response or vice versa.

Some results from other FTP servers.

ProFTPD:

EPSV
229 Entering Extended Passive Mode (|||8257|)
RETR sub.mkv
150 Opening BINARY mode data connection for sub.mkv (13313790 bytes)
QUIT
226 Trasferimento completato
221 Arrivederci.

Pure-FTPD (abort the transfer):

EPSV
229 Extended Passive mode OK (|||64013|)
RETR sub.mkv
150-Accepted data connection
150 13001.7 kbytes to download
QUIT
426 ABORT
226-Transfer aborted
226 0.000 seconds (measured here), 332.67 Mbytes per second

vsftpd

EPSV
229 Entering Extended Passive Mode (|||46192|)
RETR sub.mkv
150 Opening BINARY mode data connection for sub.mkv (13313790 bytes).
QUIT
226 Transfer complete.
221 Goodbye.

I haven't tested pyftpdlib but according to the code here. It sends the QUIT response before the transfer close.

We behave like ProFTPD and vsftpd. The test case ensures that the behavior is not changed by mistake

@codecov
Copy link

codecov bot commented Jul 2, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (3fe6de2) 86.46% compared to head (59da87a) 86.46%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #410   +/-   ##
=======================================
  Coverage   86.46%   86.46%           
=======================================
  Files          11       11           
  Lines        1603     1603           
=======================================
  Hits         1386     1386           
  Misses        151      151           
  Partials       66       66           

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@drakkan
Copy link
Collaborator Author

drakkan commented Jul 2, 2023

I confirm that pyftpdlib send transfer close response after quit

EPSV
229 Entering extended passive mode (|||48977|).
RETR sub.mkv
125 Data connection already open. Transfer starting.
QUIT
221 Goodbye.
226 Transfer complete.

tested using this sample python program

from pyftpdlib.authorizers import DummyAuthorizer
from pyftpdlib.handlers import FTPHandler
from pyftpdlib.servers import FTPServer

authorizer = DummyAuthorizer()
authorizer.add_user("nicola", "password", "/home/nicola", perm="elradfmwMT")

handler = FTPHandler
handler.authorizer = authorizer

server = FTPServer(("127.0.0.1", 2121), handler)
server.serve_forever()

Copy link
Owner

@fclairamb fclairamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM !

@fclairamb fclairamb enabled auto-merge (squash) July 19, 2023 10:24
@fclairamb fclairamb merged commit 76e3b67 into fclairamb:main Jul 19, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants