Merge pull request #1309 from chasingimpact/fix/ordinary-connect-no-path

HTTP/2: accept ordinary CONNECT without :path/:scheme; add tests (pseudo-headers + round-trip)

Author: Kriechi

CHANGELOG.rst

@@ -13,6 +13,10 @@ dev
 
 - Support for Python 3.14 has been added.
 - Support for PyPy 3.11 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**
 

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.
@@ -387,16 +389,36 @@ def _check_pseudo_header_field_acceptability(pseudo_headers: set[bytes | str] |
  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 +720,4 @@ def _check_size_limit(self) -> None:
  if self._size_limit is not None:
  while len(self) > self._size_limit:
  self.popitem(last=False)
+

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()))
+