Skip to content

Commit

Permalink
pythonGH-103472: close response in HTTPConnection._tunnel (pythonGH-1…
Browse files Browse the repository at this point in the history
…03473)

Avoid a potential `ResourceWarning` in `http.client.HTTPConnection`
by closing the proxy / tunnel's CONNECT response explicitly.

---------

(cherry picked from commit 9de0cf2)

Co-authored-by: Thomas Grainger <tagrain@gmail.com>
Co-authored-by: Gregory P. Smith <greg@krypto.org>
  • Loading branch information
2 people authored and miss-islington committed May 2, 2023
1 parent 9f191a1 commit 8f1ce8f
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 15 deletions.
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.

0 comments on commit 8f1ce8f

Please sign in to comment.