From 9de0cf20fa0485e327e57cc0864c7476da85cfad Mon Sep 17 00:00:00 2001 From: Thomas Grainger Date: Tue, 2 May 2023 04:59:42 +0100 Subject: GH-103472: close response in HTTPConnection._tunnel (#103473) Avoid a potential `ResourceWarning` in `http.client.HTTPConnection` by closing the proxy / tunnel's CONNECT response explicitly. --------- Co-authored-by: Gregory P. Smith --- Lib/http/client.py | 33 ++++++++++++---------- Lib/test/test_httplib.py | 23 +++++++++++++++ .../2023-04-12-13-04-16.gh-issue-103472.C6bOHv.rst | 2 ++ 3 files changed, 43 insertions(+), 15 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2023-04-12-13-04-16.gh-issue-103472.C6bOHv.rst diff --git a/Lib/http/client.py b/Lib/http/client.py index 0f5cd35..74f7bcb 100644 --- a/Lib/http/client.py +++ b/Lib/http/client.py @@ -941,23 +941,26 @@ class HTTPConnection: 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__.""" diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py index b4f4e2b..37f77fe 100644 --- a/Lib/test/test_httplib.py +++ b/Lib/test/test_httplib.py @@ -2390,6 +2390,29 @@ class TunnelTests(TestCase): 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) 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 0000000..01d84f0 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-04-12-13-04-16.gh-issue-103472.C6bOHv.rst @@ -0,0 +1,2 @@ +Avoid a potential :exc:`ResourceWarning` in :class:`http.client.HTTPConnection` +by closing the proxy / tunnel's CONNECT response explicitly. -- cgit v0.12