Skip to content

http3: a request whose body does not match its content-length is refused with H3_MESSAGE_ERROR instead of being served - #383

Open
MDA2AV wants to merge 1 commit into
mainfrom
fix/350-h3-content-length
Open

MDA2AV wants to merge 1 commit into
mainfrom
fix/350-h3-content-length

Conversation

@MDA2AV

@MDA2AV MDA2AV commented Oct 17, 2026 •

Copy link
Copy Markdown
Owner

Fixes #350.

What was wrong. RFC 9114 4.1.2 makes a request malformed when its content-length differs from the sum of its DATA payloads. A malformed request is a stream error of type H3_MESSAGE_ERROR. ioxide.http3 never read content-length: it was decoded into req.Headers and never compared with anything.

  • A buffered handler was handed a ten-byte Body for a request that announced a hundred.
  • A streaming handler's read ended cleanly, exactly as it would for a complete body.

A proxy that forwards the header upstream would send a body shorter than the one it announces.

What changed.

  • Parsing. Http3Request.AddField parses content-length as the field is decoded, into one internal field, ContentLengthLeft. It is -1 when there is no content-length and -2 when the value is invalid or repeated.
    • long.TryParse with NumberStyles.None accepts digits only: no sign, no whitespace, no overflow.
    • A repeated field is refused even when the values agree, as nghttp3 does.
  • Checks. Http3Connection checks the value in three places:
    • after the field section, before a streaming handler is dispatched: an invalid or repeated value is refused;
    • at each DATA frame header: a frame longer than what is still owed is refused before any of its payload is taken, and otherwise the amount owed goes down by the frame's length;
    • at the FIN: a body that is still short is refused.
  • Refusal (RefuseMalformed). It is a stream error, not a connection error:
    • The stream is reset with H3_MESSAGE_ERROR (0x10e). The connection keeps serving.
    • The stream moves to a new ParseState.Refused, which drops anything still in flight for it. Dropped bytes are still credited when the stream is paced.
    • A buffered request is never dispatched.
    • On the streaming overload the handler is already running. Its reader is failed instead of ended: chunks already queued still read, and the read that would have returned the end throws an IOException that names the mismatch. This is Http3BodyReader.Fail plus one Exception? field. The reader's docs now say so. Every other end (a fin, a reset, a teardown) is still an empty chunk, so the reader's documented contract is unchanged outside this one case.
  • HeadersPayload now sets State = FrameHeader before calling OnHeadersComplete instead of after, so a refusal there is not overwritten.
  • Cost on the hot path. One compare per request header field, which fails on length for nearly every name, and one compare per DATA frame header.

Proof. All in H3BodyTruncationTests. Its summary and the mid-frame test's rationale were updated where they had become false.

  • http3: a body shorter than its content-length is refused, not served whole: the Pending, promoted (buffered, over H3TestClient).
  • http3: a streamed body shorter than its content-length raises on read instead of ending: new.
  • http3: a body that runs past its content-length is refused with H3_MESSAGE_ERROR: new.
  • http3: an invalid or repeated content-length is refused with H3_MESSAGE_ERROR: new. Cases: ten, -10, +10, 10 and 10 + 10, each with a ten-byte body.
  • Controls:
    • control: a body that matches its content-length is served whole (existing).
    • control: a streamed body that matches its content-length reads as a clean end (new).
    • control: a body that ends where its DATA frame said reads as a clean end (existing).
  • The file's RawH3Client can now write content-length fields and reports the code its request stream was reset with (WaitForReset). The wire tests assert 0x10e itself, because H3TestClient does not surface resets.

Run both ways:

  • Without the fix (source reverted, tests kept), the four tests fail and the three controls pass:
    • ... the server answered 200 'got 10' and handed the handler a 10-byte body as if that were the whole request
    • expected [raised IOException after 10], got [ended after 10]
    • expected [reset 0x10e], got [served]
    • content-length 'ten': expected reset 0x10e, got served
  • With the fix: all pass.

One crash came up while writing this, and it was the test's fault, not the fix's. The first version of the invalid/repeated test segfaulted the E2E process inside libioxide_ngtcp2.so: two reactor threads faulted on the same instruction. Its helper disposed each QuicEngine while that engine's server still had live connections, and QuicEngine.Dispose says the engine must outlive them. The helper now stops the servers before disposing the engine, the way QuicIdentityCapTests does. There was no crash in the two full runs since.

nghttp3 refuses all four malformed cases too, but as a connection error.

  • read_stream returns ERR_MALFORMED_HTTP_MESSAGING for a short or long body, and ERR_MALFORMED_HTTP_HEADER for an invalid or repeated content-length.
  • FailProtocol then closes the whole connection with H3_MESSAGE_ERROR.
  • Probed against this branch over H3TestClient: an exact body is served, and the other four came back with status -1 and the connection closed by the server.

RFC 9114 section 8 lets an endpoint treat a stream error as a connection error, so nghttp3's behaviour is allowed. The two stacks now differ, though:

  • Pure C# resets the one stream and keeps the connection.
  • A streaming handler's read throws on pure C#. On nghttp3 it ends cleanly, through the teardown.

Suites.

  • E2E: 292 passed, 5 pending on main → 297 passed, 0 failed, 0 skipped, 4 pending.
  • Http: 88 passed, same as main.
  • Chaos: 79 passed, 1 pending, same as main.
  • ioxide.http3 builds for net10.0 and net11.0 with no warnings.

Bench. bench/any.sh Http3/ManagedBuffered (h2load built with HTTP/3, 2 reactors pinned to CPUs 2 and 6, the driver to 0-1 and 12-15). Five rounds alternate main 71aab91, the whole batch merged (for h3, this PR, #382, #380 and the rebuilt QUIC .so), and main again as the A/A control:

  • main: 1.372M req/s mean, 1.420 µs CPU/req
  • main again: 1.378M (+0.4%), 1.416 µs (-0.3%)
  • batch: 1.380M (+0.5%), 1.414 µs (-0.4%)

Flat.

Overlap. Merges cleanly with #382 (issue #351). Both touch Http3Connection.cs, in different hunks. Checked with git merge-tree against every other PR of this batch (#378, #379, #381, #376, #377, #380): all merge cleanly. #350 + #351 + #352 merged together build, and E2E passes there: 305 passed, 0 failed, 2 pending (the #195 and #349 tests).

All eight together. This batch's PRs (#376 to #383) were merged onto main 71aab91 in one scratch tree. The only conflict is the binary libioxide_ngtcp2.so between #376 and #377, rebuilt there from the merged shim with the script's defaults. Everything builds, and Unit 102, E2E 309, Http 88, Chaos 80, Tls 169 (14 kTLS cases skipped) and File 5 pass, with 0 failed and 0 pending. The repo's only pending test left is the Redis suite's (#336).

Noticed, not changed:

  • Most QUIC tests declare using var engine in the test body, so the engine is disposed before the runner's TestServer.StopAll(). That leaves a short window in which a reactor can run native code on a freed engine. QuicIdentityCapTests closes the window with try/finally; the rest of the suite does not.
  • The decoder does not refuse an uppercase field name (RFC 9114 4.2 requires lowercase), so Content-Length is never matched as content-length. nghttp3 refuses such a request.

…sed with H3_MESSAGE_ERROR instead of being served

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

http3: a request body shorter than its content-length is served instead of refused

1 participant