[robobun]

Problem

• A node:http2 server ends the session with GOAWAY(ENHANCE_YOUR_CALM) and ERR_HTTP2_SESSION_ERROR at the 100th stream over the SETTINGS_MAX_CONCURRENT_STREAMS it advertised. Node keeps the session until refusal 1002.
• Cause: JS refuses the stream after the engine opened it (src/js/node/http2.ts:4053) and charges maxSessionRejectedStreams. node:http2: rewritten inbound engine, batched write path, server push, +290 node v26.3.0 tests (79% passing) #31584 chose that budget on a wrong premise about nghttp2 (Notes).
• Reach: a client that sends 100 streams over the limit before it reads SETTINGS, like Bun's http2.connect(). No user report.

Fix

• Connection::handle_headers refuses the stream before it makes a stream entry: RST_STREAM(REFUSED_STREAM), charged to maxSessionInvalidFrames, like node.
• H2FrameParser counts the open peer streams in a Cell<u32>. The JS count is deleted.
• Verified: test/js/node/http2/node-http2-max-concurrent-streams.test.ts (22 tests fail before). Its last test runs the file on Node.js. Also all of test/js/node/http2/ and node's test-http2-* tests.
• Self-reviewed: 48 concerns raised, 35 addressed (Notes).

Background

• The engine (connection.rs) parses inbound frames. H2FrameParser embeds it and writes responses.
• A refused stream has no engine entry. refused_ids keeps later frames on its id without a second answer (Notes).
• Considered and removed: holding the slot of an ended stream until the read ends. It cannot follow node's writes (Notes).
• Notes has two questions for a maintainer: the budget move, and a limit that the client ACKed.

Downsides

• These refusals use maxSessionInvalidFrames (default 1000), not maxSessionRejectedStreams (default 100). A server that set either option sees another count.
• Cost: +24 bytes per session, at most 8 KB of refused ids, +768 bytes of .text, +59 instructions per request.

Notes

What is in this PR. It targets main and has one commit for each part:

1. node:http2: count a refused stream where the session refuses it, not in rstStream #44248, as it is (three commits). When node:http2: count a refused stream where the session refuses it, not in rstStream #44248 lands, I rebase and they go.
2. The pins: 33 tests that say when a stream gives its slot back, and two helpers (Connection::count_invalid_frame, H2FrameParser::close_unsent). No behaviour change. This was node:http2: pin the slot accounting of maxConcurrentStreams #44335.
3. The limit: the part that the Fix bullets describe.

Earlier attempt: #32675 (closed). This PR keeps its decision point (refuse in handle_headers before on_stream_open) and the O(1) count that was asked for there.

Reach. Two kinds of client, measured with the public API against a server with limit 10.

Client, requests, handler	node v26.3.0 server	Bun 1.4.3 server	this PR
Bun http2.connect(), 150 at once, handler answers after 100 ms	10 served, 140 refused	0 served, session ends	10 served, 140 refused
node client, 300 at once, handler answers after 100 ms	210 served, 90 refused	210 served, 90 refused	210 served, 90 refused
node client, 300 at once, handler answers at once	210 served, 90 refused	300 served	300 served

A client on nghttp2 holds its first flight to 100 streams, so it gets at most 100 refusals at once and sees no change. A client with no such cap (Bun's own, and grpc-js on Bun) loses every call of the burst on 1.4.x, the admitted ones too. On 1.4.x the server option maxSessionRejectedStreams: 1002 gives node's count.

History. Bun 1.3.14 did not enforce the limit: every request reached a handler. #31584 added the enforcement in JS and, after two review findings (r3337691981, r3345228319), the charge to maxSessionRejectedStreams. The second finding says "nghttp2 counts concurrency-limit REFUSED_STREAM toward the rejected-streams cap". Node's rejected_stream_count_ counts the streams that node itself turns away for memory (node_http2.cc:1035). A stream over the limit never reaches that code: nghttp2 refuses it and reports an invalid frame (nghttp2_session.c:3905), and node charges maxSessionInvalidFrames with count++ > max (node_http2.cc:1128). The flood bound that #31584 wanted stays, at 1000 in place of 100.

Repro. Server http2.createServer({ settings: { maxConcurrentStreams: 10 } }), handler stream.respond({ ":status": 200 }); stream.write("hello"). A raw client sends the preface, an empty SETTINGS frame and N complete requests in one write, then a PING.

N	node v26.3.0	Bun 1.4.3	this PR
150	140 RST_STREAM(7), session open	99 RST_STREAM(7), GOAWAY(last 219, code 11)	140 RST_STREAM(7), session open
1011	1001 RST_STREAM(7), session open	same as above	1001 RST_STREAM(7), session open
1012	1002 RST_STREAM(7), ERR_HTTP2_TOO_MANY_INVALID_FRAMES	same as above	1002 RST_STREAM(7), ERR_HTTP2_TOO_MANY_INVALID_FRAMES, GOAWAY(last 19, code 2)

Two questions for a maintainer.

1. The budget. This PR moves the refusal from maxSessionRejectedStreams (100) to maxSessionInvalidFrames (1000), which is what node does and the opposite of what node:http2: rewritten inbound engine, batched write path, server push, +290 node v26.3.0 tests (79% passing) #31584 chose. cirospaciari merged node:http2: rewritten inbound engine, batched write path, server push, +290 node v26.3.0 tests (79% passing) #31584, so the question is for him: is that move right?
2. A stream over a limit that the client ACKed. nghttp2 makes it a connection error (nghttp2_session.c:3889): node ends the session with ERR_HTTP2_ERROR and GOAWAY(INTERNAL_ERROR). Bun 1.4.x refuses the stream and counts it (99 are kept). This PR refuses the stream and counts it on the other budget (1001 are kept). RFC 9113 5.1.2 asks for a stream error. The choices are node's connection error, the old budget for this one case, or what this PR does. No test pins it.

Where it still differs from node.

• Slot timing. Bun gives a slot back when the stream closes, which is when the handler has ended the response. Node gives it back when nghttp2 has written the frame that closes the stream. Node writes at several points, some of them inside a read, and the points depend on the transport. So Bun can serve a request that node refuses: three GET requests in one write at limit 1, with a handler that answers at once, are all served by Bun 1.4.3 and by this PR, and node serves one. This PR does not change it. One test pins it and is marked Bun only.
• An earlier version of this PR held the slot until the read ended, to follow node. A review measured that the rule cannot follow node's writes: with respond(); end(); destroy() node serves 300 of 300 requests of a node client and the rule served 210. It also took 90 requests from a node client that Bun 1.4.3 serves. It is removed.
• HEADERS over the limit after goaway() or close() still get RST_STREAM(7), now on the invalid-frame budget. Node ignores them. node:http2: ignore a request that opens after the server's GOAWAY #43475 adds that.
• A GOAWAY that the engine writes for a connection error names the highest stream id that it has read, a refused one too. Node names the last stream that it processed. node:http2: name only peer streams in GOAWAY and never raise the id (RFC 9113 6.8) #37588 owns that id. The GOAWAY of close(), goaway() and destroy() is right.
• Node has two more gates on this path that Bun does not have: CanAddStream (a refusal for memory, RST_STREAM(ENHANCE_YOUR_CALM)) is tested after the limit as here, and nghttp2's glitch rate limit (node:http2: add nghttp2's glitch rate limit for ignored frames #42522).
• The engine takes the limit test when the whole HEADERS frame has arrived. nghttp2 takes it at the 9-byte frame header.
• Node resets a stream whose response ended and whose request body nobody read, with RST_STREAM(NO_ERROR), one turn later. Bun does that only when body bytes are buffered or arrive (on purpose, since node:http/https/http2: raise Node v26.3.0 compat to ~94%, sync the upstream suites, and fix the Windows/macOS transport-layer teardown bugs they exposed #32488). So an answered upload that sends nothing more keeps its slot in Bun.
• A peer RST_STREAM for a refused stream is not counted by the reset limit (streamResetBurst). Node counts it. The refusal is counted.
• WINDOW_UPDATE with increment 0 on a refused stream gets RST_STREAM(PROTOCOL_ERROR), as on any closed stream. Node ignores it. node:http2: drop frames on a closed stream, as nghttp2 does #45801 owns that rule.

What else changes.

• The refusal is the same on every entry point: core API, compat API, TLS, performServerHandshake() over a Duplex. session.settings() and server.updateSettings() set the limit that native code reads, from the call on.

• A refused stream is not the last processed stream. session.state.lastProcStreamID and the GOAWAY of close(), goaway() and destroy() name the last admitted stream. 1.4.3 names the refused stream, so a client cannot know that it can send the request again.

• A refused request with a malformed block gets one RST_STREAM. 1.4.3 sends two.

• A frame on a refused stream gets no second answer and no second charge, in the same read and in a later one. That is DATA (an empty frame and a frame that a read cuts in two also) and a trailer block. Frames on stream 3, which the server refused at limit 1:

  Client sends after the refused HEADERS frame	node v26.3.0	Bun 1.4.3	this PR
  2 DATA frames, same write	RST_STREAM(7)	RST_STREAM(7)	RST_STREAM(7)
  2 DATA frames, later write	RST_STREAM(7)	RST_STREAM(7), 2 RST_STREAM(5)	RST_STREAM(7)
  a trailer block, same write	RST_STREAM(7)	RST_STREAM(7)	RST_STREAM(7)
  a trailer block, later write	RST_STREAM(7)	2 RST_STREAM(7), RST_STREAM(1)	RST_STREAM(7)

• A slot is taken only for a stream that the engine admitted. A Stream that a host function makes for an id with no entry takes none.

• The JS count had two defects. A respond() that failed and a later reset or end of that stream gave two slots back, so a limit of 1 admitted 2 streams. An 'aborted' listener that throws kept the slot for the rest of the session. Both have a test.

• A limit that was never sent to the peer is not enforced: { maxConcurrentStreams: 5, settings: { maxConcurrentStreams: undefined } }.

• When the allowance is used up the engine stops at that frame. 1.4.3 parsed and allocated for every remaining HEADERS frame of the chunk.

Not changed. Three other rejections still end the session at the 100th: a malformed block, an oversized header list and a refusal for maxSessionMemory. 150 malformed requests: node answers 150 and keeps the session, Bun ends it at the 100th. Node's rule sets its count back to 0 for each admitted stream, which takes away a bound that Bun has today, so that change needs its own decision. A first flight with large responses therefore still ends on the memory budget.

The count. open_peer_streams goes up where handle_received_stream_id makes the Stream of a request that the engine admitted. Stream::release_peer_slot takes the bit and goes down, so a second call does nothing. It is called from free_resources and from close_unsent, and from nowhere else. It never reads Stream.state: a local half-close can write over CLOSED after a reset. assert_peer_slots checks in builds with debug assertions, once per read, that the count is the number of streams with the bit and that no CLOSED stream has it.

The refused ids. A refused stream has no engine entry, so a later frame on its id finds nothing. On 1.4.3 the engine still has the entry of a refused stream for the rest of the read, so the frames that follow in the same write get no answer. Node gives them no answer in any read. One of the pins holds that, and without a record of the refused ids this PR fails it (DATA gets RST_STREAM(STREAM_CLOSED), a trailer block is refused a second time). refused_ids is that record: a sorted Vec<u32> with one entry for each refused stream. handle_headers and the two DATA paths read it, and the first test is one compare with the last id. It keeps the last 2048 ids at most (8 KB). The default budgets end a session before it has refused that many streams, so only a server that raised them can drop an id. A frame on a dropped id gets the reply that main gives today.

A review asked for the commit of #37985 in place of the list. That commit reads every client id at or below the highest id, with no entry, as a closed stream. I did not take it. Jarred's note on #31359 (r3295037991) says why that rule needs care: Bun's own client can write stream ids out of order (#37568 is the client half). #45801 leaves that rule out for the same reason. The list is exact for the ids that this server refused and says nothing about any other id. When #37985 and #45801 land, the list can go.

Both runtimes. The tests are written for node:test and node:assert. In Bun the last test of the file runs the same file on Node.js, as node-http-agent-free-socket.test.ts does, so CI checks both. Node.js v26.3.0: 59 tests, 52 pass, 0 fail, 7 are skipped as Bun only. Six of those need a respond() that fails, and the calls that make it fail in Bun (weight: 0, a parent out of range, options that are not an object, a header block over maxSendHeaderBlockLength) do not fail on node. The seventh pins the slot timing above. This PR: 60 pass. Bun 1.4.3: 22 of the 26 tests of the new block fail. The test of #44248 that pinned the old budget is rewritten in place, as its comment asks.

The self-review. Three reviews. The first asked for tests that pass on node, and for node's slot timing or a maintainer's word. The second asked for node:test, and found approximations in the slot timing. The third measured that the slot timing cannot follow node and costs requests that Bun serves today, so it is removed. It also asked for this packaging: one PR on main, and the two questions above. Of its 48 findings, 35 are taken. Six are left as they are and are in the lists above (the other rejection budgets, the client cap, the answered upload, the frame-header timing, #connections). One needs no action. Not taken:

• The commit of h2 server: decode and discard HEADERS on a closed client stream instead of resetting or re-opening it #37985 in place of refused_ids (3 findings). The reason is under "The refused ids".
• A seeded corpus of request flights in place of the fixed tests. The tests that remain do not depend on timing inside a read.
• A CI job for the Rust tests of connection.rs. CI type-checks them and runs none of them, as for the other tests of that file. I ran them with the engine sources in a standalone crate: 28 pass.
• To delete the rewritten test of node-http2-rejected-streams.test.ts in place of the rewrite.

Suites on a debug build of this head, with a 60 s test timeout: the 261 test-http2-* files of test/js/node/test all exit 0. All 13 files of test/js/node/http2/ pass (node-http2.test.js 405 pass and 6 skip, h2-conformance.test.ts 84, node-http2-client-close.test.ts 69, node-http2-max-concurrent-streams.test.ts 60, node-http2-rejected-streams.test.ts 16). Of the 29 grpc-js files, 27 pass. test-client (3 tests) and test-resolver (5 tests) fail in this container: they need DNS, or they have a 100 ms deadline that a debug build misses. On release builds of main and of this head the same number of their tests fail (2, and 5 or 6).

Measurements. Release builds of main (d0831bc) and of this PR on that commit, same toolchain and build directory:

	main	this PR
.text (size -A)	81,316,949	81,317,717 (+768)
.rodata	22,590,412	22,590,412
.bun_builtins (the JS modules)	2,697,031	2,696,083 (-948)
file	106,858,208	106,858,208
size_of H2FrameParser (it holds the Connection)	1496	1520
size_of Connection	448	472
size_of legacy Stream	104	104

The three size_of rows are from debug builds (gdb). Host functions +0, codegen entries +0.

Instructions that each function executes for one request that is admitted, callees not counted. The same release builds, gdb single-step, the same count on every repeat.

Server, one GET request, no limit set	main	this PR
Connection::receive (the read with the HEADERS frame)	297	310
Connection::finish_header_block	1487	1517
handle_received_stream_id (2 calls)	189 + 39	199 + 40
Stream::free_resources	161	166
rewrite_read	192	192
sum	2365	2424 (+59)

Client, one http2.connect() request	main	this PR
Connection::receive (the read with the response)	486	475
Connection::finish_header_block	723	739
handle_received_stream_id (3 calls)	159 + 39 + 39	167 + 40 + 40
Stream::free_resources	104	108
rewrite_read	161	161
sum	1711	1730 (+19)

No line is new on the admitted path of finish_header_block. Its +30 and +16 come with the two new match arms: the compiler lays the function out in another way. A DATA frame pays one more test (refused_ids is empty, or the id is above its last entry). The server loses the JS count for each stream: 3 % 2 tests, 4 private-field reads, 2 writes and 1 call of the stream.id getter.

The numbers below are from 9ee2fe9, an earlier head of this PR with the same refusal path, against its base of that time.

Debug builds, gdb breakpoint hit counts, first flight at limit 10:

Per refused stream	base	this PR
Box<Stream>	1	0
engine stream map inserts	1	0
JS calls	3	0
header blocks made into JS objects	1	0

Per admitted stream nothing changes: 1 Box<Stream>, 1 map insert, the same JS calls.

sendto calls of the server for a first flight of 150 at limit 10 (maxSessionRejectedStreams raised so that the base session stays open): 153 on base, 13 with this PR.

The engine alone, 200,000 refused requests in reads of 20,000 (the engine sources in a scratch crate, release profile): 244 ns per refusal, 5.1 hash slots examined per refusal in read 1 and in read 10, 0 inserts. The same engine driven as 1.4.3 drives it (open, then refuse from the embedder): 840 ns per refusal in read 1 and 61,000 ns in read 10.

Server CPU from process.cpuUsage(), N requests in one write at limit 100, the handler holds its stream. Each cell is the median of 5 sessions. Two server processes per build, run in turns:

N	base	this PR
4,000	28 and 26 ms	7 and 6 ms
8,000	62 and 52 ms	7 and 7 ms
16,000	104 and 103 ms	8 and 11 ms
32,000	204 and 210 ms	13 and 11 ms
64,000	290 and 356 ms	17 and 17 ms

That is about 6 us per refused stream on base and about 0.2 us with this PR.

A server with no limit, 20,000 requests in one write, handler respond() then end(), the same method: base 323, 315 and 263 ms. This PR 293, 296 and 350 ms. The difference is inside the noise.

perf_event_open is not permitted in the build container, and valgrind, strace and bloaty are not installed. So there is no whole-process instruction count.

Left for later.

• Bun's node:http2 client does not hold its first flight to 100 streams before the SETTINGS of the server arrive (peerMaxConcurrentStreams). For that client the session still ends, at refusal 1002. ClientHttp2Session.request() reads #remoteSettings?.maxConcurrentStreams, which is undefined until then.
• session.settings({ enableConnectProtocol }) inside a handler applies from the next read. Node applies it from the call.
• #connections, the JS count of the streams of a session, is given back twice after a respond() that fails.
• After a respond() that fails, the server writes no RST_STREAM, and the entries of the stream stay until the peer resets it. main does the same. The slot is given back at once, as on 1.4.3. node:http2: ignore request() options in respond(), like node #43478 and node:http2: do not refuse respond() when the session is over maxSessionMemory #43419 remove two of the ways to make respond() fail.
• A stream whose streamStart makes no JS stream (it threw, or an exception was pending) stays open until the peer resets it, and it holds its slot for that time. The JS count of 1.4.3 did not count such a stream. Node resets it.
• node:http2: ignore request() options in respond(), like node #43478: respond() fails in Bun for weight: 0 and for a parent out of range. Node accepts both.
• node:http2: do not reset a stream the native layer already closed #43539: Bun answers a peer reset of some streams with its own RST_STREAM(CANCEL).
• The client side: a push response over the limit that the client advertised.

[robobun]

^{Updated 3:34 AM PT - Oct 18th, 2026}

❌ @robobun, your commit d8c5baf has 1 failures in Build #128078 (All Failures):

• step failed outside runner - exit 1 on 🍎 any aarch64 - test-bun

---

🧪   To try this PR locally:

bunx bun-pr 44337

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

bun-44337 --bun

[robobun]

Status: ready for review at d8c5baf. The code, the tests and the measurements are complete. It needs a maintainer now.

CI: the diff is green. In build 128078 180 jobs passed and 1 failed. The jobs that passed are every Linux lane (Alpine, Debian, Debian ASAN, Ubuntu), both Windows lanes, macOS x64, and one of the two macOS aarch64 shards. The job that failed did not run a test: the other macOS aarch64 shard stopped on buildkite-agent artifact download timed out after 120s for step 'darwin-aarch64-build-bun'. The same agent did the same on the build before it (127986). I already used one retrigger for it, so I do not push again.

This PR is now one PR on main, with one commit for each part:

1. node:http2: count a refused stream where the session refuses it, not in rstStream #44248, as it is.
2. The pins (this was node:http2: pin the slot accounting of maxConcurrentStreams #44335).
3. The limit: a stream over the advertised limit is refused in the engine before it is opened, on node's budget (maxSessionInvalidFrames), with an O(1) native count.

Removed since the last status: the rule that held a slot until the end of the read. A slot is free when its stream closes, as on Bun 1.4.3. The PR body says why.

Two questions wait for a maintainer. Both are in the PR body:

• The budget move itself (maxSessionRejectedStreams to maxSessionInvalidFrames), which node:http2: rewritten inbound engine, batched write path, server push, +290 node v26.3.0 tests (79% passing) #31584 chose the other way.
• A stream over a limit that the client ACKed. Node ends the session. This PR refuses the stream.

How I reproduced the bug: a server with settings: { maxConcurrentStreams: 10 } whose handler keeps each stream open, and a raw HTTP/2 client that sends the preface, an empty SETTINGS frame and 150 complete requests in one write, then a PING.

• Bun 1.4.3: 10 handlers, 99 RST_STREAM(REFUSED_STREAM), GOAWAY(ENHANCE_YOUR_CALM), ERR_HTTP2_SESSION_ERROR.
• node v26.3.0 and this branch: 10 handlers, 140 RST_STREAM(REFUSED_STREAM), the PING is answered, the session stays open.

The tests: bun bd test test/js/node/http2/node-http2-max-concurrent-streams.test.ts. On Bun 1.4.3, 22 of the 26 tests of the new block fail. With this change all 60 pass. The last test runs the file on Node.js v26.3.0: 52 pass, 7 are skipped as Bun only (report).

[robobun]

The tests of this PR on Node.js v26.3.0, for the rule from #37588 and #41305 (a test must pass on Bun and on node).

The tests are in test/js/node/http2/node-http2-max-concurrent-streams.test.ts. The file is written for node:test and node:assert, so node and Bun run the same file. In Bun, the last test of the file starts Node.js on that file, so CI checks both runtimes.

node test/js/node/http2/node-http2-max-concurrent-streams.test.ts
bun bd test test/js/node/http2/node-http2-max-concurrent-streams.test.ts

Block	Tests	node v26.3.0	Bun 1.4.3	This PR (d8c5baf)
the slots of SETTINGS_MAX_CONCURRENT_STREAMS (the pins)	33	29 pass, 4 skipped	33 pass	33 pass
a stream refused over SETTINGS_MAX_CONCURRENT_STREAMS (the limit)	26	23 pass, 3 skipped	22 fail, 4 pass	26 pass
Node.js compatibility (Bun only, it starts node)	1	not run	pass	pass

The 7 tests that node skips are marked Bun only.

• 6 need a respond() that fails. The ways to make it fail in Bun (weight: 0, a parent out of range, options that are not an object, a header block over maxSendHeaderBlockLength) do not fail on node. They guard the slot count on the failure paths of respond().
• 1 pins the slot timing of Bun: three GET requests in one write at limit 1, with a handler that answers at once, are all served. Node serves the first one. The PR body lists this as a difference that stays.

What changed since the first version of this report:

• An earlier head held a slot until the end of the read, to follow node in that last case. A review measured that the rule cannot follow node (node serves 300 of 300 requests with respond(); end(); destroy(), the rule served 210) and that it refused requests that Bun 1.4.3 serves. The rule and the tests that pinned it are removed.
• 3 tests on a session that reads from a Duplex used the PING ACK as the barrier. Node writes the ACK before the responses there. outcome() now waits for an answer to every request.
• 1 test asserted that a stream over a limit that the client ACKed is refused and the session stays open. Node ends the session. The test is removed and the difference is an open question in the PR body.

node-http2-rejected-streams.test.ts (from #44248, written for bun:test) had one test of the old budget, a stream refused over maxConcurrentStreams is counted. Its comment asks for a rewrite when the refusal moves to the other budget. It now asserts what node does: three refusals with maxSessionRejectedStreams: 3 keep the session.

[robobun]

The 11 comment-cop threads of 02:59 UTC (on src/ast, src/boringssl_sys, src/bun_alloc and src/bun_core) are on lines that this PR does not change.

I pushed the new head, which is on current main, a moment before I set the base of the PR to main. For that moment the diff held every commit of main since the old base, and the bot read that diff. The base is main now and the PR changes 5 files. I resolved the 11 threads.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 1 minute.

Check out review usage here.

View limit details

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: Advanced
• Run ID: e115a43b-c4ac-4513-9bb1-cc40e4ae3e74

📥 Commits

Reviewing files that changed from the base of the PR and between d0831bc and d8c5baf.

📒 Files selected for processing: 5

• src/js/node/http2.ts
• src/runtime/api/bun/h2/connection.rs
• src/runtime/api/bun/h2_frame_parser.rs
• test/js/node/http2/node-http2-max-concurrent-streams.test.ts
• test/js/node/http2/node-http2-rejected-streams.test.ts

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[claude]

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

Beyond the inline findings, I also checked connection-level flow control for DATA frames on a refused stream id: handle_data in src/runtime/api/bun/h2/connection.rs still debits recv_window (line 1651) before the was_refused early return (line 1662), and replenish_windows works from that same window at batch end, so the early return does not drift the connection window relative to the base's STREAM_CLOSED path. The HPACK-table sync for refused and ignored header blocks is covered by the new refused_block_keeps_the_hpack_table_in_sync unit test.

Extended reasoning...

The change moves SETTINGS_MAX_CONCURRENT_STREAMS enforcement for node:http2 servers from JS into the Rust HTTP/2 engine (connection.rs, h2_frame_parser.rs), deletes the JS-side counters, and adds two large test files. It touches adversarial wire-input handling (refused-id tracking, invalid-frame budget, per-stream slot accounting) and six inline findings are already posted; this note only records one additional concern that was examined and ruled out.

[claude]

I re-reviewed the follow-up commit (d8c5baf) and found no new bugs; a human maintainer should still look at this one, since it moves SETTINGS_MAX_CONCURRENT_STREAMS enforcement into the native HTTP/2 engine and the description leaves two budget/semantics questions open for a maintainer.

What was reviewed in this pass:

• The new MAX_REFUSED_IDS cap in remember_refused (connection.rs:946-953): the drain of the oldest half keeps the list sorted, so was_refused's binary search stays valid; the unit test exercises the overflow with the invalid-frame budget set to u32::MAX, so it reaches the cap.
• The decision order in handle_headers (connection.rs:1106-1119): refused ids are checked before the concurrency and memory gates, and only new server-side odd streams take the refusal path, so client sessions and continuation of existing streams are unaffected.
• The subprocess test now spawns through bunExe()/bunEnv and asserts a combined { stdout, exitCode, stderr } object instead of a negative stderr check.

Extended reasoning...

The PR touches src/runtime/api/bun/h2/connection.rs and h2_frame_parser.rs, the native engine that parses inbound HTTP/2 frames from untrusted peers, plus the node:http2 JS layer and two new test files (+2040/-91 across 5 files). It changes which session budget (maxSessionInvalidFrames vs maxSessionRejectedStreams) a concurrency refusal is charged to, which is a DoS-bound behavior change the description itself flags as a question for the merging maintainer of #31584. The follow-up commit addressed the unbounded refused_ids growth and the harness-spawn feedback from the prior review, and the hunt ran dry with no findings, but the size, the wire-parsing surface, and the open maintainer questions rule out approving without a human look.