From 86952ce000e32955b00532c2e3c5c8f66dd7f262 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Wed, 12 Apr 2023 14:00:25 +0100 Subject: [PATCH 1/6] GH-103472: close response in HTTPConnection._tunnel --- Lib/http/client.py | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/Lib/http/client.py b/Lib/http/client.py index 0f5cd35247ae82..74f7bcb68fb6bc 100644 --- a/Lib/http/client.py +++ b/Lib/http/client.py @@ -941,23 +941,26 @@ def _tunnel(self): del headers response = self.response_class(self.sock, method=self._method) - (version, code, message) = response._read_status() + try: + (version, code, message) = response._read_status() - if code != http.HTTPStatus.OK: - self.close() - raise OSError(f"Tunnel connection failed: {code} {message.strip()}") - while True: - line = response.fp.readline(_MAXLINE + 1) - if len(line) > _MAXLINE: - raise LineTooLong("header line") - if not line: - # for sites which EOF without sending a trailer - break - if line in (b'\r\n', b'\n', b''): - break + if code != http.HTTPStatus.OK: + self.close() + raise OSError(f"Tunnel connection failed: {code} {message.strip()}") + while True: + line = response.fp.readline(_MAXLINE + 1) + if len(line) > _MAXLINE: + raise LineTooLong("header line") + if not line: + # for sites which EOF without sending a trailer + break + if line in (b'\r\n', b'\n', b''): + break - if self.debuglevel > 0: - print('header:', line.decode()) + if self.debuglevel > 0: + print('header:', line.decode()) + finally: + response.close() def connect(self): """Connect to the host and port specified in __init__.""" From 65087626ed346b46f6c006dda27a6b056eccfd87 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Wed, 12 Apr 2023 13:04:21 +0000 Subject: [PATCH 2/6] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Library/2023-04-12-13-04-16.gh-issue-103472.C6bOHv.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2023-04-12-13-04-16.gh-issue-103472.C6bOHv.rst diff --git a/Misc/NEWS.d/next/Library/2023-04-12-13-04-16.gh-issue-103472.C6bOHv.rst b/Misc/NEWS.d/next/Library/2023-04-12-13-04-16.gh-issue-103472.C6bOHv.rst new file mode 100644 index 00000000000000..1ea26d624fdc28 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-04-12-13-04-16.gh-issue-103472.C6bOHv.rst @@ -0,0 +1 @@ +avoid a ResourceWarning in :class:`http.client.HTTPConnection` by closing tunnel CONNECT requests explicitly From f88793c03508270df1ca11d5b305b22c1a84c6a8 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Wed, 12 Apr 2023 15:08:30 +0100 Subject: [PATCH 3/6] Update Misc/NEWS.d/next/Library/2023-04-12-13-04-16.gh-issue-103472.C6bOHv.rst --- .../next/Library/2023-04-12-13-04-16.gh-issue-103472.C6bOHv.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2023-04-12-13-04-16.gh-issue-103472.C6bOHv.rst b/Misc/NEWS.d/next/Library/2023-04-12-13-04-16.gh-issue-103472.C6bOHv.rst index 1ea26d624fdc28..60bf7730af1822 100644 --- a/Misc/NEWS.d/next/Library/2023-04-12-13-04-16.gh-issue-103472.C6bOHv.rst +++ b/Misc/NEWS.d/next/Library/2023-04-12-13-04-16.gh-issue-103472.C6bOHv.rst @@ -1 +1 @@ -avoid a ResourceWarning in :class:`http.client.HTTPConnection` by closing tunnel CONNECT requests explicitly +avoid a ResourceWarning in :class:`http.client.HTTPConnection` by closing tunnel CONNECT response explicitly From 2aa460c4986f4f9961206ef0b50ed85e48eb4298 Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Tue, 25 Apr 2023 15:55:10 +0100 Subject: [PATCH 4/6] test that HTTPConnection._tunnel closes the response on exceptions --- Lib/test/test_httplib.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py index b4f4e2b14351a6..37f77fe0a320c7 100644 --- a/Lib/test/test_httplib.py +++ b/Lib/test/test_httplib.py @@ -2390,6 +2390,29 @@ def test_tunnel_debuglog(self): lines = output.getvalue().splitlines() self.assertIn('header: {}'.format(expected_header), lines) + def test_tunnel_leak(self): + sock = None + + def _create_connection(address, timeout=None, source_address=None): + nonlocal sock + sock = FakeSocket( + 'HTTP/1.1 404 NOT FOUND\r\n\r\n', + host=address[0], + port=address[1], + ) + return sock + + self.conn._create_connection = _create_connection + self.conn.set_tunnel('destination.com') + exc = None + try: + self.conn.request('HEAD', '/', '') + except OSError as e: + # keeping a reference to exc keeps response alive in the traceback + exc = e + self.assertIsNotNone(exc) + self.assertTrue(sock.file_closed) + if __name__ == '__main__': unittest.main(verbosity=2) From c2171c7a5c10c35a06e26de871ad3c360968bbb7 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Mon, 1 May 2023 20:35:10 -0700 Subject: [PATCH 5/6] minor wording and ReST update to NEWS --- .../Library/2023-04-12-13-04-16.gh-issue-103472.C6bOHv.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2023-04-12-13-04-16.gh-issue-103472.C6bOHv.rst b/Misc/NEWS.d/next/Library/2023-04-12-13-04-16.gh-issue-103472.C6bOHv.rst index 60bf7730af1822..c7cd66d14f30c1 100644 --- a/Misc/NEWS.d/next/Library/2023-04-12-13-04-16.gh-issue-103472.C6bOHv.rst +++ b/Misc/NEWS.d/next/Library/2023-04-12-13-04-16.gh-issue-103472.C6bOHv.rst @@ -1 +1,2 @@ -avoid a ResourceWarning in :class:`http.client.HTTPConnection` by closing tunnel CONNECT response explicitly +Avoid a potential :exc:`ResourceWarning` in :class:`http.client.HTTPConnection` by closing +the proxy / tunnel's CONNECT response explicitly. From bd079b179c8bc483bc47c485461370d5bfffa25e Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Mon, 1 May 2023 20:35:41 -0700 Subject: [PATCH 6/6] word wrap news. --- .../Library/2023-04-12-13-04-16.gh-issue-103472.C6bOHv.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2023-04-12-13-04-16.gh-issue-103472.C6bOHv.rst b/Misc/NEWS.d/next/Library/2023-04-12-13-04-16.gh-issue-103472.C6bOHv.rst index c7cd66d14f30c1..01d84f024bd4a6 100644 --- a/Misc/NEWS.d/next/Library/2023-04-12-13-04-16.gh-issue-103472.C6bOHv.rst +++ b/Misc/NEWS.d/next/Library/2023-04-12-13-04-16.gh-issue-103472.C6bOHv.rst @@ -1,2 +1,2 @@ -Avoid a potential :exc:`ResourceWarning` in :class:`http.client.HTTPConnection` by closing -the proxy / tunnel's CONNECT response explicitly. +Avoid a potential :exc:`ResourceWarning` in :class:`http.client.HTTPConnection` +by closing the proxy / tunnel's CONNECT response explicitly.