Skip to content

http2: DATA on an idle stream is a PROTOCOL_ERROR instead of being credited and dropped - #378

Open
MDA2AV wants to merge 1 commit into
mainfrom
fix/354-h2-idle-stream-data
Open

MDA2AV wants to merge 1 commit into
mainfrom
fix/354-h2-idle-stream-data

Conversation

@MDA2AV

@MDA2AV MDA2AV commented Oct 17, 2026 •

Copy link
Copy Markdown
Owner

Fixes #354.

What was wrong. HandleData refused DATA on stream 0 and nothing else. DATA on a stream the client had never opened (idle, RFC 9113 5.1) found no PendingRequest. It was credited back to the connection and the stream, and dropped, and the connection carried on serving.

The cause the issue gives is out of date. _highestStream has existed since #332: the highest stream id the client has opened, refused ones included. So nothing new is needed to tell an idle stream from a closed one.

What changed. One condition in HandleData. A stream id above _highestStream is a connection error PROTOCOL_ERROR, the same answer stream 0 gets. Ids at or below it belong to closed streams, not idle ones, and keep today's handling: credited and dropped. No state is added.

Proof. The pending Chaos test h2c: DATA on an idle stream is a PROTOCOL_ERROR is promoted to a Test, and gains two more cases:

  • DATA on stream 5 after a HEADERS opened stream 3, so the highest is not zero: GOAWAY PROTOCOL_ERROR.
  • A control with two cases, after each of which the connection answers the next request:
    • DATA on stream 1 after stream 3 was opened, which implicitly closes 1 (RFC 9113 5.1.1).
    • DATA on stream 3 after it was answered.

Run both ways:

  • Without the fix (test kept, source reverted): expected [GOAWAY PROTOCOL_ERROR], got [HEADERS 1].
  • The control on its own also passes on the reverted source, so the handling of closed streams is unchanged.
  • With the fix: passes. The nghttp2 twin, nghttp2: DATA on stream 0, or on an idle stream, is a PROTOCOL_ERROR, stays green.

Suites (main 71aab91 in parentheses):

Overlap. #379 (issue #353) also edits Http2Connection.Frames.cs, in BeginStreamedBody and TryComplete. The two branches merge cleanly (git merge-tree). On the merged tree, Unit is 102 passed and Chaos 80 passed, both with 0 failed and 0 pending.

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. A server that never pushes never opens an even stream id, so every even id is idle. HandleHeaders checks only <= _highestStream, though, so a HEADERS on an even id opens a request. RFC 9113 5.1.1 makes that a PROTOCOL_ERROR. As a result, DATA on an even id at or below the highest is still treated as DATA on a closed stream. The fix belongs on the HEADERS path.

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.

http2: DATA on an idle stream is accepted instead of a PROTOCOL_ERROR

1 participant