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.
diff --git a/Lib/http/client.py b/Lib/http/client.py
index 4eca93e..b339f20 100644
--- a/Lib/http/client.py
+++ b/Lib/http/client.py
@@ -901,23 +901,24 @@ def set_debuglevel(self, level):
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:
diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py
index 4abff60..5fb4592 100644
--- a/Lib/test/test_httplib.py
+++ b/Lib/test/test_httplib.py
@@ -10,6 +10,7 @@
import warnings
import unittest
+from unittest import mock
TestCase = unittest.TestCase
from test import support
@@ -2051,6 +2052,23 @@ def test_connect_with_tunnel(self):
# 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', '/', '')