Skip to content

Commit be8b4e3

Browse files
authored
Fix substring check used instead of equality for cookie header name (#1313)
* Fix substring check used instead of equality for header names The `in` operator on `bytes` performs substring search, not equality. `header[0] in b"cookie"` matches any header name that is a substring of "cookie" (e.g. b"co", b"ok", b"e"), not just b"cookie" itself. This means short header names that happen to be substrings of "cookie" get incorrectly promoted to NeverIndexedHeaderTuple when their value is under 20 bytes, potentially affecting HPACK compression behavior. Changed both occurrences to use `==` for exact comparison: - Line 91: cookie header check in _secure_headers - Line 350: :method pseudo-header check in _reject_pseudo_header_fields * Add regression tests for substring check fix - Add test data with header names that are substrings of 'cookie' but not equal to 'cookie' to verify they are not treated as sensitive - Add test for extract_method_header to verify substring names like ':me' do not falsely match ':method'
1 parent 18fa348 commit be8b4e3

File tree

3 files changed

+52
-2
lines changed

3 files changed

+52
-2
lines changed

src/h2/utilities.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ def _secure_headers(headers: Iterable[Header],
8888
"""
8989
for header in headers:
9090
assert isinstance(header[0], bytes)
91-
if header[0] in _SECURE_HEADERS or (header[0] in b"cookie" and len(header[1]) < 20):
91+
if header[0] in _SECURE_HEADERS or (header[0] == b"cookie" and len(header[1]) < 20):
9292
yield NeverIndexedHeaderTuple(header[0], header[1])
9393
else:
9494
yield header
@@ -347,7 +347,7 @@ def _reject_pseudo_header_fields(headers: Iterable[Header],
347347
msg = f"Received custom pseudo-header field {header[0]!r}"
348348
raise ProtocolError(msg)
349349

350-
if header[0] in b":method":
350+
if header[0] == b":method":
351351
method = header[1]
352352

353353
else:

tests/test_header_indexing.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,19 @@ class TestSecureHeaders:
418418
HeaderTuple(b"cookie", b"twenty byte cookie!!"),
419419
HeaderTuple(b"Cookie", b"twenty byte cookie!!"),
420420
]
421+
# Headers with names that are substrings of "cookie" but not equal to
422+
# "cookie". Before the fix, the ``in`` operator was used instead of
423+
# ``==``, which caused these to be incorrectly treated as sensitive.
424+
non_cookie_substring_headers = [
425+
(b"cook", b"short"),
426+
(b"okie", b"short"),
427+
(b"oo", b"short"),
428+
(b"cooki", b"short"),
429+
(b"ookie", b"short"),
430+
HeaderTuple(b"cook", b"short"),
431+
HeaderTuple(b"okie", b"short"),
432+
HeaderTuple(b"cooki", b"short"),
433+
]
421434

422435
server_config = h2.config.H2Configuration(client_side=False)
423436

@@ -622,3 +635,27 @@ def test_long_cookie_headers_can_be_indexed_push(self,
622635
)
623636

624637
assert c.data_to_send() == expected_frame.serialize()
638+
639+
@pytest.mark.parametrize(
640+
"headers", [example_request_headers, bytes_example_request_headers],
641+
)
642+
@pytest.mark.parametrize("header", non_cookie_substring_headers)
643+
def test_non_cookie_substring_headers_can_be_indexed(self,
644+
headers,
645+
header,
646+
frame_factory) -> None:
647+
"""
648+
Headers with names that are substrings of 'cookie' but not equal to
649+
'cookie' should not be treated as sensitive.
650+
"""
651+
send_headers = [*headers, header]
652+
expected_headers = [*headers, HeaderTuple(header[0], header[1])]
653+
654+
c = h2.connection.H2Connection()
655+
c.initiate_connection()
656+
657+
c.clear_outbound_data_buffer()
658+
c.send_headers(1, send_headers)
659+
660+
f = frame_factory.build_headers_frame(headers=expected_headers)
661+
assert c.data_to_send() == f.serialize()

tests/test_utility_functions.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,19 @@ def test_extract_header_method(self) -> None:
163163
self.example_headers_with_bytes,
164164
) == b"GET"
165165

166+
def test_extract_method_header_no_false_substring_match(self) -> None:
167+
"""
168+
Headers with names that are substrings of ':method' but not equal
169+
to ':method' should not be matched.
170+
"""
171+
headers = [
172+
(b":authority", b"example.com"),
173+
(b":path", b"/"),
174+
(b":scheme", b"https"),
175+
(b":me", b"GET"),
176+
]
177+
assert extract_method_header(headers) is None
178+
166179

167180
def test_size_limit_dict_limit() -> None:
168181
dct = SizeLimitDict(size_limit=2)

0 commit comments

Comments
 (0)