diff --git a/Lib/_py_warnings.py b/Lib/_py_warnings.py index 91a9f44b201733..67c74fdd2d0b42 100644 --- a/Lib/_py_warnings.py +++ b/Lib/_py_warnings.py @@ -646,6 +646,9 @@ def __str__(self): "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 0c5f99ec18bf43..17db686942fabe 100644 --- 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 @@ def test_ftp(self): ] 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 @@ def _test_urls(self, urls, handlers, retry=True): 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 af93d4cd75dbef..566b8087aec277 100644 --- a/Lib/urllib/request.py +++ b/Lib/urllib/request.py @@ -1535,6 +1535,7 @@ def ftp_open(self, req): 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 @@ def ftp_open(self, req): 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 @@ def setMaxConns(self, m): 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 index 00000000000000..84b6195c9262c8 --- /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`.