Validate content-length when a stream ends with trailers

[feiiiiii5]

Fixes #1328

Description

A stream that ends with a trailers section never had its content-length policed, and a content-length in the trailers was accepted as if it were the one from the header section.

H2Stream.receive_headers handles every received HEADERS block, trailers included, and called _initialize_content_length(headers) unconditionally. _track_content_length, where the comparison actually happens, was only ever called from receive_data, so end_stream was never True for a body that ends with trailers. A peer declaring content-length: 15 could send 13 bytes and then a trailers section, and the short body was accepted.

Per @Kriechi's review, content-length is now rejected in a trailers section outright: 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. Trailers that end a stream run the same length check, so they can neither reset nor redefine the expectation.

Test plan

master at bc239af1d1b85bc70482804f30a0e0e587d90a08 (4.4.1), Python 3.14, macOS. Unit level with the existing frame_factory fixture, in-memory bytes, no network.

TestContentLengthEnforcedAtTrailers in tests/test_invalid_content_lengths.py: insufficient data ended by trailers, no data at all ended by trailers, content-length in trailers rejected for several values, a matching body ended by trailers still accepted with TrailersReceived, and a request with no content-length unaffected. Four receive-side tests in tests/test_basic_logic.py used content-length: 0 as their trailer field and now use x-checksum.

Command output

Before the fix, with only the tests added:

$ pytest tests/test_invalid_content_lengths.py::TestContentLengthEnforcedAtTrailers -q
FAILED ...::test_insufficient_data_ended_by_trailers[request_headers0]
FAILED ...::test_no_data_ended_by_trailers
FAILED ...::test_content_length_rejected_in_trailers[13]
FAILED ...::test_content_length_rejected_in_trailers[15]
FAILED ...::test_content_length_rejected_in_trailers[0]
FAILED ...::test_content_length_rejected_in_trailers[banana]
6 failed, 2 passed in 0.40s

The two that passed before the fix as well are the ones pinning that valid trailers and a stream with no content-length are still accepted.

After:

$ pytest -q
1670 passed in 4.26s

$ coverage report | grep stream.py
src/h2/stream.py           466      0     98      0   100%
TOTAL                     1919      0    470      0   100%

$ ruff check src/
All checks passed!

$ mypy --strict src/h2/stream.py
Success: no issues found in 1 source file

[Kriechi]

I am not sure if I understand the problem stated here. To my understanding, a content-length header is not allowed in Trailers anyway, see https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Trailer#directives

If a HEADERS frame with END_STREAM set is received after DATA, the content length is already known or final, so there is no need to parse or validate it again.

So the only missing call is possibly _track_content_length once we receive Trailers - this would reduce the PR to a single line of code change at the right place?

[feiiiiii5]

Thanks for the review. The length check now runs before _initialize_content_length(headers) when a trailer block sets END_STREAM, so it checks against the original header value. Trailer parsing still follows, preserving the existing rejection of malformed values. I removed the save/restore path in 753a5f3. The regression cases fail 3/6 on the PR base and pass 6/6 here; pytest -q passes (1668), as do ruff check src/ and the configured strict-byte mypy command.

[Kriechi]

Trailers are not allowed to have content-length headers at all - no matter their value.

[Kriechi]

Trailers are not allowed to have content-length headers at all - no matter their value.

[feiiiiii5]

@Kriechi good catch — trailers must not carry content-length at all, so rejecting it there is the right shape. Done in d2b43d0: a trailers section containing content-length is now a ProtocolError regardless of value (RFC 9110 § 6.5.1), instead of being parsed and then ignored, and the trailers no longer reach _initialize_content_length at all, so they can neither reset nor redefine the expectation.

That made the receive-side trailer fixtures in tests/test_basic_logic.py invalid, so the four of them now use x-checksum: 0 instead of content-length: 0. The send-side ones are untouched.

Still the same fix underneath: a body that ends with trailers is policed where the stream actually ends. pytest -q is 1670 passed with coverage at 100%, ruff check src/ and mypy --strict src/h2/stream.py are clean. I also reworded the PR description to match.

[feiiiiii5]

Both of these are addressed in d2b43d0, the current head — the comments are on 753a5f3.

test_content_length_rejected_in_trailers now parametrizes over ["13", "15", "0", "banana"] and asserts ProtocolError for each, so a trailers section cannot carry a content-length at any value, not just one that disagrees with the body. That is the rule you stated: the field is not allowed there in the first place, so there is nothing to compare it against.

The rest of the change is separate from that rule and is what the earlier commits do: a trailers section that ends a stream without a content-length still runs the body-length check, so trailers can neither satisfy nor reset the expectation set by the header section. test_matching_body_ended_by_trailers_is_accepted covers the case where they agree, and still asserts TrailersReceived is emitted.

If the RFC 9110 § 6.5.1 reading is not what you want for the mismatch cases — i.e. if you would rather a content-length in trailers be ignored outright than be a protocol error — say so and I will change the assertion.