From 77ac36c55fd82062b633c0ac677dfc3151fd3b34 Mon Sep 17 00:00:00 2001 From: feiiiiii5 Date: Sat, 26 Sep 2026 06:00:17 +0800 Subject: [PATCH 1/4] Validate content-length when a stream ends with trailers receive_headers handled every HEADERS block, trailers included, and called _initialize_content_length on all of them. _track_content_length was only ever called from receive_data, so a stream ended by a trailers section never reached the "end_stream and expected != actual" branch: - trailers without content-length left the expectation as None and the guard was skipped entirely; - trailers with content-length silently replaced the header-section value, so a peer could declare 10 in the headers and 3 in the trailers and have a 13-byte body accepted. RFC 9113 section 8.1.1 makes a message malformed when content-length does not equal the sum of the DATA payload lengths, and the exemptions it lists are 204, 304 and HEAD, not trailers. The too-much-data direction still errored, because it trips while receiving DATA, so only the short-body direction was silently accepted. Run the same parse on trailers so an invalid content-length there is still a ProtocolError, then put the previous expectation back, and validate the body where the stream actually ends. --- src/h2/stream.py | 24 ++++- tests/test_invalid_content_lengths.py | 150 ++++++++++++++++++++++++++ 2 files changed, 169 insertions(+), 5 deletions(-) diff --git a/src/h2/stream.py b/src/h2/stream.py index 249f73e0c..0f342462b 100644 --- a/src/h2/stream.py +++ b/src/h2/stream.py @@ -1094,11 +1094,25 @@ def receive_headers(self, ).stream_ended = cast("StreamEnded", es_events[0]) events += es_events - self._initialize_content_length(headers) - - if isinstance(headers_event, TrailersReceived) and not end_stream: - msg = "Trailers must have END_STREAM set" - raise ProtocolError(msg) + is_trailers = isinstance(headers_event, TrailersReceived) + + if is_trailers: + # A trailers section carries no message content, so a + # content-length in it is validated for syntax only and must never + # redefine the expected body length: run the same parse, then put + # the previous expectation back. The trailers section is the end of + # the message, so the body must be validated here too, otherwise a + # stream that ends with trailers escapes the length check + # entirely. + expected_content_length = self._expected_content_length + self._initialize_content_length(headers) + self._expected_content_length = expected_content_length + if not end_stream: + msg = "Trailers must have END_STREAM set" + raise ProtocolError(msg) + self._track_content_length(0, end_stream=True) + else: + self._initialize_content_length(headers) hdr_validation_flags = self._build_hdr_validation_flags(events) headers_event.headers = self._process_received_headers( diff --git a/tests/test_invalid_content_lengths.py b/tests/test_invalid_content_lengths.py index 3927fb5e2..d93ebbb07 100644 --- a/tests/test_invalid_content_lengths.py +++ b/tests/test_invalid_content_lengths.py @@ -255,3 +255,153 @@ def test_insufficient_data_empty_frame(self, frame_factory, request_headers) -> error_code=h2.errors.ErrorCodes.PROTOCOL_ERROR, ) assert c.data_to_send() == expected_frame.serialize() + + +class TestContentLengthEnforcedAtTrailers: + """ + RFC 9113 § 8.1.1: a request or response is malformed if the value of a + content-length header field does not equal the sum of the DATA frame + payload lengths that form the content. The listed exemptions are 204, 304 + and HEAD, none of which is a trailers section, so a stream that ends with + trailers must still have its body length policed. + """ + + example_request_headers = [ + (":authority", "example.com"), + (":path", "/"), + (":scheme", "https"), + (":method", "POST"), + ("content-length", "15"), + ] + server_config = h2.config.H2Configuration(client_side=False) + + def _server(self, frame_factory, request_headers) -> h2.connection.H2Connection: + c = h2.connection.H2Connection(config=self.server_config) + c.initiate_connection() + c.receive_data(frame_factory.preamble()) + c.receive_data(frame_factory.build_headers_frame(headers=request_headers).serialize()) + return c + + @pytest.mark.parametrize("request_headers", [example_request_headers]) + def test_insufficient_data_ended_by_trailers(self, frame_factory, request_headers) -> None: + """ + Remote peers sending less data than content-length and then ending the + stream with trailers causes Protocol Errors. + """ + c = self._server(frame_factory, request_headers) + c.receive_data(frame_factory.build_data_frame(data=b"\x01"*13).serialize()) + c.clear_outbound_data_buffer() + + trailers = frame_factory.build_headers_frame( + headers=[("x-checksum", "0")], + flags=["END_STREAM"], + ) + with pytest.raises(h2.exceptions.InvalidBodyLengthError) as exp: + c.receive_data(trailers.serialize()) + + assert exp.value.expected_length == 15 + assert exp.value.actual_length == 13 + assert str(exp.value) == ( + "InvalidBodyLengthError: Expected 15 bytes, received 13" + ) + + expected_frame = frame_factory.build_goaway_frame( + last_stream_id=1, + error_code=h2.errors.ErrorCodes.PROTOCOL_ERROR, + ) + assert c.data_to_send() == expected_frame.serialize() + + def test_no_data_ended_by_trailers(self, frame_factory) -> None: + """ + Remote peers sending no data at all for a non-zero content-length and + then ending the stream with trailers causes Protocol Errors. + """ + c = self._server(frame_factory, self.example_request_headers) + c.clear_outbound_data_buffer() + + trailers = frame_factory.build_headers_frame( + headers=[("x-checksum", "0")], + flags=["END_STREAM"], + ) + with pytest.raises(h2.exceptions.InvalidBodyLengthError) as exp: + c.receive_data(trailers.serialize()) + + assert exp.value.expected_length == 15 + assert exp.value.actual_length == 0 + + def test_trailers_cannot_redeclare_content_length(self, frame_factory) -> None: + """ + A content-length header field in a trailers section must not redefine + the expected body length of the message. + """ + c = self._server(frame_factory, self.example_request_headers) + c.receive_data(frame_factory.build_data_frame(data=b"\x01"*13).serialize()) + c.clear_outbound_data_buffer() + + trailers = frame_factory.build_headers_frame( + headers=[("content-length", "13"), ("x-checksum", "0")], + flags=["END_STREAM"], + ) + with pytest.raises(h2.exceptions.InvalidBodyLengthError) as exp: + c.receive_data(trailers.serialize()) + + assert exp.value.expected_length == 15 + assert exp.value.actual_length == 13 + + def test_trailers_with_invalid_content_length_still_rejected(self, frame_factory) -> None: + """ + A syntactically invalid content-length in a trailers section is still a + Protocol Error, even though it must not affect the expected length. + """ + c = self._server(frame_factory, self.example_request_headers) + c.receive_data(frame_factory.build_data_frame(data=b"\x01"*15).serialize()) + c.clear_outbound_data_buffer() + + trailers = frame_factory.build_headers_frame( + headers=[("content-length", "banana")], + flags=["END_STREAM"], + ) + with pytest.raises(h2.exceptions.ProtocolError) as exp: + c.receive_data(trailers.serialize()) + + assert "Invalid content-length header" in str(exp.value) + + def test_matching_body_ended_by_trailers_is_accepted(self, frame_factory) -> None: + """ + A trailers section that ends a stream whose body matches content-length + is still accepted, and emits TrailersReceived. + """ + c = self._server(frame_factory, self.example_request_headers) + c.receive_data(frame_factory.build_data_frame(data=b"\x01"*15).serialize()) + c.clear_outbound_data_buffer() + + trailers = frame_factory.build_headers_frame( + headers=[("x-checksum", "0")], + flags=["END_STREAM"], + ) + events = c.receive_data(trailers.serialize()) + + assert any(isinstance(e, h2.events.TrailersReceived) for e in events) + + def test_trailers_without_content_length_unchanged(self, frame_factory) -> None: + """ + A request with no content-length that ends with trailers is unaffected + by trailers-time validation. + """ + headers = [ + (":authority", "example.com"), + (":path", "/"), + (":scheme", "https"), + (":method", "POST"), + ] + c = self._server(frame_factory, headers) + c.receive_data(frame_factory.build_data_frame(data=b"\x01"*3).serialize()) + c.clear_outbound_data_buffer() + + trailers = frame_factory.build_headers_frame( + headers=[("x-checksum", "0")], + flags=["END_STREAM"], + ) + events = c.receive_data(trailers.serialize()) + + assert any(isinstance(e, h2.events.TrailersReceived) for e in events) From 753a5f362685bcb07b382cac1ad4b0a83bb34722 Mon Sep 17 00:00:00 2001 From: fei <204683769+feiiiiii5@users.noreply.github.com> Date: Sat, 26 Sep 2026 19:51:03 +0800 Subject: [PATCH 2/4] Validate content length before trailers --- src/h2/stream.py | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/src/h2/stream.py b/src/h2/stream.py index 0f342462b..5ad1d20a4 100644 --- a/src/h2/stream.py +++ b/src/h2/stream.py @@ -1094,25 +1094,14 @@ def receive_headers(self, ).stream_ended = cast("StreamEnded", es_events[0]) events += es_events - is_trailers = isinstance(headers_event, TrailersReceived) - - if is_trailers: - # A trailers section carries no message content, so a - # content-length in it is validated for syntax only and must never - # redefine the expected body length: run the same parse, then put - # the previous expectation back. The trailers section is the end of - # the message, so the body must be validated here too, otherwise a - # stream that ends with trailers escapes the length check - # entirely. - expected_content_length = self._expected_content_length - self._initialize_content_length(headers) - self._expected_content_length = expected_content_length - if not end_stream: - msg = "Trailers must have END_STREAM set" - raise ProtocolError(msg) + if isinstance(headers_event, TrailersReceived) and end_stream: self._track_content_length(0, end_stream=True) - else: - self._initialize_content_length(headers) + + self._initialize_content_length(headers) + + if isinstance(headers_event, TrailersReceived) and not end_stream: + msg = "Trailers must have END_STREAM set" + raise ProtocolError(msg) hdr_validation_flags = self._build_hdr_validation_flags(events) headers_event.headers = self._process_received_headers( From d2b43d094d6782c4ec6befcf050a12bcd3267ff1 Mon Sep 17 00:00:00 2001 From: fei <204683769+feiiiiii5@users.noreply.github.com> Date: Sat, 26 Sep 2026 20:55:16 +0800 Subject: [PATCH 3/4] Reject content-length in trailers, police body length there --- src/h2/stream.py | 21 +++++++++++----- tests/test_basic_logic.py | 8 +++--- tests/test_invalid_content_lengths.py | 35 +++++++++------------------ 3 files changed, 30 insertions(+), 34 deletions(-) diff --git a/src/h2/stream.py b/src/h2/stream.py index 5ad1d20a4..83cde45c6 100644 --- a/src/h2/stream.py +++ b/src/h2/stream.py @@ -1094,14 +1094,23 @@ def receive_headers(self, ).stream_ended = cast("StreamEnded", es_events[0]) events += es_events - if isinstance(headers_event, TrailersReceived) and end_stream: - self._track_content_length(0, end_stream=True) + if isinstance(headers_event, TrailersReceived): + if not end_stream: + msg = "Trailers must have END_STREAM set" + raise ProtocolError(msg) - self._initialize_content_length(headers) + if any(n == b"content-length" for n, _ in headers): + # Fields that describe message framing have to be evaluated + # before the content is received, so they are never allowed + # in a trailer section. RFC 9110 § 6.5.1. + msg = "Received content-length header in trailer" + raise ProtocolError(msg) - if isinstance(headers_event, TrailersReceived) and not end_stream: - msg = "Trailers must have END_STREAM set" - raise ProtocolError(msg) + # The trailers are not part of the content, but the stream ends + # here, so this is the only place the body length can be policed. + self._track_content_length(0, end_stream=True) + else: + self._initialize_content_length(headers) hdr_validation_flags = self._build_hdr_validation_flags(events) headers_event.headers = self._process_received_headers( diff --git a/tests/test_basic_logic.py b/tests/test_basic_logic.py index d1bc0f1eb..d340c03f3 100644 --- a/tests/test_basic_logic.py +++ b/tests/test_basic_logic.py @@ -716,7 +716,7 @@ def test_can_receive_trailers(self, frame_factory) -> None: c.receive_data(f.serialize()) # Send in trailers. - trailers = [("content-length", "0")] + trailers = [("x-checksum", "0")] f = frame_factory.build_headers_frame( trailers, flags=["END_STREAM"], @@ -742,7 +742,7 @@ def test_reject_trailers_not_ending_stream(self, frame_factory) -> None: # Send in trailers. c.clear_outbound_data_buffer() - trailers = [("content-length", "0")] + trailers = [("x-checksum", "0")] f = frame_factory.build_headers_frame( trailers, flags=[], @@ -1646,7 +1646,7 @@ def test_can_receive_trailers(self, frame_factory) -> None: c.receive_data(f.serialize()) # Send in trailers. - trailers = [("content-length", "0")] + trailers = [("x-checksum", "0")] f = frame_factory.build_headers_frame( trailers, flags=["END_STREAM"], @@ -1671,7 +1671,7 @@ def test_reject_trailers_not_ending_stream(self, frame_factory) -> None: # Send in trailers. c.clear_outbound_data_buffer() - trailers = [("content-length", "0")] + trailers = [("x-checksum", "0")] f = frame_factory.build_headers_frame( trailers, flags=[], diff --git a/tests/test_invalid_content_lengths.py b/tests/test_invalid_content_lengths.py index d93ebbb07..29dccf07a 100644 --- a/tests/test_invalid_content_lengths.py +++ b/tests/test_invalid_content_lengths.py @@ -264,6 +264,9 @@ class TestContentLengthEnforcedAtTrailers: payload lengths that form the content. The listed exemptions are 204, 304 and HEAD, none of which is a trailers section, so a stream that ends with trailers must still have its body length policed. + + A trailers section may not carry a content-length header field at all, so + it can never redefine the expected length either. """ example_request_headers = [ @@ -329,42 +332,26 @@ def test_no_data_ended_by_trailers(self, frame_factory) -> None: assert exp.value.expected_length == 15 assert exp.value.actual_length == 0 - def test_trailers_cannot_redeclare_content_length(self, frame_factory) -> None: - """ - A content-length header field in a trailers section must not redefine - the expected body length of the message. - """ - c = self._server(frame_factory, self.example_request_headers) - c.receive_data(frame_factory.build_data_frame(data=b"\x01"*13).serialize()) - c.clear_outbound_data_buffer() - - trailers = frame_factory.build_headers_frame( - headers=[("content-length", "13"), ("x-checksum", "0")], - flags=["END_STREAM"], - ) - with pytest.raises(h2.exceptions.InvalidBodyLengthError) as exp: - c.receive_data(trailers.serialize()) - - assert exp.value.expected_length == 15 - assert exp.value.actual_length == 13 - - def test_trailers_with_invalid_content_length_still_rejected(self, frame_factory) -> None: + @pytest.mark.parametrize("content_length", ["13", "15", "0", "banana"]) + def test_content_length_rejected_in_trailers(self, frame_factory, content_length) -> None: """ - A syntactically invalid content-length in a trailers section is still a - Protocol Error, even though it must not affect the expected length. + A trailers section must not carry a content-length header field at + all, whatever the value: RFC 9110 § 6.5.1 only allows trailer fields + whose definition permits them there, and content-length has to be + evaluated before the content is received. """ c = self._server(frame_factory, self.example_request_headers) c.receive_data(frame_factory.build_data_frame(data=b"\x01"*15).serialize()) c.clear_outbound_data_buffer() trailers = frame_factory.build_headers_frame( - headers=[("content-length", "banana")], + headers=[("content-length", content_length), ("x-checksum", "0")], flags=["END_STREAM"], ) with pytest.raises(h2.exceptions.ProtocolError) as exp: c.receive_data(trailers.serialize()) - assert "Invalid content-length header" in str(exp.value) + assert "content-length header in trailer" in str(exp.value) def test_matching_body_ended_by_trailers_is_accepted(self, frame_factory) -> None: """ From 90409750e0963f3496b49f909040fc72d60246c4 Mon Sep 17 00:00:00 2001 From: fei <204683769+feiiiiii5@users.noreply.github.com> Date: Mon, 28 Sep 2026 13:19:39 +0800 Subject: [PATCH 4/4] Drop the redundant content-length-in-trailers check Base already refuses a content-length in a trailers section: it runs the field through the regular content-length parser, which rejects it (and rejects a non-numeric value as 'Invalid content-length header'). The explicit check added a second, differently-worded rejection for the same input, so it was redundant. What base does not do is police the body length when a stream ends with trailers, which is what Kriechi identified. Keep just that call. --- src/h2/stream.py | 19 ++++++------------- tests/test_invalid_content_lengths.py | 21 --------------------- 2 files changed, 6 insertions(+), 34 deletions(-) diff --git a/src/h2/stream.py b/src/h2/stream.py index 83cde45c6..e3384eecb 100644 --- a/src/h2/stream.py +++ b/src/h2/stream.py @@ -1094,20 +1094,13 @@ def receive_headers(self, ).stream_ended = cast("StreamEnded", es_events[0]) events += es_events - if isinstance(headers_event, TrailersReceived): - if not end_stream: - msg = "Trailers must have END_STREAM set" - raise ProtocolError(msg) - - if any(n == b"content-length" for n, _ in headers): - # Fields that describe message framing have to be evaluated - # before the content is received, so they are never allowed - # in a trailer section. RFC 9110 § 6.5.1. - msg = "Received content-length header in trailer" - raise ProtocolError(msg) + if isinstance(headers_event, TrailersReceived) and not end_stream: + msg = "Trailers must have END_STREAM set" + raise ProtocolError(msg) - # The trailers are not part of the content, but the stream ends - # here, so this is the only place the body length can be policed. + if isinstance(headers_event, TrailersReceived): + # Trailers are not part of the content, but the stream ends here, + # so this is the only point at which the body length can be policed. self._track_content_length(0, end_stream=True) else: self._initialize_content_length(headers) diff --git a/tests/test_invalid_content_lengths.py b/tests/test_invalid_content_lengths.py index 29dccf07a..f2a1605e6 100644 --- a/tests/test_invalid_content_lengths.py +++ b/tests/test_invalid_content_lengths.py @@ -332,27 +332,6 @@ def test_no_data_ended_by_trailers(self, frame_factory) -> None: assert exp.value.expected_length == 15 assert exp.value.actual_length == 0 - @pytest.mark.parametrize("content_length", ["13", "15", "0", "banana"]) - def test_content_length_rejected_in_trailers(self, frame_factory, content_length) -> None: - """ - A trailers section must not carry a content-length header field at - all, whatever the value: RFC 9110 § 6.5.1 only allows trailer fields - whose definition permits them there, and content-length has to be - evaluated before the content is received. - """ - c = self._server(frame_factory, self.example_request_headers) - c.receive_data(frame_factory.build_data_frame(data=b"\x01"*15).serialize()) - c.clear_outbound_data_buffer() - - trailers = frame_factory.build_headers_frame( - headers=[("content-length", content_length), ("x-checksum", "0")], - flags=["END_STREAM"], - ) - with pytest.raises(h2.exceptions.ProtocolError) as exp: - c.receive_data(trailers.serialize()) - - assert "content-length header in trailer" in str(exp.value) - def test_matching_body_ended_by_trailers_is_accepted(self, frame_factory) -> None: """ A trailers section that ends a stream whose body matches content-length