in ftp cache pruning, avoid changing the size of a dict while iterating over it ...

Patch by Skyler Leigh Amador.

diff --git a/Lib/test/test_urllib.py b/Lib/test/test_urllib.py
index 1a405666206bafb534e39e59d5baf58dd9188ea3..1a5013ed2696a5b507169aed5bae3c1d89211ca7 100644 (file)
--- a/Lib/test/test_urllib.py
+++ b/Lib/test/test_urllib.py
@@ -7,6 +7,7 @@ import http.client
 import email.message
 import io
 import unittest
+from unittest.mock import patch
 from test import support
 import os
 import sys
@@ -89,6 +90,26 @@ class FakeHTTPMixin(object):
         http.client.HTTPConnection = self._connection_class
 
 
+class FakeFTPMixin(object):
+    def fakeftp(self):
+        class FakeFtpWrapper(object):
+            def __init__(self,  user, passwd, host, port, dirs, timeout=None,
+                     persistent=True):
+                pass
+
+            def retrfile(self, file, type):
+                return io.BytesIO(), 0
+
+            def close(self):
+                pass
+
+        self._ftpwrapper_class = urllib.request.ftpwrapper
+        urllib.request.ftpwrapper = FakeFtpWrapper
+
+    def unfakeftp(self):
+        urllib.request.ftpwrapper = self._ftpwrapper_class
+
+
 class urlopen_FileTests(unittest.TestCase):
     """Test urlopen() opening a temporary file.
 
@@ -195,7 +216,7 @@ class ProxyTests(unittest.TestCase):
         self.env.set('NO_PROXY', 'localhost, anotherdomain.com, newdomain.com')
         self.assertTrue(urllib.request.proxy_bypass_environment('anotherdomain.com'))
 
-class urlopen_HttpTests(unittest.TestCase, FakeHTTPMixin):
+class urlopen_HttpTests(unittest.TestCase, FakeHTTPMixin, FakeFTPMixin):
     """Test urlopen() opening a fake http connection."""
 
     def check_read(self, ver):
@@ -309,6 +330,15 @@ Content-Type: text/html; charset=iso-8859-1
         self.assertFalse(e.exception.filename)
         self.assertTrue(e.exception.reason)
 
+    @patch.object(urllib.request, 'MAXFTPCACHE', 0)
+    def test_ftp_cache_pruning(self):
+        self.fakeftp()
+        try:
+            urllib.request.ftpcache['test'] = urllib.request.ftpwrapper('user', 'pass', 'localhost', 21, [])
+            urlopen('ftp://localhost')
+        finally:
+            self.unfakeftp()
+
 
     def test_userpass_inurl(self):
         self.fakehttp(b"HTTP/1.0 200 OK\r\n\r\nHello!")

diff --git a/Lib/urllib/request.py b/Lib/urllib/request.py
index 43d6caa9e79d5d42bfd3ab1867e5a61264ab5248..72e91dd5f4cc3bf2169e2653817d1eb8dc222a80 100644 (file)
--- a/Lib/urllib/request.py
+++ b/Lib/urllib/request.py
@@ -1911,7 +1911,7 @@ class URLopener:
         # XXX thread unsafe!
         if len(self.ftpcache) > MAXFTPCACHE:
             # Prune the cache, rather arbitrarily
-            for k in self.ftpcache.keys():
+            for k in list(self.ftpcache):
                 if k != key:
                     v = self.ftpcache[k]
                     del self.ftpcache[k]

diff --git a/Misc/ACKS b/Misc/ACKS
index 571f1a65e491d5bae50bad4e14fab61edee6fb1a..344a29b10e20b9c63d9e4c07658119fc6b0702ee 100644 (file)
--- a/Misc/ACKS
+++ b/Misc/ACKS
@@ -32,6 +32,7 @@ Fred Allen
 Ray Allen
 Billy G. Allie
 Kevin Altis
+Skyler Leigh Amador
 Joe Amenta
 A. Amoroso
 Mark Anacker

diff --git a/Misc/NEWS b/Misc/NEWS
index 8f05a13b19a57d4cddfb36e6416256a01cf1ea3e..f581cd458021cc8a5c56ed072918393a118029b0 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -22,6 +22,8 @@ Core and Builtins
 Library
 -------
 
+- Issue #21463: In urllib.request, fix pruning of the FTP cache.
+
 - Issue #21618: The subprocess module could fail to close open fds that were
   inherited by the calling process and already higher than POSIX resource
   limits would otherwise allow.  On systems with a functioning /proc/self/fd