From 0d72e3429a57840cf08898b14df1de9363ffb9f6 Mon Sep 17 00:00:00 2001 From: ChasingImpact Date: Tue, 7 Oct 2025 16:22:20 -0400 Subject: [PATCH 1/2] utilities/tests: align CONNECT validation with RFC 9113 s8.3 & RFC 8441 s4; add tests; changelog --- CHANGELOG.rst | 20 ++ src/h2/utilities.py | 32 ++- tests/test_connect_pseudo_headers.py | 99 +++++++++ tests/test_connect_roundtrip_and_edges.py | 234 ++++++++++++++++++++++ 4 files changed, 380 insertions(+), 5 deletions(-) create mode 100644 tests/test_connect_pseudo_headers.py create mode 100644 tests/test_connect_roundtrip_and_edges.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 06111c33..fe4d288a 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,6 +1,26 @@ Release History =============== +dev +--- + +**API Changes (Backward Incompatible)** + +- Support for Python 3.9 has been removed. + +**API Changes (Backward Compatible)** + +- Support for Python 3.14 has been added. +- Align CONNECT pseudo-header validation with RFC 9113 s8.3 and RFC 8441 s4. + Ordinary CONNECT now requires ``:method=CONNECT`` and ``:authority``, and + forbids ``:scheme``/``:path``. Extended CONNECT (e.g., WebSocket) requires + ``:scheme``, ``:path``, ``:authority`` plus ``:protocol``. (PR #1309) + + +**Bugfixes** + +- + 4.3.0 (2025-08-23) ------------------ diff --git a/src/h2/utilities.py b/src/h2/utilities.py index a7409b38..7f0541b2 100644 --- a/src/h2/utilities.py +++ b/src/h2/utilities.py @@ -385,18 +385,39 @@ def _check_pseudo_header_field_acceptability(pseudo_headers: set[bytes | str] | if invalid_response_headers: msg = f"Encountered request-only headers {invalid_response_headers}" raise ProtocolError(msg) + elif (not hdr_validation_flags.is_response_header and not hdr_validation_flags.is_trailer): - # This is a request, so we need to have seen :path, :method, and - # :scheme. - _assert_header_in_set(b":path", pseudo_headers) + # Request header block. _assert_header_in_set(b":method", pseudo_headers) - _assert_header_in_set(b":scheme", pseudo_headers) + + is_connect = (method == b"CONNECT") + is_extended_connect = is_connect and (b":protocol" in pseudo_headers) + + if is_connect and not is_extended_connect: + # Ordinary CONNECT (RFC 9113 s8.3): + # MUST NOT include :scheme or :path. + if b":scheme" in pseudo_headers or b":path" in pseudo_headers: + msg = "Ordinary CONNECT MUST NOT include :scheme or :path" + raise ProtocolError(msg) + # :authority presence is enforced elsewhere; no extra asserts here. + elif is_extended_connect: + # Extended CONNECT (RFC 8441 s4): require the regular tuple. + _assert_header_in_set(b":scheme", pseudo_headers) + _assert_header_in_set(b":path", pseudo_headers) + # :authority presence validated by host/authority checker. + else: + # Non-CONNECT requests require :scheme and :path (RFC 9113 s8.3). + _assert_header_in_set(b":scheme", pseudo_headers) + _assert_header_in_set(b":path", pseudo_headers) + invalid_request_headers = pseudo_headers & _RESPONSE_ONLY_HEADERS if invalid_request_headers: msg = f"Encountered response-only headers {invalid_request_headers}" raise ProtocolError(msg) - if method != b"CONNECT": + + # If not CONNECT, then :protocol is invalid. + if not is_connect: invalid_headers = pseudo_headers & _CONNECT_REQUEST_ONLY_HEADERS if invalid_headers: msg = f"Encountered connect-request-only headers {invalid_headers!r}" @@ -698,3 +719,4 @@ def _check_size_limit(self) -> None: if self._size_limit is not None: while len(self) > self._size_limit: self.popitem(last=False) + diff --git a/tests/test_connect_pseudo_headers.py b/tests/test_connect_pseudo_headers.py new file mode 100644 index 00000000..4ab60bfb --- /dev/null +++ b/tests/test_connect_pseudo_headers.py @@ -0,0 +1,99 @@ +"""unit tests for ordinary vs extended CONNECT validation on the client side.""" + +from __future__ import annotations + +import pytest + +from h2.config import H2Configuration +from h2.connection import H2Connection +from h2.utilities import HeaderValidationFlags, validate_outbound_headers + + +def _new_conn() -> H2Connection: + c = H2Connection( + config=H2Configuration(client_side=True, header_encoding="utf-8") + ) + c.initiate_connection() + # settings ack frame: length=0, type=4, flags=1(ACK), stream=0 + c.receive_data(b"\x00\x00\x00\x04\x01\x00\x00\x00\x00") + return c + + +def _client_req_flags() -> HeaderValidationFlags: + # client, not trailers, not response, not push + return HeaderValidationFlags( + is_client=True, + is_trailer=False, + is_response_header=False, + is_push_promise=False, + ) + + +def test_ordinary_connect_allows_no_scheme_no_path_and_send_headers_ok() -> None: + # ---- bytes for validate_outbound_headers ---- + hdrs_bytes = [ + (b":method", b"CONNECT"), + (b":authority", b"example.com:443"), + ] + # should not raise + list(validate_outbound_headers(hdrs_bytes, _client_req_flags())) + + # ---- str is fine for send_headers due to header_encoding ---- + hdrs_str = [ + (":method", "CONNECT"), + (":authority", "example.com:443"), + ] + conn = _new_conn() + # should not raise + conn.send_headers(1, hdrs_str, end_stream=True) + + +def test_ordinary_connect_rejects_path_or_scheme() -> None: + bad1 = [ + (b":method", b"CONNECT"), + (b":authority", b"example.com:443"), + (b":path", b"/"), + ] + bad2 = [ + (b":method", b"CONNECT"), + (b":authority", b"example.com:443"), + (b":scheme", b"https"), + ] + with pytest.raises(Exception): + list(validate_outbound_headers(bad1, _client_req_flags())) + with pytest.raises(Exception): + list(validate_outbound_headers(bad2, _client_req_flags())) + + +def test_extended_connect_requires_regular_tuple_and_send_headers_ok() -> None: + hdrs_bytes = [ + (b":method", b"CONNECT"), + (b":protocol", b"websocket"), + (b":scheme", b"https"), + (b":path", b"/chat?room=1"), + (b":authority", b"ws.example.com"), + ] + # should not raise + list(validate_outbound_headers(hdrs_bytes, _client_req_flags())) + + hdrs_str = [ + (":method", "CONNECT"), + (":protocol", "websocket"), + (":scheme", "https"), + (":path", "/chat?room=1"), + (":authority", "ws.example.com"), + ] + conn = _new_conn() + # should not raise + conn.send_headers(3, hdrs_str, end_stream=True) + + +def test_non_connect_still_requires_scheme_and_path() -> None: + hdrs_bytes = [ + (b":method", b"GET"), + (b":authority", b"example.com"), + # omit :scheme and :path -> should raise + ] + with pytest.raises(Exception): + list(validate_outbound_headers(hdrs_bytes, _client_req_flags())) + diff --git a/tests/test_connect_roundtrip_and_edges.py b/tests/test_connect_roundtrip_and_edges.py new file mode 100644 index 00000000..9074d3c5 --- /dev/null +++ b/tests/test_connect_roundtrip_and_edges.py @@ -0,0 +1,234 @@ +"""round-trip and edge tests for ordinary vs extended CONNECT behavior.""" + +from __future__ import annotations + + +import pytest + +from h2.config import H2Configuration +from h2.connection import H2Connection +from h2.events import RequestReceived +from h2.utilities import HeaderValidationFlags, validate_outbound_headers + + + +def _new_conn(client_side: bool) -> H2Connection: + return H2Connection( + H2Configuration(client_side=client_side, header_encoding="utf-8") + ) + + +def _xfer_once(a: H2Connection, b: H2Connection): + """ + move bytes from a -> b one step and return the events produced on b. + """ + data = a.data_to_send() + if not data: + return [] + return b.receive_data(data) + + +def _shake(client: H2Connection, server: H2Connection) -> None: + client.initiate_connection() + server.initiate_connection() + _xfer_once(client, server) + _xfer_once(server, client) + + +def _drain_events( + src: H2Connection, dst: H2Connection, max_iters: int = 10 +) -> list: + """ + keep transferring until src has nothing more to send (or max_iters reached). + return the list of all events generated on dst. + """ + out = [] + for _ in range(max_iters): + evs = _xfer_once(src, dst) + if not evs: + break + out.extend(evs) + return out + + +def _validate_bytes(hdrs) -> None: + flags = HeaderValidationFlags( + is_client=True, + is_trailer=False, + is_response_header=False, + is_push_promise=False, + ) + # force bytes; validate_outbound_headers expects (bytes, bytes) tuples + hdrs_b = [ + ( + k if isinstance(k, bytes) else k.encode("ascii"), + v if isinstance(v, bytes) else v.encode("ascii"), + ) + for (k, v) in hdrs + ] + # exhaust generator to trigger validation side-effects + list(validate_outbound_headers(hdrs_b, flags)) # noqa: B018 (intentional) + + +def test_roundtrip_ordinary_connect_no_path_ok() -> None: + client = _new_conn(True) + server = _new_conn(False) + _shake(client, server) + + # ordinary CONNECT: only :method and :authority + hdrs = [ + (":method", "CONNECT"), + (":authority", "example.com:443"), + ] + _validate_bytes(hdrs) + + stream_id = 1 + client.send_headers(stream_id, hdrs, end_stream=True) + evs = _drain_events(client, server) + + req_evs = [e for e in evs if isinstance(e, RequestReceived)] + assert req_evs, "server should receive a RequestReceived" + ps = {k: v for k, v in req_evs[0].headers if k.startswith(":")} + assert ps[":method"] == "CONNECT" + assert ps[":authority"] == "example.com:443" + assert ":scheme" not in ps and ":path" not in ps + + +def test_roundtrip_extended_connect_websocket_ok() -> None: + client = _new_conn(True) + server = _new_conn(False) + _shake(client, server) + + # extended CONNECT for WebSocket (RFC 8441 ยง4) + hdrs = [ + (":method", "CONNECT"), + (":protocol", "websocket"), + (":scheme", "https"), + (":path", "/chat?room=1"), + (":authority", "ws.example.com"), + ] + _validate_bytes(hdrs) + + stream_id = 1 + client.send_headers(stream_id, hdrs, end_stream=True) + evs = _drain_events(client, server) + + req_evs = [e for e in evs if isinstance(e, RequestReceived)] + assert req_evs + ps = {k: v for k, v in req_evs[0].headers if k.startswith(":")} + assert ps[":method"] == "CONNECT" + assert ps[":protocol"] == "websocket" + assert ps[":scheme"] == "https" + assert ps[":path"] == "/chat?room=1" + assert ps[":authority"] == "ws.example.com" + + +def test_extended_connect_rejects_if_tuple_incomplete() -> None: + # :protocol present but missing :scheme/:path => should fail validation + hdrs = [ + (":method", "CONNECT"), + (":protocol", "websocket"), + (":authority", "ws.example.com"), + # no :scheme, no :path + ] + with pytest.raises(Exception): + _validate_bytes(hdrs) + + +def test_ordinary_connect_rejects_path_or_scheme() -> None: + bad1 = [ + (":method", "CONNECT"), + (":authority", "example.com:443"), + (":path", "/"), + ] + bad2 = [ + (":method", "CONNECT"), + (":authority", "example.com:443"), + (":scheme", "https"), + ] + with pytest.raises(Exception): + _validate_bytes(bad1) + with pytest.raises(Exception): + _validate_bytes(bad2) + + +def test_pseudo_headers_must_come_first() -> None: + # a regular header before a pseudo-header should be rejected + hdrs = [ + ("host", "example.com"), + (":method", "CONNECT"), + (":authority", "example.com:443"), + ] + with pytest.raises(Exception): + _validate_bytes(hdrs) + + +def test_duplicate_pseudo_header_rejected() -> None: + hdrs = [ + (":method", "CONNECT"), + (":method", "CONNECT"), + (":authority", "example.com:443"), + ] + with pytest.raises(Exception): + _validate_bytes(hdrs) + + +def test_large_header_block_continuation_ok() -> None: + client = _new_conn(True) + server = _new_conn(False) + _shake(client, server) + + # many regular headers to force CONTINUATION frames + big = [(f"x-h{i}", "y" * 200) for i in range(200)] + hdrs = [ + (":method", "CONNECT"), + (":authority", "example.com:443"), + ] + big + + _validate_bytes(hdrs) + client.send_headers(1, hdrs, end_stream=True) + + evs = _drain_events(client, server, max_iters=20) + req_evs = [e for e in evs if isinstance(e, RequestReceived)] + assert req_evs + got = dict(req_evs[0].headers) + assert "x-h0" in got and str(got["x-h0"]).startswith("y") + + +def test_te_header_rules() -> None: + # only "te: trailers" is legal in HTTP/2 requests. anything else must be rejected. + ok = [ + (":method", "CONNECT"), + (":authority", "example.com:443"), + ("te", "trailers"), + ] + _validate_bytes(ok) # should not raise + + bad = [ + (":method", "CONNECT"), + (":authority", "example.com:443"), + ("te", "gzip"), + ] + with pytest.raises(Exception): + _validate_bytes(bad) + + +def test_multiple_concurrent_connect_streams_ok() -> None: + client = _new_conn(True) + server = _new_conn(False) + _shake(client, server) + + s1 = 1 + s2 = 3 + hdrs = [ + (":method", "CONNECT"), + (":authority", "example.internal:3128"), + ] + _validate_bytes(hdrs) + client.send_headers(s1, hdrs, end_stream=True) + client.send_headers(s2, hdrs, end_stream=True) + + evs = _drain_events(client, server, max_iters=10) + reqs = [e for e in evs if isinstance(e, RequestReceived)] + assert len(reqs) == 2 + From fd104ad7d31b897a17c05768815de5ab2565f56c Mon Sep 17 00:00:00 2001 From: ChasingImpact Date: Thu, 9 Oct 2025 17:54:33 -0400 Subject: [PATCH 2/2] lint/types: utilities.py noqa+comma; stream.py coerce memoryview -> bytes; silence settings.py PLW1641 --- src/h2/settings.py | 2 +- src/h2/stream.py | 2 +- src/h2/utilities.py | 9 +++++---- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/h2/settings.py b/src/h2/settings.py index c1be953b..659dac79 100644 --- a/src/h2/settings.py +++ b/src/h2/settings.py @@ -90,7 +90,7 @@ def __repr__(self) -> str: ) -class Settings(MutableMapping[Union[SettingCodes, int], int]): +class Settings(MutableMapping[Union[SettingCodes, int], int]): # noqa: PLW1641 """ An object that encapsulates HTTP/2 settings state. diff --git a/src/h2/stream.py b/src/h2/stream.py index d6f5845c..1f38339d 100644 --- a/src/h2/stream.py +++ b/src/h2/stream.py @@ -968,7 +968,7 @@ def send_data(self, self.state_machine.process_input(StreamInputs.SEND_DATA) df = DataFrame(self.stream_id) - df.data = data + df.data = data.tobytes() if isinstance(data, memoryview) else data if end_stream: self.state_machine.process_input(StreamInputs.SEND_END_STREAM) df.flags.add("END_STREAM") diff --git a/src/h2/utilities.py b/src/h2/utilities.py index 7f0541b2..9b492210 100644 --- a/src/h2/utilities.py +++ b/src/h2/utilities.py @@ -361,9 +361,11 @@ def _reject_pseudo_header_fields(headers: Iterable[Header], ) -def _check_pseudo_header_field_acceptability(pseudo_headers: set[bytes | str] | set[bytes] | set[str], - method: bytes | None, - hdr_validation_flags: HeaderValidationFlags) -> None: +def _check_pseudo_header_field_acceptability( # noqa: C901 + pseudo_headers: set[bytes | str] | set[bytes] | set[str], + method: bytes | None, + hdr_validation_flags: HeaderValidationFlags, +) -> None: """ Given the set of pseudo-headers present in a header block and the validation flags, confirms that RFC 7540 allows them. @@ -385,7 +387,6 @@ def _check_pseudo_header_field_acceptability(pseudo_headers: set[bytes | str] | if invalid_response_headers: msg = f"Encountered request-only headers {invalid_response_headers}" raise ProtocolError(msg) - elif (not hdr_validation_flags.is_response_header and not hdr_validation_flags.is_trailer): # Request header block.