[robobun]

Problem

• A node:http2 server accepts DATA that a client sends on a server-pushed stream. The pushed ServerHttp2Stream emits 'error' ERR_STREAM_PUSH_AFTER_EOF, then never sends END_STREAM or emits 'close'. node v26.3.0 refuses the frame.
• Cause: Connection::handle_data and begin_streamed_data (src/runtime/api/bun/h2/connection.rs:1502, :1362) enter a locally opened stream as State::Open. That is correct for a client's request stream. On a server such a stream is always a pushed stream.

Fix

• local_stream_state() gives the state to enter: Open on a client, HalfClosedRemote on a server. The existing path then sends RST_STREAM(STREAM_CLOSED) and resets the pushed stream (ERR_HTTP2_STREAM_ERROR, rstCode 5).
• Error level: node (nghttp2) ends the session with GOAWAY(STREAM_CLOSED). The engine answers DATA in each other non-receiving state with a stream error, which RFC 9113 §5.1 asks for in half-closed (remote). This PR keeps that level.
• Verified: test/js/node/http2/h2-conformance.test.ts (4 new cases, all fail on bun 1.4.3-canary 367d939). Also test/js/node/http2/ and the 256 test-http2-* node tests.
• Self-reviewed: 7 concerns raised, 3 addressed. The Notes explain the other 4.

Background

• Connection is the inbound HTTP/2 engine. H2FrameParser embeds it, encodes all outbound frames itself, and keeps its own stream map.
• So the engine never sees a PUSH_PROMISE go out. It learns of a locally opened stream only when a frame arrives on it.
• RFC 9113 §5.1: for the server, a pushed stream is reserved (local), then half-closed (remote) after the pushed response HEADERS. The client can send only RST_STREAM, PRIORITY and WINDOW_UPDATE on it.

Notes

Repro. A raw TCP client sends the preface, SETTINGS and HEADERS(1, END_STREAM). The 'stream' handler calls stream.pushStream({ ":path": "/pushed" }, (err, pushed) => { pushed.respond({ ":status": 200 }); pushed.write("p"); setTimeout(() => pushed.end("done"), 750); }), then stream.respond(); stream.end("main"). When the client sees the pushed response HEADERS on stream 2, it sends DATA(2) with 28 bytes.

	wire after the client's DATA(2)	server events
node v26.3.0	GOAWAY(code=5, "DATA: stream in half-closed(remote)"), socket closes	pushed 'error' ERR_HTTP2_ERROR, pushed 'close' rstCode 2, 'sessionError' ERR_HTTP2_ERROR
bun 1.4.3-canary (367d939)	nothing. No RST_STREAM, no GOAWAY, and DATA(2, END_STREAM) never leaves	pushed 'error' ERR_STREAM_PUSH_AFTER_EOF, no 'close'
this branch	RST_STREAM(2, code=5)	pushed 'error' ERR_HTTP2_STREAM_ERROR, pushed 'close' rstCode 5

The same three results occur when the client sends the DATA after PUSH_PROMISE and before the pushed response HEADERS (reserved (local)). node prints the same reason for both states.

Why ERR_STREAM_PUSH_AFTER_EOF. pushStream() ends the readable side of the pushed stream (endInboundHalf(pushedStream) in src/js/node/http2.ts). The engine then delivers the client's DATA through on_data, and push() after EOF errors the stream. The stream is not destroyed, so the later pushed.end() sends nothing.

The request stream case today. The same client sends DATA(1) after HEADERS(1, END_STREAM), while the response is still open. node: GOAWAY(code=5, "DATA: stream in half-closed(remote)"). bun 1.4.3-canary and this branch: RST_STREAM(1, code=5), stream 'error' ERR_HTTP2_STREAM_ERROR, rstCode 5. The pushed stream now takes this path.

nghttp2. session_on_data_received_fail_fast in lib/nghttp2_session.c ends the session with STREAM_CLOSED when the stream is shut for reading ("DATA: stream in half-closed(remote)"). The engine already differs from nghttp2 for DATA in this way. The existing test "DATA on a promised stream before its response HEADERS is refused, not delivered" asserts RST_STREAM(STREAM_CLOSED) and a live connection. node v26.3.0 as the client ends the session there: GOAWAY(code=1, "DATA: stream in reserved"). To end the session for DATA in half-closed (remote) is a change for request streams too, so it is not part of this PR.

handle_rst_stream. It has the same shim. It enters the stream as Closed, which is correct for both roles, so it does not change.

No stream on a server needs the old Open. A stream that can receive DATA on a server is a request stream, and handle_headers enters it before any DATA. The engine removes an entry only together with the embedder's entry (close_stream in the pending_engine_stream_closes drain), or after a reset that also frees the embedder's entry. A request stream whose engine entry is gone is closed, so half-closed (remote) refuses DATA there too, where Open delivered it.

Connection window. handle_data and begin_streamed_data count the frame toward the connection window before they look at the stream, and replenish_windows gives the credit back. That does not change. The stream window is no longer charged.

Related. #38082 gives a server's pushed stream the state HALF_CLOSED_REMOTE in the embedder's own stream map, for the outbound side. This PR is the inbound engine's half. #42467 changes what the engine does for DATA on a closed stream. It keeps the shim, so it does not change this case.

Tests. Each new case starts an http2.createServer() that pushes, and drives it with the raw client RawH2. The matrix is the state of the pushed stream (half-closed (remote), reserved (local)) times the delivery of the DATA frame (one read for handle_data, header plus 10 payload bytes first for begin_streamed_data). A PING travels with the first read and a second PING follows the rest of the frame. After the second PING ACK each case asserts that the only RST_STREAM or GOAWAY on the wire is RST_STREAM(2, STREAM_CLOSED), then the error code, rstCode, and that no data reached the stream. On the released build each case fails on that assertion, not on a timeout.

Self-review.

1. Addressed. The first version of the test wrote the rest of the frame from the continuation of the pushed stream's 'error' event, which runs inside the server's read callback. On Windows, uSockets does one more recv() after each read callback (packages/bun-usockets/src/loop.c). The server could then read those bytes before the deferred rstStream call from the stream's _destroy ran. That call finds no native entry and writes a second RST_STREAM, and the exact assertion fails. The test now writes only after a PING ACK arrives from the wire. The second RST_STREAM is not new with this PR: each reset that the engine starts can produce it, for request streams too. node:http2 sends RST_STREAM twice for a stream that the inbound engine reset #43510 tracks it.
2. Addressed. The test awaited the pushed stream's 'error' with no failure path. It now records the code and asserts it after the PING ACK.
3. Addressed. A pushStream() failure rejects the awaited promise at once.
4. Not changed. A push whose PUSH_PROMISE fails validation (an invalid header name) is released through kReleaseUnannouncedStream. That leaves the native stream entry and its JS context in place. A client frame on that id that resets the stream then reaches streamError for the destroyed stream, and the session's stream count goes down a second time. RST_STREAM and a WINDOW_UPDATE with a zero increment do this today. DATA now does it too, where before it was delivered to the destroyed stream. The cause is the release path, not the shim. node:http2 server: a client frame on the id of a push that failed validation makes session.close() cut open streams #43509 tracks it.
5. Not changed. RFC 9113 §5.1 asks for a connection error for DATA in reserved (local). The engine cannot tell reserved (local) from half-closed (remote) for a pushed stream, and nghttp2 gives both the same answer.
6. Rejected. A Rust unit test for the server shim: CI does not run the unit tests of this crate.
7. Rejected. A longer waitFor guard than the 2 s default: the raw-client tests in this file use the default.

Suites run on the debug build. test/js/node/http2/h2-conformance.test.ts, h2-push-refusal-staged, h2-late-rst-staged, node-http2-continuation, node-http2-invalid-padding, node-http2-settings-ack-ordering, node-http2-client-close, and all 256 test/js/node/test/parallel/test-http2-*.js pass. Two kinds of failure occur on this machine and do not involve a pushed stream. The "stream release after a queued END_STREAM" cases in the conformance file fail in some runs, also when run alone (#42357 covers them). In node-http2.test.js, tests that spawn a debug-build server take 3 to 4 s against the 5 s default and some time out under load. They pass when run alone.

[coderabbitai]

Warning

Review paused — included plan limit reached

Keep your review moving with free on-demand reviews.

• Run this review for free

On-demand reviews are free for one more day.

• Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Promotion and pricing details

On-demand reviews are free for one more day. After that, they cost $0.25 per reviewed file.

Review limit details

Or wait 53 seconds for your next included review.

Check out review usage here.

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: oven-sh/bun/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 41422da8-aa63-4507-aa09-d4341e4c6bf1

📥 Commits

Reviewing files that changed from the base of the PR and between 26e7a4b and 3a671da.

📒 Files selected for processing (3)

• src/runtime/api/bun/h2/connection.rs
• src/runtime/api/bun/h2/stream.rs
• test/js/node/http2/h2-conformance.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

Status: the fix is pushed. CI is running.

How I reproduced it: a raw TCP HTTP/2 client connects to http2.createServer(). The 'stream' handler calls stream.pushStream(), responds on the pushed stream, and keeps it open. When the client sees the pushed response HEADERS on stream 2, it sends DATA(2).

• bun 1.4.3-canary (367d939) delivers the DATA. The pushed stream emits ERR_STREAM_PUSH_AFTER_EOF, sends no END_STREAM, and never emits 'close'.
• node v26.3.0 answers GOAWAY(STREAM_CLOSED).
• This branch answers RST_STREAM(2, STREAM_CLOSED), and the pushed stream closes with ERR_HTTP2_STREAM_ERROR.

The 4 new cases in test/js/node/http2/h2-conformance.test.ts fail on the released build and pass on a debug build of this branch.

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

• 🟡 src/runtime/api/bun/h2/connection.rs — A server whose pushStream() failed header validation can have its session stream count driven below zero by a client DATA frame, so session.close() never completes. On the base the DATA was delivered and dropped; after this change the shim at connection.rs:1502-1508 enters the never-announced even id as HalfClosedRemote and connection.rs:1536 calls on_stream_reset, which reaches streamError in http2.ts:4034-4039 and decrements #connections a second time after kReleaseUnannouncedStream at http2.ts:4416-4420 already did. Fix: kReleaseUnannouncedStream must also free the legacy stream entry (or on_stream_reset must skip streams already released) so no second streamError can fire for that id.

  Extended reasoning...

  Server code calls pushStream with a header the encoder rejects. http2.ts:3278 calls kReleaseUnannouncedStream, which decrements #connections and clears only sctx via set_stream_context at h2_frame_parser.rs:6305-6309. The legacy entry in the embedder map stays, with its js identifier. A client sends DATA on that even id. is_local_stream at h2_frame_parser.rs:3945-3949 returns true because the legacy entry exists. The shim inserts HalfClosedRemote. can_receive_data fails. connection.rs:1536 calls on_stream_reset. rewrite_stream_ctx at h2_frame_parser.rs:3511-3513 falls back to the legacy entry's identifier, the destroyed JS stream. streamError at http2.ts:4036 decrements #connections again. Net effect: #connections is one lower than the number of live streams. session.close() waits for #connections === 0 (http2.ts:4085, 4178, 4265); with the count at -1 after all streams end, it never fires, so graceful close hangs until the socket dies. The dismissing finders noted RST_STREAM and WINDOW_UPDATE(0) already reach this on the base. DATA is a new route added by this diff, and a…

  Verification: pre-existing / nit — acknowledged in diff: PR self-review note 4 ("RST_STREAM and a WINDOW_UPDATE with a zero increment do this today. DATA now does it too ... The cause is the release path, not the shim") is accurate; the root cause and an equivalent attacker route already exist on the base. Mechanism verified. Trigger: server app calls pushStream() with a header pushPromise rejects… | nit…

• 🟡 src/runtime/api/bun/h2/connection.rs — Clients that send DATA on a pushed stream can now receive two RST_STREAM(2, STREAM_CLOSED) frames from a Bun server, where the base sent none. The engine writes the first at connection.rs:1381/:1531 and calls on_stream_reset (:1385/:1536); the JS pushed stream's _destroy schedules rstNextTick via setImmediate (http2.ts:2639), and if the next read batch drains the legacy entry first (h2_frame_parser.rs:3618) the rst_stream host fn finds no entry and writes a second frame (h2_frame_parser.rs:5062-5073). Fix: mark the legacy stream so the deferred rstNextTick is a no-op after an engine-initiated reset, for request and pushed streams alike. [also at: src/runtime/api/bun/h2/connection.rs:1531 - Windows servers that push can now send two RST_STREAM frames for the same pushed stream when a client sends DATA on it; on the base no reset was sent at all.]

  Extended reasoning...

  Server pushes stream 2; client sends DATA(2). The shim enters HalfClosedRemote (:1366/:1506), the Rst arm sends RST_STREAM(2,5) and calls sink.on_stream_reset. on_stream_reset (h2_frame_parser.rs:4168-4215) sets the legacy entry CLOSED, dispatches onStreamError, then free_resources queues the id in pending_engine_stream_closes. JS destroys the pushed stream; _destroy (http2.ts:2639) does setImmediate(rstNextTick). Normally the immediate runs before the next socket read and rst_stream host fn finds the CLOSED entry so end_stream returns at h2_frame_parser.rs:2150. But if another read batch for the same socket runs first (Windows probe recv after the callback, packages/bun-usockets/src/loop.c Windows branch; Linux repeat-recv when a read fills the buffer, loop.c:764; or a JS-fed Duplex calling parser.read() inside the same tick), the drain at h2_frame_parser.rs:3614-3631 removes the legacy entry, and rst_stream then takes the no-entry branch at :5052-5075 and hand-writes a second RST_STREAM(2, 5). The base delivered the DATA and…

  Verification: nit — acknowledged in diff: the PR description's self-review item 1 names exactly this race ("On Windows, uSockets does one more recv() after each read callback... That call finds no native entry and writes a second RST_STREAM") and its bound ("not new with this PR: each reset that the engine starts can produce it, for request streams too") is accurate; this PR routes a new input class (DATA…

[robobun]

Both findings are correct. Both root causes exist on main, and this PR does not change them. I filed a tracker for each.

Stream count after a push that failed validation: #43509. I confirmed it on bun 1.4.3-canary (367d939) with the two triggers that exist today. After a pushStream() that fails with ERR_INVALID_HTTP_TOKEN, a client RST_STREAM or a WINDOW_UPDATE with a zero increment on stream 2 makes session.close() destroy the session while stream 1 is still open. This PR adds DATA as a third trigger, where main delivers that DATA to the destroyed stream and drops it. The cause is the release path: setStreamContext(id, undefined) keeps the native entry and its JS context. The probe and a possible fix are in the issue.

Second RST_STREAM: #43510. It is deterministic on main for a pushed stream on a client: a raw server that sends DATA on a reserved stream gets RST_STREAM(2, code=5) twice. For other streams it needs a second read before the deferred rstStream call, as you describe. This PR sends DATA on a pushed stream down the same reset path that a request stream already takes. The new test writes to the server only after a PING ACK, so it does not depend on that order.

[robobun]

^{Updated 12:25 PM PT - Sep 19th, 2026}

✅ @robobun, your commit 3a671da6d616552d19130c02a79175086e405744 passed in Build #118401! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 43494

That installs a local version of the PR into your bun-43494 executable, so you can run:

bun-43494 --bun