gh-140691: urllib.request: Close FTP control socket if data socket can't connect...

Co-authored-by: codenamenam <bluetire27@gmail.com>

diff --git a/Lib/_py_warnings.py b/Lib/_py_warnings.py
index 91a9f44b20173371e9efa7125e57a027f7d0893e..67c74fdd2d0b421da16cdc648fda2b7767baefbc 100644 (file)
--- a/Lib/_py_warnings.py
+++ b/Lib/_py_warnings.py
@@ -646,6 +646,9 @@ class WarningMessage(object):
                     "line : %r}" % (self.message, self._category_name,
                                     self.filename, self.lineno, self.line))
 
+    def __repr__(self):
+        return f'<{type(self).__qualname__} {self}>'
+
 
 class catch_warnings(object):
 

diff --git a/Lib/test/test_urllib2net.py b/Lib/test/test_urllib2net.py
index 0c5f99ec18bf43994392e2f972c4990e339e60a1..17db686942fabe55fbc40c0e6685ee7f4c77971c 100644 (file)
--- a/Lib/test/test_urllib2net.py
+++ b/Lib/test/test_urllib2net.py
@@ -1,9 +1,13 @@
+import contextlib
 import errno
+import sysconfig
 import unittest
+from unittest import mock
 from test import support
 from test.support import os_helper
 from test.support import socket_helper
 from test.support import ResourceDenied
+from test.support.warnings_helper import check_no_resource_warning
 
 import os
 import socket
@@ -143,6 +147,43 @@ class OtherNetworkTests(unittest.TestCase):
             ]
         self._test_urls(urls, self._extra_handlers())
 
+    @support.requires_resource('walltime')
+    @unittest.skipIf(sysconfig.get_platform() == 'linux-ppc64le',
+                     'leaks on PPC64LE (gh-140691)')
+    def test_ftp_no_leak(self):
+        # gh-140691: When the data connection (but not control connection)
+        # cannot be made established, we shouldn't leave an open socket object.
+
+        class MockError(OSError):
+            pass
+
+        orig_create_connection = socket.create_connection
+        def patched_create_connection(address, *args, **kwargs):
+            """Simulate REJECTing connections to ports other than 21"""
+            host, port = address
+            if port != 21:
+                raise MockError()
+            return orig_create_connection(address, *args, **kwargs)
+
+        url = 'ftp://www.pythontest.net/README'
+        entry = url, None, urllib.error.URLError
+        no_cache_handlers = [urllib.request.FTPHandler()]
+        cache_handlers = self._extra_handlers()
+        with mock.patch('socket.create_connection', patched_create_connection):
+            with check_no_resource_warning(self):
+                # Try without CacheFTPHandler
+                self._test_urls([entry], handlers=no_cache_handlers,
+                                retry=False)
+            with check_no_resource_warning(self):
+                # Try with CacheFTPHandler (uncached)
+                self._test_urls([entry], cache_handlers, retry=False)
+            with check_no_resource_warning(self):
+                # Try with CacheFTPHandler (cached)
+                self._test_urls([entry], cache_handlers, retry=False)
+        # Try without the mock: the handler should not use a closed connection
+        with check_no_resource_warning(self):
+            self._test_urls([url], cache_handlers, retry=False)
+
     def test_file(self):
         TESTFN = os_helper.TESTFN
         f = open(TESTFN, 'w')
@@ -234,18 +275,16 @@ class OtherNetworkTests(unittest.TestCase):
                 else:
                     req = expected_err = None
 
+                if expected_err:
+                    context = self.assertRaises(expected_err)
+                else:
+                    context = contextlib.nullcontext()
+
                 with socket_helper.transient_internet(url):
-                    try:
+                    f = None
+                    with context:
                         f = urlopen(url, req, support.INTERNET_TIMEOUT)
-                    # urllib.error.URLError is a subclass of OSError
-                    except OSError as err:
-                        if expected_err:
-                            msg = ("Didn't get expected error(s) %s for %s %s, got %s: %s" %
-                                   (expected_err, url, req, type(err), err))
-                            self.assertIsInstance(err, expected_err, msg)
-                        else:
-                            raise
-                    else:
+                    if f is not None:
                         try:
                             with time_out, \
                                  socket_peer_reset, \

diff --git a/Lib/urllib/request.py b/Lib/urllib/request.py
index af93d4cd75dbef2016cc11cdbae058a459d878ef..566b8087aec2771e1f5dc3b776bbd68bd67e56e2 100644 (file)
--- a/Lib/urllib/request.py
+++ b/Lib/urllib/request.py
@@ -1535,6 +1535,7 @@ class FTPHandler(BaseHandler):
         dirs, file = dirs[:-1], dirs[-1]
         if dirs and not dirs[0]:
             dirs = dirs[1:]
+        fw = None
         try:
             fw = self.connect_ftp(user, passwd, host, port, dirs, req.timeout)
             type = file and 'I' or 'D'
@@ -1552,8 +1553,12 @@ class FTPHandler(BaseHandler):
                 headers += "Content-length: %d\n" % retrlen
             headers = email.message_from_string(headers)
             return addinfourl(fp, headers, req.full_url)
-        except ftplib.all_errors as exp:
-            raise URLError(f"ftp error: {exp}") from exp
+        except Exception as exp:
+            if fw is not None and not fw.keepalive:
+                fw.close()
+            if isinstance(exp, ftplib.all_errors):
+                raise URLError(f"ftp error: {exp}") from exp
+            raise
 
     def connect_ftp(self, user, passwd, host, port, dirs, timeout):
         return ftpwrapper(user, passwd, host, port, dirs, timeout,
@@ -1577,14 +1582,15 @@ class CacheFTPHandler(FTPHandler):
 
     def connect_ftp(self, user, passwd, host, port, dirs, timeout):
         key = user, host, port, '/'.join(dirs), timeout
-        if key in self.cache:
-            self.timeout[key] = time.time() + self.delay
-        else:
-            self.cache[key] = ftpwrapper(user, passwd, host, port,
-                                         dirs, timeout)
-            self.timeout[key] = time.time() + self.delay
+        conn = self.cache.get(key)
+        if conn is None or not conn.keepalive:
+            if conn is not None:
+                conn.close()
+            conn = self.cache[key] = ftpwrapper(user, passwd, host, port,
+                                                dirs, timeout)
+        self.timeout[key] = time.time() + self.delay
         self.check_cache()
-        return self.cache[key]
+        return conn
 
     def check_cache(self):
         # first check for old ones

diff --git a/Misc/NEWS.d/next/Library/2025-10-31-15-06-26.gh-issue-140691.JzHGtg.rst b/Misc/NEWS.d/next/Library/2025-10-31-15-06-26.gh-issue-140691.JzHGtg.rst
new file mode 100644 (file)
index 0000000..84b6195
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2025-10-31-15-06-26.gh-issue-140691.JzHGtg.rst
@@ -0,0 +1,3 @@
+In :mod:`urllib.request`, when opening a FTP URL fails because a data
+connection cannot be made, the control connection's socket is now closed to
+avoid a :exc:`ResourceWarning`.