bpo-43332: Buffer proxy connection setup packets before sending. (GH-24780)

We now buffer the CONNECT request + tunnel HTTP headers into a single
send call.  This prevents the OS from generating multiple network
packets for connection setup when not necessary, improving efficiency.
(cherry picked from commit c25910a135)

Co-authored-by: Gregory P. Smith <greg@krypto.org>
This commit is contained in:
Miss Islington (bot) 2021-03-07 23:59:37 -08:00 committed by GitHub
parent 2c0a0b04a4
commit c6e7cf1ee0
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 32 additions and 10 deletions

View file

@ -886,23 +886,24 @@ class HTTPConnection:
self.debuglevel = level
def _tunnel(self):
connect_str = "CONNECT %s:%d HTTP/1.0\r\n" % (self._tunnel_host,
self._tunnel_port)
connect_bytes = connect_str.encode("ascii")
self.send(connect_bytes)
connect = b"CONNECT %s:%d HTTP/1.0\r\n" % (
self._tunnel_host.encode("ascii"), self._tunnel_port)
headers = [connect]
for header, value in self._tunnel_headers.items():
header_str = "%s: %s\r\n" % (header, value)
header_bytes = header_str.encode("latin-1")
self.send(header_bytes)
self.send(b'\r\n')
headers.append(f"{header}: {value}\r\n".encode("latin-1"))
headers.append(b"\r\n")
# Making a single send() call instead of one per line encourages
# the host OS to use a more optimal packet size instead of
# potentially emitting a series of small packets.
self.send(b"".join(headers))
del headers
response = self.response_class(self.sock, method=self._method)
(version, code, message) = response._read_status()
if code != http.HTTPStatus.OK:
self.close()
raise OSError("Tunnel connection failed: %d %s" % (code,
message.strip()))
raise OSError(f"Tunnel connection failed: {code} {message.strip()}")
while True:
line = response.fp.readline(_MAXLINE + 1)
if len(line) > _MAXLINE:

View file

@ -10,6 +10,7 @@ import threading
import warnings
import unittest
from unittest import mock
TestCase = unittest.TestCase
from test import support
@ -2021,6 +2022,23 @@ class TunnelTests(TestCase):
# This test should be removed when CONNECT gets the HTTP/1.1 blessing
self.assertNotIn(b'Host: proxy.com', self.conn.sock.data)
def test_tunnel_connect_single_send_connection_setup(self):
"""Regresstion test for https://bugs.python.org/issue43332."""
with mock.patch.object(self.conn, 'send') as mock_send:
self.conn.set_tunnel('destination.com')
self.conn.connect()
self.conn.request('GET', '/')
mock_send.assert_called()
# Likely 2, but this test only cares about the first.
self.assertGreater(
len(mock_send.mock_calls), 1,
msg=f'unexpected number of send calls: {mock_send.mock_calls}')
proxy_setup_data_sent = mock_send.mock_calls[0][1][0]
self.assertIn(b'CONNECT destination.com', proxy_setup_data_sent)
self.assertTrue(
proxy_setup_data_sent.endswith(b'\r\n\r\n'),
msg=f'unexpected proxy data sent {proxy_setup_data_sent!r}')
def test_connect_put_request(self):
self.conn.set_tunnel('destination.com')
self.conn.request('PUT', '/', '')

View file

@ -0,0 +1,3 @@
Improves the networking efficiency of :mod:`http.client` when using a proxy
via :meth:`~HTTPConnection.set_tunnel`. Fewer small send calls are made
during connection setup.