GH-103472: close response in HTTPConnection._tunnel (#103473)

graingert · gpshead · web-flow · commit 9de0cf20fa04 · 2023-05-02T03:59:42.000Z
Avoid a potential `ResourceWarning` in `http.client.HTTPConnection`
by closing the proxy / tunnel's CONNECT response explicitly.

---------

Co-authored-by: Gregory P. Smith &lt;greg@krypto.org&gt;
diff --git 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__."""
diff --git 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)
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
@@ -0,0 +1,2 @@
+Avoid a potential :exc:`ResourceWarning` in :class:`http.client.HTTPConnection`
+by closing the proxy / tunnel's CONNECT response explicitly.
