[2.7] bpo-27973 - Fix for urllib.urlretrieve() failing on second ftp transfer (python#1040)

orsenthil · web-flow · commit f82e59ac4020 · 2019-12-30T21:14:56.000-08:00
* bpo-27973: Fix urllib.urlretrieve failing on subsequent ftp transfers from the same host.

* bpo-35411: Skip test_urllibnet FTP tests on Travis CI.
diff --git a/Lib/test/test_urllibnet.py b/Lib/test/test_urllibnet.py
@@ -1,5 +1,7 @@
+import tempfile
 import unittest
 from test import test_support
+from test.test_urllib2net import skip_ftp_test_on_travis
 
 import socket
 import urllib
@@ -213,14 +215,49 @@ def test_context_argument(self):
         self.assertIn("Python", response.read())
 
 
+class urlopen_FTPTest(unittest.TestCase):
+    FTP_TEST_FILE = 'ftp://www.pythontest.net/README'
+    NUM_FTP_RETRIEVES = 3
+
+    @skip_ftp_test_on_travis
+    def test_multiple_ftp_retrieves(self):
+
+        with test_support.transient_internet(self.FTP_TEST_FILE):
+            try:
+                for _ in range(self.NUM_FTP_RETRIEVES):
+                    with tempfile.NamedTemporaryFile() as fp:
+                        urllib.FancyURLopener().retrieve(self.FTP_TEST_FILE, fp.name)
+            except IOError as e:
+                self.fail("Failed FTP retrieve while accessing ftp url "
+                          "multiple times.\n Error message was : %s" % e)
+
+    @skip_ftp_test_on_travis
+    def test_multiple_ftp_urlopen_same_host(self):
+        with test_support.transient_internet(self.FTP_TEST_FILE):
+            ftp_fds_to_close = []
+            try:
+                for _ in range(self.NUM_FTP_RETRIEVES):
+                    fd = urllib.urlopen(self.FTP_TEST_FILE)
+                    # test ftp open without closing fd as a supported scenario.
+                    ftp_fds_to_close.append(fd)
+            except IOError as e:
+                self.fail("Failed FTP binary file open. "
+                          "Error message was: %s" % e)
+            finally:
+                # close the open fds
+                for fd in ftp_fds_to_close:
+                    fd.close()
+
+
 def test_main():
     test_support.requires('network')
     with test_support.check_py3k_warnings(
             ("urllib.urlopen.. has been removed", DeprecationWarning)):
         test_support.run_unittest(URLTimeoutTest,
                                   urlopenNetworkTests,
                                   urlretrieveNetworkTests,
-                                  urlopen_HttpsTests)
+                                  urlopen_HttpsTests,
+                                  urlopen_FTPTest)
 
 if __name__ == "__main__":
     test_main()
diff --git a/Lib/urllib.py b/Lib/urllib.py
@@ -934,7 +934,13 @@ def retrfile(self, file, type):
         return (ftpobj, retrlen)
 
     def endtransfer(self):
+        if not self.busy:
+            return
         self.busy = 0
+        try:
+            self.ftp.voidresp()
+        except ftperrors():
+            pass
 
     def close(self):
         self.keepalive = False
diff --git a/Misc/NEWS.d/next/Library/2019-12-30-07-59-34.bpo-27973.mgWXH1.rst b/Misc/NEWS.d/next/Library/2019-12-30-07-59-34.bpo-27973.mgWXH1.rst
@@ -0,0 +1,2 @@
+Fix urllib.urlretrieve failing on subsequent ftp transfers from the same
+host.

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+Fix urllib.urlretrieve failing on subsequent ftp transfers from the same`
	`2`	`+host.`