Skip to content

Commit 9de0cf2

Browse files
graingertgpshead
andauthored
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 <greg@krypto.org>
1 parent 690df4c commit 9de0cf2

File tree

3 files changed

+43
-15
lines changed

3 files changed

+43
-15
lines changed

Lib/http/client.py

+18-15
Original file line numberDiff line numberDiff line change
@@ -941,23 +941,26 @@ def _tunnel(self):
941941
del headers
942942

943943
response = self.response_class(self.sock, method=self._method)
944-
(version, code, message) = response._read_status()
944+
try:
945+
(version, code, message) = response._read_status()
945946

946-
if code != http.HTTPStatus.OK:
947-
self.close()
948-
raise OSError(f"Tunnel connection failed: {code} {message.strip()}")
949-
while True:
950-
line = response.fp.readline(_MAXLINE + 1)
951-
if len(line) > _MAXLINE:
952-
raise LineTooLong("header line")
953-
if not line:
954-
# for sites which EOF without sending a trailer
955-
break
956-
if line in (b'\r\n', b'\n', b''):
957-
break
947+
if code != http.HTTPStatus.OK:
948+
self.close()
949+
raise OSError(f"Tunnel connection failed: {code} {message.strip()}")
950+
while True:
951+
line = response.fp.readline(_MAXLINE + 1)
952+
if len(line) > _MAXLINE:
953+
raise LineTooLong("header line")
954+
if not line:
955+
# for sites which EOF without sending a trailer
956+
break
957+
if line in (b'\r\n', b'\n', b''):
958+
break
958959

959-
if self.debuglevel > 0:
960-
print('header:', line.decode())
960+
if self.debuglevel > 0:
961+
print('header:', line.decode())
962+
finally:
963+
response.close()
961964

962965
def connect(self):
963966
"""Connect to the host and port specified in __init__."""

Lib/test/test_httplib.py

+23
Original file line numberDiff line numberDiff line change
@@ -2390,6 +2390,29 @@ def test_tunnel_debuglog(self):
23902390
lines = output.getvalue().splitlines()
23912391
self.assertIn('header: {}'.format(expected_header), lines)
23922392

2393+
def test_tunnel_leak(self):
2394+
sock = None
2395+
2396+
def _create_connection(address, timeout=None, source_address=None):
2397+
nonlocal sock
2398+
sock = FakeSocket(
2399+
'HTTP/1.1 404 NOT FOUND\r\n\r\n',
2400+
host=address[0],
2401+
port=address[1],
2402+
)
2403+
return sock
2404+
2405+
self.conn._create_connection = _create_connection
2406+
self.conn.set_tunnel('destination.com')
2407+
exc = None
2408+
try:
2409+
self.conn.request('HEAD', '/', '')
2410+
except OSError as e:
2411+
# keeping a reference to exc keeps response alive in the traceback
2412+
exc = e
2413+
self.assertIsNotNone(exc)
2414+
self.assertTrue(sock.file_closed)
2415+
23932416

23942417
if __name__ == '__main__':
23952418
unittest.main(verbosity=2)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Avoid a potential :exc:`ResourceWarning` in :class:`http.client.HTTPConnection`
2+
by closing the proxy / tunnel's CONNECT response explicitly.

0 commit comments

Comments
 (0)