Skip to content

Commit 9e69861

Browse files
ofek-kapsclaude
andcommitted
test: use real ResponseHandler to assert _skip_payload behavioral state
Address review feedback: instead of asserting that set_response_params is called with skip_payload=True on a mock, instantiate a real ResponseHandler and assert proto._skip_payload is True after the CONNECT 200 handshake. This validates the actual HTTP parser configuration (response_with_body=False) that prevents tunnel response body bytes from blocking the TLS upgrade (RFC-9110 §9.3.6). Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
1 parent 86ea5fb commit 9e69861

1 file changed

Lines changed: 22 additions & 10 deletions

File tree

tests/test_proxy.py

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
from yarl import URL
1111

1212
import aiohttp
13+
from aiohttp.client_proto import ResponseHandler
1314
from aiohttp.client_reqrep import (
1415
ClientRequest,
1516
ClientRequestArgs,
@@ -1184,8 +1185,14 @@ async def test_https_connect_skip_payload_on_200( # type: ignore[misc]
11841185
11851186
Per RFC-9110 §9.3.6 a client MUST ignore any Content-Length or
11861187
Transfer-Encoding header fields in a successful response to CONNECT.
1187-
Verify that set_response_params is called with skip_payload=True so the
1188-
HTTP parser does not try to read a body from the 200 tunnel response.
1188+
1189+
This test uses a real ResponseHandler instance so we can assert that
1190+
``_skip_payload`` is actually set to ``True`` on the protocol object
1191+
after the CONNECT 200 handshake. ``_skip_payload=True`` causes the
1192+
underlying ``HttpResponseParser`` to be configured with
1193+
``response_with_body=False``, which means any body bytes advertised
1194+
by Content-Length / Transfer-Encoding in the tunnel response are
1195+
silently discarded rather than blocking the TLS upgrade.
11891196
"""
11901197
event_loop = asyncio.get_running_loop()
11911198
proxy_req = ClientRequestBase(
@@ -1227,7 +1234,10 @@ async def test_https_connect_skip_payload_on_200( # type: ignore[misc]
12271234
with mock.patch.object(
12281235
connector, "_resolve_host", autospec=True, return_value=[r]
12291236
):
1230-
tr, proto = mock.Mock(), mock.Mock()
1237+
# Use a real ResponseHandler so we can assert its internal state
1238+
# rather than only checking that a mock method was called.
1239+
tr = mock.Mock()
1240+
proto = ResponseHandler(loop=event_loop)
12311241
with mock.patch.object(
12321242
event_loop,
12331243
"create_connection",
@@ -1250,13 +1260,15 @@ async def test_https_connect_skip_payload_on_200( # type: ignore[misc]
12501260
req, [], aiohttp.ClientTimeout()
12511261
)
12521262

1253-
# proto is the mock protocol returned by create_connection.
1254-
# The connector calls protocol.set_response_params(...) on it
1255-
# directly, so we assert on proto's auto-mock attribute.
1256-
proto.set_response_params.assert_called_once_with(
1257-
read_until_eof=True,
1258-
timeout_ceil_threshold=mock.ANY,
1259-
skip_payload=True,
1263+
# Verify that the connector configured the real protocol
1264+
# to skip the CONNECT response body. This flag causes
1265+
# HttpResponseParser to use response_with_body=False, so
1266+
# any Content-Length / Transfer-Encoding bytes in the
1267+
# tunnel handshake are not consumed before TLS is started.
1268+
assert proto._skip_payload is True, (
1269+
"ResponseHandler._skip_payload must be True after a "
1270+
"CONNECT 200 so that proxy body bytes are not read "
1271+
"before the TLS upgrade (RFC-9110 §9.3.6)"
12601272
)
12611273

12621274
proxy_resp.close()

0 commit comments

Comments
 (0)