PR #41329 - oven-sh/bun

Fixes #43893

Stacked on #40180, which lands first. The base is that branch.

Problem

A node:http2 server that lowers initialWindowSize mid-upload stalls compliant clients. The engine keeps each stream's receive window at the old size (src/runtime/api/bun/h2/connection.rs:679), so it sends no stream WINDOW_UPDATE.
The send side (src/runtime/api/bun/h2_frame_parser.rs:3820) ignores a decrease, so a bun client overruns a smaller window. An increase replaces the WINDOW_UPDATE credit.
rewrite_read reads bytes that arrived during a dispatch without an engine sync. It misses a queued setLocalWindowSize() growth, and the session ends with FLOW_CONTROL_ERROR (ERR_HTTP2_ERROR).

Fix

On the SETTINGS ACK, move each stream's receive window by the change. New streams start with the ACKed value, as in nghttp2.
When the peer changes the value, move each stream's send window by the change.
Sync the engine before each read of queued bytes (sync_engine()).
Verified: seven new tests in test/js/node/http2/h2-conformance.test.ts. Six fail on main and on #40180. Also test/js/node/http2/. Self-reviewed: 5 concerns raised, 5 addressed.

Background

RFC 9113 section 6.9.2: a change to SETTINGS_INITIAL_WINDOW_SIZE moves every open stream's window by the difference.
The engine sends a stream WINDOW_UPDATE after the peer uses half of the stream window.
Considered the delta at settings() time, but the peer applies it later. Both sides see the ACK.

Downsides

A server that lowers the window now enforces it. A bun 1.4.3 client overruns it and gets ERR_HTTP2_ERROR. Today that upload finishes.
No window change: one compare per SETTINGS frame and per ACK. Release size is unchanged (text 80,656,721 bytes).

Refs #40180, #39873.

Notes

Reproduction: the script from the report, a 4 MiB upload with a shrink to 1024 after the first DATA. Results before and after this change:

server client main this PR
bun node stalls at 66559 bytes finishes
node bun GOAWAY FLOW_CONTROL_ERROR finishes
bun bun finishes (the client overruns, the server does not check) finishes
node node finishes finishes
A frame trace of bun against bun with this change: after SETTINGS(INITIAL_WINDOW_SIZE=1024) and WINDOW_UPDATE(+65496), the client has 985 bytes of window and sends 985. After that, the server grants 1024 bytes each round, and the client sends 1024.
#43893: the script from the issue with a 65536-byte body ends with ERR_HTTP2_ERROR ("Protocol error") on canary 1.4.3-canary.1+367d939d9 and on #40180 alone. With this PR the server receives 65536 bytes, and 1,000,000 bytes too. setLocalWindowSize() inside a frame callback queues its growth in pending_recv_window_growth, because the engine is borrowed. The old drain loop at the end of rewrite_read fed the queued bytes to the engine without that growth. sync_engine() is the old sync block of rewrite_read, and it now runs before each drain too.
#39873 rewrites rewrite_read and removes the drain loop that this PR changes. The same rule must hold there: every change that the binding makes is in the engine before each Connection::receive() call. The two tests with bytes that arrive during a dispatch protect that rule.
Why this needs #40180: the ACK delta uses the settings snapshot that each SETTINGS frame carries. On main, setLocalWindowSize(n) raises local_settings.initial_window_size with no SETTINGS frame. The next snapshot would carry that value, and the ACK would move open streams by a change that the peer never applied. #40180 lands first. The base of this PR is the #40180 branch (head 0aa0a9343c), so GitHub shows only this change. After #40180 merges, this branch gets a rebase onto main.
The seven new tests:
- the server lowers the window mid-upload (receive side),
- the ACK of that change arrives while the server is inside a dispatch (the engine sync),
- setLocalWindowSize() in a 'stream' handler, with a peer that answers the WINDOW_UPDATE inside the same handler (#43893). The test also checks its premise: the DATA arrived while the handler ran.
- a stream opens between a SETTINGS frame and its ACK (new streams start with the ACKed value). This one passes on main too. It fails if a stream starts with the unACKed value, because the delta then counts twice.
- the first or a later server SETTINGS lowers the window (send side, two cases),
- a raise after WINDOW_UPDATE credit: main sends 100000 bytes where section 6.9.2 allows 110000.
The downside, measured with the report's script. This PR as the server and canary 1.4.3-canary.1+367d939d9 as the client: the client gets ERR_HTTP2_SESSION_ERROR, and the server reports ERR_HTTP2_ERROR. Canary with canary: the upload finishes, because the client overruns the window and the server does not check. This PR with this PR, and with node v26.3.0: the upload finishes.
Binary size, from release builds of the base (#40180 at 0aa0a9343c) and of this PR. size gives text 80,656,721, data 110,300 and bss 1,822,960 for both, and both files are 80,823,840 bytes long. The hashes differ, and only the PR binary passes the #43893 script. nm -S on bun-profile: Connection::receive goes from 11056 to 11120 bytes, and rewrite_read (3018) becomes rewrite_read (1851) plus sync_engine (1206). That is +103 bytes, inside alignment padding. bloaty is not installed here, so the numbers come from size and nm.
Work per call when no window changes: the SETTINGS ACK branch computes one delta and skips the loop. on_remote_settings computes one delta and skips the loop. Before, it compared every open stream on every SETTINGS frame. rewrite_read calls sync_engine() once per read, which is the same block as before, and once more per drain only after a re-entrant read. No path allocates.
History: #33602 made the same send-side change. It was closed as superseded by #31584, but the current code applies only an increase. #30344 was the Zig version. #30342 reported hangs with a raised window. #40360 (for #40358) changed the ACK order in the same function.
bun-v1.3.14 moved stream windows on the SETTINGS ACK, but always from 65535 (handleSettingsFrame). The current engine has no such step.
The self-review asked for the stack on #40180 (the first version had its own copy of the setLocalWindowSize() lines), the engine sync, the two send-side tests, and the reference list. It also said that #40180 should land first.
Two gaps that reviews found are on main too, and this PR does not change them: stream.state.localWindowSize reports a fixed value (node reports the remaining credit), and setLocalWindowSize() has no recv_reduction (#41344 has that fix).
Also run with the debug build: node-http2.test.js (398 pass), node-http2-settings-ack-ordering.test.ts, and test-http2-window-size.js, test-http2-client-setLocalWindowSize.js, test-http2-server-setLocalWindowSize.js, test-http2-session-settings.js, test-http2-session-stream-state.js, test-http2-misbehaving-flow-control.js, test-http2-generic-streams.js, test-http2-pipe.js, test-http2-backpressure.js and 15 more test-http2-* files.
grpc-js does not change window sizes, so this change does not affect it. Four tests in test/js/third_party/grpc-js/test-client.test.ts fail on a debug build of main too.

server	client	main	this PR
bun	node	stalls at 66559 bytes	finishes
node	bun	GOAWAY FLOW_CONTROL_ERROR	finishes
bun	bun	finishes (the client overruns, the server does not check)	finishes
node	node	finishes	finishes

[human-review] gate passed · iteration 1 · 3 files touched

fails on main (without fix)

ASAN without fix: 7 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" "test/js/node/http2/h2-conformance.test.ts"
bun test v1.4.3 (367d939d9)

test/js/node/http2/h2-conformance.test.ts:
(pass) connection preface & SETTINGS handshake (checklist §1) > server sends a SETTINGS frame first (§1.4) [426.52ms]
(pass) connection preface & SETTINGS handshake (checklist §1) > server ACKs the client's SETTINGS frame (§3.5) [123.82ms]
(pass) connection preface & SETTINGS handshake (checklist §1) > a SETTINGS frame with a non-zero stream id is a PROTOCOL_ERROR (§3.5) [76.10ms]
(pass) connection preface & SETTINGS handshake (checklist §1) > a SETTINGS frame whose length is not a multiple of 6 is a FRAME_SIZE_ERROR (§3.5) [41.50ms]
(pass) connection preface & SETTINGS handshake (checklist §1) > a SETTINGS ACK that carries a payload is a FRAME_SIZE_ERROR (§3.5) [33.02ms]
(pass) PING (checklist §3.7) > server replies to PING with a PING ACK echoing the payload [36.94ms]
(pass) PING (checklist §3.7) > a PING with length != 8 is a FRAME_SIZE_ERROR [34.74ms]
(pass) PING (checklist §3.7) > a PING on a non-zero str
... (truncated)

release without fix: 6 FAILED
bun test v1.4.3-canary.1 (b82b79639)

test/js/node/http2/h2-conformance.test.ts:
(pass) connection preface & SETTINGS handshake (checklist §1) > server sends a SETTINGS frame first (§1.4) [9.18ms]
(pass) connection preface & SETTINGS handshake (checklist §1) > server ACKs the client's SETTINGS frame (§3.5) [2.77ms]
(pass) connection preface & SETTINGS handshake (checklist §1) > a SETTINGS frame with a non-zero stream id is a PROTOCOL_ERROR (§3.5) [2.20ms]
(pass) connection preface & SETTINGS handshake (checklist §1) > a SETTINGS frame whose length is not a multiple of 6 is a FRAME_SIZE_ERROR (§3.5) [1.18ms]
(pass) connection preface & SETTINGS handshake (checklist §1) > a SETTINGS ACK that carries a payload is a FRAME_SIZE_ERROR (§3.5) [0.94ms]
(pass) PING (checklist §3.7) > server replies to PING with a PING ACK echoing the payload [0.90ms]
(pass) PING (checklist §3.7) > a PING with length != 8 is a FRAME_SIZE_ERROR [0.97ms]
(pass) PING (checklist §3.7) > a PING on a non-zero stream id is a PROTOCOL_ERROR [0.78ms]
(pass) WINDOW_UPDATE (checklist §6) > a connection-level WINDOW_UPDATE with a 0 increment is a PROTOCOL_ERROR [0.88ms]
(pass) WINDOW_UPDATE
... (truncated)

passes on PR (with fix)

ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" "test/js/node/http2/h2-conformance.test.ts"
bun test v1.4.3 (367d939d9)

test/js/node/http2/h2-conformance.test.ts:
(pass) connection preface & SETTINGS handshake (checklist §1) > server sends a SETTINGS frame first (§1.4) [362.21ms]
(pass) connection preface & SETTINGS handshake (checklist §1) > server ACKs the client's SETTINGS frame (§3.5) [104.68ms]
(pass) connection preface & SETTINGS handshake (checklist §1) > a SETTINGS frame with a non-zero stream id is a PROTOCOL_ERROR (§3.5) [79.10ms]
(pass) connection preface & SETTINGS handshake (checklist §1) > a SETTINGS frame whose length is not a multiple of 6 is a FRAME_SIZE_ERROR (§3.5) [39.17ms]
(pass) connection preface & SETTINGS handshake (checklist §1) > a SETTINGS ACK that carries a payload is a FRAME_SIZE_ERROR (§3.5) [34.72ms]
(pass) PING (checklist §3.7) > server replies to PING with a PING ACK echoing the payload [30.74ms]
(pass) PING (checklist §3.7) > a PING with length != 8 is a FRAME_SIZE_ERROR [35.61ms]
(pass) PING (checklist §3.7) > a PING on a non-zero str
... (truncated)

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 981ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/8] gen generated_host_exports.rs
generated_host_exports.rs: 121 exports (host=5, lazy=10, generic=106, rust=0); 245 extern-C blocks audited
[2/7] rustc bun_runtime 
[3/7] link bun-profile
ld.lld: warning: Linking two modules of different target triples: 'obj/codegen/GeneratedSSLConfig.cpp.o' is 'x86_64-pc-linux-gnu' whereas 'rust-target/x86_64-unknown-linux-gnu/deps/libbun_jsc-9cc23114361cb5d7.rlib(bun_jsc-9cc23114361cb5d7.bun_jsc.dbc349fa102b7826-cgu.0.rcgu.o at 207024)' is 'x86_64-unknown-linux-gnu'


ld.lld: warning: Linking two modules of different target triples: 'obj/codegen/GeneratedSocketConfig.cpp.o' is 'x86_64-pc-linux-gnu' whereas 'rust-target/x86_64-unknown-linux-gnu/deps/libbun_jsc-9cc23114361cb5d7.rlib(bun_jsc-9cc23114361cb5d7.bun_jsc.dbc349fa102b7826-cgu.0.rcgu.o at 207024)' is 'x86_64-unknown-linux-gnu'


ld.lld: warning: Linking two modules of different target triples: 'obj/codegen/GeneratedSocketConfigHandlers.cpp.o' is 'x86_64-pc-linux-gnu' whereas 'rust-target/x86_64-unknown-linux-gn
... (truncated)

diff hotspot

src/runtime/api/bun/h2/connection.rs      |  25 +-
 src/runtime/api/bun/h2_frame_parser.rs    |  61 ++--
 test/js/node/http2/h2-conformance.test.ts | 451 ++++++++++++++++++++++++++----
 3 files changed, 449 insertions(+), 88 deletions(-)

gate history · 4 passed · 0 rejected · iteration 1

evidence per changed file

file                                       reads  edits  tests
src/runtime/api/bun/h2/connection.rs           7      7     66
src/runtime/api/bun/h2_frame_parser.rs        18     14     64
test/js/node/http2/h2-conformance.test.ts     12     16     64

#41329 node:http2: apply INITIAL_WINDOW_SIZE changes to open streams

Problem

Fix

Background

Downsides

node:http2: apply INITIAL_WINDOW_SIZE changes to open streams