in ftp cache pruning, avoid changing the size of a dict while iterating over it (closes #21463)

Patch by Skyler Leigh Amador.

4 files changed, 35 insertions, 2 deletions

diff --git a/Lib/test/test_urllib.py b/Lib/test/test_urllib.py
index 1a40566..1a5013e 100644
--- 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 43d6caa..72e91dd 100644
--- 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 571f1a6..344a29b 100644
--- 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 8f05a13..f581cd4 100644
--- 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