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

[3.11] GH-103472: close response in HTTPConnection._tunnel (GH-103473) #104077

Merged
merged 1 commit into from
May 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 18 additions & 15 deletions Lib/http/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -918,23 +918,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__."""
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 @@ -2249,6 +2249,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)
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Avoid a potential :exc:`ResourceWarning` in :class:`http.client.HTTPConnection`
by closing the proxy / tunnel's CONNECT response explicitly.