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 by cirospaciari · Pull Request #32488 · oven-sh/bun

cirospaciari · 2026-06-18T14:06:57Z

What this does

Raises node:http, node:https, and node:http2 compatibility — measured by running Node v26.3.0's own test/parallel + test/sequential http/https/http2 suites unmodified under Bun — from ~77% to ~94%, and syncs the vendored copies of those suites to v26.3.0.

module	before	after
node:http	319/405 (78.8%)	382/405 (94.3%)
node:https	45/65 (69.2%)	61/65 (93.8%)
node:http2	215/278 (77.3%)	259/278 (93.2%)
total	579/748 (77.4%)	702/748 (93.9%)

(Counts are upstream Node v26.3.0 test files run with the CI runner config, exit 0 = pass. Every vendored http/https/http2 file passes; this PR adds no test/expectations.txt entries — it only removes them. The remaining gap is upstream files that are not vendored, listed under Not vendored below.)

Fixes #28656 — http2.createSecureServer({ allowHTTP1: true }) answered an HTTPS/1.1 request with an empty reply (curl exit 52): the handshake completed with no ALPN protocol negotiated and the connection closed without a response. On this branch the same repro gets HTTP/1.1 200 + the body (bun 1.3.14 still reproduces the bug). Covered by the synced upstream test-http2-https-fallback*.js suites and the allowHTTP1 cases in node-http2.test.js.

Server (node:http / node:https)

headersTimeout / requestTimeout enforcement with Node's connectionsCheckingInterval sweep and raw 408 reply; keepAliveTimeout closes idle keep-alive connections (0 = no timeout); server.setTimeout / res.setTimeout / req.setTimeout arm real per-socket timers, on TLS too. The per-connection idle timer is refreshed in place across the keep-alive cycle (no clearTimeout+setTimeout per request).
Connection lifecycle parity: 'connection'/'secureConnection' at accept/handshake time, 'close' emitted whenever the TCP connection closes, closeIdleConnections/closeAllConnections counting, socket-error abort, and connection sockets are now real net.Socket instances with Node's socket.parser surface (incoming, free(), kOnTimeout slot, freed before 'upgrade'/'connect').
Parse errors follow Node's clientError contract (error reaches the client, bytesParsed/rawPacket, specific HPE_* codes incl. HPE_PAUSED_H2_UPGRADE for the full 24-byte HTTP/2 preface, premature-EOF, chunked+Content-Length, bare CR, oversized chunk extensions with 413/431 replies).
HTTP/1.1 pipelining (queued responses with res.socket === null, ordered flush including buffered 1xx, bounded by read backpressure, never advances after a must-close response); maxRequestsPerSocket (503 + 'dropRequest'); half-close handling. Pipelining lives in the node-http template instantiation, so Bun.serve keeps its existing async-pipeline-denied behaviour and fast paths unchanged. Queued responses are bounded by Node's two gates: transport backpressure and state.outgoingData >= writableHighWaterMark (the bytes buffered across every queued response), so a client that pipelines without reading cannot grow the queue without bound.
Upgrade-with-body, CONNECT tunnels (raw data + 'end' after handoff), incoming/outgoing trailers (trailer-section size cap honours per-server maxHeaderSize); the captured trailer section is given the parser's standard post-padding before the 8-byte field-value scanner runs over it, fixing an out-of-bounds read of up to 3 bytes past the section's allocation (ASAN heap-buffer-overflow in tryConsumeFieldValue, caught in review).
Strict request parsing only for the node compat layer (llhttp-style method validation and friends are gated on the node-http flags); leading bare LF/CR before the request-line is tolerated like llhttp's s_start state.
calculateLenientFlags ported from Node so client httpValidation reaches the response parser; per-server insecureHTTPParser/maxHeaderSize (smaller-than-default values are honoured for buffered partial headers too); createServer({ highWaterMark }) plumbed to req/res; https pfx/minVersion/maxVersion/ciphers/ALPN properties (with validateObject(options) and the falsy-ALPN default like Node); req.socket.servername/authorized/authorizationError; client/agent fixes (Node-exact messages, domain binding via domain.run forwarding args/return, parser reentrancy, abort/half-open lifecycle); NODE_TLS_REJECT_UNAUTHORIZED is now === "0" only.
A Connection: close response whose body was still in the native send buffer when res.end() returned was silently truncated: socket.end()'s native path shut the transport down once the socket's own stream buffer was empty, ignoring the response bytes uWS still held under backpressure, so the FIN overtook them (~1.4 MB of an 8 MiB body reached the client on macOS, where the small loopback send buffer leaves most of the body in userspace; Node v26.3.0 delivers every byte). When the in-flight response still has buffered data the close is now handed to uWS — HTTP_CONNECTION_CLOSE makes its drain path shut the socket down right after the last byte flushes, the same sequencing Node gets from destroySoon(). Three regression tests in node-http-backpressure.test.ts (client-requested close, server-set Connection: close, one-shot res.end(body)), each failing without the fix.

node:http2

nghttp2-style session errors and teardown for protocol violations (bad preface, oversized frames, flow-control violations, HEADERS on closed streams, unsolicited PING ACK, GOAWAY parity), flood/resource limits (maxSessionMemory byte-exact, maxSessionInvalidFrames, PING/SETTINGS-ACK floods).
Flow control: stream windows are only replenished while the JS readable is consuming (a paused reader backpressures the peer), and a response that ends before the request body finished no longer stalls the connection — fixes a real-world server bug covered by a new wire-level conformance test.
Settings parity (empty initial SETTINGS, customSettings/remoteCustomSettings, enableConnectProtocol, strictSingleValueFields; settings parsing is transactional and the wire SETTINGS frame carries only the current call's keys while localSettings.customSettings stays cumulative like Node); the 'localSettings' event reports the values the ACK actually acknowledged.
Writable parity (_write callback completes asynchronously so write() returns false past the highWaterMark and 'drain' fires; over a native socket the deferral is process.nextTick, over a JS-side socket / duplexPair it is setImmediate so JS-side delivery has run first); client.close() waits for outstanding SETTINGS ACKs like Node's kMaybeDestroy hasPendingData() check.
Session idle timer under kTimeout with write-progress suppression (a buffer that drained to zero between fires counts as progress), respondWithFile/respondWithFD fd ownership and delivery, AsyncLocalStorage context on client streams, request queueing before connect, per-request AbortSignal, performServerHandshake, allowHTTP1 fallback headers, Node-exact error wording.
session.ping() matches Node: throws ERR_HTTP2_PING_LENGTH for a non-8-byte payload, validateFunction(callback), returns false (callback gets ERR_HTTP2_PING_CANCEL) at maxOutstandingPings, and each call is an AsyncResource('HTTP2PING').
client.request() validates option types before checking session state (Node order), and stream.close(NaN) rejects like Node's default-parameter form. The server rejects an inbound :protocol pseudo-header when its local enableConnectProtocol is 0 (RFC 8441 §4, like nghttp2 — the request never reaches 'stream').
Server-initiated push streams close cleanly on END_STREAM (the diagnostics http2.server.stream.close channel now publishes with closed=true && destroyed=false && rstCode=0 for the non-error path and destroyed=true for the error path, matching node's onStreamClose/_destroy split).
Client stream events run in the async context captured when request() was called even when that context is empty: the "never captured" default is a sentinel now, so a stream requested outside any AsyncLocalStorage scope observes an empty store in 'response'/'data'/'end' instead of the session's connect-time store (caught in review; new regression test).

node:net — fatal write errors and connect semantics

macOS (kqueue) could truncate a hung-up socket's stream: EV_EOF is reported on the same readable event as a connection's final data, and the event handler honored it even when its read loop had stopped early (short read, repeat budget, or the data callback pausing the socket), ending and closing the socket with bytes still queued in the kernel. A tls.connect() client whose peer did end(big); destroySoon() observed 'end' short of the payload — the darwin-aarch64-only CI failure an earlier draft had quarantined. Now a hung-up event is drained to recv() == 0/EAGAIN first (matching Linux, where the EOF is only ever discovered by recv() == 0); a hard read error after that drain has delivered data is reported as the end-of-stream the FIN already announced (an RST behind a FIN — Node's reader also stops at the FIN) while a pure RST still reports its error; and the EOF is deferred for a socket the data callback paused mid-burst. Deterministic regression test; test-tls-client-destroy-soon.js is un-quarantined and the two recv-short TLS decode tests that failed on macOS now pass.
A send(2) that fails after the peer reset the connection now reaches JS. The errno travels usockets → write_check_error → $write (negative errno; -1 stays the legacy closed/shutdown sentinel) → node:net fails the pending write callback exactly like Node's onWriteComplete (lib/internal/stream_base_commons.js#L81-L92). A fatal flush of natively-buffered data (write already acknowledged to JS) is surfaced from the writable dispatch through the socket's 'error' handler with the errno-derived code. ENOBUFS stays transient and macOS EPROTOTYPE is reported as ECONNRESET, matching libuv's uv__try_write.
This replaces an earlier branch of this PR that synthesized read ECONNRESET from a duplicate EOF dispatch. Node treats a repeated UV_EOF as a no-op, and that synthesized path destroyed healthy sockets mid-read (the test-net-write-slow CI timeout). With the real mechanism in place, test-http2-max-invalid-frames and test-http2-reset-flood pass deterministically (verified run-for-run against the Node v26.3.0 binary), test-net-write-slow is clean under 8× parallel load, and double-connect.test.ts no longer needs test.failing.
New regression test: test/js/node/net/node-net.test.ts "a write after the peer reset the connection fails with a write error".
socket.connect() no longer force-resume()s the stream. Like Node's afterConnect it only does read(0), so data that arrives before a 'data' listener is attached stays buffered instead of being emitted to nobody and lost, and a socket the user never reads follows Node's lifecycle: no 'end'/auto-destroy while data sits unread, while the handle stops holding the event loop once the peer's FIN arrives (so the process can still exit). The pre-existing tls-syscall-fault "FIN before close_notify drained" test asserted the old auto-flow side effect (await 'close' on a never-read client) and was the deterministic debian-x64-asan timeout; it now consumes the client like Node requires. New regression test: "a connected socket is not flowing until the user reads from it".

Fixed from review

Bun.serve was silently made lenient. The new "skip empty lines before the request-line" (llhttp's s_start) went into the shared templated parser with no IsNodeHttp gate, so Bun.serve answered 200 to a request prefixed with \r\n, a bare \n, or a bare \r — main returns 400. Gated to node-http; serve.test.ts now asserts the 400 for all three prefixes.
A missing Host header fired a bogus clientError with HPE_INTERNAL. Node never fires 'clientError' for it — parserOnIncoming answers the 400 itself — so Bun now replies byte-for-byte with Node.
Unbounded pipelined response buffering (see Server section): Node's outgoingData >= writableHighWaterMark gate had no equivalent. With 50k pipelined requests and the head response held open, Node plateaus at 2148 dispatched; Bun climbed past 6385 with RSS following.
H2FrameParser leaked on a throwing handler. process.exit() never unwinds — the VM is destructed from inside the exit() call — so a native frame still on the stack (on_native_read) never returns to drop its +1, and neither the cork slot nor the queued auto-flush task is released. The parser stranded at refcount 4 and deinit() never ran. The self-keepalive refs now use a counted RAII guard, and finalize releases the stranded ones while the VM is shutting down. (Reproduces on main 10/10 under ASAN — pre-existing, not a regression.)
A terminated Worker leaked its loop state: EventLoop::deinit() never reclaimed deferred_tasks, finalizer-closed listen sockets were only queued for close before the loop was freed, and the HPACK scratch buffer's thread_local destructor is not guaranteed to run. (Also pre-existing on main — a worker running a plain net.createServer() reproduces the orphaned poll 5/5 under ASAN.)
h2 sessions re-reported a post-teardown RST. Node's socketOnError starts from const session = this[kSession] and does nothing when the session is gone; Bun had no destroyed-guard, so a peer RST racing our own teardown re-entered destroy(error). It also now ignores ECONNRESET once a GOAWAY has been received, like Node — verified against v26.3.0, where GOAWAY-then-RST yields ['goaway','goaway','close'] and no error.
The Windows h2 read ECONNRESET family had a single root cause. On graceful session close Bun stopped reading the socket, so the peer's late GOAWAY sat unread in the kernel buffer and the close sent RST instead of FIN. Node's finishSessionClose explicitly socket.resume()s on graceful close for exactly this reason (core.js v26.3.0). Reproduced on Linux with the socket fault-injection layer (recv → ECONNRESET after the GOAWAY exchange — the identical uncaught-error signature) and fixed by matching Node: resume() before end() in both session classes.
node:https never saw a premature EOF. Both TLS EOF paths (peer close_notify and the raw FIN behind it) force-closed the socket without dispatching the end event, so the clientError (HPE_INVALID_EOF_STATE) that fires over plain http was silently swallowed over https — along with CONNECT/Upgrade half-open and pipeline-drain-after-FIN. The dispatch is scoped to uWS HTTP server sockets; every other TLS kind synthesizes its JS 'end' from the close event and keeps the historical force-close. Covered by a new test that fails on the unfixed build.
The h2 session idle timer misjudged an active transfer as idle. It compared instantaneous buffer levels between expiries, so a response that filled and fully drained within one period sampled as idle and 'timeout' fired mid-transfer (Windows CI's slower loopback hit the gap reliably). Now judged by a monotonic written counter like Node's callTimeout.
A fatally-failed native h2 write hung the session forever. The parser treated a kernel-rejected send as "wrote 0" and re-buffered, waiting for a drain that can never come; on Windows the peer's RST routinely completes the send before the read path observes anything, so nothing ever closed the socket — the flood tests timed out only there. A fatal errno now tears the transport down from the deferred tick (reproduced and verified on Linux via socket fault injection), and Windows reports real WSA codes instead of a generic fatal sentinel.
Queued pipelined responses under-counted their output, deadlocking the read gate. Node's _storeHeader counts the serialized header block in outputData; Bun counted only body chunks, so power-of-two chunks could land outgoingData exactly on writableHighWaterMark and pause reads one request earlier than Node — hanging a client that pipelines the unblocking request behind the crossing one (test-http-pipeline-socket-parser-typeerror). Headers are now accounted once per queued response.
Process exit stranded FileSink refs (piped stdout with a pending write leaked at every exit — flagged by LSan once the worker-teardown fix stopped masking it). Released from finalize while the VM shuts down, same pattern as the H2FrameParser fix.
More quarantine entries removed, none added: the grpc-js pair (passes 45/0 debug, 4× clean under ASAN), the http/fetch ASAN trio, and the Windows half-close entry are gone from test/expectations.txt; remaining entries are down to platform-environmental causes (FinalizationRegistry timing on musl, darwin CI routing).
Session write-progress was read from a frozen mirror. The idle-timer fix initially compared socket.bytesWritten, but the JS getter only mirrors the native counter on drain events - which the parser's direct native writes never raise. Now reads the native handle's live counter (with the JS getter as the fallback for duplexPair-backed sessions).
Error-path teardown now matches Node's finishSessionClose literally: end() first, hard destroy one setImmediate later - Node's own documented Windows-ECONNRESET avoidance (core.js v26.3.0). The immediate destroySoon() destroyed before the peer drained our final GOAWAY, turning its close abortive.
A refused connect stopped failing grpc-style clients. The fatal-write teardown closed via close_and_detach, severing the JS wrapper before the close could dispatch - http2.connect to a nonexistent server emitted neither 'error' nor 'close' and grpc-js calls waited out their full deadline on all platforms. The teardown now closes without detaching, and leaves not-yet-established sockets to the connect-error path.
macOS's racy EPROTOTYPE no longer kills healthy connections. libuv retries it (RETRY_ON_WRITE_ERROR); Bun renamed it to ECONNRESET, which the new fatal-write handling then acted on - spuriously tearing down darwin h2 sessions mid-transfer. Now classified with the transient errors and retried.
Write retries no longer unpause a paused socket. All nine backpressured-write re-arm sites set the poll to READABLE|WRITABLE absolutely, silently resuming reads the application had paused; they now go through one pause-preserving helper.
failWrite applies the same listener policy as SocketEmitEndNT: a failed flush on an orphaned socket (no callback, no 'error' listener - an h2 teardown racing the peer's reset) closes quietly instead of surfacing an unhandled write ECONNRESET.
Session destroy() is now idempotent, like Node's (if (this.destroyed) return; opens Node's, v26.3.0). A second destroy re-ran the teardown and re-emitted 'error'; on the Windows agents a surfaced socket reset had consumed grpc-js's once('error') absorber, and the received-GOAWAY handler's second destroy then threw an unhandled "Session closed with error code 8" on every Windows lane. Guarded with a one-shot latch (not the destroyed getter, which reads "socket detached" and is set by #onError pre-destroy) that releases when a validation-failure destroy throws — pinned by the conformance suite.
Windows peer-FIN delivery on the libuv path: UV_DISCONNECT — AFD's only signal for a peer FIN with no read outstanding — was never requested, so a client FIN against a half-closed server socket never fired and server.close() waited forever. Now armed unconditionally, delivered one-shot (AFD re-reports it forever once signaled; a level-triggered re-fire on a paused socket starved the write side), and mapped to the EOF hint only for sockets whose write side is already shut down — unlike kqueue's EV_EOF, AFD can signal DISCONNECT while data is still in flight, and an unconditional EOF mapping truncated in-flight transfers; for a shut-down socket no data-bearing flow remains, and that is exactly the state that hung.
Fatal-write errno whitelist in the h2 transport latch: the teardown latch treated every errno below -1 as connection death. It now enumerates the peer-gone class (EPIPE/ECONNRESET/ECONNABORTED/ENOTCONN/ETIMEDOUT/ENET*/EHOSTUNREACH + the matching raw WSA codes); anything unclassified keeps the historical re-buffer behavior instead of killing a live session (macOS returns racy errnos from send() on healthy sockets).
Received error-GOAWAY after our own graceful close is mutual teardown: grpc-js forceShutdown destroys server sessions with NGHTTP2_CANCEL, racing a GOAWAY(8) against a client that already close()d. Node never observes that frame (its socket is torn down first); on the Windows agents it deterministically arrived and destroyed the closing session with an unhandled error. A session that is closed with zero live streams now destroys cleanly on an error GOAWAY; a session that did not initiate close keeps Node's exact throw (verified side-by-side with v26.3.0).
Deferred stream-error sweep respects destroyed streams — precisely: the native per-stream error sweep defers each emission with process.nextTick, and session destroy's synchronous pass can tear the same stream down first. Re-destroying a listener-less destroyed stream re-emitted the error as an uncaught exception (Windows); skipping all destroyed streams swallowed grpc's terminal status codes on darwin (1 CANCELLED across ten suites). The sweep now skips only listener-less destroyed streams — listened ones keep the delivery, de-duped by Node's errorEmitted semantics.
Windows event-loop wedge resolved at the accounting layer — the root of the "server.close() waits forever" class (mechanism and A/B proof in Windows: uv_run's alive-guard wedges teardown states with zero ref'd handles (server.close() waits forever class) #34158): uv_run's alive-guard skips timer processing and I/O polling when the loop has no ref'd handles, and every uSockets handle is uv_unref'd by design, so a graceful server.close() awaiting a half-closed connection's teardown wedged forever once the server's KeepAlive dropped. Three pieces land here, in dependency order:
- Truthful request accounting on tunnel handoff: a raw 'upgrade'/CONNECT exchange never released pending_requests (the tunneled socket's close had no release path), so the server's all-closed promise silently never resolved for such connections. The handoff now marks the response TUNNELED and rides the existing single-fire mark_request_as_done path.
- Truthful websocket close accounting: server-initiated ws.close()/terminate() pre-set the closed flag and on_close then skipped the active_connections decrement — every server-initiated close leaked the count forever (server.stop()'s promise never resolved; reproducible on current releases) — and nothing re-evaluated server teardown when websockets drained. on_close now owns the single decrement and notifies the server.
- Graceful-stop keeps the server's ref until the drain completes (Node parity: server.close() keeps the process alive until connections end), so the Windows loop keeps polling through teardown. Direct loop-layer fixes (always-run tick with a bounded deadline) were tried, CI-validated as unwedging, and deliberately reverted — they change Windows timer punctuality globally; that avenue is documented in Windows: uv_run's alive-guard wedges teardown states with zero ref'd handles (server.close() waits forever class) #34158.
Silent write-path jam on unclassified send errnos fixed (the darwin h2 silent-death class): an errno that was neither would-block/transient nor in the peer-gone set fell between two layers — the socket layer reported it fatally without re-arming the writable poll, and the h2 transport's whitelist re-buffered the bytes waiting for a drain that could never come — leaving a half-alive session that reads frames but never writes again and dies silently later (wire-tape-proven on the darwin agents: the client never even sends its SETTINGS ACK; macOS's racy EPROTOTYPE on healthy sockets is the natural trigger, previously "handled" by an infinite silent retry). Deterministically reproduced on Linux via fault injection (×8 recovers / ×9 jammed forever). Unclassified errnos now get a bounded retry window (32 consecutive failures, libuv-style) and then surface like peer-gone errors; the h2 fatal check returns to the blanket form; and a second silent-loss bug found in the same audit — node:net's drain-path flush discarding fatal errnos, acking silently truncated streams as flushed — now fails the pending write and emits 'error'. Fault-injection regression tests cover burst-recovery and sustained-failure surfacing at both the h2 and net layers.
Stale fatal-write latch fixed (the darwin h2 silent-death residual): a failing write latched the deferred fatal teardown, but when the same-cycle retry succeeded and drained the buffer, the deferred flush still destroyed the now-healthy session — silently, with no error on any channel. Reproduced on Linux by fault injection at the exact SETTINGS-ACK window the darwin wire tapes identified. The teardown now verifies the buffer is still undrained before closing.
Peer resets now reach write-only and paused polls on Windows (the aarch64 PING-flood exit hang): libuv never subscribed AFD_POLL_ABORT unless readable polling was armed, and masked its report — so an RST against a socket armed for writable+disconnect was structurally invisible (vendored patch fixes request + report). A paused socket then discriminates without violating the pause: MSG_PEEK separates graceful FIN (deferred until resume, as before) from a reset — including a reset behind buffered data, recovered via SO_ERROR — which errors immediately like Node's paused sockets (an abandoned, never-resumed socket otherwise never learns its peer died and pins the process forever).
Windows net-reset class un-quarantined and green: the three main-quarantined test-net-*reset* tests ("reset not surfaced as ECONNRESET") pass on the Windows lanes with the UV_DISCONNECT delivery fixes — the entries are removed rather than carried.
Staged/event-taped twins added for the platform-intermittent scenarios (half-close-mid-upload teardown; PING-flood teardown over both loopback families and the dual-stack default path; late-RST tolerance; push-refusal conformance) plus a default-localhost loopback contract test: same contracts as the originals, but every stage runs under its own deadline and failures print the full event/frame tape instead of a silent timeout. These located every root cause fixed above, and they stay as regression coverage.
test-http2-ping-flood.js is dropped rather than shipped red: it was added by this branch's suite sync (never on main — the old expectations machinery removed http2 files from runs entirely), and on windows-11-aarch64 it cannot pass for any runtime: its never-reading flood client cannot observe the teardown reset on Windows (SO_ERROR doesn't latch a received RST, a zero-byte send() still succeeds, MSG_PEEK sees only the buffered data, and the one-shot AFD DISCONNECT report is consumed by the FIN that node's own finishSessionClose shape — ported verbatim here — sends first). The flood-detection contract is covered on every lane by the staged twin instead.
uWS cleanups (Jarred): the runtime node-http-compat flag is gone (the mode is a template instantiation — setUsingNodeHttpCompat(bool) → enableNodeHttpCompat()), three [[maybe_unused]] markers removed, the per-response booleans folded into the flags word, and the repeated byte loads in the header/trailer value loops hoisted into a local.

Follow-ups (recorded, not in this PR)

Replace the -errno / -1 sentinel protocol on the write path with an explicit WriteResult::Fatal(errno) variant, and plumb Windows WSA→errno translation so Windows surfaces fatal writes too.
Surface the fatal-flush errno on the remaining internal_flush call sites (on_open deferred flush, end_buffered, flush()), and don't re-buffer a chunk whose send already failed fatally.
h2_frame_parser's direct native writes treat a negative write_maybe_corked result as "wrote 0, re-buffer"; the session should fail the stream instead.
A trailer section whose bytes are malformed (NUL, bare CR/LF in a field line) currently parses to "no trailers" while Node's llhttp rejects the whole message; strict per-byte trailer validation (and the clientError it implies) is recorded here rather than widening this PR.
A Node-v26.3.0 source audit of net/tls/http/https/http2 (every finding double-verified with a Node citation) produced 34 further divergences for this area and 9 for node:tls (handed to the tls PR); they will be addressed in follow-up batches.

Tests

78 upstream tests added (all passing on at least one platform), 84 resynced to v26.3.0 content.
Adopted the standalone server-timeout suite contributed via node:http: enforce server headersTimeout, requestTimeout, setTimeout() and keepAliveTimeout #32942 (closed in favor of this PR) as test/js/node/http/node-http-server-timeouts.test.ts: raw-socket probes of headersTimeout, requestTimeout, server.setTimeout, and keepAliveTimeout, including the guard that requestTimeout stops at request completion and never fires while a slow handler is still streaming its response. All six pass on this branch; five of six fail on Bun without this PR (the knobs never fire).
The huge-ArrayBuffer structuredClone tests (added here for the 2GiB serialization-buffer growth crash) now use the smallest buffer that still exercises that growth path and treat a host OOM kill of the child (SIGKILL, no output) as an environment skip; any other signal, nonzero exit, or wrong round-trip still fails. Their previous ~5GB peak was OOM-killed on the Linux x64 CI runners.
test/expectations.txt: −135 entries, +0. This PR does not quarantine a single test. The eight named-pipe tests that cannot run on Windows are gated in-file with common.skip() and a reason, which is how the upstream suite gates a platform gap — and an improvement on main, where they carry if (common.isWindows) return;, a bare top-level return that reports a false PASS without running an assertion.
The --expose-internals harness shim gained internal/http, internal/options, and internal/streams/state entries so more upstream tests run unmodified.
Bun-authored tests updated where they asserted the old divergent behaviour (http2 wire error codes, clientError handling, abort-signal error code, TLS option message wording, server-timeout auto-destroy).

Not vendored (the 46-file gap to 100%)

Upstream files that are deleted, not quarantined — they exercise Node-internal machinery Bun has no equivalent for, and they never passed on main either (main only appeared green because expectations.txt skipped them):

internalBinding('http2') shape / monkey-patching and require('internal/http2/util') — binding, client-onconnect-errors, info-headers-errors, respond-errors, respond-nghttperrors, respond-with-fd-errors, server-push-stream-errors, util-headers-list, util-update-options-buffer, options-max-headers-exceeds-nghttp2, socket-proxy, allow-http1-upgrade-ws
NODE_DEBUG=http2 output, perf_hooks 'http2' entries, nghttp2-exact frame padding, async_hooks resource lifecycle events, and adopting a raw net.Socket into an h2 server via emit('connection')

test-http2-ping-flood.js is deleted for a different reason — a platform gap rather than missing internals. Its client floods PINGs and never reads; flood detection, session teardown and server.close() all complete, but on windows-11-aarch64 the process then cannot exit: a paused socket with the FIN deferred behind buffered data cannot observe the peer's later RST without consuming the stream (AFD's one-shot DISCONNECT report is consumed by the FIN, SO_ERROR does not latch a received RST, and a zero-byte send() still succeeds). Node's own client would sit in the identical state — the test simply never ran on this lane before this PR un-skipped the h2 suite (Node's CI has no windows-11-aarch64). Removed rather than quarantined, per this PR's no-new-expectations.txt rule. The reset-observability work it prompted (AFD ABORT subscription, paused-socket terminal probes, sweep-based escalation) is kept and is what un-quarantines the test-net-*reset* trio.

Known platform gap (unchanged by this PR)

Bun.serve({ unix: ... }) does not bind a Windows named pipe, so eight upstream unix-socket http/https tests common.skip() on Windows. The client side already works via net.connect. Not a regression — main hides the same gap behind a bare return.

no test proof · iteration 2 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/http/serve.test.ts test/js/bun/net/socket-syscall-fault.test.ts test/js/bun/websocket/websocket-server.test.ts test/js/node/net/net-syscall-fault.test.ts test/js/node/net/node-net.test.ts test/js/node/tls/tls-syscall-fault.test.ts test/js/web/fetch/fetch-leak.test.ts

robobun · 2026-06-18T14:07:09Z

^{Updated 8:23 PM PT - Jul 15th, 2026}

❌ @cirospaciari, your commit a4267b5 has 1 failures in Build #73542 (All Failures):

test/js/node/test/parallel/test-worker-message-port-transfer-terminate.js - SIGABRT on 🐧 13 x64-asan

🧪 To try this PR locally:

bunx bun-pr 32488

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

bun-32488 --bun

github-actions · 2026-06-18T14:14:55Z

Found 17 issues this PR may fix:

Fastify timeout settings not respected when using Bun runtime with NestJS and Fastify adapter #17287 - PR adds headersTimeout, requestTimeout, keepAliveTimeout which Fastify depends on
node:http: Server.closeAllConnections() shuts down the listening socket #31301 - PR explicitly fixes closeAllConnections() to not shut down the listening socket
node:http keep-alive server drops the next reused request after a Content-Length response finalized by a deferred end() (graceful FIN, no Connection: close) #31889 - PR fixes keep-alive connection reuse and HTTP pipelining queue with ordered flush
Bun does not upgrade and hangs when using node:http #32195 - PR adds CONNECT method support and upgrade body delivery improvements
Bun http.request() emits ECONNRESET after successful HTTP 101 Upgrade response #32222 - PR fixes upgrade body delivery and abort/half-open socket lifecycle
node:http bug in edge case usage of res.addTrailers #26171 - PR adds proper addTrailers implementation with chunked encoding support
HTTP/2 GOAWAY drops in-flight requests instead of allowing them to complete #26719 - PR fixes GOAWAY teardown to allow in-flight streams to complete
HTTP/2 Flow Control Bug in node:http2 - Requests Hang Indefinitely #30342 - PR fixes SETTINGS_INITIAL_WINDOW_SIZE delta calculation per RFC 9113 §6.5.3
http2.createSecureServer({ allowHTTP1: true }) returns empty response over HTTPS #28656 - PR implements allowHTTP1 fallback for http2.createSecureServer
HTTP/1.1 fallback broken for node:http2 secure server (allowHTTP1 ignored, ALPN only advertises h2) #26721 - PR fixes ALPN negotiation to advertise http/1.1 when allowHTTP1 is true
support "allowHTTP1" option in http2 #15419 - PR implements the allowHTTP1 option for HTTP/2 secure servers
Cannot modify default TLS cipher suite #18865 - PR adds ciphers, minVersion, maxVersion, secureProtocol TLS options to node:https
AbortSignal.timeout() is not respected for http.request #31167 - PR adds AbortSignal per-request support and abort lifecycle fixes on the client side
@fastify/http-proxy with HTTP/2 spuriously emits FST_REPLY_FROM_HTTP2_REQUEST_TIMEOUT after idle on Bun #30307 - PR fixes HTTP/2 GOAWAY and session lifecycle to prevent spurious timeouts after idle
node:http2 requests hang indefinitely after idle — regression in Bun 1.3.14 (firebase-admin FCM, production) #31881 - PR fixes HTTP/2 session management and GOAWAY handling that caused hangs after idle
request.headersDistinct is undefined in node:http #24268 - PR adds headersDistinct property on request and response objects
Bun's HTTP/2 server potentially ignores Nginx's HPACK for header compression setting SETTINGS_HEADER_TABLE_SIZE=0 #19152 - PR adds customSettings support including proper SETTINGS_HEADER_TABLE_SIZE handling

If this is helpful, copy the block below into the PR description to auto-close these issues on merge.

Fixes #17287
Fixes #31301
Fixes #31889
Fixes #32195
Fixes #32222
Fixes #26171
Fixes #26719
Fixes #30342
Fixes #28656
Fixes #26721
Fixes #15419
Fixes #18865
Fixes #31167
Fixes #30307
Fixes #31881
Fixes #24268
Fixes #19152

🤖 Generated with Claude Code

github-actions · 2026-06-18T14:19:56Z

This PR may be a duplicate of:

node:https: add addContext and other tls.Server methods to https.Server #32435 - Also adds TLS server methods (addContext, etc.) to node:https, overlapping with this PR's HTTPS server improvements
node:http: expose Server#_connections and validate getConnections callback #32429 - Implements Server#getConnections, which this PR's connection-counting work also covers
node:http: clamp negative IncomingMessage.setTimeout seconds to 0 #32308 - Fixes negative IncomingMessage.setTimeout clamping, superseded by this PR's timeout rework
http2: give Http2Stream its own per-stream idle timer #30308 - Adds per-stream idle timers to Http2Stream, overlapping with this PR's HTTP/2 timeout improvements
http2: reclaim closed-stream entries from the session streams map #30416 - Reclaims closed-stream entries from h2 session streams map, overlapping with this PR's session management
node:http2: honor res.sendDate/removeHeader and emit Keep-Alive on the allowHTTP1 fallback #28657 - Fixes allowHTTP1 fallback header mangling, which this PR also implements
fix: let clientError handler send response before closing socket #28642 - Fixes clientError handler to send response before closing socket, superseded by this PR's clientError contract
http: support client upgrade event #28828 - Adds client upgrade event support, overlapping with this PR's upgrade/CONNECT work
http: support CONNECT method in node:http client #31574 - Adds CONNECT method support in node:http client, which this PR also implements
node:http: hand off upgrade socket to userland #30664 - Hands off upgrade socket to userland, overlapping with this PR's upgrade support
fix(node:http): make upgrade socket.write() actually send data #28347 - Fixes upgrade socket.write() to actually send data, superseded by this PR's upgrade rework
fix(http): preserve server reference across close() for closeAllConnections() #30505 - Preserves server reference across close() for closeAllConnections(), superseded by this PR's connection management
node:http2: populate internal/http2/util for --expose-internals tests [1tpjlb] #29825 - Populates internal/http2/util for tests, overlapping with this PR's HTTP/2 improvements

🤖 Generated with Claude Code

coderabbitai · 2026-06-22T19:50:08Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

Walkthrough

Adds Node-compatible HTTP/HTTPS/HTTP/2 features including SNI accessors, insecure parser mode, HTTP/1.1 pipelining with queued responses, request trailers, timeout enforcement, upgrade/tunnel handling, domain lifecycle tracking, and HTTP/2 protocol validation, plus 200+ conformance tests.

Changes

Cohort / File(s)	Summary
TLS SNI and Socket Metadata `packages/bun-usockets/src/context.c`, `packages/bun-usockets/src/crypto/openssl.c`, `packages/bun-usockets/src/internal/internal.h`, `packages/bun-usockets/src/libusockets.h`	Adds exported `us_socket_sni_servername()` accessor returning TLS ServerName Indication from ClientHello via `SSL_get_servername()`, with internal helper declaration.
HTTP Parser and Protocol Constants `packages/bun-uws/src/HttpErrors.h`, `packages/bun-uws/src/HttpParser.h`, `src/jsc/ErrorCode.rs`, `src/jsc/bindings/ErrorCode.ts`	Adds `HTTP_ERROR_413_PAYLOAD_TOO_LARGE` error code, `HTTP_PARSER_ERROR_LF_EXPECTED` and `HTTP_PARSER_ERROR_CHUNK_EXTENSIONS_OVERFLOW` parser error constants, `nodeCompatMonotonicMs()` clock helper, and `ERR_HTTP_REQUEST_TIMEOUT` error code with supporting string mappings.
HTTP/1 Chunked Encoding and Trailers `packages/bun-uws/src/ChunkedEncoding.h`	Extends `consumeHexNumber()` with optional chunk-extensions-consumed counter, adds trailer state constants (`STATE_IS_TRAILERS`, `STATE_IS_TRAILERS_DONE`, `MAX_TRAILER_SECTION_SIZE`), introduces `isCompleteTrailerSection()` helper, and updates `ChunkIterator` to capture and forward trailer-section pointers during parsing.
HTTP/1 Parser Compatibility `packages/bun-uws/src/HttpParser.h`	Adds strict request-method validation, insecure header-value mode, deferred invalid transfer-encoding errors, tunnel-mode entry detection, chunk-extension overflow checking, and per-chunk extension size limits with fallback buffer sizing based on max header size.
HTTP/1 Server Lifecycle and Pipelining `packages/bun-uws/src/App.h`, `packages/bun-uws/src/HttpContext.h`, `packages/bun-uws/src/HttpContextData.h`, `packages/bun-uws/src/HttpResponse.h`	Adds `setUsingNodeHttpCompat()` flag, extends `setFlags()` with `useInsecureHTTPParser`, adds pipelining state tracking (queued count, reads-paused, tunnel-after-body), request timing fields (`lastMessageStartMs`, `headersCompleted`), trailer storage (`nodeHttpResponseTrailers`), and `shouldCloseConnection()` helper; updates response finalization for trailers and conditional chunked termination.
HTTP/1 Server JS Bindings `src/jsc/bindings/NodeHTTP.cpp`, `src/jsc/bindings/node/JSNodeHTTPServerSocket.h`, `src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp`, `src/jsc/bindings/node/JSNodeHTTPServerSocketPrototype.cpp`	Extends server flag configuration with `use_insecure_http_parser`, adds `Server__setOnConnection` binding, introduces pipelined response queuing (`appendPipelinedResponse`, `startPipelinedResponse`), TLS accessors (`sniServername`, `peerCertificateVerificationError`), request/response trailer APIs (`takeRequestTrailers`, `setResponseTrailers`), timeout detection (`isRequestTimedOut`), and prototype bindings for all new methods.
HTTP/1 Server Runtime `src/runtime/server/mod.rs`, `src/runtime/server/server_body.rs`, `src/uws_sys/App.rs`, `src/uws_sys/libuwsockets.cpp`	Adds `on_connection` JS callback field and `server_set_on_connection_` implementation, extends `set_flags()` signature with `use_insecure_http_parser` parameter, updates filter handler ABI to use raw `us_socket_t*`, and threads insecure-parser flag through C/FFI bindings.
HTTP Client and Message Objects `src/js/node/_http_client.ts`, `src/js/node/_http_common.ts`, `src/js/node/_http_incoming.ts`, `src/js/node/_http_outgoing.ts`	Adds `httpValidation` option (`strict`/`relaxed`/`insecure`), parser reentrancy queuing via `kPendingParserData`, per-stream `maxHeaderSize`, lenient header-value validation modes via `checkInvalidHeaderChar(lenient)`, `highWaterMark` propagation from server options, read-timer unref on incoming chunks, and lenient-mode header validation in `OutgoingMessage`.
HTTP Server JS Implementation `src/js/node/_http_server.ts`	Implements full Node-compatible server with per-socket parser mirroring, TLS option handling via `processPfxOptions`, request/connection timeout intervals, pipelined response queuing with `advanceResponsePipeline`, trailers-only branch for `internalEnd()`, parser-error detection for HTTP/2 preface, Node-like error mapping, and unique-header joining (`;` separator).
HTTP Utilities and Timers `src/js/internal/http.ts`, `src/js/internal/timers.ts`, `src/js/node/domain.ts`	Moves `kDeferredTimeouts` removal and `onConnection` callback support in HTTP binding, changes `kTimeout` to use globally-registered `Symbol.for("::buntimeout::")` for cross-module access, implements domain stack lifecycle with `enter()/exit()` push/pop semantics and `d.run(fn, ...args)` with enter/exit guarantees and function return value propagation.
TLS and HTTPS `src/js/internal/tls.ts`, `src/js/node/tls.ts`, `src/js/node/https.ts`, `src/js/node/net.ts`	Adds shared TLS helpers (`tlsStringToProtocolVersion`, `secureProtocolToVersionRange`, `processPfxOptions` for PKCS#12), `getAllowUnauthorized()` lazy warning, explicit `authorizationError: null` initialization, delegated PFX handling, net socket abort handling with `$makeAbortError(...)`, half-close EOF with synthesized `ECONNRESET`, post-connect `read(0)` startup, and explicit `handle.close(callback)`.
HTTP/2 Engine and Protocol `src/runtime/api/bun/h2/connection.rs`, `src/runtime/api/bun/h2/wire.rs`, `src/runtime/api/bun/h2_frame_parser.rs`	Introduces `PendingLocalSettings` for per-submission SETTINGS tracking, extends `Sink` trait with `on_error(lib_error_code: i32)`, `on_stream_open() -> bool` refusal capability, `is_stream_reading(stream_id) -> bool` receive-window gating, and `on_remote_custom_setting(id, value)` hook; adds ACK flood guard, enforces peer-acknowledged header-list limits, upgrades stream flow-control violations to session-level GOAWAY, and introduces `lib_error` module with nghttp2 constants.
HTTP/2 Frame Parser JS Bridge `src/runtime/api/h2.classes.ts`	Adds `setStreamReading` proto handler to frame-parser configuration for receive-window stream-reading gating.
HTTP/2 Node Implementation `src/js/node/http2.ts`	Significantly expands HTTP/2 compatibility with Node-accurate `ERR_INVALID_ARG_TYPE` formatting, `NghttpError` wrapper, `kStrictSingleValueFields` enforcement, session idle timer (`kTimeout`) with refresh logic, async-context swapping, write-callback deferred-flush detection, stream lifecycle correctness (chaining, trailer re-entrancy guards, proper `_final()` empty-DATA prevention), queued-request cancellation with `ERR_HTTP2_STREAM_CANCEL`, file-descriptor ownership tracking, and new `performServerHandshake(socket, options?)` export.
HTTP Test Suite `test/js/node/test/parallel/test-http-.js`, `test/js/node/test/parallel/test-https-.js`, `test/js/node/test/sequential/test-http.js`, `test/js/node/test/sequential/test-https.js`, `test/js/node/http2/.ts`, `test/js/bun/test/parallel/test-http-.ts`	Adds 200+ tests covering HTTP client/server, HTTPS, HTTP/2, chunked encoding, trailers, pipelining, timeouts, keep-alive, upgrades, parser edge cases, insecure modes, header validation, unique headers, domain integration, and conformance scenarios.
Test Expectations and Utilities `test/expectations.txt`, `test/js/node/test/common/index.js`, `test/js/node/test/parallel/test-http-*.ts`	Updates HTTP/2 and HTTP test expectations, adds Bun virtual modules for `internal/http`, `internal/streams/state`, and `internal/options` shims, and updates test assertions for new error messages and validation formatting.

Possibly related issues

node:http keep-alive server drops the next reused request after a Content-Length response finalized by a deferred end() (graceful FIN, no Connection: close) #31889: The PR's HTTP/1.1 pipelining, response queuing, and deferred-end() handling directly address the keep-alive request-ordering and response-closure behavior reported in this issue.

Possibly related PRs

oven-sh/bun#31175: Both PRs harden HTTP request-line parsing in packages/bun-uws/src/HttpParser.h around method/request-target validation, with overlapping changes to the same parser code level.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the main compatibility uplift, suite sync, and dependent net/tls fixes.
Description check	✅ Passed	The description is detailed and covers purpose plus verification, though it doesn’t use the exact template headings.

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

coderabbitai

Actionable comments posted: 15

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

src/runtime/api/bun/h2_frame_parser.rs (1)

6089-6205: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Reset explicit_settings for each SETTINGS submission.

load_settings_from_js_value() seeds the mask from self.explicit_settings, so once a bit is set it is serialized by every later settings() call even when that key is absent. For example, after headerTableSize is set once, settings({ maxFrameSize }) still sends SETTINGS_HEADER_TABLE_SIZE. Start the mask at 0 for the current options and commit that per-submission mask only after validation succeeds.
Proposed fix
-        let mut explicit_settings = self.explicit_settings.get();
+        let mut explicit_settings = 0;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/runtime/api/bun/h2_frame_parser.rs` around lines 6089 - 6205, The issue
is that explicit_settings is initialized by fetching the value from
self.explicit_settings, which means previously set bits persist across multiple
calls to load_settings_from_js_value(). This causes settings from previous
submissions to be included in later submissions even when they are not present
in the current options. Initialize explicit_settings to 0 instead of
self.explicit_settings.get() so that each call starts with a fresh mask, and
only the settings present in the current options parameter will have their
corresponding bits set before the mask is committed back to
self.explicit_settings.set(explicit_settings).

src/js/node/http2.ts (1)

2947-2955: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Keep FD ownership scoped to the file-response operation.

kOwnsFd is sticky stream state. If respondWithFile() fails and onError leaves the stream usable, a later respondWithFD() inherits kOwnsFd === true, causing caller-owned descriptors to be closed or passed with autoClose: true.

Proposed fix

-function doSendFileFD(options, fd, headers, err, stat) {
+function doSendFileFD(options, ownsFd, fd, headers, err, stat) {
   const onError = options.onError;
-  const ownsFd = this[kOwnsFd] === true;
   if (err) {
     if (ownsFd && err.code !== "EBADF") {
       tryClose(fd);
@@
-    if (this[kOwnsFd] === true) tryClose(fd);
+    if (ownsFd) tryClose(fd);
     return;
   }
@@
     fd: fd,
@@
-    autoClose: this[kOwnsFd] === true,
+    autoClose: ownsFd,
@@
-    if (ownsFd) fileStream.destroy();
+    if (ownsFd) fileStream.destroy();

-    this[kOwnsFd] = true;
     fs.open(path, "r", afterOpen.bind(this, options || {}, headers));

-  fs.fstat(fd, doSendFileFD.bind(this, options, fd, headers));
+  fs.fstat(fd, doSendFileFD.bind(this, options, true, fd, headers));

-      fs.fstat(fd.fd, doSendFileFD.bind(this, options, fd, headers));
+      fs.fstat(fd.fd, doSendFileFD.bind(this, options, false, fd, headers));
     } else {
-      fs.fstat(fd, doSendFileFD.bind(this, options, fd, headers));
+      fs.fstat(fd, doSendFileFD.bind(this, options, false, fd, headers));
     }

Also applies to: 3011-3016, 3050-3058, 3285-3286, 3344-3354

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/js/node/http2.ts` around lines 2947 - 2955, The kOwnsFd flag is
persisting as sticky stream state across multiple file-response operations,
causing subsequent calls like respondWithFD() to incorrectly inherit ownership
settings from previous failed operations. In the doSendFileFD function and the
related error handling paths at the locations mentioned (3011-3016, 3050-3058,
3285-3286, 3344-3354), ensure that kOwnsFd is reset to false or cleared after
each file-response operation completes, whether it succeeds or fails. This will
prevent the ownership flag from persisting and affecting later operations on the
same stream, ensuring that each file-response operation has its own scoped FD
ownership state rather than inheriting from previous operations.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/bun-uws/src/ChunkedEncoding.h`:
- Around line 132-142: The chunkExtensionsConsumed counter is being reset to 0
when a valid chunk-size line is found (in the return statement where
STATE_HAS_SIZE is set) before the caller has a chance to validate the
accumulated extensions. Add a bounds check at the increment point where
++*chunkExtensionsConsumed occurs to enforce the maximum extension size limit
before allowing further increments. If the counter would exceed the configured
maximum limit, return STATE_IS_ERROR to prevent bypassing the extension size
restriction when oversized extensions arrive within a single buffer.
- Around line 176-192: The MAX_TRAILER_SECTION_SIZE constant is hard-coded to 16
KiB, but trailers should be validated against the server's configured
maxHeaderSize limit instead. Modify the getNextChunk() function signature to
accept the active header limit as a parameter, and thread this limit through to
ChunkIterator as well. When validating the trailerSection size, replace the
comparison against the hard-coded MAX_TRAILER_SECTION_SIZE with a comparison
against the passed-in header limit value to ensure external protocol lengths are
bounded by the validated per-server configuration.

In `@src/js/internal/http.ts`:
- Line 30: The onConnection property uses an optional marker (?) which allows
TypeScript callers to omit passing the 7th argument, but the native binding
jsHTTPSetCustomOptions requires exactly 7 arguments to be passed. Remove the
optional marker from onConnection and instead make the type union explicitly
allow undefined, changing from onConnection?: (socketHandle: any) => undefined
to onConnection: ((socketHandle: any) => undefined) | undefined. This ensures
callers must always provide the parameter slot while still allowing undefined as
a valid value.

In `@src/js/node/_http_server.ts`:
- Around line 1124-1127: The Server.prototype.setTimeout method directly assigns
the msecs parameter without validating it first. Add validation to ensure msecs
is a non-negative integer before assigning it to this.timeout. If msecs fails
validation (such as being negative, NaN, or non-numeric), throw an appropriate
error with a descriptive message to match Node.js behavior and prevent
unexpected runtime issues.
- Around line 2178-2262: The try-finally block in the advanceResponsePipeline
function that replays buffered operations lacks error handling. If a write() or
end() call throws during the replay loop, the error will propagate uncaught and
leave the socket in an inconsistent state. Add a catch block to the existing
try-finally structure that wraps the ops replay loop, and within the catch
block, destroy both the response and socket to gracefully handle the error and
prevent further operations on the compromised connection.

In `@src/js/node/domain.ts`:
- Around line 60-69: The `run` method in the domain class needs to be modified
to properly handle callback arguments and return values. Update the method
signature to accept variadic arguments, forward all arguments when invoking the
callback function `fn`, and return the result of the callback execution instead
of always returning `this`. Ensure the callback is executed with the domain
bound as `this` context, and preserve the actual return value from the callback
to maintain Node.js compatibility.

In `@src/js/node/http2.ts`:
- Around line 4262-4269: The socket binding in the code snippet sets
socket[kBoundSession] before later fallible work completes, which can poison the
socket if construction fails. Move the socket binding statement to occur after
all validation and initialization work has succeeded, and add error handling to
clear the binding if any subsequent operations throw. Additionally, apply this
same transactional binding pattern to ClientHttp2Session by using
bindHttp2SessionSocket(socket, this) in the appropriate locations where sockets
are attached in ClientHttp2Session, ensuring consistency in how both server and
client sessions safely bind sockets.
- Around line 2604-2611: The line `code = code || 0` uses a logical OR operator
that coerces all falsy values (NaN, null, false, empty string) to 0 before
validation runs, allowing invalid input to pass validation as NGHTTP2_NO_ERROR.
Replace this permissive default assignment with a strict nullish coalescing
operator or explicit check that only defaults undefined or null to 0, ensuring
that invalid caller input like NaN or false is properly caught by the
validateInteger call that follows.
- Around line 5761-5769: The current condition in the signal validation block
checks both that options is an object AND that options.signal is truthy before
calling validateAbortSignal(). This causes falsey signal values like null,
false, or 0 to bypass validation entirely. Modify the condition to check if
options.signal exists as a property (using hasOwnProperty or similar) rather
than relying on truthiness, so that validateAbortSignal() is called for any
provided signal value including falsey ones, allowing the validation function to
properly reject invalid types.
- Around line 4618-4627: The current condition for detecting outbound progress
in the HTTP/2 timeout logic only refreshes the timeout when
sessionHasPendingWrite is true or nativeBuffered is greater than 0. When
nativeBuffered drains to zero (from a previously non-zero value), the code skips
the comparison between nativeBuffered and session[kTimeoutBytesSnapshot],
causing the timeout to potentially fire immediately despite data draining during
the interval. Fix this by also checking if nativeBuffered has changed from the
snapshot value (session[kTimeoutBytesSnapshot]) even when nativeBuffered equals
zero, and if it has changed, update the snapshot by setting
session[kTimeoutBytesSnapshot] = nativeBuffered and call
session[kTimeout]?.refresh() to properly treat the draining of buffered bytes as
outbound progress.

In `@src/js/node/tls.ts`:
- Around line 462-474: The issue is that getAllowUnauthorized() and
rejectUnauthorizedDefault() handle the NODE_TLS_REJECT_UNAUTHORIZED environment
variable inconsistently: getAllowUnauthorized() only treats "0" as disabling
verification and emits a warning, while rejectUnauthorizedDefault() also treats
"false" as disabling verification without a warning. To fix this, modify
rejectUnauthorizedDefault() to use !getAllowUnauthorized() as its implementation
instead of directly checking the environment variable, which ensures both
functions use the same single source of truth and consistently emit warnings for
all cases where verification is disabled.

In `@src/runtime/api/bun/h2_frame_parser.rs`:
- Around line 6204-6207: The settings parsing is not transactional because
local_settings and explicit_settings are being set into their cells via
self.local_settings.set() and self.explicit_settings.set() before customSettings
is fully validated. If customSettings validation fails, the earlier mutations
persist and can cause incorrect state in subsequent calls. Parse all settings
including customSettings validation into local temporaries first, then only
after all validation succeeds, commit them to the cells by calling
self.local_settings.set(), self.explicit_settings.set(), and writing to
self.custom_settings. Apply this same pattern to all locations where settings
are mutated (including the range mentioned at 6269-6276).
- Around line 5845-5852: Extract the byte-based session memory comparison logic
into a shared helper method, e.g. `is_over_session_memory_limit()`, that returns
a boolean. Replace the inline comparison in the `on_stream_open()` function with
a call to this helper. Apply this same helper consistently to all other
max-session-memory rejection paths mentioned in the applicable range (around
lines 7021-7031) to ensure uniform byte-based limit checking throughout the
code. Keep the existing `get_session_memory_usage()` method available for
logging and reporting purposes only, avoiding its use in budget gate decisions
where the MiB flooring could permit nearly one extra megabyte over the
configured limit.

In `@src/runtime/api/bun/h2/connection.rs`:
- Around line 707-730: In the note_outbound_ack method, add a brief inline
comment above or next to the condition check `self.obq_ack_pending <
MAX_OUTBOUND_ACK_QUEUE` to clarify that the flood error is intentionally
triggered when the 1000th ACK is queued (not 1001st) to match nghttp2's
behavior. The comment should explain that the counter is incremented before the
comparison, so the check fires on reaching the threshold value, documenting this
intentional off-by-one behavior for nghttp2 parity.

In `@src/runtime/server/server_body.rs`:
- Around line 3535-3555: The on_connection_callback method invokes
callback.call() with no mechanism to keep the server alive from the JS side,
creating a potential use-after-free if the callback re-entrantly disposes the
server. Add an RAII ref/deref guard using scopeguard before the callback.call()
invocation in on_connection_callback to pin the server lifetime, incrementing a
reference count before the call and decrementing on scope exit. Follow the same
pattern as used in on_upgrade_callback (referenced above) to ensure consistent
protection.

---

Outside diff comments:
In `@src/js/node/http2.ts`:
- Around line 2947-2955: The kOwnsFd flag is persisting as sticky stream state
across multiple file-response operations, causing subsequent calls like
respondWithFD() to incorrectly inherit ownership settings from previous failed
operations. In the doSendFileFD function and the related error handling paths at
the locations mentioned (3011-3016, 3050-3058, 3285-3286, 3344-3354), ensure
that kOwnsFd is reset to false or cleared after each file-response operation
completes, whether it succeeds or fails. This will prevent the ownership flag
from persisting and affecting later operations on the same stream, ensuring that
each file-response operation has its own scoped FD ownership state rather than
inheriting from previous operations.

In `@src/runtime/api/bun/h2_frame_parser.rs`:
- Around line 6089-6205: The issue is that explicit_settings is initialized by
fetching the value from self.explicit_settings, which means previously set bits
persist across multiple calls to load_settings_from_js_value(). This causes
settings from previous submissions to be included in later submissions even when
they are not present in the current options. Initialize explicit_settings to 0
instead of self.explicit_settings.get() so that each call starts with a fresh
mask, and only the settings present in the current options parameter will have
their corresponding bits set before the mask is committed back to
self.explicit_settings.set(explicit_settings).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2f235446-1cab-41b7-b484-9bcd9aa45cb0

📥 Commits

Reviewing files that changed from the base of the PR and between eae8038 and 4a12f3e.

📒 Files selected for processing (237)

packages/bun-usockets/src/context.c
packages/bun-usockets/src/crypto/openssl.c
packages/bun-usockets/src/internal/internal.h
packages/bun-usockets/src/libusockets.h
packages/bun-uws/src/App.h
packages/bun-uws/src/ChunkedEncoding.h
packages/bun-uws/src/HttpContext.h
packages/bun-uws/src/HttpContextData.h
packages/bun-uws/src/HttpErrors.h
packages/bun-uws/src/HttpParser.h
packages/bun-uws/src/HttpResponse.h
packages/bun-uws/src/HttpResponseData.h
src/js/internal/http.ts
src/js/internal/timers.ts
src/js/internal/tls.ts
src/js/node/_http_client.ts
src/js/node/_http_common.ts
src/js/node/_http_incoming.ts
src/js/node/_http_outgoing.ts
src/js/node/_http_server.ts
src/js/node/domain.ts
src/js/node/http2.ts
src/js/node/https.ts
src/js/node/net.ts
src/js/node/tls.ts
src/jsc/ErrorCode.rs
src/jsc/bindings/ErrorCode.ts
src/jsc/bindings/NodeHTTP.cpp
src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp
src/jsc/bindings/node/JSNodeHTTPServerSocket.h
src/jsc/bindings/node/JSNodeHTTPServerSocketPrototype.cpp
src/runtime/api/bun/h2/connection.rs
src/runtime/api/bun/h2/wire.rs
src/runtime/api/bun/h2_frame_parser.rs
src/runtime/api/h2.classes.ts
src/runtime/server/mod.rs
src/runtime/server/server_body.rs
src/uws_sys/App.rs
src/uws_sys/libuwsockets.cpp
test/expectations.txt
test/js/bun/test/parallel/test-http-host-array-should-throw-in-request.ts
test/js/bun/test/parallel/test-http-should-emit-timeout-event-when-using-server-setTimeout.ts
test/js/bun/test/parallel/test-http-should-emit-timeout-event.ts
test/js/node/http/node-http.test.ts
test/js/node/http2/h2-conformance.test.ts
test/js/node/http2/node-http2.test.js
test/js/node/net/node-net.test.ts
test/js/node/test/common/index.js
test/js/node/test/parallel/test-http-abort-stream-end.js
test/js/node/test/parallel/test-http-agent-domain-reused-gc.js
test/js/node/test/parallel/test-http-agent-keepalive-delay.js
test/js/node/test/parallel/test-http-agent-maxtotalsockets.js
test/js/node/test/parallel/test-http-agent-remove.js
test/js/node/test/parallel/test-http-allow-content-length-304.js
test/js/node/test/parallel/test-http-autoselectfamily.js
test/js/node/test/parallel/test-http-buffer-sanity.js
test/js/node/test/parallel/test-http-chunk-extensions-limit.js
test/js/node/test/parallel/test-http-chunk-problem.js
test/js/node/test/parallel/test-http-client-abort-keep-alive-queued-unix-socket.js
test/js/node/test/parallel/test-http-client-abort-unix-socket.js
test/js/node/test/parallel/test-http-client-close-with-default-agent.js
test/js/node/test/parallel/test-http-client-finished.js
test/js/node/test/parallel/test-http-client-immediate-error.js
test/js/node/test/parallel/test-http-client-keep-alive-hint.js
test/js/node/test/parallel/test-http-client-pipe-end.js
test/js/node/test/parallel/test-http-client-reject-unexpected-agent.js
test/js/node/test/parallel/test-http-client-request-options.js
test/js/node/test/parallel/test-http-client-response-domain.js
test/js/node/test/parallel/test-http-client-response-timeout.js
test/js/node/test/parallel/test-http-client-spurious-aborted.js
test/js/node/test/parallel/test-http-client-timeout-event.js
test/js/node/test/parallel/test-http-client-timeout-on-connect.js
test/js/node/test/parallel/test-http-client-timeout-option.js
test/js/node/test/parallel/test-http-client-timeout.js
test/js/node/test/parallel/test-http-client-with-create-connection.js
test/js/node/test/parallel/test-http-connect-req-res.js
test/js/node/test/parallel/test-http-connect.js
test/js/node/test/parallel/test-http-content-length-mismatch.js
test/js/node/test/parallel/test-http-correct-hostname.js
test/js/node/test/parallel/test-http-date-header.js
test/js/node/test/parallel/test-http-decoded-auth.js
test/js/node/test/parallel/test-http-double-content-length.js
test/js/node/test/parallel/test-http-dump-req-when-res-ends.js
test/js/node/test/parallel/test-http-early-hints-invalid-argument.js
test/js/node/test/parallel/test-http-end-throw-socket-handling.js
test/js/node/test/parallel/test-http-expect-handling.js
test/js/node/test/parallel/test-http-extra-response.js
test/js/node/test/parallel/test-http-flush-headers.js
test/js/node/test/parallel/test-http-flush-response-headers.js
test/js/node/test/parallel/test-http-generic-streams.js
test/js/node/test/parallel/test-http-head-throw-on-response-body-write.js
test/js/node/test/parallel/test-http-header-badrequest.js
test/js/node/test/parallel/test-http-header-obstext.js
test/js/node/test/parallel/test-http-header-read.js
test/js/node/test/parallel/test-http-header-value-relaxed.js
test/js/node/test/parallel/test-http-highwatermark.js
test/js/node/test/parallel/test-http-host-headers.js
test/js/node/test/parallel/test-http-hostname-typechecking.js
test/js/node/test/parallel/test-http-incoming-pipelined-socket-destroy.js
test/js/node/test/parallel/test-http-insecure-parser-per-stream.js
test/js/node/test/parallel/test-http-insecure-parser.js
test/js/node/test/parallel/test-http-invalidheaderfield.js
test/js/node/test/parallel/test-http-invalidheaderfield2.js
test/js/node/test/parallel/test-http-keep-alive-drop-requests.js
test/js/node/test/parallel/test-http-keep-alive-empty-line.mjs
test/js/node/test/parallel/test-http-keep-alive-max-requests.js
test/js/node/test/parallel/test-http-localaddress.js
test/js/node/test/parallel/test-http-many-ended-pipelines.js
test/js/node/test/parallel/test-http-max-header-size-per-stream.js
test/js/node/test/parallel/test-http-max-http-headers.js
test/js/node/test/parallel/test-http-multiple-headers.js
test/js/node/test/parallel/test-http-no-read-no-dump.js
test/js/node/test/parallel/test-http-outgoing-drain-writable-length.js
test/js/node/test/parallel/test-http-outgoing-finished.js
test/js/node/test/parallel/test-http-outgoing-proto.js
test/js/node/test/parallel/test-http-outgoing-renderHeaders.js
test/js/node/test/parallel/test-http-parser-finish-error.js
test/js/node/test/parallel/test-http-parser-free.js
test/js/node/test/parallel/test-http-parser-freed-before-upgrade.js
test/js/node/test/parallel/test-http-parser-freed-during-execute.js
test/js/node/test/parallel/test-http-parser-memory-retention.js
test/js/node/test/parallel/test-http-parser-multiple-execute.js
test/js/node/test/parallel/test-http-parser-timeout-reset.js
test/js/node/test/parallel/test-http-parser.js
test/js/node/test/parallel/test-http-pause.js
test/js/node/test/parallel/test-http-pipeline-assertionerror-finish.js
test/js/node/test/parallel/test-http-pipeline-flood.js
test/js/node/test/parallel/test-http-pipeline-outgoing-destroy.js
test/js/node/test/parallel/test-http-proxy.js
test/js/node/test/parallel/test-http-raw-headers.js
test/js/node/test/parallel/test-http-readable-data-event.js
test/js/node/test/parallel/test-http-req-close-robust-from-tampering.js
test/js/node/test/parallel/test-http-req-res-close.js
test/js/node/test/parallel/test-http-request-end-twice.js
test/js/node/test/parallel/test-http-request-end.js
test/js/node/test/parallel/test-http-request-method-delete-payload.js
test/js/node/test/parallel/test-http-response-add-header-after-sent.js
test/js/node/test/parallel/test-http-response-readable.js
test/js/node/test/parallel/test-http-response-remove-header-after-sent.js
test/js/node/test/parallel/test-http-response-setheaders.js
test/js/node/test/parallel/test-http-response-status-message.js
test/js/node/test/parallel/test-http-response-statuscode.js
test/js/node/test/parallel/test-http-response-writehead-returns-this.js
test/js/node/test/parallel/test-http-same-map.js
test/js/node/test/parallel/test-http-server-client-error.js
test/js/node/test/parallel/test-http-server-close-all.js
test/js/node/test/parallel/test-http-server-close-idle-wait-response.js
test/js/node/test/parallel/test-http-server-close-idle.js
test/js/node/test/parallel/test-http-server-connection-list-when-close.js
test/js/node/test/parallel/test-http-server-destroy-socket-on-client-error.js
test/js/node/test/parallel/test-http-server-drop-connections-in-cluster.js
test/js/node/test/parallel/test-http-server-headers-timeout-delayed-headers.js
test/js/node/test/parallel/test-http-server-headers-timeout-interrupted-headers.js
test/js/node/test/parallel/test-http-server-headers-timeout-keepalive.js
test/js/node/test/parallel/test-http-server-headers-timeout-pipelining.js
test/js/node/test/parallel/test-http-server-keep-alive-timeout.js
test/js/node/test/parallel/test-http-server-keepalive-end.js
test/js/node/test/parallel/test-http-server-method.query.js
test/js/node/test/parallel/test-http-server-multiheaders.js
test/js/node/test/parallel/test-http-server-multiple-client-error.js
test/js/node/test/parallel/test-http-server-non-utf8-header.js
test/js/node/test/parallel/test-http-server-options-highwatermark.js
test/js/node/test/parallel/test-http-server-options-incoming-message.js
test/js/node/test/parallel/test-http-server-options-server-response.js
test/js/node/test/parallel/test-http-server-reject-chunked-with-content-length.js
test/js/node/test/parallel/test-http-server-reject-cr-no-lf.js
test/js/node/test/parallel/test-http-server-request-timeout-delayed-body.js
test/js/node/test/parallel/test-http-server-request-timeout-delayed-headers.js
test/js/node/test/parallel/test-http-server-request-timeout-interrupted-body.js
test/js/node/test/parallel/test-http-server-request-timeout-interrupted-headers.js
test/js/node/test/parallel/test-http-server-request-timeout-keepalive.js
test/js/node/test/parallel/test-http-server-request-timeout-pipelining.js
test/js/node/test/parallel/test-http-server-request-timeout-upgrade.js
test/js/node/test/parallel/test-http-server-stale-close.js
test/js/node/test/parallel/test-http-server-unconsume.js
test/js/node/test/parallel/test-http-server.js
test/js/node/test/parallel/test-http-set-cookies.js
test/js/node/test/parallel/test-http-set-header-chain.js
test/js/node/test/parallel/test-http-set-timeout-server.js
test/js/node/test/parallel/test-http-set-timeout.js
test/js/node/test/parallel/test-http-set-trailers.js
test/js/node/test/parallel/test-http-socket-encoding-error.js
test/js/node/test/parallel/test-http-status-code.js
test/js/node/test/parallel/test-http-status-message.js
test/js/node/test/parallel/test-http-timeout-overflow.js
test/js/node/test/parallel/test-http-transfer-encoding-repeated-chunked.js
test/js/node/test/parallel/test-http-unix-socket.js
test/js/node/test/parallel/test-http-upgrade-server-callback.js
test/js/node/test/parallel/test-http-upgrade-server-with-body-and-extras.mjs
test/js/node/test/parallel/test-http-upgrade-server-with-body-error.mjs
test/js/node/test/parallel/test-http-upgrade-server-with-body.mjs
test/js/node/test/parallel/test-http-upgrade-server-with-large-body-unread.mjs
test/js/node/test/parallel/test-http-upgrade-server-with-large-body.mjs
test/js/node/test/parallel/test-http-url.parse-basic.js
test/js/node/test/parallel/test-http-url.parse-https.request.js
test/js/node/test/parallel/test-http-write-callbacks.js
test/js/node/test/parallel/test-http-zero-length-write.js
test/js/node/test/parallel/test-https-agent-additional-options.js
test/js/node/test/parallel/test-https-agent-keylog.js
test/js/node/test/parallel/test-https-agent-session-eviction.js
test/js/node/test/parallel/test-https-agent-sni.js
test/js/node/test/parallel/test-https-agent.js
test/js/node/test/parallel/test-https-argument-of-creating.js
test/js/node/test/parallel/test-https-autoselectfamily.js
test/js/node/test/parallel/test-https-byteswritten.js
test/js/node/test/parallel/test-https-client-renegotiation-limit.js
test/js/node/test/parallel/test-https-insecure-parse-per-stream.js
test/js/node/test/parallel/test-https-keep-alive-drop-requests.js
test/js/node/test/parallel/test-https-localaddress.js
test/js/node/test/parallel/test-https-max-header-size-per-stream.js
test/js/node/test/parallel/test-https-max-headers-count.js
test/js/node/test/parallel/test-https-options-boolean-check.js
test/js/node/test/parallel/test-https-pfx.js
test/js/node/test/parallel/test-https-resume-after-renew.js
test/js/node/test/parallel/test-https-selfsigned-no-keycertsign-no-crash.js
test/js/node/test/parallel/test-https-server-close-all.js
test/js/node/test/parallel/test-https-server-close-idle.js
test/js/node/test/parallel/test-https-set-timeout-server.js
test/js/node/test/parallel/test-https-strict.js
test/js/node/test/parallel/test-https-timeout-server-2.js
test/js/node/test/parallel/test-https-timeout-server.js
test/js/node/test/parallel/test-https-unix-socket-self-signed.js
test/js/node/test/parallel/test-tls-options-boolean-check.js
test/js/node/test/sequential/test-http-econnrefused.js
test/js/node/test/sequential/test-http-keep-alive-large-write.js
test/js/node/test/sequential/test-http-regr-gh-2928.js
test/js/node/test/sequential/test-http-server-keep-alive-timeout-slow-client-headers.js
test/js/node/test/sequential/test-http-server-keep-alive-timeout-slow-server.js
test/js/node/test/sequential/test-http-server-request-timeouts-mixed.js
test/js/node/test/sequential/test-http2-max-session-memory.js
test/js/node/test/sequential/test-http2-ping-flood.js
test/js/node/test/sequential/test-http2-settings-flood.js
test/js/node/test/sequential/test-http2-timeout-large-write-file.js
test/js/node/test/sequential/test-http2-timeout-large-write.js
test/js/node/test/sequential/test-https-connect-localport.js
test/js/node/test/sequential/test-https-server-keep-alive-timeout.js
test/regression/issue/25190.test.ts

💤 Files with no reviewable changes (5)

test/js/node/test/parallel/test-http-client-with-create-connection.js
test/js/node/test/parallel/test-http-client-response-timeout.js
test/js/node/test/parallel/test-http-client-pipe-end.js
test/js/node/test/parallel/test-http-unix-socket.js
test/js/node/test/parallel/test-https-unix-socket-self-signed.js

coderabbitai

Actionable comments posted: 16

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

src/js/node/http2.ts (1)

2949-3055: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Make fd ownership per operation, not sticky stream state.

respondWithFile() sets this[kOwnsFd] = true, but respondWithFD() never clears it. If a file response fails or overlaps before headers are sent, a later respondWithFD() on the same stream can inherit ownership and close a caller-owned fd.

Proposed fix

 function doSendFileFD(options, fd, headers, err, stat) {
   const onError = options.onError;
-  const ownsFd = this[kOwnsFd] === true;
+  const ownsFd = options[kOwnsFd] === true;
@@
-    if (this[kOwnsFd] === true) tryClose(fd);
+    if (ownsFd) tryClose(fd);
@@
-    if (this[kOwnsFd] === true) tryClose(fd);
+    if (ownsFd) tryClose(fd);
@@
-    autoClose: this[kOwnsFd] === true,
+    autoClose: ownsFd,
@@
-    this[kOwnsFd] = true;
+    options[kOwnsFd] = true;
     fs.open(path, "r", afterOpen.bind(this, options || {}, headers));

Also applies to: 3285-3286, 3344-3354

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/js/node/http2.ts` around lines 2949 - 3055, The fd ownership is currently
stored as persistent stream state in `this[kOwnsFd]`, which causes a file
descriptor opened by `respondWithFile()` to remain marked as owned even after
that operation completes. If a subsequent `respondWithFD()` call fails before
headers are sent, it will incorrectly close the caller-owned fd. Instead, make
fd ownership per-operation by determining ownership locally at the start of the
operation (where `respondWithFile()` sets ownership true and `respondWithFD()`
sets it false) and storing this decision in a local variable. Replace all
references to `this[kOwnsFd]` throughout the operation with this local ownership
variable, and do not persist the ownership state back to the instance property
after the operation completes.

src/js/node/_http_server.ts (1)

2503-2511: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Buffer informational responses while a pipelined response is queued.

Queued responses have socket === null; writeProcessing()/writeEarlyHints() still reach _writeRaw() and can throw, while writeContinue() silently drops the 100 Continue. Buffer _writeRaw/informational ops like write() and end() so they replay when the response gets the socket.

Also applies to: 2597-2611
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/js/node/_http_server.ts` around lines 2503 - 2511, The _writeRaw method
in ServerResponse.prototype needs to buffer write operations when the response
has no socket assigned (socket === null for pipelined responses) instead of
immediately executing them. Currently, informational responses from methods like
writeProcessing() and writeEarlyHints() can throw or be silently dropped when
reaching _writeRaw with a null socket. Implement buffering logic to queue the
chunk, encoding, and callback when the socket is null, then replay these
buffered operations once the response receives a socket assignment. Apply the
same buffering logic to the corresponding methods mentioned at lines 2597-2611
to ensure all informational operations (write, end, etc.) are properly buffered
and replayed.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/bun-uws/src/ChunkedEncoding.h`:
- Around line 207-218: The code in the STATE_IS_TRAILERS block accepts any bytes
matching the trailer end delimiter without validating their format as proper
header fields. Before transitioning the state to STATE_IS_TRAILERS_DONE and
emitting the final chunk, validate the trailerSection contents through a
header-specific parser that enforces token/value format validation and respects
the useInsecureHTTPParser setting, similar to how regular headers are validated.
This validation must occur immediately after isCompleteTrailerSection returns
true but before any state change or processing, ensuring malformed trailers are
rejected before req.trailers is exposed or the next pipelined request is parsed.

In `@packages/bun-uws/src/HttpContext.h`:
- Around line 329-334: The httpResponseData->lastMessageStartMs and
httpResponseData->headersCompleted state is not being reset for bodyless
requests (GET, HEAD, Content-Length: 0), causing incorrect timeout behavior on
slow async responses. Add logic to reset lastMessageStartMs to 0 and
headersCompleted to false when the parser determines there is no request body in
the httpContextData->flags.usingNodeHttpCompat block, matching the reset pattern
already present in the fin data path. Apply this same fix to both occurrences
mentioned (the main section and the also applies section around lines 456-462).
- Around line 727-736: The condition checking nodeHttpQueuedPipelinedCount only
preserves the socket when there are pipelined responses queued, but it fails to
account for the case where a current response is still pending. Modify the if
statement condition to also check whether an HTTP response is currently pending
(in addition to the existing nodeHttpQueuedPipelinedCount check) so that
half-closed sockets remain open both when responses are queued behind the
current one and when the current response itself is still being prepared by the
async server.

In `@packages/bun-uws/src/HttpResponseData.h`:
- Around line 151-184: The nodeHttpQueuedPipelinedCount counter declared as
uint16_t can overflow when more than 65535 pipelined responses are queued,
causing it to wrap around to 0 while responses remain queued, which breaks the
logic in shouldCloseConnection() and read-resumption checks. Fix this by either
widening nodeHttpQueuedPipelinedCount from uint16_t to uint32_t or size_t to
accommodate larger queue depths, or by adding saturation logic at the increment
site in HttpContext.h to ensure the counter never exceeds its maximum value (if
the counter is less than UINT16_MAX before incrementing, proceed with the
increment; otherwise, cap it at UINT16_MAX).

In `@src/js/internal/tls.ts`:
- Around line 111-145: The processPfxOptions function assigns parsed CA
certificates to the string key _pfxExtraCACerts only when pfxCAs has values,
which means user-provided _pfxExtraCACerts from the input options could be
spoofed and persist downstream. Either create a private exported symbol to use
instead of the string key _pfxExtraCACerts when storing the parsed CA values, or
unconditionally clear the _pfxExtraCACerts field from the output object before
conditionally assigning the parsed values to ensure downstream code only trusts
actual parsed CA material.

In `@src/js/node/_http_client.ts`:
- Around line 251-263: The httpValidation option is being validated and stored
in this.httpValidation but is not being wired into the actual parser
configuration, making it a no-op option. Find where the parser is being
initialized with the insecureHTTPParser flag and update that logic to also
consider the httpValidation value. Map the httpValidation options ("strict",
"relaxed", "insecure") to the appropriate parser leniency settings so that the
validation option actually affects the parsing behavior instead of just being
accepted and ignored.

In `@src/js/node/_http_outgoing.ts`:
- Around line 110-132: The `_isLenientHeaderValidation()` method treats any
non-"strict" value for `httpValidation` as lenient, but the server-side
`storeHTTPOptions()` function does not validate the `httpValidation` option
before it is stored. Add validation for `httpValidation` in `storeHTTPOptions()`
using `validateOneOf()` to restrict it to only the allowed values ["strict",
"relaxed", "insecure"], similar to how `insecureHTTPParser` is currently
validated. This ensures that only valid, explicitly allowed values are accepted
and prevents typos or garbage values from bypassing strict header validation.

In `@src/js/node/_http_server.ts`:
- Around line 711-728: The CONNECT request handling path contains early returns
before the HTTPS IncomingMessage flag is restored, causing the HTTPS state to
leak into subsequent request construction. In the CONNECT code path (around
lines 746-778), locate all early return statements and add a call to
setIsNextIncomingMessageHTTPS(prevIsNextIncomingMessageHTTPS) immediately before
each return to restore the previous HTTPS state, mirroring the restoration that
occurs at line 817 for the normal flow.
- Around line 2410-2417: The `writableCorked` property getter unconditionally
dereferences the socket, but for queued pipelined responses where `res.socket`
now returns `null`, this causes an error. Add a guard check in the
`writableCorked` getter (and the other related locations mentioned at lines
2450-2453) similar to the one in the socket getter that checks if
`this[kPipelinedQueuedState]` is undefined before accessing the socket. When it
is a queued pipelined response, return an appropriate default value (such as 0)
instead of attempting to dereference the socket.
- Around line 316-343: The ternary operator that merges pfxExtraCAs with ca
values uses the public Array.isArray function to check if ca is an array, but
this file should use the builtin-safe $isArray intrinsic instead for
tamper-resistance consistency. Replace Array.isArray(ca) with $isArray(ca) in
the ternary conditional expression on the line containing the ca assignment that
handles pfxExtraCAs merging.
- Around line 230-238: In the releaseServerParserShim function, replace the call
to parser.free() with a call to an internal no-op method instead. The
parser.free() method is exposed to userland and can be overwritten to throw
errors, which could prevent proper cleanup of parser.socket and tracked state
during close/upgrade operations. Use the internal implementation directly to
ensure cleanup always completes successfully.
- Around line 1172-1193: The isHttp2Preface function only checks the first 16
bytes of the HTTP/2 preface but the code reports bytesParsed = 24, creating a
mismatch. Extend the kHttp2PrefaceStart constant array to include the complete
24-byte HTTP/2 connection preface (the current 16-byte "PRI * HTTP/2.0\r\n"
string plus the additional 8 bytes that follow it), then ensure the
isHttp2Preface function validates against the full 24-byte preface before the
error is reported with bytesParsed = 24.
- Around line 2012-2016: In the header serialization conditional check, replace
the call to uniqueHeaders.has(key) with the builtin-safe version
uniqueHeaders.$has(key) to prevent user-overridable behavior. This ensures that
the membership check for uniqueHeaders uses the native Set method rather than a
potentially overridden one, in accordance with coding guidelines for built-in JS
modules.

In `@src/js/node/http2.ts`:
- Around line 150-155: The current conditional logic in the alias
synchronization block only handles cases where one of maxHeaderListSize or
maxHeaderSize is undefined, but when both are provided, they can end up with
different values despite representing the same SETTINGS id. Add an additional
condition to handle the case where both submitted.maxHeaderListSize and
submitted.maxHeaderSize are defined, and synchronize them to a single value
(such as the one that will be serialized) to ensure they remain aliased and
prevent an impossible local state where they differ.

In `@src/js/node/net.ts`:
- Around line 566-569: The code currently assigns an unwrapped SNI context to
state.selected without validating it is a legitimate SecureContext object. When
innerContext is extracted and truthy, add a validation check to ensure it is an
instance of state.server[kNativeSecureContextCtor] (similar to the check done in
the else-if branch for the direct context parameter) before assigning it to
state.selected. If the innerContext fails this validation, the code should fall
through to the else-if branch or handle it appropriately to prevent invalid
objects from bypassing the SecureContext type check.
- Around line 1057-1062: In the ConnResetException handling block where
listenerCount("error") is checked before calling self.destroy(er), add a
one-shot no-op listener to the "error" event before the destroy call. This
guards against the race condition where error listeners can be removed between
the listenerCount check and the deferred error emission. Install this temporary
listener using once("error", () => {}) pattern immediately before
self.destroy(er) to ensure there is always a listener to handle the error,
mirroring the approach used in the nearby SocketEmitEndNT reset path.

---

Outside diff comments:
In `@src/js/node/_http_server.ts`:
- Around line 2503-2511: The _writeRaw method in ServerResponse.prototype needs
to buffer write operations when the response has no socket assigned (socket ===
null for pipelined responses) instead of immediately executing them. Currently,
informational responses from methods like writeProcessing() and
writeEarlyHints() can throw or be silently dropped when reaching _writeRaw with
a null socket. Implement buffering logic to queue the chunk, encoding, and
callback when the socket is null, then replay these buffered operations once the
response receives a socket assignment. Apply the same buffering logic to the
corresponding methods mentioned at lines 2597-2611 to ensure all informational
operations (write, end, etc.) are properly buffered and replayed.

In `@src/js/node/http2.ts`:
- Around line 2949-3055: The fd ownership is currently stored as persistent
stream state in `this[kOwnsFd]`, which causes a file descriptor opened by
`respondWithFile()` to remain marked as owned even after that operation
completes. If a subsequent `respondWithFD()` call fails before headers are sent,
it will incorrectly close the caller-owned fd. Instead, make fd ownership
per-operation by determining ownership locally at the start of the operation
(where `respondWithFile()` sets ownership true and `respondWithFD()` sets it
false) and storing this decision in a local variable. Replace all references to
`this[kOwnsFd]` throughout the operation with this local ownership variable, and
do not persist the ownership state back to the instance property after the
operation completes.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3bbe5b95-af18-4e69-b127-617f895e1f2e

📥 Commits

Reviewing files that changed from the base of the PR and between 4a12f3e and d0b6fcc.

📒 Files selected for processing (31)

packages/bun-usockets/src/context.c
packages/bun-usockets/src/crypto/openssl.c
packages/bun-usockets/src/internal/internal.h
packages/bun-usockets/src/libusockets.h
packages/bun-uws/src/App.h
packages/bun-uws/src/ChunkedEncoding.h
packages/bun-uws/src/HttpContext.h
packages/bun-uws/src/HttpContextData.h
packages/bun-uws/src/HttpErrors.h
packages/bun-uws/src/HttpParser.h
packages/bun-uws/src/HttpResponse.h
packages/bun-uws/src/HttpResponseData.h
src/js/internal/http.ts
src/js/internal/timers.ts
src/js/internal/tls.ts
src/js/node/_http_client.ts
src/js/node/_http_common.ts
src/js/node/_http_incoming.ts
src/js/node/_http_outgoing.ts
src/js/node/_http_server.ts
src/js/node/domain.ts
src/js/node/http2.ts
src/js/node/https.ts
src/js/node/net.ts
src/js/node/tls.ts
src/jsc/ErrorCode.rs
src/jsc/bindings/ErrorCode.ts
src/jsc/bindings/NodeHTTP.cpp
src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp
src/jsc/bindings/node/JSNodeHTTPServerSocket.h
src/jsc/bindings/node/JSNodeHTTPServerSocketPrototype.cpp

💤 Files with no reviewable changes (6)

src/jsc/bindings/ErrorCode.ts
src/jsc/bindings/node/JSNodeHTTPServerSocketPrototype.cpp
src/jsc/ErrorCode.rs
src/jsc/bindings/NodeHTTP.cpp
src/jsc/bindings/node/JSNodeHTTPServerSocket.h
src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp

coderabbitai

Caution

Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

Actionable comments posted: 16

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

src/js/node/http2.ts (1)

2949-3055: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Make fd ownership per operation, not sticky stream state.

respondWithFile() sets this[kOwnsFd] = true, but respondWithFD() never clears it. If a file response fails or overlaps before headers are sent, a later respondWithFD() on the same stream can inherit ownership and close a caller-owned fd.

Proposed fix

 function doSendFileFD(options, fd, headers, err, stat) {
   const onError = options.onError;
-  const ownsFd = this[kOwnsFd] === true;
+  const ownsFd = options[kOwnsFd] === true;
@@
-    if (this[kOwnsFd] === true) tryClose(fd);
+    if (ownsFd) tryClose(fd);
@@
-    if (this[kOwnsFd] === true) tryClose(fd);
+    if (ownsFd) tryClose(fd);
@@
-    autoClose: this[kOwnsFd] === true,
+    autoClose: ownsFd,
@@
-    this[kOwnsFd] = true;
+    options[kOwnsFd] = true;
     fs.open(path, "r", afterOpen.bind(this, options || {}, headers));

Also applies to: 3285-3286, 3344-3354

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/js/node/http2.ts` around lines 2949 - 3055, The fd ownership is currently
stored as persistent stream state in `this[kOwnsFd]`, which causes a file
descriptor opened by `respondWithFile()` to remain marked as owned even after
that operation completes. If a subsequent `respondWithFD()` call fails before
headers are sent, it will incorrectly close the caller-owned fd. Instead, make
fd ownership per-operation by determining ownership locally at the start of the
operation (where `respondWithFile()` sets ownership true and `respondWithFD()`
sets it false) and storing this decision in a local variable. Replace all
references to `this[kOwnsFd]` throughout the operation with this local ownership
variable, and do not persist the ownership state back to the instance property
after the operation completes.

src/js/node/_http_server.ts (1)

2503-2511: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Buffer informational responses while a pipelined response is queued.

Queued responses have socket === null; writeProcessing()/writeEarlyHints() still reach _writeRaw() and can throw, while writeContinue() silently drops the 100 Continue. Buffer _writeRaw/informational ops like write() and end() so they replay when the response gets the socket.

Also applies to: 2597-2611
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/js/node/_http_server.ts` around lines 2503 - 2511, The _writeRaw method
in ServerResponse.prototype needs to buffer write operations when the response
has no socket assigned (socket === null for pipelined responses) instead of
immediately executing them. Currently, informational responses from methods like
writeProcessing() and writeEarlyHints() can throw or be silently dropped when
reaching _writeRaw with a null socket. Implement buffering logic to queue the
chunk, encoding, and callback when the socket is null, then replay these
buffered operations once the response receives a socket assignment. Apply the
same buffering logic to the corresponding methods mentioned at lines 2597-2611
to ensure all informational operations (write, end, etc.) are properly buffered
and replayed.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/bun-uws/src/ChunkedEncoding.h`:
- Around line 207-218: The code in the STATE_IS_TRAILERS block accepts any bytes
matching the trailer end delimiter without validating their format as proper
header fields. Before transitioning the state to STATE_IS_TRAILERS_DONE and
emitting the final chunk, validate the trailerSection contents through a
header-specific parser that enforces token/value format validation and respects
the useInsecureHTTPParser setting, similar to how regular headers are validated.
This validation must occur immediately after isCompleteTrailerSection returns
true but before any state change or processing, ensuring malformed trailers are
rejected before req.trailers is exposed or the next pipelined request is parsed.

In `@packages/bun-uws/src/HttpContext.h`:
- Around line 329-334: The httpResponseData->lastMessageStartMs and
httpResponseData->headersCompleted state is not being reset for bodyless
requests (GET, HEAD, Content-Length: 0), causing incorrect timeout behavior on
slow async responses. Add logic to reset lastMessageStartMs to 0 and
headersCompleted to false when the parser determines there is no request body in
the httpContextData->flags.usingNodeHttpCompat block, matching the reset pattern
already present in the fin data path. Apply this same fix to both occurrences
mentioned (the main section and the also applies section around lines 456-462).
- Around line 727-736: The condition checking nodeHttpQueuedPipelinedCount only
preserves the socket when there are pipelined responses queued, but it fails to
account for the case where a current response is still pending. Modify the if
statement condition to also check whether an HTTP response is currently pending
(in addition to the existing nodeHttpQueuedPipelinedCount check) so that
half-closed sockets remain open both when responses are queued behind the
current one and when the current response itself is still being prepared by the
async server.

In `@packages/bun-uws/src/HttpResponseData.h`:
- Around line 151-184: The nodeHttpQueuedPipelinedCount counter declared as
uint16_t can overflow when more than 65535 pipelined responses are queued,
causing it to wrap around to 0 while responses remain queued, which breaks the
logic in shouldCloseConnection() and read-resumption checks. Fix this by either
widening nodeHttpQueuedPipelinedCount from uint16_t to uint32_t or size_t to
accommodate larger queue depths, or by adding saturation logic at the increment
site in HttpContext.h to ensure the counter never exceeds its maximum value (if
the counter is less than UINT16_MAX before incrementing, proceed with the
increment; otherwise, cap it at UINT16_MAX).

In `@src/js/internal/tls.ts`:
- Around line 111-145: The processPfxOptions function assigns parsed CA
certificates to the string key _pfxExtraCACerts only when pfxCAs has values,
which means user-provided _pfxExtraCACerts from the input options could be
spoofed and persist downstream. Either create a private exported symbol to use
instead of the string key _pfxExtraCACerts when storing the parsed CA values, or
unconditionally clear the _pfxExtraCACerts field from the output object before
conditionally assigning the parsed values to ensure downstream code only trusts
actual parsed CA material.

In `@src/js/node/_http_client.ts`:
- Around line 251-263: The httpValidation option is being validated and stored
in this.httpValidation but is not being wired into the actual parser
configuration, making it a no-op option. Find where the parser is being
initialized with the insecureHTTPParser flag and update that logic to also
consider the httpValidation value. Map the httpValidation options ("strict",
"relaxed", "insecure") to the appropriate parser leniency settings so that the
validation option actually affects the parsing behavior instead of just being
accepted and ignored.

In `@src/js/node/_http_outgoing.ts`:
- Around line 110-132: The `_isLenientHeaderValidation()` method treats any
non-"strict" value for `httpValidation` as lenient, but the server-side
`storeHTTPOptions()` function does not validate the `httpValidation` option
before it is stored. Add validation for `httpValidation` in `storeHTTPOptions()`
using `validateOneOf()` to restrict it to only the allowed values ["strict",
"relaxed", "insecure"], similar to how `insecureHTTPParser` is currently
validated. This ensures that only valid, explicitly allowed values are accepted
and prevents typos or garbage values from bypassing strict header validation.

In `@src/js/node/_http_server.ts`:
- Around line 711-728: The CONNECT request handling path contains early returns
before the HTTPS IncomingMessage flag is restored, causing the HTTPS state to
leak into subsequent request construction. In the CONNECT code path (around
lines 746-778), locate all early return statements and add a call to
setIsNextIncomingMessageHTTPS(prevIsNextIncomingMessageHTTPS) immediately before
each return to restore the previous HTTPS state, mirroring the restoration that
occurs at line 817 for the normal flow.
- Around line 2410-2417: The `writableCorked` property getter unconditionally
dereferences the socket, but for queued pipelined responses where `res.socket`
now returns `null`, this causes an error. Add a guard check in the
`writableCorked` getter (and the other related locations mentioned at lines
2450-2453) similar to the one in the socket getter that checks if
`this[kPipelinedQueuedState]` is undefined before accessing the socket. When it
is a queued pipelined response, return an appropriate default value (such as 0)
instead of attempting to dereference the socket.
- Around line 316-343: The ternary operator that merges pfxExtraCAs with ca
values uses the public Array.isArray function to check if ca is an array, but
this file should use the builtin-safe $isArray intrinsic instead for
tamper-resistance consistency. Replace Array.isArray(ca) with $isArray(ca) in
the ternary conditional expression on the line containing the ca assignment that
handles pfxExtraCAs merging.
- Around line 230-238: In the releaseServerParserShim function, replace the call
to parser.free() with a call to an internal no-op method instead. The
parser.free() method is exposed to userland and can be overwritten to throw
errors, which could prevent proper cleanup of parser.socket and tracked state
during close/upgrade operations. Use the internal implementation directly to
ensure cleanup always completes successfully.
- Around line 1172-1193: The isHttp2Preface function only checks the first 16
bytes of the HTTP/2 preface but the code reports bytesParsed = 24, creating a
mismatch. Extend the kHttp2PrefaceStart constant array to include the complete
24-byte HTTP/2 connection preface (the current 16-byte "PRI * HTTP/2.0\r\n"
string plus the additional 8 bytes that follow it), then ensure the
isHttp2Preface function validates against the full 24-byte preface before the
error is reported with bytesParsed = 24.
- Around line 2012-2016: In the header serialization conditional check, replace
the call to uniqueHeaders.has(key) with the builtin-safe version
uniqueHeaders.$has(key) to prevent user-overridable behavior. This ensures that
the membership check for uniqueHeaders uses the native Set method rather than a
potentially overridden one, in accordance with coding guidelines for built-in JS
modules.

In `@src/js/node/http2.ts`:
- Around line 150-155: The current conditional logic in the alias
synchronization block only handles cases where one of maxHeaderListSize or
maxHeaderSize is undefined, but when both are provided, they can end up with
different values despite representing the same SETTINGS id. Add an additional
condition to handle the case where both submitted.maxHeaderListSize and
submitted.maxHeaderSize are defined, and synchronize them to a single value
(such as the one that will be serialized) to ensure they remain aliased and
prevent an impossible local state where they differ.

In `@src/js/node/net.ts`:
- Around line 566-569: The code currently assigns an unwrapped SNI context to
state.selected without validating it is a legitimate SecureContext object. When
innerContext is extracted and truthy, add a validation check to ensure it is an
instance of state.server[kNativeSecureContextCtor] (similar to the check done in
the else-if branch for the direct context parameter) before assigning it to
state.selected. If the innerContext fails this validation, the code should fall
through to the else-if branch or handle it appropriately to prevent invalid
objects from bypassing the SecureContext type check.
- Around line 1057-1062: In the ConnResetException handling block where
listenerCount("error") is checked before calling self.destroy(er), add a
one-shot no-op listener to the "error" event before the destroy call. This
guards against the race condition where error listeners can be removed between
the listenerCount check and the deferred error emission. Install this temporary
listener using once("error", () => {}) pattern immediately before
self.destroy(er) to ensure there is always a listener to handle the error,
mirroring the approach used in the nearby SocketEmitEndNT reset path.

---

Outside diff comments:
In `@src/js/node/_http_server.ts`:
- Around line 2503-2511: The _writeRaw method in ServerResponse.prototype needs
to buffer write operations when the response has no socket assigned (socket ===
null for pipelined responses) instead of immediately executing them. Currently,
informational responses from methods like writeProcessing() and
writeEarlyHints() can throw or be silently dropped when reaching _writeRaw with
a null socket. Implement buffering logic to queue the chunk, encoding, and
callback when the socket is null, then replay these buffered operations once the
response receives a socket assignment. Apply the same buffering logic to the
corresponding methods mentioned at lines 2597-2611 to ensure all informational
operations (write, end, etc.) are properly buffered and replayed.

In `@src/js/node/http2.ts`:
- Around line 2949-3055: The fd ownership is currently stored as persistent
stream state in `this[kOwnsFd]`, which causes a file descriptor opened by
`respondWithFile()` to remain marked as owned even after that operation
completes. If a subsequent `respondWithFD()` call fails before headers are sent,
it will incorrectly close the caller-owned fd. Instead, make fd ownership
per-operation by determining ownership locally at the start of the operation
(where `respondWithFile()` sets ownership true and `respondWithFD()` sets it
false) and storing this decision in a local variable. Replace all references to
`this[kOwnsFd]` throughout the operation with this local ownership variable, and
do not persist the ownership state back to the instance property after the
operation completes.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3bbe5b95-af18-4e69-b127-617f895e1f2e

📥 Commits

Reviewing files that changed from the base of the PR and between 4a12f3e and d0b6fcc.

📒 Files selected for processing (31)

packages/bun-usockets/src/context.c
packages/bun-usockets/src/crypto/openssl.c
packages/bun-usockets/src/internal/internal.h
packages/bun-usockets/src/libusockets.h
packages/bun-uws/src/App.h
packages/bun-uws/src/ChunkedEncoding.h
packages/bun-uws/src/HttpContext.h
packages/bun-uws/src/HttpContextData.h
packages/bun-uws/src/HttpErrors.h
packages/bun-uws/src/HttpParser.h
packages/bun-uws/src/HttpResponse.h
packages/bun-uws/src/HttpResponseData.h
src/js/internal/http.ts
src/js/internal/timers.ts
src/js/internal/tls.ts
src/js/node/_http_client.ts
src/js/node/_http_common.ts
src/js/node/_http_incoming.ts
src/js/node/_http_outgoing.ts
src/js/node/_http_server.ts
src/js/node/domain.ts
src/js/node/http2.ts
src/js/node/https.ts
src/js/node/net.ts
src/js/node/tls.ts
src/jsc/ErrorCode.rs
src/jsc/bindings/ErrorCode.ts
src/jsc/bindings/NodeHTTP.cpp
src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp
src/jsc/bindings/node/JSNodeHTTPServerSocket.h
src/jsc/bindings/node/JSNodeHTTPServerSocketPrototype.cpp

💤 Files with no reviewable changes (6)

src/jsc/bindings/ErrorCode.ts
src/jsc/bindings/node/JSNodeHTTPServerSocketPrototype.cpp
src/jsc/ErrorCode.rs
src/jsc/bindings/NodeHTTP.cpp
src/jsc/bindings/node/JSNodeHTTPServerSocket.h
src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp

🛑 Comments failed to post (16)

packages/bun-uws/src/ChunkedEncoding.h (1)
207-218: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Validate trailer fields before accepting the message.

This accepts any bytes ending in \r\n\r\n as trailers and emits the final chunk without applying the header token/value validation used for regular headers. Malformed trailer fields should fail before req.trailers is exposed or the next pipelined request is parsed. Thread trailer bytes through a trailer-specific header parser that honors useInsecureHTTPParser and the active size limit. As per coding guidelines, “Validate untrusted input BEFORE any processing, allocation, or side effect.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/bun-uws/src/ChunkedEncoding.h` around lines 207 - 218, The code in
the STATE_IS_TRAILERS block accepts any bytes matching the trailer end delimiter
without validating their format as proper header fields. Before transitioning
the state to STATE_IS_TRAILERS_DONE and emitting the final chunk, validate the
trailerSection contents through a header-specific parser that enforces
token/value format validation and respects the useInsecureHTTPParser setting,
similar to how regular headers are validated. This validation must occur
immediately after isCompleteTrailerSection returns true but before any state
change or processing, ensuring malformed trailers are rejected before
req.trailers is exposed or the next pipelined request is parsed.
Source: Coding guidelines
packages/bun-uws/src/HttpContext.h (2)
329-334: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Clear request-timeout state for bodyless requests.

This starts the Node request-timeout window when headers complete, but the only visible completion reset is the fin data path. GET, HEAD, and Content-Length: 0 requests can leave lastMessageStartMs nonzero after the full request is already received, so a slow async response may be timed out as if the request body were still arriving.

Reset lastMessageStartMs and headersCompleted when the parser determines there is no request body, matching the fin path.

Also applies to: 456-462
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/bun-uws/src/HttpContext.h` around lines 329 - 334, The
httpResponseData->lastMessageStartMs and httpResponseData->headersCompleted
state is not being reset for bodyless requests (GET, HEAD, Content-Length: 0),
causing incorrect timeout behavior on slow async responses. Add logic to reset
lastMessageStartMs to 0 and headersCompleted to false when the parser determines
there is no request body in the httpContextData->flags.usingNodeHttpCompat
block, matching the reset pattern already present in the fin data path. Apply
this same fix to both occurrences mentioned (the main section and the also
applies section around lines 456-462).
727-736: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Keep half-closed sockets open while the current response is pending.

This preserves the socket only when pipelined responses are queued. A valid client can send a single request, half-close its write side, and still expect the async server response; with HTTP_RESPONSE_PENDING and nodeHttpQueuedPipelinedCount == 0, this falls through to the close path and can drop that response.
Proposed fix
-            if (httpResponseData->nodeHttpQueuedPipelinedCount > 0) {
+            if ((httpResponseData->state & HttpResponseData<SSL>::HTTP_RESPONSE_PENDING)
+                || httpResponseData->nodeHttpQueuedPipelinedCount > 0) {
                 httpResponseData->nodeHttpReceivedFIN = true;
                 return s;
             }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/bun-uws/src/HttpContext.h` around lines 727 - 736, The condition
checking nodeHttpQueuedPipelinedCount only preserves the socket when there are
pipelined responses queued, but it fails to account for the case where a current
response is still pending. Modify the if statement condition to also check
whether an HTTP response is currently pending (in addition to the existing
nodeHttpQueuedPipelinedCount check) so that half-closed sockets remain open both
when responses are queued behind the current one and when the current response
itself is still being prepared by the async server.
packages/bun-uws/src/HttpResponseData.h (1)
151-184: ⚠️ Potential issue | 🔴 Critical
🧩 Analysis chain

🏁 Script executed:
#!/bin/bash
# Verify all mutation sites for nodeHttpQueuedPipelinedCount.
rg -n -C4 '\bnodeHttpQueuedPipelinedCount\b'
Repository: oven-sh/bun

Length of output: 6745

🏁 Script executed:
# Search for any constants or limits related to pipelined requests, queue size, or request buffering
rg -i "max.*pipeline|pipeline.*max|pipelined.*limit|queue.*size|max.*queued" --type cpp --type h
Repository: oven-sh/bun

Length of output: 3328

🏁 Script executed:
# Check if there's saturation logic or bounds checking before the increment at HttpContext.h:360
sed -n '340,380p' packages/bun-uws/src/HttpContext.h
Repository: oven-sh/bun

Length of output: 2413

🏁 Script executed:
# Search for any requestor memory limits that might implicitly cap the queue
rg "backlog|buffer.*limit|max.*request|request.*limit" packages/bun-uws/src/HttpContext.h -A 2
Repository: oven-sh/bun

Length of output: 37

🏁 Script executed:
# Search for HTTP/request parsing limits or buffer constraints that might cap the queue
rg "getBufferedAmount|backpressure|read.*pause|onData|onHeadersComplete" packages/bun-uws/src/HttpContext.h -B 2 -A 2 | head -60
Repository: oven-sh/bun

Length of output: 3392

🏁 Script executed:
# Check if there are any validation checks or early returns that prevent excessive pipelined requests
sed -n '320,370p' packages/bun-uws/src/HttpContext.h | cat -n
Repository: oven-sh/bun

Length of output: 3514

🏁 Script executed:
# Search for any connection-level constants, limits, or initialization that might cap requests
rg "struct.*HttpResponseData|class.*HttpResponseData" packages/bun-uws/src/ -A 30 | head -80
Repository: oven-sh/bun

Length of output: 2726

🏁 Script executed:
# Check if there's any saturation or clamping logic around the increment
sed -n '355,365p' packages/bun-uws/src/HttpContext.h | cat -n
Repository: oven-sh/bun

Length of output: 785

🏁 Script executed:
# Search for any node:http request queueing or memory limits
rg "nodeHttpQueuedPipelinedCount|pipelined.*queue" packages/bun-uws/src/ --type cpp --type h -B 1 -A 1
Repository: oven-sh/bun

Length of output: 2194

🏁 Script executed:
# Check if socket buffer size provides an implicit practical cap
rg "getBufferedAmount|BUFFER|buffer.*size|MAX" packages/bun-uws/src/HttpContext.h -i | head -30
Repository: oven-sh/bun

Length of output: 2147
Use a wider counter or add saturation logic to prevent nodeHttpQueuedPipelinedCount overflow.

The counter (uint16_t, max 65535) is incremented unconditionally at HttpContext.h:360 with no bounds check. If 65,536+ pipelined responses queue before draining—reachable if requests arrive faster than backpressure flushes—the counter wraps to 0 while responses remain queued. This causes shouldCloseConnection() and read-resumption logic to misread the queue as empty, leading to premature connection closure or read resumption despite pending responses.

Replace the bare increment with saturation (if (count < UINT16_MAX) count++) or widen to uint32_t/size_t.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/bun-uws/src/HttpResponseData.h` around lines 151 - 184, The
nodeHttpQueuedPipelinedCount counter declared as uint16_t can overflow when more
than 65535 pipelined responses are queued, causing it to wrap around to 0 while
responses remain queued, which breaks the logic in shouldCloseConnection() and
read-resumption checks. Fix this by either widening nodeHttpQueuedPipelinedCount
from uint16_t to uint32_t or size_t to accommodate larger queue depths, or by
adding saturation logic at the increment site in HttpContext.h to ensure the
counter never exceeds its maximum value (if the counter is less than UINT16_MAX
before incrementing, proceed with the increment; otherwise, cap it at
UINT16_MAX).
src/js/internal/tls.ts (1)
111-145: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don’t keep internal PFX CA state on a user-spoofable string key.

_pfxExtraCACerts is copied from user options and only overwritten when parsed PFX CAs exist; downstream TLS code then trusts that field as parsed CA material. Store it on a private exported symbol, or clear it unconditionally before assigning parsed values.
Suggested hardening
 function processPfxOptions(options) {
-  if (options == null || options.pfx == null) return options;
+  if (options == null || options.pfx == null) return options;
   NativeSecureContext ??= $zig("SecureContext.zig", "js.getConstructor");
   const out = { ...options };
+  out._pfxExtraCACerts = undefined;
   const keys = out.key == null ? [] : Array.isArray(out.key) ? [...out.key] : [out.key];
   const certs = out.cert == null ? [] : Array.isArray(out.cert) ? [...out.cert] : [out.cert];
   const pfxCAs = [];
A symbol shared through internal/tls would be stronger than a string key if consumers can be updated in this PR.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/js/internal/tls.ts` around lines 111 - 145, The processPfxOptions
function assigns parsed CA certificates to the string key _pfxExtraCACerts only
when pfxCAs has values, which means user-provided _pfxExtraCACerts from the
input options could be spoofed and persist downstream. Either create a private
exported symbol to use instead of the string key _pfxExtraCACerts when storing
the parsed CA values, or unconditionally clear the _pfxExtraCACerts field from
the output object before conditionally assigning the parsed values to ensure
downstream code only trusts actual parsed CA material.
src/js/node/_http_client.ts (1)
251-263: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Wire httpValidation into the parser flags.

The constructor validates and stores this.httpValidation, but the parser setup still derives leniency only from req.insecureHTTPParser; strict/relaxed/insecure therefore risks becoming an accepted no-op option instead of changing response parsing behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/js/node/_http_client.ts` around lines 251 - 263, The httpValidation
option is being validated and stored in this.httpValidation but is not being
wired into the actual parser configuration, making it a no-op option. Find where
the parser is being initialized with the insecureHTTPParser flag and update that
logic to also consider the httpValidation value. Map the httpValidation options
("strict", "relaxed", "insecure") to the appropriate parser leniency settings so
that the validation option actually affects the parsing behavior instead of just
being accepted and ignored.
src/js/node/_http_outgoing.ts (1)
110-132: ⚠️ Potential issue | 🔴 Critical
🧩 Analysis chain

🏁 Script executed:
#!/bin/bash
# Description: Inspect all httpValidation writers/readers to confirm values are validated
# before OutgoingMessage.prototype._isLenientHeaderValidation observes them.

rg -nP -C4 --type=ts '\bhttpValidation\b'
Repository: oven-sh/bun

Length of output: 3725

🏁 Script executed:
rg -nP 'httpValidation' src/js/node/_http_server.ts -A3 -B3
Repository: oven-sh/bun

Length of output: 37

🏁 Script executed:
fd -t f '.*http_server.*' src/js/node/ && rg -nP 'httpValidation' src/js/node/ --type=ts
Repository: oven-sh/bun

Length of output: 1434

🏁 Script executed:
cat -n src/js/node/_http_server.ts | head -300
Repository: oven-sh/bun

Length of output: 11709

🏁 Script executed:
sed -n '300,450p' src/js/node/_http_server.ts
Repository: oven-sh/bun

Length of output: 5082

🏁 Script executed:
rg -nP 'storeHTTPOptions' src/js/node/_http_server.ts -A20
Repository: oven-sh/bun

Length of output: 1838

🏁 Script executed:
sed -n '3304,3360p' src/js/node/_http_server.ts
Repository: oven-sh/bun

Length of output: 2433

🏁 Script executed:
sed -n '3360,3400p' src/js/node/_http_server.ts
Repository: oven-sh/bun

Length of output: 1705

🏁 Script executed:
sed -n '3400,3450p' src/js/node/_http_server.ts
Repository: oven-sh/bun

Length of output: 1281
ServerResponse path lacks validation for httpValidation — arbitrary values will be treated as lenient.

The server-side storeHTTPOptions() does not validate the httpValidation option (only insecureHTTPParser is validated), but the _isLenientHeaderValidation() helper reads this.req?.socket?.server?.httpValidation and treats any non-"strict" value as lenient. This violates the fail-closed requirement: a typo, null, or any garbage value bypasses strict header validation.

ClientRequest properly validates httpValidation against ["strict", "relaxed", "insecure"] before storage, but ServerResponse inherits an unvalidated value. Add validateOneOf() for httpValidation in storeHTTPOptions(), or explicitly allow-list only "relaxed" and "insecure" in the helper.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/js/node/_http_outgoing.ts` around lines 110 - 132, The
`_isLenientHeaderValidation()` method treats any non-"strict" value for
`httpValidation` as lenient, but the server-side `storeHTTPOptions()` function
does not validate the `httpValidation` option before it is stored. Add
validation for `httpValidation` in `storeHTTPOptions()` using `validateOneOf()`
to restrict it to only the allowed values ["strict", "relaxed", "insecure"],
similar to how `insecureHTTPParser` is currently validated. This ensures that
only valid, explicitly allowed values are accepted and prevents typos or garbage
values from bypassing strict header validation.
Source: Coding guidelines
src/js/node/_http_server.ts (6)
230-238: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don’t call the user-mutable parser shim method during cleanup.

socket.parser is exposed to userland, so parser.free() can be overwritten and throw during close/upgrade cleanup before parser.socket and tracked state are released. Call the internal no-op directly instead.
🛡️ Proposed fix
-  parser.free();
+  serverParserShimFree.$call(parser);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/js/node/_http_server.ts` around lines 230 - 238, In the
releaseServerParserShim function, replace the call to parser.free() with a call
to an internal no-op method instead. The parser.free() method is exposed to
userland and can be overwritten to throw errors, which could prevent proper
cleanup of parser.socket and tracked state during close/upgrade operations. Use
the internal implementation directly to ensure cleanup always completes
successfully.
316-343: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use the $isArray intrinsic for builtin tamper-resistance.

Line 342 routes TLS option normalization through user-overridable Array.isArray; this file already uses $isArray elsewhere for builtin-safe checks. As per coding guidelines, built-in JS modules must use $-prefixed intrinsics/private APIs instead of public globals on internal paths.
🛡️ Proposed fix
-      ca = ca == null ? pfxExtraCAs : Array.isArray(ca) ? [...ca, ...pfxExtraCAs] : [ca, ...pfxExtraCAs];
+      ca = ca == null ? pfxExtraCAs : $isArray(ca) ? [...ca, ...pfxExtraCAs] : [ca, ...pfxExtraCAs];
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/js/node/_http_server.ts` around lines 316 - 343, The ternary operator
that merges pfxExtraCAs with ca values uses the public Array.isArray function to
check if ca is an array, but this file should use the builtin-safe $isArray
intrinsic instead for tamper-resistance consistency. Replace Array.isArray(ca)
with $isArray(ca) in the ternary conditional expression on the line containing
the ca assignment that handles pfxExtraCAs merging.
Source: Coding guidelines

711-728: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Restore the HTTPS IncomingMessage flag before CONNECT early returns.

The CONNECT path returns before Line 817 restores setIsNextIncomingMessageHTTPS(prevIsNextIncomingMessageHTTPS), leaking the HTTPS state into later request construction after an HTTPS CONNECT request.
🐛 Proposed fix
-        const http_req = new RequestClass(kHandle, url, method, headersObject, headersArray, handle, hasBody, socket);
+        let http_req;
+        try {
+          http_req = new RequestClass(kHandle, url, method, headersObject, headersArray, handle, hasBody, socket);
+        } finally {
+          setIsNextIncomingMessageHTTPS(prevIsNextIncomingMessageHTTPS);
+        }
...
-        setIsNextIncomingMessageHTTPS(prevIsNextIncomingMessageHTTPS);
Also applies to: 746-778, 817-817
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/js/node/_http_server.ts` around lines 711 - 728, The CONNECT request
handling path contains early returns before the HTTPS IncomingMessage flag is
restored, causing the HTTPS state to leak into subsequent request construction.
In the CONNECT code path (around lines 746-778), locate all early return
statements and add a call to
setIsNextIncomingMessageHTTPS(prevIsNextIncomingMessageHTTPS) immediately before
each return to restore the previous HTTPS state, mirroring the restoration that
occurs at line 817 for the normal flow.
1172-1193: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Match the full HTTP/2 preface before reporting 24 parsed bytes.

isHttp2Preface() only checks the first 16 bytes but then reports bytesParsed = 24. This can classify a partial/prefix match as HPE_PAUSED_H2_UPGRADE with an impossible parsed length.
🐛 Proposed fix
 const kHttp2PrefaceStart = [
   0x50, 0x52, 0x49, 0x20, 0x2a, 0x20, 0x48, 0x54, 0x54, 0x50, 0x2f, 0x32, 0x2e, 0x30, 0x0d, 0x0a,
-]; // "PRI * HTTP/2.0\r\n"
+  0x0d, 0x0a, 0x53, 0x4d, 0x0d, 0x0a, 0x0d, 0x0a,
+]; // "PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/js/node/_http_server.ts` around lines 1172 - 1193, The isHttp2Preface
function only checks the first 16 bytes of the HTTP/2 preface but the code
reports bytesParsed = 24, creating a mismatch. Extend the kHttp2PrefaceStart
constant array to include the complete 24-byte HTTP/2 connection preface (the
current 16-byte "PRI * HTTP/2.0\r\n" string plus the additional 8 bytes that
follow it), then ensure the isHttp2Preface function validates against the full
24-byte preface before the error is reported with bytesParsed = 24.
2012-2016: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use the Set intrinsic for uniqueHeaders membership checks.

uniqueHeaders.has(key) routes header serialization through a user-overridable Set method. Use uniqueHeaders.$has(key) for builtin-safe behavior. As per coding guidelines, built-in JS modules must use private $ methods for internal Map/Set access.
🛡️ Proposed fix
-        if (valueLength >= 2 && (key === "cookie" || (uniqueHeaders != null && uniqueHeaders.has(key)))) {
+        if (valueLength >= 2 && (key === "cookie" || (uniqueHeaders != null && uniqueHeaders.$has(key)))) {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/js/node/_http_server.ts` around lines 2012 - 2016, In the header
serialization conditional check, replace the call to uniqueHeaders.has(key) with
the builtin-safe version uniqueHeaders.$has(key) to prevent user-overridable
behavior. This ensures that the membership check for uniqueHeaders uses the
native Set method rather than a potentially overridden one, in accordance with
coding guidelines for built-in JS modules.
Source: Coding guidelines

2410-2417: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard writableCorked for queued pipelined responses.

res.socket now returns null while queued, but writableCorked still dereferences it unconditionally. Reading res.writableCorked in a pipelined handler can throw.
🐛 Proposed fix
 Object.defineProperty(ServerResponse.prototype, "writableCorked", {
   get() {
-    return this.socket.writableCorked;
+    return this.socket?.writableCorked ?? 0;
   },
   set(_value) {},
 });
Also applies to: 2450-2453
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/js/node/_http_server.ts` around lines 2410 - 2417, The `writableCorked`
property getter unconditionally dereferences the socket, but for queued
pipelined responses where `res.socket` now returns `null`, this causes an error.
Add a guard check in the `writableCorked` getter (and the other related
locations mentioned at lines 2450-2453) similar to the one in the socket getter
that checks if `this[kPipelinedQueuedState]` is undefined before accessing the
socket. When it is a queued pipelined response, return an appropriate default
value (such as 0) instead of attempting to dereference the socket.
src/js/node/http2.ts (1)
150-155: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Keep maxHeaderSize and maxHeaderListSize aliased when both are provided.

The loop can leave localSettings.maxHeaderListSize and localSettings.maxHeaderSize with different pre-ACK values when both aliases are submitted. Since they represent the same SETTINGS id, mirror the value that will be serialized instead of exposing an impossible local state.
Proposed fix
-  if (submitted.maxHeaderListSize !== undefined && submitted.maxHeaderSize === undefined) {
+  if (submitted.maxHeaderListSize !== undefined) {
     target.maxHeaderSize = submitted.maxHeaderListSize;
-  } else if (submitted.maxHeaderSize !== undefined && submitted.maxHeaderListSize === undefined) {
+  } else if (submitted.maxHeaderSize !== undefined) {
     target.maxHeaderListSize = submitted.maxHeaderSize;
   }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/js/node/http2.ts` around lines 150 - 155, The current conditional logic
in the alias synchronization block only handles cases where one of
maxHeaderListSize or maxHeaderSize is undefined, but when both are provided,
they can end up with different values despite representing the same SETTINGS id.
Add an additional condition to handle the case where both
submitted.maxHeaderListSize and submitted.maxHeaderSize are defined, and
synchronize them to a single value (such as the one that will be serialized) to
ensure they remain aliased and prevent an impossible local state where they
differ.
src/js/node/net.ts (2)
566-569: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Validate the unwrapped SNI context before selecting it.

SNICallback is user-controlled, and any object with a truthy context property currently bypasses the native SecureContext check. That can pass a plain object/boolean into TLS resume instead of failing with "Invalid SNI context". As per coding guidelines, validate representation at every boundary.
Proposed fix
-  const innerContext = typeof context === "object" ? context.context : undefined;
-  if (innerContext) {
+  const NativeSecureContext = state.server?.[kNativeSecureContextCtor];
+  const innerContext = typeof context === "object" ? context.context : undefined;
+  if (NativeSecureContext && innerContext instanceof NativeSecureContext) {
     state.selected = innerContext;
-  } else if (state.server?.[kNativeSecureContextCtor] && context instanceof state.server[kNativeSecureContextCtor]) {
+  } else if (NativeSecureContext && context instanceof NativeSecureContext) {
     state.selected = context;
   } else {
     state.failed = new Error("Invalid SNI context");
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/js/node/net.ts` around lines 566 - 569, The code currently assigns an
unwrapped SNI context to state.selected without validating it is a legitimate
SecureContext object. When innerContext is extracted and truthy, add a
validation check to ensure it is an instance of
state.server[kNativeSecureContextCtor] (similar to the check done in the else-if
branch for the direct context parameter) before assigning it to state.selected.
If the innerContext fails this validation, the code should fall through to the
else-if branch or handle it appropriately to prevent invalid objects from
bypassing the SecureContext type check.
Source: Coding guidelines

1057-1062: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard the synthesized reset error against listener removal.

This branch checks listenerCount("error") before destroy(er), but the error emission can still be deferred. Mirror the nearby SocketEmitEndNT reset path and install a one-shot no-op listener before destroying, otherwise a listener removed between the check and emission can still crash as an uncaught error.
Proposed fix
         if (self.listenerCount("error") > 0) {
+          self.once("error", () => {});
           const er = new ConnResetException("read ECONNRESET") as Error & { errno?: number; syscall?: string };
           er.errno = process.platform === "win32" ? -4077 : process.platform === "linux" ? -104 : -54;
           er.syscall = "read";
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/js/node/net.ts` around lines 1057 - 1062, In the ConnResetException
handling block where listenerCount("error") is checked before calling
self.destroy(er), add a one-shot no-op listener to the "error" event before the
destroy call. This guards against the race condition where error listeners can
be removed between the listenerCount check and the deferred error emission.
Install this temporary listener using once("error", () => {}) pattern
immediately before self.destroy(er) to ensure there is always a listener to
handle the error, mirroring the approach used in the nearby SocketEmitEndNT
reset path.

coderabbitai

Actionable comments posted: 12

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

src/runtime/api/bun/h2_frame_parser.rs (1)

6237-6269: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Reject standard SETTINGS IDs in customSettings.

These entries are documented as non-standard, but IDs like 2 are accepted and serialized as SETTINGS_ENABLE_PUSH; values such as { customSettings: { 2: 2 } } put an invalid standard setting on the wire and trigger peer protocol errors. Reject IDs already handled by the standard SETTINGS fields.

Proposed validation guard

-                    if setting_id > 0xFFFF {
+                    if setting_id > 0xFFFF
+                        || matches!(setting_id, 0x1 | 0x2 | 0x3 | 0x4 | 0x5 | 0x6 | 0x8)
+                    {
                         return global_object
                             .err_http2_invalid_setting_value_range_error(
                                 "Invalid custom setting identifier",

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/runtime/api/bun/h2_frame_parser.rs` around lines 6237 - 6269, After the
existing range validation that checks if setting_id > 0xFFFF, add an additional
validation to reject standard HTTP/2 SETTINGS IDs (IDs 1-6) that should not be
allowed in customSettings. Insert a check that returns an error if the
setting_id falls within the standard range of reserved SETTINGS identifiers,
preventing invalid standard settings from being added to the custom_settings
collection via the with_mut call.

src/js/node/http2.ts (2)

2947-2955: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Make fd ownership per operation, not stream-global.

this[kOwnsFd] is sticky after respondWithFile(). If that call fails and onError recovers by calling respondWithFD(), the caller-owned fd is treated as owned here and can be closed via tryClose(), autoClose, or the stream close handler. Pass ownership into doSendFileFD()/afterOpen() instead of storing it on the stream.

Proposed direction

-function doSendFileFD(options, fd, headers, err, stat) {
+function doSendFileFD(ownsFd, options, fd, headers, err, stat) {
   const onError = options.onError;
-  const ownsFd = this[kOwnsFd] === true;
   if (err) {
     if (ownsFd && err.code !== "EBADF") {
       tryClose(fd);
@@
-    if (this[kOwnsFd] === true) tryClose(fd);
+    if (ownsFd) tryClose(fd);
@@
-    if (this[kOwnsFd] === true) tryClose(fd);
+    if (ownsFd) tryClose(fd);
@@
-    autoClose: this[kOwnsFd] === true,
+    autoClose: ownsFd,
@@
-function afterOpen(options, headers, err, fd) {
+function afterOpen(options, headers, err, fd) {
@@
-  fs.fstat(fd, doSendFileFD.bind(this, options, fd, headers));
+  fs.fstat(fd, doSendFileFD.bind(this, true, options, fd, headers));
 }
@@
-    this[kOwnsFd] = true;
     fs.open(path, "r", afterOpen.bind(this, options || {}, headers));
@@
-      fs.fstat(fd.fd, doSendFileFD.bind(this, options, fd, headers));
+      fs.fstat(fd.fd, doSendFileFD.bind(this, false, options, fd, headers));
     } else {
-      fs.fstat(fd, doSendFileFD.bind(this, options, fd, headers));
+      fs.fstat(fd, doSendFileFD.bind(this, false, options, fd, headers));
     }

Also applies to: 3035-3058, 3083-3086, 3285-3286, 3344-3354

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/js/node/http2.ts` around lines 2947 - 2955, The fd ownership is currently
stored as a sticky property on the stream object via this[kOwnsFd], which
persists incorrectly across multiple file operations. Instead of relying on this
stream-level property, refactor the code to pass the ownership information as a
parameter into the doSendFileFD() and afterOpen() functions. This way, each
operation tracks whether it owns the fd independently, preventing the
caller-owned fd from being incorrectly closed when a previous respondWithFile()
call failed and onError recovery attempts to use respondWithFD() with a
caller-provided fd.

Source: Coding guidelines

5411-5479: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Destroy open client streams synchronously during session destroy.

The server path uses destroyStreamForSessionDestroy() before emitErrorToAllStreams(), matching the helper’s invariant that writes immediately after session.destroy() observe destroyed streams. The client path only emits parser errors, whose handlers defer teardown, so open client streams can remain writable/readable for a tick after ClientHttp2Session.destroy() returns.

Proposed fix

     if (parser) {
       // node cancels streams still open when their session is destroyed: each gets
       // ERR_HTTP2_STREAM_CANCEL (or the session error when one was provided), with the CANCEL
       // rst code.
       if (this[kSessionDestroyError] == null && error == null) {
         this[kSessionDestroyError] = createPendingStreamCancelError();
       }
       // Like Node's Http2Stream._destroy: a received GOAWAY's code takes
       // precedence over the destroy code when streams are torn down.
-      parser.emitErrorToAllStreams(this[kGoawayCode] || (code !== undefined ? code : constants.NGHTTP2_CANCEL));
+      const streamRstCode = this[kGoawayCode] || (code !== undefined ? code : constants.NGHTTP2_CANCEL);
+      parser.forEachStream(
+        FunctionPrototypeBind.$call(
+          destroyStreamForSessionDestroy,
+          undefined,
+          this[kSessionDestroyError] || error,
+          streamRstCode,
+        ),
+      );
+      parser.emitErrorToAllStreams(streamRstCode);
       parser.detach();
     }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/js/node/http2.ts` around lines 5411 - 5479, The destroy() method only
emits parser errors to streams without synchronously destroying them first,
allowing open streams to remain writable/readable for a tick after destroy()
returns. Before calling parser.emitErrorToAllStreams() on the parser object, add
a call to destroyStreamForSessionDestroy() to synchronously destroy all open
streams immediately, matching the server path behavior and ensuring streams are
torn down during the destroy() call rather than being deferred.

♻️ Duplicate comments (1)

src/js/node/_http_server.ts (1)

1172-1194: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Require the full HTTP/2 preface before reporting HPE_PAUSED_H2_UPGRADE.

Line 1175 checks only the first 16 bytes, but Line 1193 reports the 24-byte HTTP/2 preface as parsed. A partial PRI * HTTP/2.0\r\n packet is misclassified as an HTTP/2 upgrade pause.

Proposed fix

-const kHttp2PrefaceStart = [
+const kHttp2Preface = [
   0x50, 0x52, 0x49, 0x20, 0x2a, 0x20, 0x48, 0x54, 0x54, 0x50, 0x2f, 0x32, 0x2e, 0x30, 0x0d, 0x0a,
-]; // "PRI * HTTP/2.0\r\n"
+  0x0d, 0x0a, 0x53, 0x4d, 0x0d, 0x0a, 0x0d, 0x0a,
+]; // "PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n"
 function isHttp2Preface(rawPacket: ArrayBuffer) {
-  if (rawPacket.byteLength < kHttp2PrefaceStart.length) return false;
-  const bytes = new Uint8Array(rawPacket, 0, kHttp2PrefaceStart.length);
-  for (let i = 0; i < kHttp2PrefaceStart.length; i++) {
-    if (bytes[i] !== kHttp2PrefaceStart[i]) return false;
+  if (rawPacket.byteLength < kHttp2Preface.length) return false;
+  const bytes = new Uint8Array(rawPacket, 0, kHttp2Preface.length);
+  for (let i = 0; i < kHttp2Preface.length; i++) {
+    if (bytes[i] !== kHttp2Preface[i]) return false;
   }
   return true;
 }

-    err.bytesParsed = 24;
+    err.bytesParsed = kHttp2Preface.length;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/js/node/_http_server.ts` around lines 1172 - 1194, The kHttp2PrefaceStart
array currently contains only the first 16 bytes of the HTTP/2 connection
preface, but the error handler in onServerClientError reports 24 bytes as parsed
when HPE_PAUSED_H2_UPGRADE is detected. Update the kHttp2PrefaceStart array to
include all 24 bytes of the complete HTTP/2 preface (the additional 8 bytes
after "PRI * HTTP/2.0\r\n") so that the isHttp2Preface function only returns
true when the full preface is present, preventing partial packets from being
misclassified as HTTP/2 upgrade attempts.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/js/internal/tls.ts`:
- Around line 126-128: The passphrase fallback logic in the per-entry pfx
handling is using strict undefined checking instead of falsy-fallback semantics,
which diverges from Node.js behavior. At the point where entryPassphrase is
checked (line 127), change the condition from checking if entryPassphrase is not
undefined to instead using a truthy check, so that falsy values like empty
strings, null, or false do not override the top-level passphrase and instead
fall back to it, matching Node.js v26.3.0 compatibility.

In `@src/js/node/_http_client.ts`:
- Around line 783-790: In the domain binding logic where reqDomain is checked
for the add function, the order of operations needs to be reversed. Currently,
res.domain is being assigned to reqDomain before calling reqDomain.add(res), but
this should be done in the opposite order. Move the reqDomain.add(res) call to
execute before the res.domain assignment to ensure that domain implementations
can properly register the response emitter without the add method returning
early.

In `@src/js/node/_http_server.ts`:
- Around line 1008-1023: The pipeline advancement calls at lines following the
isPipelined check, within the finish event listener setup, and at other
locations (around line 2228) are advancing the pipeline unconditionally even
when the response requires closing the connection. To fix this, add a condition
before each advanceResponsePipeline call to check if the connection should be
closed (e.g., by examining the Connection header, HTTP version compatibility, or
close-delimited status), and only call advanceResponsePipeline if the connection
is not required to close. Apply this check to all three locations mentioned: the
direct advanceResponsePipeline call after the handle.finished check, the
advanceResponsePipeline binding in the finish event listener, and the similar
calls around line 2228.
- Around line 2412-2416: Queued pipelined responses need to buffer informational
and flush operations instead of throwing or silently dropping them. Modify the
methods writeEarlyHints(), writeProcessing(), writeInformation(),
writeContinue(), and flushHeaders() to check if kPipelinedQueuedState is defined
and queue these operations (similar to how write/end operations are queued)
rather than attempting to call this.socket.write() which will fail on null
socket. Store the queued operations in a buffer and replay them after the socket
is assigned to the response during socket assignment.
- Around line 416-422: The issue is in the setupConnectionsTracking function
where the delay assignment uses the OR operator (||) which treats 0 as falsy and
defaults to 30_000, even though 0 is a valid value for
connectionsCheckingInterval. Replace the || operator with the nullish coalescing
operator (??) when assigning the delay constant so that only null or undefined
values trigger the fallback to 30_000, while explicitly set 0 values are
preserved as distinct from unset values.

In `@src/js/node/https.ts`:
- Around line 503-509: The createServer function spreads the options parameter
without validating it first, causing primitive inputs to be coerced into objects
instead of being rejected. Import validateObject from internal/validators at the
top of the file if not already present, then add a call to
validateObject(options, "options") immediately after the conditional check that
handles the function-as-first-argument case and before any object spread
operation. This ensures options is validated as an object before spreading,
matching Node.js behavior and preventing silent coercion of invalid inputs like
strings or numbers.

In `@src/runtime/api/bun/h2_frame_parser.rs`:
- Around line 6089-6092: The issue is that explicit_settings and custom_settings
are using persistent parser state that accumulates across multiple calls,
causing re-emission of previously sent settings even when omitted in the current
submission. Instead of using self.explicit_settings and self.custom_settings
directly in the number_setting! macro and when calling set_settings() and
write_settings_payload(), create local per-submission variables (a new
explicit_settings mask and custom_settings vector) that are initialized fresh
for each SETTINGS frame being parsed. Populate these local variables during the
current submission's parsing, then pass these local per-submission variables to
set_settings() and write_settings_payload() calls to ensure only the current
submission's settings are serialized, rather than the cumulative state.

In `@src/runtime/api/bun/h2/connection.rs`:
- Around line 1103-1112: The invalid frame budget comparison is checking the old
count value before incrementing, which causes the limit to trigger one frame
late. In the code block where self.invalid_frame_count is incremented (around
the saturating_add call), you must compare the new incremented value against
self.max_invalid_frames instead of comparing the original count variable. This
same off-by-one issue also appears in another location in the file around lines
1293-1298, so apply the same fix to both occurrences to ensure the budget is
enforced correctly from the first invalid frame.
- Around line 20-23: The issue is that when acknowledging SETTINGS, the code
pops the oldest submission from pending_local_settings_acks but then passes
self.local_settings to on_local_settings(), which can cause a later
unacknowledged submission to be incorrectly reported as acknowledged. Capture
the full Settings snapshot that was popped from pending_local_settings_acks
queue and pass that specific snapshot to the on_local_settings() method (which
appears to be called around lines 300-304 and 628-640) instead of passing
self.local_settings, ensuring the sink is notified with the exact acknowledged
SETTINGS snapshot.
- Around line 341-345: The `send_go_away()` method accepts a `code: ErrorCode`
parameter but never uses it, always hardcoding `wire::lib_error::PROTO` instead
when calling `local_connection_error()`. Since this is a public method that is
currently uncalled, fix the API contract by either removing the unused `code`
parameter from the function signature (and update the documentation comment to
clarify it only sends error GOAWAY frames with PROTO code), or update the
implementation to use the provided `code` parameter and pass it to
`local_connection_error()` instead of the hardcoded PROTO value. Choose the
approach that best fits your intended semantics for graceful versus error GOAWAY
frames.

In `@test/js/node/http2/h2-conformance.test.ts`:
- Line 558: Remove the dynamic require() statement for Writable from line 558
and add it to the module-scope imports at the top of the file alongside other
dependencies like http2 and net. Import Writable directly from node:stream at
the beginning of the file using the standard import pattern, maintaining any
necessary TypeScript type assertions for consistency with the existing import
style in this test file.

In `@test/js/node/test/common/index.js`:
- Around line 1319-1325: The getOptionValue method has a hardcoded return false
for the "--insecure-http-parser" case, but it should instead query the actual
flag state from process.execArgv to reflect flags parsed by parseTestFlags.
Replace the hardcoded false return with a check that determines whether
"--insecure-http-parser" exists in process.execArgv and returns true if present
or false if absent, ensuring tests that set this flag receive the correct value.

---

Outside diff comments:
In `@src/js/node/http2.ts`:
- Around line 2947-2955: The fd ownership is currently stored as a sticky
property on the stream object via this[kOwnsFd], which persists incorrectly
across multiple file operations. Instead of relying on this stream-level
property, refactor the code to pass the ownership information as a parameter
into the doSendFileFD() and afterOpen() functions. This way, each operation
tracks whether it owns the fd independently, preventing the caller-owned fd from
being incorrectly closed when a previous respondWithFile() call failed and
onError recovery attempts to use respondWithFD() with a caller-provided fd.
- Around line 5411-5479: The destroy() method only emits parser errors to
streams without synchronously destroying them first, allowing open streams to
remain writable/readable for a tick after destroy() returns. Before calling
parser.emitErrorToAllStreams() on the parser object, add a call to
destroyStreamForSessionDestroy() to synchronously destroy all open streams
immediately, matching the server path behavior and ensuring streams are torn
down during the destroy() call rather than being deferred.

In `@src/runtime/api/bun/h2_frame_parser.rs`:
- Around line 6237-6269: After the existing range validation that checks if
setting_id > 0xFFFF, add an additional validation to reject standard HTTP/2
SETTINGS IDs (IDs 1-6) that should not be allowed in customSettings. Insert a
check that returns an error if the setting_id falls within the standard range of
reserved SETTINGS identifiers, preventing invalid standard settings from being
added to the custom_settings collection via the with_mut call.

---

Duplicate comments:
In `@src/js/node/_http_server.ts`:
- Around line 1172-1194: The kHttp2PrefaceStart array currently contains only
the first 16 bytes of the HTTP/2 connection preface, but the error handler in
onServerClientError reports 24 bytes as parsed when HPE_PAUSED_H2_UPGRADE is
detected. Update the kHttp2PrefaceStart array to include all 24 bytes of the
complete HTTP/2 preface (the additional 8 bytes after "PRI * HTTP/2.0\r\n") so
that the isHttp2Preface function only returns true when the full preface is
present, preventing partial packets from being misclassified as HTTP/2 upgrade
attempts.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: fc379c9a-b81c-4cae-9235-55da0f8846c9

📥 Commits

Reviewing files that changed from the base of the PR and between d0b6fcc and 038565b.

📒 Files selected for processing (237)

packages/bun-usockets/src/context.c
packages/bun-usockets/src/crypto/openssl.c
packages/bun-usockets/src/internal/internal.h
packages/bun-usockets/src/libusockets.h
packages/bun-uws/src/App.h
packages/bun-uws/src/ChunkedEncoding.h
packages/bun-uws/src/HttpContext.h
packages/bun-uws/src/HttpContextData.h
packages/bun-uws/src/HttpErrors.h
packages/bun-uws/src/HttpParser.h
packages/bun-uws/src/HttpResponse.h
packages/bun-uws/src/HttpResponseData.h
src/js/internal/http.ts
src/js/internal/timers.ts
src/js/internal/tls.ts
src/js/node/_http_client.ts
src/js/node/_http_common.ts
src/js/node/_http_incoming.ts
src/js/node/_http_outgoing.ts
src/js/node/_http_server.ts
src/js/node/domain.ts
src/js/node/http2.ts
src/js/node/https.ts
src/js/node/net.ts
src/js/node/tls.ts
src/jsc/ErrorCode.rs
src/jsc/bindings/ErrorCode.ts
src/jsc/bindings/NodeHTTP.cpp
src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp
src/jsc/bindings/node/JSNodeHTTPServerSocket.h
src/jsc/bindings/node/JSNodeHTTPServerSocketPrototype.cpp
src/runtime/api/bun/h2/connection.rs
src/runtime/api/bun/h2/wire.rs
src/runtime/api/bun/h2_frame_parser.rs
src/runtime/api/h2.classes.ts
src/runtime/server/mod.rs
src/runtime/server/server_body.rs
src/uws_sys/App.rs
src/uws_sys/libuwsockets.cpp
test/expectations.txt
test/js/bun/test/parallel/test-http-host-array-should-throw-in-request.ts
test/js/bun/test/parallel/test-http-should-emit-timeout-event-when-using-server-setTimeout.ts
test/js/bun/test/parallel/test-http-should-emit-timeout-event.ts
test/js/node/http/node-http.test.ts
test/js/node/http2/h2-conformance.test.ts
test/js/node/http2/node-http2.test.js
test/js/node/net/node-net.test.ts
test/js/node/test/common/index.js
test/js/node/test/parallel/test-http-abort-stream-end.js
test/js/node/test/parallel/test-http-agent-domain-reused-gc.js
test/js/node/test/parallel/test-http-agent-keepalive-delay.js
test/js/node/test/parallel/test-http-agent-maxtotalsockets.js
test/js/node/test/parallel/test-http-agent-remove.js
test/js/node/test/parallel/test-http-allow-content-length-304.js
test/js/node/test/parallel/test-http-autoselectfamily.js
test/js/node/test/parallel/test-http-buffer-sanity.js
test/js/node/test/parallel/test-http-chunk-extensions-limit.js
test/js/node/test/parallel/test-http-chunk-problem.js
test/js/node/test/parallel/test-http-client-abort-keep-alive-queued-unix-socket.js
test/js/node/test/parallel/test-http-client-abort-unix-socket.js
test/js/node/test/parallel/test-http-client-close-with-default-agent.js
test/js/node/test/parallel/test-http-client-finished.js
test/js/node/test/parallel/test-http-client-immediate-error.js
test/js/node/test/parallel/test-http-client-keep-alive-hint.js
test/js/node/test/parallel/test-http-client-pipe-end.js
test/js/node/test/parallel/test-http-client-reject-unexpected-agent.js
test/js/node/test/parallel/test-http-client-request-options.js
test/js/node/test/parallel/test-http-client-response-domain.js
test/js/node/test/parallel/test-http-client-response-timeout.js
test/js/node/test/parallel/test-http-client-spurious-aborted.js
test/js/node/test/parallel/test-http-client-timeout-event.js
test/js/node/test/parallel/test-http-client-timeout-on-connect.js
test/js/node/test/parallel/test-http-client-timeout-option.js
test/js/node/test/parallel/test-http-client-timeout.js
test/js/node/test/parallel/test-http-client-with-create-connection.js
test/js/node/test/parallel/test-http-connect-req-res.js
test/js/node/test/parallel/test-http-connect.js
test/js/node/test/parallel/test-http-content-length-mismatch.js
test/js/node/test/parallel/test-http-correct-hostname.js
test/js/node/test/parallel/test-http-date-header.js
test/js/node/test/parallel/test-http-decoded-auth.js
test/js/node/test/parallel/test-http-double-content-length.js
test/js/node/test/parallel/test-http-dump-req-when-res-ends.js
test/js/node/test/parallel/test-http-early-hints-invalid-argument.js
test/js/node/test/parallel/test-http-end-throw-socket-handling.js
test/js/node/test/parallel/test-http-expect-handling.js
test/js/node/test/parallel/test-http-extra-response.js
test/js/node/test/parallel/test-http-flush-headers.js
test/js/node/test/parallel/test-http-flush-response-headers.js
test/js/node/test/parallel/test-http-generic-streams.js
test/js/node/test/parallel/test-http-head-throw-on-response-body-write.js
test/js/node/test/parallel/test-http-header-badrequest.js
test/js/node/test/parallel/test-http-header-obstext.js
test/js/node/test/parallel/test-http-header-read.js
test/js/node/test/parallel/test-http-header-value-relaxed.js
test/js/node/test/parallel/test-http-highwatermark.js
test/js/node/test/parallel/test-http-host-headers.js
test/js/node/test/parallel/test-http-hostname-typechecking.js
test/js/node/test/parallel/test-http-incoming-pipelined-socket-destroy.js
test/js/node/test/parallel/test-http-insecure-parser-per-stream.js
test/js/node/test/parallel/test-http-insecure-parser.js
test/js/node/test/parallel/test-http-invalidheaderfield.js
test/js/node/test/parallel/test-http-invalidheaderfield2.js
test/js/node/test/parallel/test-http-keep-alive-drop-requests.js
test/js/node/test/parallel/test-http-keep-alive-empty-line.mjs
test/js/node/test/parallel/test-http-keep-alive-max-requests.js
test/js/node/test/parallel/test-http-localaddress.js
test/js/node/test/parallel/test-http-many-ended-pipelines.js
test/js/node/test/parallel/test-http-max-header-size-per-stream.js
test/js/node/test/parallel/test-http-max-http-headers.js
test/js/node/test/parallel/test-http-multiple-headers.js
test/js/node/test/parallel/test-http-no-read-no-dump.js
test/js/node/test/parallel/test-http-outgoing-drain-writable-length.js
test/js/node/test/parallel/test-http-outgoing-finished.js
test/js/node/test/parallel/test-http-outgoing-proto.js
test/js/node/test/parallel/test-http-outgoing-renderHeaders.js
test/js/node/test/parallel/test-http-parser-finish-error.js
test/js/node/test/parallel/test-http-parser-free.js
test/js/node/test/parallel/test-http-parser-freed-before-upgrade.js
test/js/node/test/parallel/test-http-parser-freed-during-execute.js
test/js/node/test/parallel/test-http-parser-memory-retention.js
test/js/node/test/parallel/test-http-parser-multiple-execute.js
test/js/node/test/parallel/test-http-parser-timeout-reset.js
test/js/node/test/parallel/test-http-parser.js
test/js/node/test/parallel/test-http-pause.js
test/js/node/test/parallel/test-http-pipeline-assertionerror-finish.js
test/js/node/test/parallel/test-http-pipeline-flood.js
test/js/node/test/parallel/test-http-pipeline-outgoing-destroy.js
test/js/node/test/parallel/test-http-proxy.js
test/js/node/test/parallel/test-http-raw-headers.js
test/js/node/test/parallel/test-http-readable-data-event.js
test/js/node/test/parallel/test-http-req-close-robust-from-tampering.js
test/js/node/test/parallel/test-http-req-res-close.js
test/js/node/test/parallel/test-http-request-end-twice.js
test/js/node/test/parallel/test-http-request-end.js
test/js/node/test/parallel/test-http-request-method-delete-payload.js
test/js/node/test/parallel/test-http-response-add-header-after-sent.js
test/js/node/test/parallel/test-http-response-readable.js
test/js/node/test/parallel/test-http-response-remove-header-after-sent.js
test/js/node/test/parallel/test-http-response-setheaders.js
test/js/node/test/parallel/test-http-response-status-message.js
test/js/node/test/parallel/test-http-response-statuscode.js
test/js/node/test/parallel/test-http-response-writehead-returns-this.js
test/js/node/test/parallel/test-http-same-map.js
test/js/node/test/parallel/test-http-server-client-error.js
test/js/node/test/parallel/test-http-server-close-all.js
test/js/node/test/parallel/test-http-server-close-idle-wait-response.js
test/js/node/test/parallel/test-http-server-close-idle.js
test/js/node/test/parallel/test-http-server-connection-list-when-close.js
test/js/node/test/parallel/test-http-server-destroy-socket-on-client-error.js
test/js/node/test/parallel/test-http-server-drop-connections-in-cluster.js
test/js/node/test/parallel/test-http-server-headers-timeout-delayed-headers.js
test/js/node/test/parallel/test-http-server-headers-timeout-interrupted-headers.js
test/js/node/test/parallel/test-http-server-headers-timeout-keepalive.js
test/js/node/test/parallel/test-http-server-headers-timeout-pipelining.js
test/js/node/test/parallel/test-http-server-keep-alive-timeout.js
test/js/node/test/parallel/test-http-server-keepalive-end.js
test/js/node/test/parallel/test-http-server-method.query.js
test/js/node/test/parallel/test-http-server-multiheaders.js
test/js/node/test/parallel/test-http-server-multiple-client-error.js
test/js/node/test/parallel/test-http-server-non-utf8-header.js
test/js/node/test/parallel/test-http-server-options-highwatermark.js
test/js/node/test/parallel/test-http-server-options-incoming-message.js
test/js/node/test/parallel/test-http-server-options-server-response.js
test/js/node/test/parallel/test-http-server-reject-chunked-with-content-length.js
test/js/node/test/parallel/test-http-server-reject-cr-no-lf.js
test/js/node/test/parallel/test-http-server-request-timeout-delayed-body.js
test/js/node/test/parallel/test-http-server-request-timeout-delayed-headers.js
test/js/node/test/parallel/test-http-server-request-timeout-interrupted-body.js
test/js/node/test/parallel/test-http-server-request-timeout-interrupted-headers.js
test/js/node/test/parallel/test-http-server-request-timeout-keepalive.js
test/js/node/test/parallel/test-http-server-request-timeout-pipelining.js
test/js/node/test/parallel/test-http-server-request-timeout-upgrade.js
test/js/node/test/parallel/test-http-server-stale-close.js
test/js/node/test/parallel/test-http-server-unconsume.js
test/js/node/test/parallel/test-http-server.js
test/js/node/test/parallel/test-http-set-cookies.js
test/js/node/test/parallel/test-http-set-header-chain.js
test/js/node/test/parallel/test-http-set-timeout-server.js
test/js/node/test/parallel/test-http-set-timeout.js
test/js/node/test/parallel/test-http-set-trailers.js
test/js/node/test/parallel/test-http-socket-encoding-error.js
test/js/node/test/parallel/test-http-status-code.js
test/js/node/test/parallel/test-http-status-message.js
test/js/node/test/parallel/test-http-timeout-overflow.js
test/js/node/test/parallel/test-http-transfer-encoding-repeated-chunked.js
test/js/node/test/parallel/test-http-unix-socket.js
test/js/node/test/parallel/test-http-upgrade-server-callback.js
test/js/node/test/parallel/test-http-upgrade-server-with-body-and-extras.mjs
test/js/node/test/parallel/test-http-upgrade-server-with-body-error.mjs
test/js/node/test/parallel/test-http-upgrade-server-with-body.mjs
test/js/node/test/parallel/test-http-upgrade-server-with-large-body-unread.mjs
test/js/node/test/parallel/test-http-upgrade-server-with-large-body.mjs
test/js/node/test/parallel/test-http-url.parse-basic.js
test/js/node/test/parallel/test-http-url.parse-https.request.js
test/js/node/test/parallel/test-http-write-callbacks.js
test/js/node/test/parallel/test-http-zero-length-write.js
test/js/node/test/parallel/test-https-agent-additional-options.js
test/js/node/test/parallel/test-https-agent-keylog.js
test/js/node/test/parallel/test-https-agent-session-eviction.js
test/js/node/test/parallel/test-https-agent-sni.js
test/js/node/test/parallel/test-https-agent.js
test/js/node/test/parallel/test-https-argument-of-creating.js
test/js/node/test/parallel/test-https-autoselectfamily.js
test/js/node/test/parallel/test-https-byteswritten.js
test/js/node/test/parallel/test-https-client-renegotiation-limit.js
test/js/node/test/parallel/test-https-insecure-parse-per-stream.js
test/js/node/test/parallel/test-https-keep-alive-drop-requests.js
test/js/node/test/parallel/test-https-localaddress.js
test/js/node/test/parallel/test-https-max-header-size-per-stream.js
test/js/node/test/parallel/test-https-max-headers-count.js
test/js/node/test/parallel/test-https-options-boolean-check.js
test/js/node/test/parallel/test-https-pfx.js
test/js/node/test/parallel/test-https-resume-after-renew.js
test/js/node/test/parallel/test-https-selfsigned-no-keycertsign-no-crash.js
test/js/node/test/parallel/test-https-server-close-all.js
test/js/node/test/parallel/test-https-server-close-idle.js
test/js/node/test/parallel/test-https-set-timeout-server.js
test/js/node/test/parallel/test-https-strict.js
test/js/node/test/parallel/test-https-timeout-server-2.js
test/js/node/test/parallel/test-https-timeout-server.js
test/js/node/test/parallel/test-https-unix-socket-self-signed.js
test/js/node/test/parallel/test-tls-options-boolean-check.js
test/js/node/test/sequential/test-http-econnrefused.js
test/js/node/test/sequential/test-http-keep-alive-large-write.js
test/js/node/test/sequential/test-http-regr-gh-2928.js
test/js/node/test/sequential/test-http-server-keep-alive-timeout-slow-client-headers.js
test/js/node/test/sequential/test-http-server-keep-alive-timeout-slow-server.js
test/js/node/test/sequential/test-http-server-request-timeouts-mixed.js
test/js/node/test/sequential/test-http2-max-session-memory.js
test/js/node/test/sequential/test-http2-ping-flood.js
test/js/node/test/sequential/test-http2-settings-flood.js
test/js/node/test/sequential/test-http2-timeout-large-write-file.js
test/js/node/test/sequential/test-http2-timeout-large-write.js
test/js/node/test/sequential/test-https-connect-localport.js
test/js/node/test/sequential/test-https-server-keep-alive-timeout.js
test/regression/issue/25190.test.ts

💤 Files with no reviewable changes (5)

test/js/node/test/parallel/test-http-client-with-create-connection.js
test/js/node/test/parallel/test-http-client-response-timeout.js
test/js/node/test/parallel/test-https-unix-socket-self-signed.js
test/js/node/test/parallel/test-http-unix-socket.js
test/js/node/test/parallel/test-http-client-pipe-end.js

coderabbitai

Actionable comments posted: 16

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)

src/runtime/api/bun/h2_frame_parser.rs (1)

6242-6274: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Reject standard SETTINGS IDs in customSettings.

Line 6255 only checks <= 0xffff, so customSettings: { "4": 1 } emits SETTINGS_INITIAL_WINDOW_SIZE without updating local_settings or the pending ACK metadata. That can desynchronize Bun’s flow-control/header/frame-size state from what was actually sent on the wire. Also reject non-finite or fractional values before the as u32 cast.

As per coding guidelines, “Validate untrusted input BEFORE any processing, allocation, or side effect.”

Suggested fix

                     if setting_id > 0xFFFF {
                         return global_object
                             .err_http2_invalid_setting_value_range_error(
                                 "Invalid custom setting identifier",
                             )
                             .throw();
                     }
+                    if matches!(setting_id, 0x1 | 0x2 | 0x3 | 0x4 | 0x5 | 0x6 | 0x8) {
+                        return global_object
+                            .err_http2_invalid_setting_value_range_error(
+                                "Invalid custom setting identifier",
+                            )
+                            .throw();
+                    }
 
                     // Validate setting value is in range [0, 2^32-1]
                     let setting_value = iter.value;
                     if setting_value.is_number() {
                         let value = setting_value.as_number();
-                        if value < 0.0 || value > MAX_HEADER_TABLE_SIZE_F64 {
+                        if !value.is_finite()
+                            || value.fract() != 0.0
+                            || value < 0.0
+                            || value > MAX_HEADER_TABLE_SIZE_F64
+                        {
                             return global_object
                                 .err_http2_invalid_setting_value_range_error(
                                     "Invalid custom setting value",
                                 )
                                 .throw();
                         }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/runtime/api/bun/h2_frame_parser.rs` around lines 6242 - 6274, The
customSettings validation is incomplete and allows standard HTTP/2 SETTINGS IDs
(0-5) to be passed through, which can desynchronize Bun's internal state. After
parsing setting_id and validating it is within [0, 0xFFFF], add an additional
check to reject any setting_id values that are in the reserved standard range
(0-5). Additionally, before casting setting_value to u32, add validation to
reject non-finite values (check for NaN and Infinity using appropriate float
methods) and reject fractional values (check if the value differs from its
truncated integer form) to prevent silent data loss during the as u32 cast in
the staged_custom.push call.

Source: Coding guidelines

src/js/node/http2.ts (3)

3020-3058: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Handle zero-length file ranges before creating the read stream.

For an empty file, or offset at/past EOF, statOptions.length can become 0 or negative, so Line 3057 computes end as offset - 1 after already setting Content-Length. That turns a valid empty response into a read-stream range error or an invalid negative length. Clamp remaining bytes to 0 and finish the native stream without creating a file stream when there is no body.

Proposed fix

   if (stat.isFile()) {
+    const remaining = Math.max(0, stat.size - +statOptions.offset);
     statOptions.length =
       statOptions.length < 0
-        ? stat.size - +statOptions.offset
-        : Math.min(stat.size - +statOptions.offset, statOptions.length);
+        ? remaining
+        : Math.min(remaining, statOptions.length);
@@
   const finishNativeStream = closeWritableForFileResponse(this);
+
+  if (statOptions.length === 0) {
+    if (ownsFd) tryClose(fd);
+    finishNativeStream(() => {});
+    return;
+  }
 
   const stream = this;
   const fileStream = fs.createReadStream(null, {
@@
-    end: typeof statOptions.length === "number" ? statOptions.length + (statOptions.offset || 0) - 1 : undefined,
+    end: typeof statOptions.length === "number" ? statOptions.length + (statOptions.offset || 0) - 1 : undefined,

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/js/node/http2.ts` around lines 3020 - 3058, The issue is that when
statOptions.length becomes zero or negative (for empty files or when offset
exceeds file size), the code still creates a read stream with an invalid end
value calculated on line 3057, causing range errors. After computing
statOptions.length on lines 3020-3023, add a check: if statOptions.length is
less than or equal to 0, call the finishNativeStream() function to properly end
the response without creating the fileStream, then return early. This ensures
zero-length responses are handled correctly without attempting to create invalid
read stream ranges.

5469-5480: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Destroy open client streams synchronously before detaching the parser.

ServerHttp2Session.destroy() uses destroyStreamForSessionDestroy() before emitErrorToAllStreams(), but the client path only emits native errors and detaches. Open client streams can remain writable until the async native teardown runs, so a write immediately after session.destroy() can observe a live stream instead of Node’s synchronous cancelled/destroyed state.

Proposed fix

     const parser = this.#parser;
     if (parser) {
@@
-      parser.emitErrorToAllStreams(this[kGoawayCode] || (code !== undefined ? code : constants.NGHTTP2_CANCEL));
+      const streamRstCode = this[kGoawayCode] || (code !== undefined ? code : constants.NGHTTP2_CANCEL);
+      const streamDestroyError = this[kSessionDestroyError] ?? error;
+      parser.forEachStream(
+        FunctionPrototypeBind.$call(destroyStreamForSessionDestroy, undefined, streamDestroyError, streamRstCode),
+      );
+      parser.emitErrorToAllStreams(streamRstCode);
       parser.detach();
     }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/js/node/http2.ts` around lines 5469 - 5480, The client stream destruction
path is not synchronously destroying open streams before detaching the parser,
unlike the ServerHttp2Session.destroy() implementation which calls
destroyStreamForSessionDestroy() before emitErrorToAllStreams(). Add a
synchronous destruction of all open client streams using
destroyStreamForSessionDestroy() before the parser.emitErrorToAllStreams() call
to ensure streams transition to a cancelled/destroyed state immediately rather
than remaining writable during asynchronous native teardown.

2947-3058: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Keep fd ownership per response operation, not on the stream.

Line 3285 leaves kOwnsFd set on the stream. If respondWithFile() fails or statCheck cancels and user code falls back to respondWithFD() on the same stream, the caller-owned fd is treated as owned and can be closed by Line 3055 or the error paths. Pass ownership through the async callback chain instead of storing stale mutable state on the stream.

Proposed fix

-function doSendFileFD(options, fd, headers, err, stat) {
+function doSendFileFD(ownsFd, options, fd, headers, err, stat) {
   const onError = options.onError;
-  const ownsFd = this[kOwnsFd] === true;
@@
-    if (this[kOwnsFd] === true) tryClose(fd);
+    if (ownsFd) tryClose(fd);
@@
-    if (this[kOwnsFd] === true) tryClose(fd);
+    if (ownsFd) tryClose(fd);
@@
-    autoClose: this[kOwnsFd] === true,
+    autoClose: ownsFd,

-  fs.fstat(fd, doSendFileFD.bind(this, options, fd, headers));
+  fs.fstat(fd, doSendFileFD.bind(this, true, options, fd, headers));

-    this[kOwnsFd] = true;
     fs.open(path, "r", afterOpen.bind(this, options || {}, headers));

-      fs.fstat(fd.fd, doSendFileFD.bind(this, options, fd, headers));
+      fs.fstat(fd.fd, doSendFileFD.bind(this, false, fd, headers));
     } else {
-      fs.fstat(fd, doSendFileFD.bind(this, options, fd, headers));
+      fs.fstat(fd, doSendFileFD.bind(this, false, fd, headers));
     }

Also applies to: 3285-3354

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/js/node/http2.ts` around lines 2947 - 3058, The `kOwnsFd` flag is being
stored as persistent mutable state on the stream, which causes problems when
multiple response operations occur on the same stream (e.g., if
`respondWithFile()` fails and user code falls back to `respondWithFD()`).
Instead of checking `this[kOwnsFd]` in the `doSendFileFD` function and relying
on stream-level state, pass the fd ownership information through the async
callback chain as a parameter. This ensures each response operation
independently tracks whether it owns the fd, rather than inheriting stale
ownership state from previous failed operations. Remove the stream-level
`kOwnsFd` assignment and refactor the callback signatures to carry the ownership
flag through the operation sequence.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/bun-uws/src/HttpParser.h`:
- Around line 1105-1114: The chunk-extension overflow validation in the HTTP
parser is currently performed after the dataHandler is called, allowing
oversized chunk extensions to be processed and delivered to user code before
rejection. Move the check for chunkedExtensionsByteCount >
MAX_CHUNK_EXTENSION_SIZE to occur before the dataHandler call in the
ChunkIterator loops, so untrusted input is validated before side effects. Apply
this change to all three affected loop locations (around lines 1105, 1179, and
1248) while keeping the existing post-loop overflow guard for fragmented
extension lines that don't yield a complete chunk.
- Line 981: The current comparison using std::max<uint64_t>(MAX_FALLBACK_SIZE,
maxHeaderSize) enforces MAX_FALLBACK_SIZE as a floor value, which prevents
respecting smaller maxHeaderSize configurations. Replace this comparison logic
at line 981 and the additional occurrences around lines 1165-1168 to treat
maxHeaderSize of 0 as the default (using MAX_FALLBACK_SIZE), but otherwise honor
the configured maxHeaderSize cap. Use a conditional expression (ternary
operator) to check if maxHeaderSize is non-zero and use it directly, or fall
back to MAX_FALLBACK_SIZE only when maxHeaderSize equals 0, ensuring
user-configured resource limits are actually enforced rather than silently
overridden by the larger default.

In `@src/js/internal/http.ts`:
- Around line 194-199: The trailer handling code at the EOF path is accessing
the socket handle through the public user-visible self.socket property, which
can be replaced or intercepted with a getter that throws or skips trailers.
Instead of reading socketHandle from self.socket?.[kHandle], capture and store
the socket handle as a private field at object construction time (before the
request is exposed to user code), or access it directly via a native binding.
Update the code to use the privately stored socket handle rather than
dereferencing through the public socket property to ensure internal logic cannot
be subverted by user code overriding the socket property.

In `@src/js/node/_http_client.ts`:
- Around line 251-263: The httpValidation option is validated and stored in the
constructor but is never actually used when initializing the parser in the
tickOnSocket() method. The parser initialization code only references
insecureHTTPParser and the existing lenient logic, meaning the "relaxed" and
"insecure" modes for httpValidation have no effect on parsing behavior. Either
add logic in the parser initialization section (around lines 906-912 where
initialize() is called) to map the this.httpValidation modes to appropriate
parser flags alongside the existing lenient flag handling, or alternatively,
update the validation code to throw an error if httpValidation is set to
"relaxed" or "insecure" since those modes are not currently supported.

In `@src/js/node/_http_outgoing.ts`:
- Around line 106-133: The _isLenientHeaderValidation function is vulnerable to
prototype pollution because it uses regular property access that accepts
inherited values. To fix this, replace all property lookups for httpValidation
and insecureHTTPParser with own-property checks using
Object.prototype.hasOwnProperty.call() or Object.hasOwn(). This applies to
checks on this object, this.req?.socket?.server object, and any other objects
being accessed. Ensure that only values that are directly owned properties (not
inherited from the prototype chain) are used to determine the lenient validation
behavior.

In `@src/js/node/_http_server.ts`:
- Around line 2412-2416: The `writableCorked` property is throwing an error for
queued pipelined responses instead of returning a neutral cork count. Add a
guard check at the beginning of the `writableCorked` property implementation
that checks if `this[kPipelinedQueuedState] !== undefined` (matching the pattern
used for the socket check in the same section), and return 0 as the neutral cork
count for queued responses. Apply this same guard pattern to the related code at
lines 2450-2453 to ensure consistency across all cork-related operations.
- Around line 1172-1193: The kHttp2PrefaceStart array currently contains only
the first 16 bytes of the HTTP/2 connection preface, but the onServerClientError
function reports a 24-byte preface when returning HPE_PAUSED_H2_UPGRADE. This
causes the isHttp2Preface function to incorrectly match partial prefaces. Extend
the kHttp2PrefaceStart array to include the complete 24-byte HTTP/2 connection
preface by adding the remaining 8 bytes that follow "PRI * HTTP/2.0\r\n",
ensuring the matcher accurately identifies the full preface before the error is
reported.
- Around line 230-238: The releaseServerParserShim function calls parser.free()
directly, but since socket.parser is exposed to user code, users can replace or
tamper with the free method before teardown, causing the internal cleanup to
throw. Instead of calling the user-tamperable parser.free() method, use a
private symbol to store a reference to the actual internal parser release
function during parser creation, and call that private symbol reference directly
in releaseServerParserShim to ensure reliable teardown regardless of user code
modifications.

In `@src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp`:
- Around line 408-416: The responses in the local pipelined vector become
unrooted when socket->m_pipelinedResponses is cleared via std::exchange before
Bun__NodeHTTPResponse_onClose callbacks execute, creating a GC safety issue if
GC occurs during callbacks. Either prevent clearing m_pipelinedResponses by
using a separate boolean flag (like m_notifyingResponses) to prevent reentrant
delivery while keeping the vector rooted and attached to the socket, or wrap the
pipelined responses in a GC-rooted Strong holder before invoking the callbacks
so they remain protected during callback execution.
- Around line 175-178: The trailer name and value strings on lines 175 and 177
are being constructed using fromUTF8ReplacingInvalidSequences(), which corrupts
raw HTTP trailer bytes (particularly obs-text bytes 0x80–0xFF) by replacing
invalid UTF-8 sequences with U+FFFD. Replace both
fromUTF8ReplacingInvalidSequences() calls with a Latin-1 string constructor that
creates a WTF::String directly from the raw bytes while preserving the exact
byte values as-is, mapping each byte 0–255 directly to Unicode code points
U+0000–U+00FF. Keep the same reinterpret_cast and data access patterns, but use
the appropriate Latin-1 constructor method instead of the UTF-8 replacement
function.

In `@src/jsc/bindings/node/JSNodeHTTPServerSocketPrototype.cpp`:
- Around line 108-111: Replace the `isTrue()` method call with
`toBoolean(globalObject)` in the argument being passed to the
`upgradeToTunnelMode` method. The current code uses
`callFrame->argument(0).isTrue()` which only accepts the literal boolean `true`,
but the documented behavior specifies "with a truthy argument," meaning any
JavaScript truthy value (numbers, strings, objects, etc.) should be accepted.
Change it to `callFrame->argument(0).toBoolean(globalObject)` to properly coerce
any truthy value to a boolean, matching the standard pattern used consistently
throughout the codebase.
- Around line 162-163: The conversion of headersTimeout and requestTimeout from
double to uint64_t lacks upper-bound checking, which can lead to undefined
behavior when a finite double value exceeds uint64_t::max and is cast. Add
defensive clamping to std::numeric_limits<uint64_t>::max() before casting both
headersTimeout and requestTimeout in the assignments to headersTimeoutMs and
requestTimeoutMs respectively, ensuring that any finite double value above the
maximum uint64_t is clamped to the maximum representable uint64_t value rather
than causing undefined behavior during the cast.

In `@src/jsc/bindings/NodeHTTP.cpp`:
- Around line 41-43: The JSFunction registration for jsHTTPSetCustomOptions at
line 1188 declares an incorrect arity of 2, while the actual C++ function
implementation asserts argumentCount() == 7 and the JavaScript call site passes
7 arguments. Locate the JSC::JSFunction::create call that registers
jsHTTPSetCustomOptions and change the arity parameter from 2 to 7 to match the
expected argument count of server, requireHostHeader, useStrictMethodValidation,
insecureHTTPParser, maxHeaderSize, onClientError, and onConnection.

In `@src/runtime/api/bun/h2_frame_parser.rs`:
- Around line 6286-6317: The SETTINGS state is being committed too early in the
update_settings method. The calls to self.local_settings.set(),
self.explicit_settings.set(), and self.custom_settings.with_mut() should be
moved to execute after the remoteCustomSettings parsing block completes, not
before. Currently, if the remoteCustomSettings array iteration or property
access fails (lines 6300-6317), the function returns an error but the staged
settings have already been installed, leaving the system in an inconsistent
state where the SETTINGS frame was never sent. Reorganize the code so that the
settings commit operations execute only after all fallible operations like the
remoteCustomSettings parsing are completed successfully.

In `@src/runtime/api/bun/h2/connection.rs`:
- Around line 618-631: The condition checking for unsolicited SETTINGS ACKs in
the block starting around line 620 has a logic error. Currently it only returns
false when both local_settings_acked is true AND pending_local_settings_acks is
empty, which allows the first unsolicited ACK to incorrectly pass through when
local_settings_acked is false. Fix this by simplifying the condition to return
false whenever pending_local_settings_acks is empty, regardless of the
local_settings_acked state. This ensures that ACKs are only processed when there
are actual pending settings to acknowledge.

In `@test/js/node/http2/h2-conformance.test.ts`:
- Around line 564-571: The write method in the Writable object uses
setTimeout(cb, 1) which introduces timing-sensitive behavior that can cause
flakiness under load. Replace this setTimeout call with setImmediate(cb) or
process.nextTick(cb) to provide a non-time-based async yield that maintains the
same backpressure simulation intent without relying on wall-clock timing.

---

Outside diff comments:
In `@src/js/node/http2.ts`:
- Around line 3020-3058: The issue is that when statOptions.length becomes zero
or negative (for empty files or when offset exceeds file size), the code still
creates a read stream with an invalid end value calculated on line 3057, causing
range errors. After computing statOptions.length on lines 3020-3023, add a
check: if statOptions.length is less than or equal to 0, call the
finishNativeStream() function to properly end the response without creating the
fileStream, then return early. This ensures zero-length responses are handled
correctly without attempting to create invalid read stream ranges.
- Around line 5469-5480: The client stream destruction path is not synchronously
destroying open streams before detaching the parser, unlike the
ServerHttp2Session.destroy() implementation which calls
destroyStreamForSessionDestroy() before emitErrorToAllStreams(). Add a
synchronous destruction of all open client streams using
destroyStreamForSessionDestroy() before the parser.emitErrorToAllStreams() call
to ensure streams transition to a cancelled/destroyed state immediately rather
than remaining writable during asynchronous native teardown.
- Around line 2947-3058: The `kOwnsFd` flag is being stored as persistent
mutable state on the stream, which causes problems when multiple response
operations occur on the same stream (e.g., if `respondWithFile()` fails and user
code falls back to `respondWithFD()`). Instead of checking `this[kOwnsFd]` in
the `doSendFileFD` function and relying on stream-level state, pass the fd
ownership information through the async callback chain as a parameter. This
ensures each response operation independently tracks whether it owns the fd,
rather than inheriting stale ownership state from previous failed operations.
Remove the stream-level `kOwnsFd` assignment and refactor the callback
signatures to carry the ownership flag through the operation sequence.

In `@src/runtime/api/bun/h2_frame_parser.rs`:
- Around line 6242-6274: The customSettings validation is incomplete and allows
standard HTTP/2 SETTINGS IDs (0-5) to be passed through, which can desynchronize
Bun's internal state. After parsing setting_id and validating it is within [0,
0xFFFF], add an additional check to reject any setting_id values that are in the
reserved standard range (0-5). Additionally, before casting setting_value to
u32, add validation to reject non-finite values (check for NaN and Infinity
using appropriate float methods) and reject fractional values (check if the
value differs from its truncated integer form) to prevent silent data loss
during the as u32 cast in the staged_custom.push call.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 10576267-97f5-4bcd-871c-84dafac1a4b2

📥 Commits

Reviewing files that changed from the base of the PR and between 038565b and 0713d35.

📒 Files selected for processing (223)

packages/bun-usockets/src/context.c
packages/bun-usockets/src/crypto/openssl.c
packages/bun-usockets/src/internal/internal.h
packages/bun-usockets/src/libusockets.h
packages/bun-uws/src/App.h
packages/bun-uws/src/ChunkedEncoding.h
packages/bun-uws/src/HttpContext.h
packages/bun-uws/src/HttpContextData.h
packages/bun-uws/src/HttpErrors.h
packages/bun-uws/src/HttpParser.h
packages/bun-uws/src/HttpResponse.h
packages/bun-uws/src/HttpResponseData.h
src/js/internal/http.ts
src/js/internal/timers.ts
src/js/internal/tls.ts
src/js/node/_http_client.ts
src/js/node/_http_common.ts
src/js/node/_http_incoming.ts
src/js/node/_http_outgoing.ts
src/js/node/_http_server.ts
src/js/node/domain.ts
src/js/node/http2.ts
src/js/node/https.ts
src/js/node/net.ts
src/js/node/tls.ts
src/jsc/ErrorCode.rs
src/jsc/bindings/ErrorCode.ts
src/jsc/bindings/NodeHTTP.cpp
src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp
src/jsc/bindings/node/JSNodeHTTPServerSocket.h
src/jsc/bindings/node/JSNodeHTTPServerSocketPrototype.cpp
src/runtime/api/bun/h2/connection.rs
src/runtime/api/bun/h2/wire.rs
src/runtime/api/bun/h2_frame_parser.rs
src/runtime/api/h2.classes.ts
src/runtime/server/mod.rs
src/runtime/server/server_body.rs
src/uws_sys/App.rs
src/uws_sys/libuwsockets.cpp
test/expectations.txt
test/js/bun/test/parallel/test-http-host-array-should-throw-in-request.ts
test/js/bun/test/parallel/test-http-should-emit-timeout-event-when-using-server-setTimeout.ts
test/js/bun/test/parallel/test-http-should-emit-timeout-event.ts
test/js/node/http/node-http.test.ts
test/js/node/http2/h2-conformance.test.ts
test/js/node/http2/node-http2.test.js
test/js/node/net/node-net.test.ts
test/js/node/test/common/index.js
test/js/node/test/parallel/test-http-abort-stream-end.js
test/js/node/test/parallel/test-http-agent-domain-reused-gc.js
test/js/node/test/parallel/test-http-agent-keepalive-delay.js
test/js/node/test/parallel/test-http-agent-maxtotalsockets.js
test/js/node/test/parallel/test-http-agent-remove.js
test/js/node/test/parallel/test-http-allow-content-length-304.js
test/js/node/test/parallel/test-http-autoselectfamily.js
test/js/node/test/parallel/test-http-buffer-sanity.js
test/js/node/test/parallel/test-http-chunk-extensions-limit.js
test/js/node/test/parallel/test-http-chunk-problem.js
test/js/node/test/parallel/test-http-client-abort-keep-alive-queued-unix-socket.js
test/js/node/test/parallel/test-http-client-abort-unix-socket.js
test/js/node/test/parallel/test-http-client-close-with-default-agent.js
test/js/node/test/parallel/test-http-client-finished.js
test/js/node/test/parallel/test-http-client-immediate-error.js
test/js/node/test/parallel/test-http-client-keep-alive-hint.js
test/js/node/test/parallel/test-http-client-pipe-end.js
test/js/node/test/parallel/test-http-client-reject-unexpected-agent.js
test/js/node/test/parallel/test-http-client-request-options.js
test/js/node/test/parallel/test-http-client-response-domain.js
test/js/node/test/parallel/test-http-client-response-timeout.js
test/js/node/test/parallel/test-http-client-spurious-aborted.js
test/js/node/test/parallel/test-http-client-timeout-event.js
test/js/node/test/parallel/test-http-client-timeout-on-connect.js
test/js/node/test/parallel/test-http-client-timeout-option.js
test/js/node/test/parallel/test-http-client-timeout.js
test/js/node/test/parallel/test-http-client-with-create-connection.js
test/js/node/test/parallel/test-http-connect-req-res.js
test/js/node/test/parallel/test-http-connect.js
test/js/node/test/parallel/test-http-content-length-mismatch.js
test/js/node/test/parallel/test-http-correct-hostname.js
test/js/node/test/parallel/test-http-date-header.js
test/js/node/test/parallel/test-http-decoded-auth.js
test/js/node/test/parallel/test-http-double-content-length.js
test/js/node/test/parallel/test-http-dump-req-when-res-ends.js
test/js/node/test/parallel/test-http-early-hints-invalid-argument.js
test/js/node/test/parallel/test-http-end-throw-socket-handling.js
test/js/node/test/parallel/test-http-expect-handling.js
test/js/node/test/parallel/test-http-extra-response.js
test/js/node/test/parallel/test-http-flush-headers.js
test/js/node/test/parallel/test-http-flush-response-headers.js
test/js/node/test/parallel/test-http-generic-streams.js
test/js/node/test/parallel/test-http-head-throw-on-response-body-write.js
test/js/node/test/parallel/test-http-header-badrequest.js
test/js/node/test/parallel/test-http-header-obstext.js
test/js/node/test/parallel/test-http-header-read.js
test/js/node/test/parallel/test-http-header-value-relaxed.js
test/js/node/test/parallel/test-http-highwatermark.js
test/js/node/test/parallel/test-http-host-headers.js
test/js/node/test/parallel/test-http-hostname-typechecking.js
test/js/node/test/parallel/test-http-incoming-pipelined-socket-destroy.js
test/js/node/test/parallel/test-http-insecure-parser-per-stream.js
test/js/node/test/parallel/test-http-insecure-parser.js
test/js/node/test/parallel/test-http-invalidheaderfield.js
test/js/node/test/parallel/test-http-invalidheaderfield2.js
test/js/node/test/parallel/test-http-keep-alive-drop-requests.js
test/js/node/test/parallel/test-http-keep-alive-empty-line.mjs
test/js/node/test/parallel/test-http-keep-alive-max-requests.js
test/js/node/test/parallel/test-http-localaddress.js
test/js/node/test/parallel/test-http-many-ended-pipelines.js
test/js/node/test/parallel/test-http-max-header-size-per-stream.js
test/js/node/test/parallel/test-http-max-http-headers.js
test/js/node/test/parallel/test-http-multiple-headers.js
test/js/node/test/parallel/test-http-no-read-no-dump.js
test/js/node/test/parallel/test-http-outgoing-drain-writable-length.js
test/js/node/test/parallel/test-http-outgoing-finished.js
test/js/node/test/parallel/test-http-outgoing-proto.js
test/js/node/test/parallel/test-http-outgoing-renderHeaders.js
test/js/node/test/parallel/test-http-parser-finish-error.js
test/js/node/test/parallel/test-http-parser-free.js
test/js/node/test/parallel/test-http-parser-freed-before-upgrade.js
test/js/node/test/parallel/test-http-parser-freed-during-execute.js
test/js/node/test/parallel/test-http-parser-memory-retention.js
test/js/node/test/parallel/test-http-parser-multiple-execute.js
test/js/node/test/parallel/test-http-parser-timeout-reset.js
test/js/node/test/parallel/test-http-parser.js
test/js/node/test/parallel/test-http-pause.js
test/js/node/test/parallel/test-http-pipeline-assertionerror-finish.js
test/js/node/test/parallel/test-http-pipeline-flood.js
test/js/node/test/parallel/test-http-pipeline-outgoing-destroy.js
test/js/node/test/parallel/test-http-proxy.js
test/js/node/test/parallel/test-http-raw-headers.js
test/js/node/test/parallel/test-http-readable-data-event.js
test/js/node/test/parallel/test-http-req-close-robust-from-tampering.js
test/js/node/test/parallel/test-http-req-res-close.js
test/js/node/test/parallel/test-http-request-end-twice.js
test/js/node/test/parallel/test-http-request-end.js
test/js/node/test/parallel/test-http-request-method-delete-payload.js
test/js/node/test/parallel/test-http-response-add-header-after-sent.js
test/js/node/test/parallel/test-http-response-readable.js
test/js/node/test/parallel/test-http-response-remove-header-after-sent.js
test/js/node/test/parallel/test-http-response-setheaders.js
test/js/node/test/parallel/test-http-response-status-message.js
test/js/node/test/parallel/test-http-response-statuscode.js
test/js/node/test/parallel/test-http-response-writehead-returns-this.js
test/js/node/test/parallel/test-http-same-map.js
test/js/node/test/parallel/test-http-server-client-error.js
test/js/node/test/parallel/test-http-server-close-all.js
test/js/node/test/parallel/test-http-server-close-idle-wait-response.js
test/js/node/test/parallel/test-http-server-close-idle.js
test/js/node/test/parallel/test-http-server-connection-list-when-close.js
test/js/node/test/parallel/test-http-server-destroy-socket-on-client-error.js
test/js/node/test/parallel/test-http-server-drop-connections-in-cluster.js
test/js/node/test/parallel/test-http-server-headers-timeout-delayed-headers.js
test/js/node/test/parallel/test-http-server-headers-timeout-interrupted-headers.js
test/js/node/test/parallel/test-http-server-headers-timeout-keepalive.js
test/js/node/test/parallel/test-http-server-headers-timeout-pipelining.js
test/js/node/test/parallel/test-http-server-keep-alive-timeout.js
test/js/node/test/parallel/test-http-server-keepalive-end.js
test/js/node/test/parallel/test-http-server-method.query.js
test/js/node/test/parallel/test-http-server-multiheaders.js
test/js/node/test/parallel/test-http-server-multiple-client-error.js
test/js/node/test/parallel/test-http-server-non-utf8-header.js
test/js/node/test/parallel/test-http-server-options-highwatermark.js
test/js/node/test/parallel/test-http-server-options-incoming-message.js
test/js/node/test/parallel/test-http-server-options-server-response.js
test/js/node/test/parallel/test-http-server-reject-chunked-with-content-length.js
test/js/node/test/parallel/test-http-server-reject-cr-no-lf.js
test/js/node/test/parallel/test-http-server-request-timeout-delayed-body.js
test/js/node/test/parallel/test-http-server-request-timeout-delayed-headers.js
test/js/node/test/parallel/test-http-server-request-timeout-interrupted-body.js
test/js/node/test/parallel/test-http-server-request-timeout-interrupted-headers.js
test/js/node/test/parallel/test-http-server-request-timeout-keepalive.js
test/js/node/test/parallel/test-http-server-request-timeout-pipelining.js
test/js/node/test/parallel/test-http-server-request-timeout-upgrade.js
test/js/node/test/parallel/test-http-server-stale-close.js
test/js/node/test/parallel/test-http-server-unconsume.js
test/js/node/test/parallel/test-http-server.js
test/js/node/test/parallel/test-http-set-cookies.js
test/js/node/test/parallel/test-http-set-header-chain.js
test/js/node/test/parallel/test-http-set-timeout-server.js
test/js/node/test/parallel/test-http-set-timeout.js
test/js/node/test/parallel/test-http-set-trailers.js
test/js/node/test/parallel/test-http-socket-encoding-error.js
test/js/node/test/parallel/test-http-status-code.js
test/js/node/test/parallel/test-http-status-message.js
test/js/node/test/parallel/test-http-timeout-overflow.js
test/js/node/test/parallel/test-http-transfer-encoding-repeated-chunked.js
test/js/node/test/parallel/test-http-unix-socket.js
test/js/node/test/parallel/test-http-upgrade-server-callback.js
test/js/node/test/parallel/test-http-upgrade-server-with-body-and-extras.mjs
test/js/node/test/parallel/test-http-upgrade-server-with-body-error.mjs
test/js/node/test/parallel/test-http-upgrade-server-with-body.mjs
test/js/node/test/parallel/test-http-upgrade-server-with-large-body-unread.mjs
test/js/node/test/parallel/test-http-upgrade-server-with-large-body.mjs
test/js/node/test/parallel/test-http-url.parse-basic.js
test/js/node/test/parallel/test-http-url.parse-https.request.js
test/js/node/test/parallel/test-http-write-callbacks.js
test/js/node/test/parallel/test-http-zero-length-write.js
test/js/node/test/parallel/test-https-agent-additional-options.js
test/js/node/test/parallel/test-https-agent-keylog.js
test/js/node/test/parallel/test-https-agent-session-eviction.js
test/js/node/test/parallel/test-https-agent-sni.js
test/js/node/test/parallel/test-https-agent.js
test/js/node/test/parallel/test-https-argument-of-creating.js
test/js/node/test/parallel/test-https-autoselectfamily.js
test/js/node/test/parallel/test-https-byteswritten.js
test/js/node/test/parallel/test-https-client-renegotiation-limit.js
test/js/node/test/parallel/test-https-insecure-parse-per-stream.js
test/js/node/test/parallel/test-https-keep-alive-drop-requests.js
test/js/node/test/parallel/test-https-localaddress.js
test/js/node/test/parallel/test-https-max-header-size-per-stream.js
test/js/node/test/parallel/test-https-max-headers-count.js
test/js/node/test/parallel/test-https-options-boolean-check.js
test/js/node/test/parallel/test-https-pfx.js
test/js/node/test/parallel/test-https-resume-after-renew.js
test/js/node/test/parallel/test-https-selfsigned-no-keycertsign-no-crash.js
test/js/node/test/parallel/test-https-server-close-all.js
test/js/node/test/parallel/test-https-server-close-idle.js
test/js/node/test/parallel/test-https-set-timeout-server.js
test/js/node/test/parallel/test-https-strict.js
test/js/node/test/parallel/test-https-timeout-server-2.js
test/js/node/test/parallel/test-https-timeout-server.js
test/js/node/test/parallel/test-https-unix-socket-self-signed.js
test/js/node/test/parallel/test-tls-options-boolean-check.js

💤 Files with no reviewable changes (62)

test/js/node/test/parallel/test-http-socket-encoding-error.js
test/js/node/test/parallel/test-https-agent.js
test/js/node/test/parallel/test-http-server-request-timeout-pipelining.js
test/js/node/test/parallel/test-http-status-message.js
test/js/node/test/parallel/test-http-timeout-overflow.js
test/js/node/test/parallel/test-https-timeout-server.js
test/js/node/test/parallel/test-http-client-with-create-connection.js
test/js/node/test/parallel/test-http-server-reject-chunked-with-content-length.js
test/js/node/test/parallel/test-http-unix-socket.js
test/js/node/test/parallel/test-http-server-stale-close.js
test/js/node/test/parallel/test-https-byteswritten.js
test/js/node/test/parallel/test-https-agent-session-eviction.js
test/js/node/test/parallel/test-http-client-response-timeout.js
test/js/node/test/parallel/test-http-server-unconsume.js
test/js/node/test/parallel/test-http-set-header-chain.js
test/js/node/test/parallel/test-http-set-cookies.js
test/js/node/test/parallel/test-http-upgrade-server-with-body-error.mjs
test/js/node/test/parallel/test-https-client-renegotiation-limit.js
test/js/node/test/parallel/test-http-url.parse-https.request.js
test/js/node/test/parallel/test-https-selfsigned-no-keycertsign-no-crash.js
test/js/node/test/parallel/test-http-server-reject-cr-no-lf.js
test/js/node/test/parallel/test-https-unix-socket-self-signed.js
test/js/node/test/parallel/test-https-pfx.js
test/js/node/test/parallel/test-http-write-callbacks.js
test/js/node/test/parallel/test-https-agent-keylog.js
test/js/node/test/parallel/test-tls-options-boolean-check.js
test/js/node/test/parallel/test-https-resume-after-renew.js
test/js/node/test/parallel/test-https-strict.js
test/js/node/test/parallel/test-https-keep-alive-drop-requests.js
test/js/node/test/parallel/test-http-url.parse-basic.js
test/js/node/test/parallel/test-http-client-pipe-end.js
test/js/node/test/parallel/test-https-localaddress.js
test/js/node/test/parallel/test-http-server-request-timeout-upgrade.js
test/js/node/test/parallel/test-https-max-header-size-per-stream.js
test/js/node/test/parallel/test-http-upgrade-server-with-large-body-unread.mjs
test/js/node/test/parallel/test-http-zero-length-write.js
test/js/node/test/parallel/test-http-transfer-encoding-repeated-chunked.js
test/js/node/test/parallel/test-http-status-code.js
test/js/node/test/parallel/test-https-argument-of-creating.js
test/js/node/test/parallel/test-http-upgrade-server-with-body-and-extras.mjs
test/js/node/test/parallel/test-https-agent-additional-options.js
test/js/node/test/parallel/test-https-max-headers-count.js
test/js/node/test/parallel/test-http-server-request-timeout-delayed-body.js
test/js/node/test/parallel/test-http-server.js
test/js/node/test/parallel/test-http-server-request-timeout-interrupted-headers.js
test/js/node/test/parallel/test-http-set-timeout-server.js
test/js/node/test/parallel/test-http-set-timeout.js
test/js/node/test/parallel/test-https-autoselectfamily.js
test/js/node/test/parallel/test-http-upgrade-server-with-body.mjs
test/js/node/test/parallel/test-http-server-request-timeout-delayed-headers.js
test/js/node/test/parallel/test-https-options-boolean-check.js
test/js/node/test/parallel/test-http-server-request-timeout-interrupted-body.js
test/js/node/test/parallel/test-https-timeout-server-2.js
test/js/node/test/parallel/test-https-server-close-all.js
test/js/node/test/parallel/test-http-upgrade-server-with-large-body.mjs
test/js/node/test/parallel/test-https-set-timeout-server.js
test/js/node/test/parallel/test-http-upgrade-server-callback.js
test/js/node/test/parallel/test-http-set-trailers.js
test/js/node/test/parallel/test-https-agent-sni.js
test/js/node/test/parallel/test-http-server-request-timeout-keepalive.js
test/js/node/test/parallel/test-https-server-close-idle.js
test/js/node/test/parallel/test-https-insecure-parse-per-stream.js

coderabbitai

Actionable comments posted: 8

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

src/runtime/api/bun/h2_frame_parser.rs (1)

6101-6109: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Reject non-finite SETTINGS numbers before casting.

NaN passes these < / > range checks and then casts to 0, so session.settings({ maxFrameSize: NaN }) or customSettings: { 10: NaN } can silently serialize the wrong value instead of throwing.

Suggested guard

-                        if value < ($min as f64) || value > $max {
+                        if !value.is_finite() || value < ($min as f64) || value > $max {
                             return global_object
                                 .err_http2_invalid_setting_value_range_error($err)
                                 .throw();
                         }

-                if value < 0.0 || value > MAX_WINDOW_SIZE_F64 {
+                if !value.is_finite() || value < 0.0 || value > MAX_WINDOW_SIZE_F64 {
                     return global_object
                         .err_http2_invalid_setting_value_range_error(
                             "Expected initialWindowSize to be a number between 0 and 2^32-1",

-                        if value < 0.0 || value > MAX_HEADER_TABLE_SIZE_F64 {
+                        if !value.is_finite() || value < 0.0 || value > MAX_HEADER_TABLE_SIZE_F64 {
                             return global_object
                                 .err_http2_invalid_setting_value_range_error(
                                     "Invalid custom setting value",

As per coding guidelines, “Numbers from JS or the wire: handle NaN, ±Infinity, negatives, out-of-range before casting.”

Also applies to: 6143-6155, 6267-6276

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/runtime/api/bun/h2_frame_parser.rs` around lines 6101 - 6109, In the
h2_frame_parser.rs file where SETTINGS values are validated, add a check to
ensure the number value is finite before performing the range validation. After
getting the value from v.as_number() but before the existing range check using
`<` and `>` operators, insert a guard condition to reject NaN and ±Infinity
values (e.g., check that value.is_finite() returns true). If the value is not
finite, throw the invalid setting value range error. Apply this same fix pattern
to all three locations mentioned: the initial range check block with the `value
< ($min as f64) || value > $max` condition, and the two other similar validation
blocks at the additional line ranges referenced.

Source: Coding guidelines

src/js/node/http2.ts (1)

2947-2955: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Capture FD ownership per file-send operation.

kOwnsFd is stream-global but respondWithFile() and respondWithFD() both complete asynchronously. If a failed/cancelled respondWithFile() is followed by respondWithFD(), or a second response attempt races the first fs.open/fstat, later callbacks can observe the wrong ownership and either close a caller-owned fd or leak an fd opened by respondWithFile(). Pass ownsFd into doSendFileFD()/afterOpen() and use that local everywhere instead of reading mutable stream state.

Suggested direction

-function doSendFileFD(options, fd, headers, err, stat) {
+function doSendFileFD(options, fd, headers, ownsFd, err, stat) {
   const onError = options.onError;
-  const ownsFd = this[kOwnsFd] === true;
   if (err) {
     if (ownsFd && err.code !== "EBADF") {
       tryClose(fd);
@@
-    if (this[kOwnsFd] === true) tryClose(fd);
+    if (ownsFd) tryClose(fd);
     return;
   }
@@
-    if (this[kOwnsFd] === true) tryClose(fd);
+    if (ownsFd) tryClose(fd);
@@
-    autoClose: this[kOwnsFd] === true,
+    autoClose: ownsFd,

 function afterOpen(options, headers, err, fd) {
@@
-  fs.fstat(fd, doSendFileFD.bind(this, options, fd, headers));
+  fs.fstat(fd, doSendFileFD.bind(this, options, fd, headers, true));
 }
@@
-    this[kOwnsFd] = true;
     fs.open(path, "r", afterOpen.bind(this, options || {}, headers));
@@
-      fs.fstat(fd.fd, doSendFileFD.bind(this, options, fd, headers));
+      fs.fstat(fd.fd, doSendFileFD.bind(this, options, fd.fd, headers, false));
     } else {
-      fs.fstat(fd, doSendFileFD.bind(this, options, fd, headers));
+      fs.fstat(fd, doSendFileFD.bind(this, options, fd, headers, false));
     }

Also applies to: 3011-3016, 3050-3058, 3285-3286, 3344-3354

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/js/node/http2.ts` around lines 2947 - 2955, The doSendFileFD function and
afterOpen function are reading kOwnsFd from mutable stream state, which causes
race conditions when respondWithFile and respondWithFD execute concurrently or
when operations fail and retry. Refactor these functions to accept ownsFd as a
parameter instead of reading this[kOwnsFd] directly. Update all call sites to
doSendFileFD and afterOpen (including the locations around lines 3011-3016,
3050-3058, 3285-3286, and 3344-3354) to pass the ownsFd value as an argument at
the time of the call, ensuring each async operation captures its own ownership
state rather than referencing shared mutable state.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/bun-uws/src/ChunkedEncoding.h`:
- Around line 203-215: The maxTrailerSectionSize bounds check is occurring after
the completion check for isCompleteTrailerSection, allowing an oversized trailer
section to be accepted as complete if it terminates with a newline character.
Reorder the validation logic so that the maxTrailerSectionSize check happens
before the completion check that returns the fin chunk. This ensures that any
trailer section exceeding the size limit is rejected before being treated as a
complete message, preventing the acceptance of malformed or adversarial data.

In `@packages/bun-uws/src/HttpContext.h`:
- Around line 727-736: The condition on the if statement checking
nodeHttpQueuedPipelinedCount only preserves half-open sockets when there are
queued pipelined responses, but it fails to account for a current in-flight
response that is still pending or being written. Modify the if condition to also
check whether there is an active in-flight response in addition to checking for
queued pipelined responses. This ensures the connection remains open long enough
to write the current response even when no subsequent responses are queued. The
fix should ensure that nodeHttpReceivedFIN is set to true whenever either a
current response is in-flight OR pipelined responses are queued.

In `@src/js/node/_http_server.ts`:
- Around line 340-342: The code uses `Array.isArray(ca)` which can be tampered
with by overriding user globals and prototypes, violating tamper-resistance
requirements. Replace `Array.isArray(ca)` with the tamper-safe intrinsic
`$isArray(ca)` in the conditional expression. Additionally, locate the
`uniqueHeaders.has(key)` call referenced in lines 2012-2016 and replace it with
the corresponding Set intrinsic to maintain consistency with the
tamper-resistance coding guidelines for built-in JS modules.
- Around line 2290-2305: The bufferPipelinedWrite function accepts and queues
invalid chunk values before validating them, which corrupts the queued.bytes
counter when chunk.length is read later. Apply the same chunk validation logic
used in the non-queued write/end path to validate chunk types and values (such
as null, 0, or plain objects) before pushing to queued.ops.push. This ensures
invalid chunks are rejected synchronously with proper errors rather than
deferring failures until replay, preventing queued.bytes corruption.
- Around line 711-728: The HTTPS state flag is restored at line 817, but early
returns in the CONNECT handler path (around lines 773/778) cause the flag to not
be restored, leaving stale state for subsequent requests. Additionally, since
the flag is used across user-configurable constructors like RequestClass,
bracket its restoration more tightly. Move the restoration of the previous HTTPS
state (using prevIsNextIncomingMessageHTTPS) from line 817 to immediately after
the RequestClass construction at line 727, ensuring the flag is restored right
after its intended use rather than much later in the control flow.

In `@src/js/node/https.ts`:
- Around line 514-517: The ALPN defaulting condition in the https.ts file uses
strict equality checks for undefined, which causes misalignment with Node's
behavior when ALPNProtocols is explicitly set to null. Change the condition from
checking `options.ALPNProtocols === undefined && options.ALPNCallback ===
undefined` to use falsy checks instead (such as `!options.ALPNProtocols &&
!options.ALPNCallback`) so that any falsy value (null, undefined, etc.) is
treated as "not set" and triggers the default protocol assignment of http/1.1,
matching Node v26.3.0's behavior.

In `@src/runtime/api/bun/h2_frame_parser.rs`:
- Around line 6288-6290: The settings state is being committed to the parser too
early in `load_settings_from_js_value()`. The three set calls for
local_settings, explicit_settings, and custom_settings at lines 6288-6290 should
not execute immediately because if `set_settings()` later rejects the frame due
to maxOutstandingSettings being exceeded, the parser state has already been
modified with the rejected settings. Instead, keep these settings staged and
return them from `load_settings_from_js_value()` without committing them. Then
modify the code path in `set_settings()` that handles the outstanding-settings
gate to only call these three set methods after confirming the frame is
accepted. Apply the same fix pattern to the other occurrence mentioned at lines
6330-6332.
- Around line 2474-2480: The pending_settings_window_submissions structure only
captures standard settings via settings.to_engine_settings() but does not
include custom_settings, which gets attached later in on_local_settings(). This
causes ACKs to potentially report custom settings from a later submission or
miss them entirely. Modify the PendingLocalSettings structure to include a
custom_settings field alongside the standard settings field, then capture
self.custom_settings when pushing to pending_settings_window_submissions (both
at the current location and at the other referenced location around lines
5631-5645). Finally, update on_local_settings() to use the custom_settings from
the ACK-specific pending submission snapshot instead of self.custom_settings
when building localSettings.customSettings.

---

Outside diff comments:
In `@src/js/node/http2.ts`:
- Around line 2947-2955: The doSendFileFD function and afterOpen function are
reading kOwnsFd from mutable stream state, which causes race conditions when
respondWithFile and respondWithFD execute concurrently or when operations fail
and retry. Refactor these functions to accept ownsFd as a parameter instead of
reading this[kOwnsFd] directly. Update all call sites to doSendFileFD and
afterOpen (including the locations around lines 3011-3016, 3050-3058, 3285-3286,
and 3344-3354) to pass the ownsFd value as an argument at the time of the call,
ensuring each async operation captures its own ownership state rather than
referencing shared mutable state.

In `@src/runtime/api/bun/h2_frame_parser.rs`:
- Around line 6101-6109: In the h2_frame_parser.rs file where SETTINGS values
are validated, add a check to ensure the number value is finite before
performing the range validation. After getting the value from v.as_number() but
before the existing range check using `<` and `>` operators, insert a guard
condition to reject NaN and ±Infinity values (e.g., check that value.is_finite()
returns true). If the value is not finite, throw the invalid setting value range
error. Apply this same fix pattern to all three locations mentioned: the initial
range check block with the `value < ($min as f64) || value > $max` condition,
and the two other similar validation blocks at the additional line ranges
referenced.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: fa9d9a7c-7256-474a-9cbf-44eeec920a42

📥 Commits

Reviewing files that changed from the base of the PR and between 0713d35 and 3c41a0c.

📒 Files selected for processing (237)

packages/bun-usockets/src/context.c
packages/bun-usockets/src/crypto/openssl.c
packages/bun-usockets/src/internal/internal.h
packages/bun-usockets/src/libusockets.h
packages/bun-uws/src/App.h
packages/bun-uws/src/ChunkedEncoding.h
packages/bun-uws/src/HttpContext.h
packages/bun-uws/src/HttpContextData.h
packages/bun-uws/src/HttpErrors.h
packages/bun-uws/src/HttpParser.h
packages/bun-uws/src/HttpResponse.h
packages/bun-uws/src/HttpResponseData.h
src/js/internal/http.ts
src/js/internal/timers.ts
src/js/internal/tls.ts
src/js/node/_http_client.ts
src/js/node/_http_common.ts
src/js/node/_http_incoming.ts
src/js/node/_http_outgoing.ts
src/js/node/_http_server.ts
src/js/node/domain.ts
src/js/node/http2.ts
src/js/node/https.ts
src/js/node/net.ts
src/js/node/tls.ts
src/jsc/ErrorCode.rs
src/jsc/bindings/ErrorCode.ts
src/jsc/bindings/NodeHTTP.cpp
src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp
src/jsc/bindings/node/JSNodeHTTPServerSocket.h
src/jsc/bindings/node/JSNodeHTTPServerSocketPrototype.cpp
src/runtime/api/bun/h2/connection.rs
src/runtime/api/bun/h2/wire.rs
src/runtime/api/bun/h2_frame_parser.rs
src/runtime/api/h2.classes.ts
src/runtime/server/mod.rs
src/runtime/server/server_body.rs
src/uws_sys/App.rs
src/uws_sys/libuwsockets.cpp
test/expectations.txt
test/js/bun/test/parallel/test-http-host-array-should-throw-in-request.ts
test/js/bun/test/parallel/test-http-should-emit-timeout-event-when-using-server-setTimeout.ts
test/js/bun/test/parallel/test-http-should-emit-timeout-event.ts
test/js/node/http/node-http.test.ts
test/js/node/http2/h2-conformance.test.ts
test/js/node/http2/node-http2.test.js
test/js/node/net/node-net.test.ts
test/js/node/test/common/index.js
test/js/node/test/parallel/test-http-abort-stream-end.js
test/js/node/test/parallel/test-http-agent-domain-reused-gc.js
test/js/node/test/parallel/test-http-agent-keepalive-delay.js
test/js/node/test/parallel/test-http-agent-maxtotalsockets.js
test/js/node/test/parallel/test-http-agent-remove.js
test/js/node/test/parallel/test-http-allow-content-length-304.js
test/js/node/test/parallel/test-http-autoselectfamily.js
test/js/node/test/parallel/test-http-buffer-sanity.js
test/js/node/test/parallel/test-http-chunk-extensions-limit.js
test/js/node/test/parallel/test-http-chunk-problem.js
test/js/node/test/parallel/test-http-client-abort-keep-alive-queued-unix-socket.js
test/js/node/test/parallel/test-http-client-abort-unix-socket.js
test/js/node/test/parallel/test-http-client-close-with-default-agent.js
test/js/node/test/parallel/test-http-client-finished.js
test/js/node/test/parallel/test-http-client-immediate-error.js
test/js/node/test/parallel/test-http-client-keep-alive-hint.js
test/js/node/test/parallel/test-http-client-pipe-end.js
test/js/node/test/parallel/test-http-client-reject-unexpected-agent.js
test/js/node/test/parallel/test-http-client-request-options.js
test/js/node/test/parallel/test-http-client-response-domain.js
test/js/node/test/parallel/test-http-client-response-timeout.js
test/js/node/test/parallel/test-http-client-spurious-aborted.js
test/js/node/test/parallel/test-http-client-timeout-event.js
test/js/node/test/parallel/test-http-client-timeout-on-connect.js
test/js/node/test/parallel/test-http-client-timeout-option.js
test/js/node/test/parallel/test-http-client-timeout.js
test/js/node/test/parallel/test-http-client-with-create-connection.js
test/js/node/test/parallel/test-http-connect-req-res.js
test/js/node/test/parallel/test-http-connect.js
test/js/node/test/parallel/test-http-content-length-mismatch.js
test/js/node/test/parallel/test-http-correct-hostname.js
test/js/node/test/parallel/test-http-date-header.js
test/js/node/test/parallel/test-http-decoded-auth.js
test/js/node/test/parallel/test-http-double-content-length.js
test/js/node/test/parallel/test-http-dump-req-when-res-ends.js
test/js/node/test/parallel/test-http-early-hints-invalid-argument.js
test/js/node/test/parallel/test-http-end-throw-socket-handling.js
test/js/node/test/parallel/test-http-expect-handling.js
test/js/node/test/parallel/test-http-extra-response.js
test/js/node/test/parallel/test-http-flush-headers.js
test/js/node/test/parallel/test-http-flush-response-headers.js
test/js/node/test/parallel/test-http-generic-streams.js
test/js/node/test/parallel/test-http-head-throw-on-response-body-write.js
test/js/node/test/parallel/test-http-header-badrequest.js
test/js/node/test/parallel/test-http-header-obstext.js
test/js/node/test/parallel/test-http-header-read.js
test/js/node/test/parallel/test-http-header-value-relaxed.js
test/js/node/test/parallel/test-http-highwatermark.js
test/js/node/test/parallel/test-http-host-headers.js
test/js/node/test/parallel/test-http-hostname-typechecking.js
test/js/node/test/parallel/test-http-incoming-pipelined-socket-destroy.js
test/js/node/test/parallel/test-http-insecure-parser-per-stream.js
test/js/node/test/parallel/test-http-insecure-parser.js
test/js/node/test/parallel/test-http-invalidheaderfield.js
test/js/node/test/parallel/test-http-invalidheaderfield2.js
test/js/node/test/parallel/test-http-keep-alive-drop-requests.js
test/js/node/test/parallel/test-http-keep-alive-empty-line.mjs
test/js/node/test/parallel/test-http-keep-alive-max-requests.js
test/js/node/test/parallel/test-http-localaddress.js
test/js/node/test/parallel/test-http-many-ended-pipelines.js
test/js/node/test/parallel/test-http-max-header-size-per-stream.js
test/js/node/test/parallel/test-http-max-http-headers.js
test/js/node/test/parallel/test-http-multiple-headers.js
test/js/node/test/parallel/test-http-no-read-no-dump.js
test/js/node/test/parallel/test-http-outgoing-drain-writable-length.js
test/js/node/test/parallel/test-http-outgoing-finished.js
test/js/node/test/parallel/test-http-outgoing-proto.js
test/js/node/test/parallel/test-http-outgoing-renderHeaders.js
test/js/node/test/parallel/test-http-parser-finish-error.js
test/js/node/test/parallel/test-http-parser-free.js
test/js/node/test/parallel/test-http-parser-freed-before-upgrade.js
test/js/node/test/parallel/test-http-parser-freed-during-execute.js
test/js/node/test/parallel/test-http-parser-memory-retention.js
test/js/node/test/parallel/test-http-parser-multiple-execute.js
test/js/node/test/parallel/test-http-parser-timeout-reset.js
test/js/node/test/parallel/test-http-parser.js
test/js/node/test/parallel/test-http-pause.js
test/js/node/test/parallel/test-http-pipeline-assertionerror-finish.js
test/js/node/test/parallel/test-http-pipeline-flood.js
test/js/node/test/parallel/test-http-pipeline-outgoing-destroy.js
test/js/node/test/parallel/test-http-proxy.js
test/js/node/test/parallel/test-http-raw-headers.js
test/js/node/test/parallel/test-http-readable-data-event.js
test/js/node/test/parallel/test-http-req-close-robust-from-tampering.js
test/js/node/test/parallel/test-http-req-res-close.js
test/js/node/test/parallel/test-http-request-end-twice.js
test/js/node/test/parallel/test-http-request-end.js
test/js/node/test/parallel/test-http-request-method-delete-payload.js
test/js/node/test/parallel/test-http-response-add-header-after-sent.js
test/js/node/test/parallel/test-http-response-readable.js
test/js/node/test/parallel/test-http-response-remove-header-after-sent.js
test/js/node/test/parallel/test-http-response-setheaders.js
test/js/node/test/parallel/test-http-response-status-message.js
test/js/node/test/parallel/test-http-response-statuscode.js
test/js/node/test/parallel/test-http-response-writehead-returns-this.js
test/js/node/test/parallel/test-http-same-map.js
test/js/node/test/parallel/test-http-server-client-error.js
test/js/node/test/parallel/test-http-server-close-all.js
test/js/node/test/parallel/test-http-server-close-idle-wait-response.js
test/js/node/test/parallel/test-http-server-close-idle.js
test/js/node/test/parallel/test-http-server-connection-list-when-close.js
test/js/node/test/parallel/test-http-server-destroy-socket-on-client-error.js
test/js/node/test/parallel/test-http-server-drop-connections-in-cluster.js
test/js/node/test/parallel/test-http-server-headers-timeout-delayed-headers.js
test/js/node/test/parallel/test-http-server-headers-timeout-interrupted-headers.js
test/js/node/test/parallel/test-http-server-headers-timeout-keepalive.js
test/js/node/test/parallel/test-http-server-headers-timeout-pipelining.js
test/js/node/test/parallel/test-http-server-keep-alive-timeout.js
test/js/node/test/parallel/test-http-server-keepalive-end.js
test/js/node/test/parallel/test-http-server-method.query.js
test/js/node/test/parallel/test-http-server-multiheaders.js
test/js/node/test/parallel/test-http-server-multiple-client-error.js
test/js/node/test/parallel/test-http-server-non-utf8-header.js
test/js/node/test/parallel/test-http-server-options-highwatermark.js
test/js/node/test/parallel/test-http-server-options-incoming-message.js
test/js/node/test/parallel/test-http-server-options-server-response.js
test/js/node/test/parallel/test-http-server-reject-chunked-with-content-length.js
test/js/node/test/parallel/test-http-server-reject-cr-no-lf.js
test/js/node/test/parallel/test-http-server-request-timeout-delayed-body.js
test/js/node/test/parallel/test-http-server-request-timeout-delayed-headers.js
test/js/node/test/parallel/test-http-server-request-timeout-interrupted-body.js
test/js/node/test/parallel/test-http-server-request-timeout-interrupted-headers.js
test/js/node/test/parallel/test-http-server-request-timeout-keepalive.js
test/js/node/test/parallel/test-http-server-request-timeout-pipelining.js
test/js/node/test/parallel/test-http-server-request-timeout-upgrade.js
test/js/node/test/parallel/test-http-server-stale-close.js
test/js/node/test/parallel/test-http-server-unconsume.js
test/js/node/test/parallel/test-http-server.js
test/js/node/test/parallel/test-http-set-cookies.js
test/js/node/test/parallel/test-http-set-header-chain.js
test/js/node/test/parallel/test-http-set-timeout-server.js
test/js/node/test/parallel/test-http-set-timeout.js
test/js/node/test/parallel/test-http-set-trailers.js
test/js/node/test/parallel/test-http-socket-encoding-error.js
test/js/node/test/parallel/test-http-status-code.js
test/js/node/test/parallel/test-http-status-message.js
test/js/node/test/parallel/test-http-timeout-overflow.js
test/js/node/test/parallel/test-http-transfer-encoding-repeated-chunked.js
test/js/node/test/parallel/test-http-unix-socket.js
test/js/node/test/parallel/test-http-upgrade-server-callback.js
test/js/node/test/parallel/test-http-upgrade-server-with-body-and-extras.mjs
test/js/node/test/parallel/test-http-upgrade-server-with-body-error.mjs
test/js/node/test/parallel/test-http-upgrade-server-with-body.mjs
test/js/node/test/parallel/test-http-upgrade-server-with-large-body-unread.mjs
test/js/node/test/parallel/test-http-upgrade-server-with-large-body.mjs
test/js/node/test/parallel/test-http-url.parse-basic.js
test/js/node/test/parallel/test-http-url.parse-https.request.js
test/js/node/test/parallel/test-http-write-callbacks.js
test/js/node/test/parallel/test-http-zero-length-write.js
test/js/node/test/parallel/test-https-agent-additional-options.js
test/js/node/test/parallel/test-https-agent-keylog.js
test/js/node/test/parallel/test-https-agent-session-eviction.js
test/js/node/test/parallel/test-https-agent-sni.js
test/js/node/test/parallel/test-https-agent.js
test/js/node/test/parallel/test-https-argument-of-creating.js
test/js/node/test/parallel/test-https-autoselectfamily.js
test/js/node/test/parallel/test-https-byteswritten.js
test/js/node/test/parallel/test-https-client-renegotiation-limit.js
test/js/node/test/parallel/test-https-insecure-parse-per-stream.js
test/js/node/test/parallel/test-https-keep-alive-drop-requests.js
test/js/node/test/parallel/test-https-localaddress.js
test/js/node/test/parallel/test-https-max-header-size-per-stream.js
test/js/node/test/parallel/test-https-max-headers-count.js
test/js/node/test/parallel/test-https-options-boolean-check.js
test/js/node/test/parallel/test-https-pfx.js
test/js/node/test/parallel/test-https-resume-after-renew.js
test/js/node/test/parallel/test-https-selfsigned-no-keycertsign-no-crash.js
test/js/node/test/parallel/test-https-server-close-all.js
test/js/node/test/parallel/test-https-server-close-idle.js
test/js/node/test/parallel/test-https-set-timeout-server.js
test/js/node/test/parallel/test-https-strict.js
test/js/node/test/parallel/test-https-timeout-server-2.js
test/js/node/test/parallel/test-https-timeout-server.js
test/js/node/test/parallel/test-https-unix-socket-self-signed.js
test/js/node/test/parallel/test-tls-options-boolean-check.js
test/js/node/test/sequential/test-http-econnrefused.js
test/js/node/test/sequential/test-http-keep-alive-large-write.js
test/js/node/test/sequential/test-http-regr-gh-2928.js
test/js/node/test/sequential/test-http-server-keep-alive-timeout-slow-client-headers.js
test/js/node/test/sequential/test-http-server-keep-alive-timeout-slow-server.js
test/js/node/test/sequential/test-http-server-request-timeouts-mixed.js
test/js/node/test/sequential/test-http2-max-session-memory.js
test/js/node/test/sequential/test-http2-ping-flood.js
test/js/node/test/sequential/test-http2-settings-flood.js
test/js/node/test/sequential/test-http2-timeout-large-write-file.js
test/js/node/test/sequential/test-http2-timeout-large-write.js
test/js/node/test/sequential/test-https-connect-localport.js
test/js/node/test/sequential/test-https-server-keep-alive-timeout.js
test/regression/issue/25190.test.ts

💤 Files with no reviewable changes (5)

test/js/node/test/parallel/test-http-client-response-timeout.js
test/js/node/test/parallel/test-http-unix-socket.js
test/js/node/test/parallel/test-http-client-pipe-end.js
test/js/node/test/parallel/test-https-unix-socket-self-signed.js
test/js/node/test/parallel/test-http-client-with-create-connection.js

robobun mentioned this pull request Jun 20, 2026

Implement source phase imports (TC39 Stage 3) #31677

Open

cirospaciari marked this pull request as ready for review June 22, 2026 19:30

cirospaciari requested a review from Jarred-Sumner as a code owner June 22, 2026 19:30

cirospaciari force-pushed the claude/node-http-http2-compat branch from 4a12f3e to d494014 Compare June 22, 2026 19:43

coderabbitai Bot reviewed Jun 22, 2026

View reviewed changes

cirospaciari force-pushed the claude/node-http-http2-compat branch from d494014 to d0b6fcc Compare June 22, 2026 20:09

coderabbitai Bot reviewed Jun 22, 2026

View reviewed changes

claude Bot reviewed Jun 22, 2026

View reviewed changes

Comment thread packages/bun-uws/src/HttpResponseData.h Outdated

cirospaciari force-pushed the claude/node-http-http2-compat branch from d0b6fcc to 038565b Compare June 22, 2026 22:29

coderabbitai Bot reviewed Jun 22, 2026

View reviewed changes

claude Bot reviewed Jun 22, 2026

View reviewed changes

Comment thread packages/bun-uws/src/ChunkedEncoding.h

Comment thread src/js/node/_http_outgoing.ts Outdated

cirospaciari force-pushed the claude/node-http-http2-compat branch from 038565b to 6a5afa4 Compare June 22, 2026 23:42

cirospaciari commented Jun 22, 2026

View reviewed changes

Comment thread src/js/node/_http_server.ts

coderabbitai Bot reviewed Jun 23, 2026

View reviewed changes

claude Bot reviewed Jun 23, 2026

View reviewed changes

Comment thread packages/bun-uws/src/HttpParser.h Outdated

Comment thread packages/bun-uws/src/HttpParser.h Outdated

cirospaciari force-pushed the claude/node-http-http2-compat branch from 0713d35 to 3c41a0c Compare June 23, 2026 00:21

coderabbitai Bot reviewed Jun 23, 2026

View reviewed changes

cirospaciari force-pushed the claude/node-http-http2-compat branch 2 times, most recently from 382f66e to 4d334f5 Compare June 23, 2026 01:54

claude Bot reviewed Jun 23, 2026

View reviewed changes

Comment thread src/js/node/_http_server.ts

Comment thread src/jsc/bindings/node/JSNodeHTTPServerSocketPrototype.cpp Outdated

cirospaciari force-pushed the claude/node-http-http2-compat branch from 4d334f5 to 11d762b Compare June 23, 2026 03:04

claude Bot reviewed Jun 23, 2026

View reviewed changes

Comment thread src/runtime/api/bun/h2_frame_parser.rs Outdated

Comment thread src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp Outdated

cirospaciari force-pushed the claude/node-http-http2-compat branch from 11d762b to 22246c9 Compare June 23, 2026 21:04

coderabbitai Bot mentioned this pull request Jul 26, 2026

Bun.serve: reject Transfer-Encoding on an HTTP/1.0 request (RFC 9112 6.1) #35864

Merged

Uh oh!

Conversation

cirospaciari commented Jun 18, 2026 • edited by robobun Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this does

Server (node:http / node:https)

node:http2

node:net — fatal write errors and connect semantics

Fixed from review

Follow-ups (recorded, not in this PR)

Tests

Not vendored (the 46-file gap to 100%)

Known platform gap (unchanged by this PR)

Uh oh!

robobun commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jun 18, 2026

Uh oh!

github-actions Bot commented Jun 18, 2026

Uh oh!

coderabbitai Bot commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Possibly related issues

Possibly related PRs

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cirospaciari commented Jun 18, 2026 •

edited by robobun

Loading

robobun commented Jun 18, 2026 •

edited

Loading

coderabbitai Bot commented Jun 22, 2026 •

edited

Loading