~sschwarzer/ftputil

abab579857a96813322c7282fbb4f72cd9954320 — Stefan Schwarzer 6 years ago 15bfff1
Fix invalid 226 reply bug (ticket 102)

Under FTP server load, a 226 reply from the server may not be "seen"
while a remote file is closed in `FTPFile.close`. If the reply is only
seen in in the `pwd` call in `FTPHost._available_child`, `ftplib`
raises an `error_reply` exception since no 226 is expected from a
`pwd` call.

This changeset fixes the bug by explicitly catching the `error_reply`
exception. This is the same approach as for timed-out child sessions,
which cause an `error_temp` exception.

The fix will cause `_available_child` to ignore the child session
whose `pwd` caused the `error_reply` exception. However, the child
will be tried again next time `_available_child` is called and should
then have a working `pwd` call (unless the child session timed out,
in which case it will cause an `error_temp`).
2 files changed, 25 insertions(+), 0 deletions(-)

M ftputil/host.py
M test/test_real_ftp.py
M ftputil/host.py => ftputil/host.py +7 -0
@@ 168,6 168,13 @@ class FTPHost(object):
                # Timed-out sessions raise `error_temp`.
                except ftplib.error_temp:
                    continue
                # Under high load, a 226 status response from a
                # previous download may arrive too late, so that it's
                # "seen" in the `pwd` call. For now, skip the
                # potential child session; it will be considered again
                # when `_available_child` is called the next time.
                except ftplib.error_reply:
                    continue
                else:
                    # Everything's ok; use this `FTPHost` instance.
                    return host

M test/test_real_ftp.py => test/test_real_ftp.py +18 -0
@@ 701,6 701,24 @@ class TestFTPFiles(RealFTPTest):
            pass
        self.assertTrue(file_obj2 is file_obj3)

    def test_no_delayed_226_children(self):
        REMOTE_FILE_NAME = "debian-keyring.tar.gz"
        host = self.host
        # Implicitly create child host object.
        with host.open(REMOTE_FILE_NAME, "rb") as file_obj1:
            pass
        # Monkey-patch file to simulate an FTP server timeout below.
        def timed_out_pwd():
            raise ftplib.error_reply("delayed 226 reply")
        file_obj1._host._session.pwd = timed_out_pwd
        # Try to get a file - which shouldn't be the timed-out file.
        with host.open(REMOTE_FILE_NAME, "rb") as file_obj2:
            self.assertTrue(file_obj1 is not file_obj2)
        # Re-use closed and not timed-out child session.
        with host.open(REMOTE_FILE_NAME, "rb") as file_obj3:
            pass
        self.assertTrue(file_obj2 is file_obj3)


class TestChmod(RealFTPTest):