Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 8 commits into from
May 2, 2023
33 changes: 18 additions & 15 deletions Lib/http/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -941,23 +941,26 @@ def _tunnel(self):
del headers

response = self.response_class(self.sock, method=self._method)
(version, code, message) = response._read_status()
gpshead marked this conversation as resolved.
Show resolved Hide resolved
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()
gpshead marked this conversation as resolved.
Show resolved Hide resolved
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()
gpshead marked this conversation as resolved.
Show resolved Hide resolved

def connect(self):
"""Connect to the host and port specified in __init__."""
Expand Down
23 changes: 23 additions & 0 deletions Lib/test/test_httplib.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
gpshead marked this conversation as resolved.
Show resolved Hide resolved
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)
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
avoid a ResourceWarning in :class:`http.client.HTTPConnection` by closing tunnel CONNECT response explicitly