Skip to content

http/http2: node v26.3.0 compat — HTTP/1 fallback + upgrade handoff, http2 session errors, perf_hooks and frame framing (+11 upstream tests) - #34432

Merged
cirospaciari merged 56 commits into
mainfrom
ciro/node-http-v26-compat
Aug 4, 2026
Merged

Conversation

@cirospaciari

@cirospaciari cirospaciari commented Jul 17, 2026

Copy link
Copy Markdown
Member

Compat fixes against the Node v26.3.0 test suite, covering both node:http (the HTTP/1 server, its JS fallback, and parser bounds) and node:http2 (session error scope, perf_hooks instrumentation, frame framing, and a silent client hang). Every behavioural claim below was measured against the v26.3.0 binary, not inferred.

module before after
test-http-* 373/392 (95.2%) 382/392 (97.4%)  +9
test-http2-* 254/272 (93.4%) 256/272 (94.1%)  +2
already-vendored +5 tests fixed that were red on main

1. http.Server ignored any socket it did not accept itself

server.emit('connection', socket) was a no-op. Node registers connectionListener in the Server constructor; we registered nothing — Node reports 1 'connection' listener, Bun reported 0.

The parse path already existed. node:http2 has served arbitrary sockets this way since it needed an HTTP/1 fallback for ALPN: connectionListenerHTTP1 drives llhttp from JS over any Duplex, and createHttp1FallbackResponseHandle stands in for the native response handle. Both move to internal/http1_server_fallback so node:http can register the same listener, skipping sockets its native listener already owns.

Unblocks test-http-generic-streams, test-http-insecure-parser-per-stream, test-http-max-header-size-per-stream, and test-http2-autoselect-protocol (which sniffs the client preface on a raw socket and routes to h1Server.emit('connection', …) or h2Server.emit('connection', …) — the h2 branch already worked; the HTTP/1 branch is what http.Server now answers).

2. http2's HTTP/1 fallback ignored the server's parser options

It called parser.initialize(HTTPParser.REQUEST, {}) where Node passes maxHeaderSize, the lenient flags from calculateLenientFlags, and maxHeadersCount. An http2 server with allowHTTP1 silently dropped maxHeaderSize, insecureHTTPParser and httpValidation on every fallback connection.

3. --insecure-http-parser was parsed by nothing

_http_common hardcoded const insecureHTTPParser = false behind a TODO. Adds the flag, backed by a process-wide atomic set during CLI parsing and read through a binding — Node's getOptionValue('--insecure-http-parser'). Leniency now resolves through the shared calculateLenientFlags behind one serverIsLenient helper, so httpValidation keeps precedence over the flag. The server's httpValidation list was ["default","insecure","relaxed"], rejecting Node's 'strict' and accepting a 'default' Node rejects; the client already had Node's list, and the server now matches.

4. writeHead() must freeze the body framing, not just the status line

Node's _storeHeader runs eagerly in writeHead() and picks chunked while _contentLength is still null; a later end(chunk) can't add a Content-Length once _header exists. We render headers lazily, so end(chunk) still had the body and derived a length Node never sends. writeHead() already snapshots the status line to emulate the freeze — this extends it to the framing decision.

It deliberately does not apply to the implicit writeHead that end(chunk) drives: Node assigns _contentLength first, so that path still sends Content-Length. Freezing it there would switch every response from an app whose writeHead is patched (on-headers, so compression/morgan/serve-static) to chunked.

case before after node
end('bye') Content-Length Content-Length Content-Length
writeHead(200, {…}); end('bye') Content-Length chunked chunked
writeHead(200); end() Content-Length chunked chunked
writeHead; write; end chunked chunked chunked
setHeader; end Content-Length Content-Length Content-Length
patched writeHead + end('bye') Content-Length Content-Length Content-Length

5. The chunked Transfer-Encoding has to render after Connection

Node's _storeHeader writes Connection (and Keep-Alive) first, then the chunked Transfer-Encoding. We pushed it into the flat header array, which writeHead sends before writeAutoHeaders renders Date/Connection/Keep-Alive:

ours: Content-Type | Transfer-Encoding | Date | Connection | Keep-Alive
node: Content-Type | Date | Connection | Keep-Alive | Transfer-Encoding

It's carried as an auto-header bit and rendered last instead. This fixes five tests that were already red before this branchtest-http-1.0, test-http-keep-alive-max-requests, test-http-keep-alive-pipeline-max-requests, test-http-server-keep-alive-defaults, test-http-server-keep-alive-max-requests-null. Four of them read like a truncated response ("...Connection: close" with no trailing CRLF against a /Connection: close\r\n/m regex) but nothing was truncated; the header was simply last in the block.

6. Request headers are bounded the way llhttp bounds them

maxHeaderSize is Node's llhttp budget, and llhttp only charges what it hands to its callbacks: on_url, then each field name and value. It never charges the method, " HTTP/1.1\r\n", the ": " separators or the "\r\n" line endings. uWS compared a raw offset from the request line against the same number, so it rejected header blocks Node accepts — Node took a 16376-byte value where we stopped at 16325.

Verified on the basis, not just the boundary: the largest accepted value now matches exactly (16376), and adding 10 filler headers shifts that boundary by 60 — 10 × name+value — where counting raw bytes would shift it by 100. Trailing OWS is charged like llhttp too (40 headers × 400 trailing spaces → 200 OK, 41 → 431, same as Node).

Two bounds still guard raw bytes (the fallback buffer, and a value whose terminator hasn't arrived); those get the framing back as explicit slack, since the framing llhttp ignores is finite — at most 4 bytes per header, and headers are capped at 200.

7. maxConnections / 'drop' did nothing on http.Server

Both were implemented only in net.Server's JS accept path; http.Server is served by the native listener and never consulted them. Applies the same gate on accept, with net.Server's 'drop' payload shape. cluster's child was also missing Node's && !self.dropMaxConnection.

8. Pipelined dispatch is bounded like Node's flood prevention

A client that flooded pipelined requests while never reading responses made the server consume and dispatch every one of them — the existing native gate never ran for write()+end() handlers (each exchange completes before the next dispatch, so no dispatch was ever "pipelined"), and the JS gate's response.pause() was a silent no-op (doPause refuses ENDED responses, and the in-flight response has always ended by then). One recv buffer holds thousands of pipelined requests and the parse loop dispatched them all in a single synchronous burst.

Dispatch is now gated on outgoing backpressure in both branches; pausing stops the request loop at the next request boundary, parking the unconsumed remainder on the parser and replaying it, in order, before the socket reads fresh bytes. Reads resume only once the outgoing bytes, the response queue and the parked spill have all drained.

Making the replay correct exposed three pre-existing pipelined-request bugs, each fixed: a body delivered before its reader was armed was dropped outright (now parked and drained); body delivery resolved the JS wrapper through the socket's current response — the wrong response entirely while pipelining — and read an empty callback slot (the response now remembers the wrapper that armed it); and a response queued after its predecessor had already detached had nothing to advance the pipeline (the dispatch now kicks it when queueing onto an idle connection).

Verified against the v26.3.0 binary: test-http-pipeline-flood passes, a 300-request pipelined burst against a slow reader arrives complete and in order, and 100 pipelined POSTs (plus 60 over TLS) echo their bodies byte-for-byte in order under the same backpressure.

9. The JS fallback hands off Upgrade and CONNECT

The fallback parsed an Upgrade or CONNECT request and dispatched it as an ordinary 'request', so a WebSocket handshake over a foreign socket got a normal HTTP response and the tunnel bytes that followed were parsed as HTTP. It now follows Node's parserOnIncoming: the upgrade verdict only sticks for CONNECT or when an 'upgrade' listener exists (otherwise the request falls through with req.upgrade cleared); when it sticks, llhttp stops at the end of the message, the parser is freed, and the socket is handed to 'upgrade'/'connect' with the first tunnel bytes as bodyHead — or destroyed when nobody is listening. All four dispositions verified byte-for-byte against the v26.3.0 binary, and the fallback now honors the auto-header bits exactly (a CodeRabbit find: sendDate = false and removed Date/Connection headers used to diverge on this path).

10. httpValidation: 'relaxed' and 'insecure' are now distinct past the JS layer

The server passed native a single "insecure" bool, so the two modes were indistinguishable once the option crossed into the parser. llhttp separates them: LENIENT_HEADERS relaxes control bytes in field values, LENIENT_TRANSFER_ENCODING accepts a chunked coding with another value after it (e.g. a duplicate Transfer-Encoding: chunked). Node's 'relaxed' enables only the former — relaxing Transfer-Encoding for it would open request smuggling on a mode whose contract is header values only.

A two-bit leniency field now threads setServerCustomOptions/setServerAppFlagsServer__setAppFlagsuws_app_set_flagsApp::setFlagsHttpContextData → the parser: bit 0 is the existing lenient-headers behaviour (relaxed + insecure), bit 1 is lenient transfer-encoding (kLenientAll surface only). The Transfer-Encoding + Content-Length conflict is still rejected under both, matching llhttp.

Unblocks test-http-header-value-relaxed, which asserts exactly this split: relaxed still answers a duplicate Transfer-Encoding with 400 while insecure accepts it.


11. An unencodable header block failed the stream, not the session

A header block the HPACK encoder cannot emit is a COMPRESSION_ERROR (9) against the session in nghttp2, so Node reports ERR_HTTP2_SESSION_ERROR on the session and on the in-flight request; we reset the stream instead. Three parts: the encode-failure paths now schedule a session error and leave the stream open (the teardown is what errors the request, which is how Node's request-side error appears); ERR_HTTP2_SESSION_ERROR carries the numeric code again (Node passes the raw code at core.js:760/:1611 and uses nameForErrorCode only for stream errors at :2487, so the named variant was a pure deviation); and the dispatch waits for the deferred tick, because it is detected inside the caller's own submit() and Node delivers session errors from the event loop.

pushStream() with an unencodable block follows the same rule: the callback receives the reserved stream so the caller can attach handlers, and the session then fails — matching Node down to the uncaught-error behaviour when no handlers are attached.

12. perf_hooks recorded nothing for http2

PerformanceObserver({ type: "http2" }) was accepted silently and never fired. It now receives Http2Session and Http2Stream entries with Node's detail shape (framesReceived, framesSent, streamCount, maxConcurrentStreams, pingRTT, streamAverageDuration, type, and per-stream id, timeToFirstByte, timeToFirstByteSent, timeToFirstHeader, bytesRead, bytesWritten).

Frame counts come from the engine rather than a JS approximation: a &Cell<u64> parameter on FrameHeader::write makes the compiler enforce counting at every outbound site, and the engine pushes inbound counts to the embedder through a new Sink method — reading them lazily does not work, because the engine is mutably borrowed during dispatch. Counters are snapshotted where Node's own accounting stops, and GOAWAY is excluded since it terminates the session.

Unblocks test-http2-perf_hooks (vendored byte-identical; its framesReceived === 7 assertion is exact, and passes 25/25 across runs).

13. END_STREAM now rides the final DATA frame

Every response body was followed by a separate empty DATA frame carrying END_STREAM. Node flags the last DATA frame instead. This covers end(chunk) — the common case — which needed bridging the window where Writable marks the stream ending only after end()'s own synchronous write has dispatched. Client frame counts for stream.end(data) and write(data, cb => end()) now match Node exactly (4 and 5).

14. A 'data' listener attached before connect() hung the client

Attaching 'data' to a Duplex before http2.connect() put it in flowing mode, so the peer's first frames arrived before the connect callback ran — and the client then never wrote its connection preface. A hang with no error. Three compounding bugs:

  • flush()'s JS-socket arm cleared the write buffer after the onWrite dispatch regardless of the result, re-latching backpressure only on a boolean false. The handlers return -1/0/1, so that check was dead and a refusal destroyed the preface.
  • With the bytes retained, a synchronous transport re-enters flush() from inside its own dispatch, sending them twice — the peer saw a duplicate preface and answered GOAWAY. Fixed with a consumed-prefix offset plus a re-entrancy guard.
  • The client constructor queued onConnect on nextTick before assigning #parser; the parser's construction re-enters JS, which can drain the tick queue, so onConnect could run against a half-built session.

15. Smaller http2 parity fixes

  • close() extends the unACKed-SETTINGS grace to outstanding pings — Node always delivers a ping callback its RTT on a healthy session, never a cancel error.
  • session.socket reads undefined once the socket has detached, even if the proxy was handed out earlier.

Testing

  • All 11 added tests pass on Bun and on the Node v26.3.0 oracle.
  • test-http-* + test-https-* + test-http2-*: the only remaining failures are ones that fail identically with this branch's changes reverted to origin/main on the same tree — i.e. pre-existing.
  • Smuggling defences re-checked: test-http-chunked-smuggling, test-http-transfer-encoding-smuggling, test-http-header-overflow all pass.
  • On the current head: all 261 vendored test-http2-* files pass (0 failures), and every vendored test-http-* file passes individually. node-http2.test.js 311 pass / 0 fail, node-http.test.ts 133 pass / 0 fail, grpc test-server 45 pass / 0 fail.

Not addressed here

Root-caused while tracing the above; each needs its own PR, and each is a real refactor rather than a tweak:

  • socket.destroy() synchronously flushes buffered reads where Node defers them, so a once('clientError') handler is re-entered between its listener being removed and the next being registered (test-http-server-multiple-client-error). Deferring us_socket_close is not the fix — Node's uv_close stops reads immediately and defers only the callback. The fix is deferring on_close's JS dispatch across a tick, which means carrying CloseTeardown (an RAII that runs mark_inactive and branches on reconnect), the handlers.enter() guard and a Strong root, plus a task slot on every socket.
  • req.rawTrailers is empty when res.end() precedes the trailers — uWS stops delivering the request body once the response ends, so a JS-level fix just trades empty trailers for no 'end' event.
  • Unportable: test-http-same-map (V8 %HaveSameMap), test-http-client-immediate-error (fakes sock._handle.connect() returning a UV errno — Bun's net.Socket.connect never calls it), test-http-agent-domain-reused-gc (internal/js_stream_socket).
  • http2: write(x); end() still emits a trailing empty DATA frame. Packing it needs same-tick frame coalescing, i.e. rewriting a frame already serialised into the cork buffer — the hottest path in the h2 engine, and not worth that risk for one frame.
  • http2: socket._handle.hasRef() is still missing (Node exposes it; internal/dgram has the precedent). It is net-scope, so it belongs with a net/tls blast-radius run rather than here.

no test proof · iteration 22 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/spawn/spawn-maxbuf.test.ts

…er maxConnections

Three node:http v26.3.0 compat gaps, each with the upstream test it unblocks.

--insecure-http-parser was parsed by nothing: _http_common hardcoded
`const insecureHTTPParser = false` behind a TODO. Add the flag, back it with a
process-wide atomic set during CLI parsing, and read it through a
getInsecureHTTPParser binding. The server also has to fall back to it: it passed
`!!server.insecureHTTPParser`, which coerces an unset option to false and drops
the flag, where Node resolves the same choice through calculateLenientFlags ->
isLenient().

writeHead() has to freeze the body framing, not just the status line. Node's
_storeHeader runs eagerly and picks chunked while _contentLength is still null,
and a later end(chunk) cannot add a Content-Length once _header exists. Headers
are rendered lazily here, so end(chunk) still has the body and derives a length
Node never sends. Record the frozen choice in writeHead and honor it at render
time through the existing forceChunked sentinel. Only one shape changes:
`writeHead(200, {...}); end('bye')` now frames chunked, matching Node.

maxConnections / 'drop' were implemented only in net.Server's JS accept path;
http.Server is served by the native listener and never consulted them, so the
limit did nothing and 'drop' never fired. Apply the same gate when the
connection is accepted. cluster's child was also missing Node's
`&& !self.dropMaxConnection` in its round-robin accept decision.

Adds the three upstream tests: test-http-insecure-parser,
test-http-chunk-extensions-limit, test-http-server-drop-connections-in-cluster.
@robobun

robobun commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator
Updated 3:31 PM PT - Aug 4th, 2026

@robobun, your commit e758733 has 2 failures in Build #88971 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 34432

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

bun-34432 --bun

…mpat

# Conflicts:
#	src/js/node/_http_server.ts
The writeHead framing freeze was also firing on the implicit writeHead that
end(chunk) drives through callWriteHeadIfObservable. Node assigns _contentLength
before _implicitHeader reaches _storeHeader, so that path still sends a
Content-Length; freezing it to chunked regressed every response from an app whose
writeHead is patched (the on-headers package, so compression/morgan/serve-static).
Only the two end() call sites suppress the freeze — the write() paths still have
no body in hand and must stay chunked.

Resolve the parser leniency through the shared calculateLenientFlags instead of a
hand-rolled subset, behind one serverIsLenient helper. The inline version had no
'strict' arm and let httpValidation 'strict'/'relaxed' fall through to the global
flag, so `--insecure-http-parser` made a server lenient that had explicitly asked
for strict. The httpAllowHalfOpen setter pushes the same native bit and was still
sending `!!this.insecureHTTPParser`, silently reverting leniency to strict when
toggled after listen(); it now shares the helper.

The server's httpValidation list was ["default", "insecure", "relaxed"], rejecting
Node's 'strict' and accepting a 'default' Node rejects. The client already had
Node's list; match it.

Also restore the localPort/remoteFamily fallbacks net.Server uses when building
the 'drop' payload, and unexport the new atomic.
http.Server ignored any socket the native listener did not accept itself:
`server.emit('connection', socket)` was a no-op, because unlike Node — which
registers connectionListener in the Server constructor — nothing was listening.
Node registers 1 listener, we registered 0.

The parse path for this already existed. node:http2 has served arbitrary sockets
this way since it needed an HTTP/1 fallback for ALPN: connectionListenerHTTP1
drives llhttp from JS over any Duplex, and createHttp1FallbackResponseHandle
stands in for the native response handle, rendering the header block to the
socket from the same renderNativeHeaders() output. Move both into
internal/http1_server_fallback so node:http can register the same listener,
skipping the sockets its native listener already owns.

That fallback was also ignoring most of the server's parser configuration. It
called parser.initialize(HTTPParser.REQUEST, {}) where Node passes
maxHeaderSize, the lenient flags from calculateLenientFlags, and maxHeadersCount
— so an http2 server with allowHTTP1 silently dropped maxHeaderSize,
insecureHTTPParser and httpValidation on every fallback connection. Pass them,
and set parser.socket/socket.parser like Node does.

Adds test-http-generic-streams, test-http-insecure-parser-per-stream and
test-http-max-header-size-per-stream.
test-http2-autoselect-protocol sniffs the client preface on a raw net socket and
hands it to whichever server matches: h2Server.emit('connection', socket) for the
h2 preface, h1Server.emit('connection', socket) otherwise. The h2 branch already
worked; the HTTP/1 branch is what http.Server now answers.

Read maxHeadersCount into a local — reading a property twice trips the lint rule.
@cirospaciari cirospaciari changed the title http: 3 node v26.3.0 compat fixes (+3 upstream tests, test-http 95.2% -> 96.0%) http/http2: 5 node v26.3.0 compat fixes (+7 upstream tests; test-http 95.2%->96.7%, test-http2 93.4%->93.8%) Jul 17, 2026
maxHeaderSize is Node's llhttp budget, and llhttp only charges what it hands to
its callbacks: on_url, then each field name and each field value. It never
charges the method, " HTTP/1.1\r\n", the ": " separators or the "\r\n" line
endings. uWS compared a raw offset from the start of the request line against
the same number, so it rejected header blocks Node accepts — measured against
the v26.3.0 binary, Node took a 16376-byte value where we stopped at 16325.

Count name + value like llhttp's TrackHeader and fail at >=, seeded with the URL
from the request line. Two bounds still guard raw bytes (the fallback buffer, and
the in-loop check for a value whose terminator has not arrived yet); those get
the framing back as explicit slack, since the framing llhttp ignores is finite —
at most 4 bytes per header, and headers are capped at 200. The in-loop check
skips leading OWS rather than taking the slack: llhttp is handed the value with
that whitespace already removed, and a value that never terminates has to
overflow exactly where llhttp would instead of waiting for more data
(test-http-header-overflow).

Verified against Node v26.3.0 on both the boundary and the basis: the largest
accepted value now matches exactly (16376), and adding 10 filler headers shifts
that boundary by 60 — 10x name+value — where counting raw bytes would shift it
by 100.

Adds test-http-max-http-headers.
Node's _storeHeader writes Connection (and Keep-Alive) first and the chunked
Transfer-Encoding after them. We pushed Transfer-Encoding into the flat header
array, which writeHead sends before writeAutoHeaders renders Date/Connection/
Keep-Alive, so it went out too early:

  ours: Content-Type | Transfer-Encoding | Date | Connection | Keep-Alive
  node: Content-Type | Date | Connection | Keep-Alive | Transfer-Encoding

Carry it as an auto-header bit instead and render it last, where Node puts it,
still setting HTTP_WROTE_TRANSFER_ENCODING_HEADER so uWS chunk-frames the body.

Fixes five tests that were already failing before this branch: test-http-1.0,
test-http-keep-alive-max-requests, test-http-keep-alive-pipeline-max-requests,
test-http-server-keep-alive-defaults and
test-http-server-keep-alive-max-requests-null. Four of them read like a
truncated response — `"...Connection: close"` with no trailing CRLF against a
/Connection: close\r\n/m regex — but nothing was truncated; the header was
simply last in the block.
This test pinned `Content-Length: 5` for `writeHead(200); end("hello")`, which
was our own divergence — Node freezes the framing in writeHead() while
_contentLength is still null and sends Transfer-Encoding: chunked. Now that we
match, the test's wait condition (`data.endsWith("hello")`) can never fire,
because a chunked body ends with the terminator, so it timed out instead of
failing.

Assert the bytes Node actually sends, verified against the v26.3.0 binary:

  HTTP/1.1 200 OK ... Transfer-Encoding: chunked\r\n\r\n5\r\nhello\r\n0\r\n\r\n

The point of the test is unchanged: the 200 that follows a 304 on a reused
socket still carries framing and a body, so the per-request reset really does
clear the no-body flag. The sibling 204 / HEAD tests keep Content-Length — those
responses have no body, and the freeze deliberately skips them.
Moving Transfer-Encoding out of the flat header array and onto an auto-header
bit left the JS HTTP/1 fallback behind: it detects chunking by scanning the flat
array for the header name, so it stopped seeing it and framed with
Content-Length instead. That hit both consumers — http2's allowHTTP1 responses
and any socket http.Server serves through connectionListener.

Read the bit where the framing is decided, so it suppresses the Content-Length
this path would otherwise invent, and render the header last, next to the
Connection line, like the native writeAutoHeaders and Node's _storeHeader.
Emitting both a Content-Length and a chunked Transfer-Encoding would be a
smuggling shape (RFC 9112 6.1), not just a cosmetic difference.

The fallback's response for `writeHead(200); end("hello")` is now byte-identical
to Node v26.3.0's.
@cirospaciari
cirospaciari marked this pull request as ready for review July 17, 2026 18:32
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

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

Changes

HTTP parser limits now use Node-compatible accounting and framing slack. Runtime configuration supports insecure parsing and leniency flags. Shared HTTP/1 fallback handling, response framing, connection limits, paused-read replay, and HTTP/2 instrumentation are updated with corresponding Node compatibility tests.

HTTP compatibility

Layer / File(s) Summary
Parser limits and leniency
packages/bun-uws/src/HttpParser.h, packages/bun-uws/src/HttpContext.h, src/runtime/..., src/js/node/_http_server.ts, src/jsc/bindings/NodeHTTP.cpp, src/uws_sys/*
Header accounting, transfer-encoding handling, insecure-parser defaults, and numeric leniency flags are threaded through runtime and native server configuration.
HTTP/1 fallback integration
src/js/internal/http1_server_fallback.ts, src/js/node/_http_server.ts, src/js/node/http2.ts
HTTP/1 response writing, socket parsing, request tracking, error handling, cleanup, and HTTP/2 fallback integration use shared implementations.
Response framing and read backpressure
src/js/node/_http_server.ts, src/jsc/bindings/NodeHTTP.cpp, src/runtime/server/NodeHTTPResponse.rs, src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp
Chunked framing, implicit header handling, pipeline advancement, paused-read replay, and response-body delivery are updated.
Connection ownership and limits
src/js/internal/async_hooks.ts, src/js/internal/cluster/child.ts, src/js/node/net.ts, src/js/node/_http_server.ts
The shared owner symbol and max-connection/drop behavior are updated for server and cluster paths.
HTTP/2 lifecycle and observability
src/js/node/http2.ts, src/runtime/api/bun/h2/*, src/runtime/api/h2.classes.ts
HTTP/2 stream finalization, deferred compression errors, frame counters, performance entries, push handling, and teardown scheduling are updated.
Compatibility regression coverage
test/js/node/http/node-http.test.ts, test/js/node/http2/node-http2.test.js, test/js/node/test/parallel/*
Tests cover header limits, chunk extensions, insecure and relaxed parsing, generic streams, framing, protocol selection, connection handoff, cluster drops, flood prevention, and HTTP/2 instrumentation.

Possibly related PRs

  • oven-sh/bun#22756: Also modifies HttpParser.h header parsing and post-padded buffer handling.
  • oven-sh/bun#32488: Also updates the node:http parser compatibility path in HttpParser.h.
  • oven-sh/bun#34356: Relates to paused node:http reads and resuming buffered request data.

Suggested reviewers: jarred-sumner, robobun

🚥 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 HTTP and HTTP/2 Node v26.3.0 compatibility work, including the main fallback and protocol fixes.
Description check ✅ Passed The description explains the changes, test results, coverage improvements, remaining limitations, and verification against Node v26.3.0.

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🤖 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 575-582: Update the raw-header fallback validation around
MAX_HEADER_FRAMING_SLACK and the headerNread handling to account for leading
SP/HTAB separately rather than treating it as fixed framing slack. Consume
leading OWS independently, preserve Node-valid fragmented headers whose logical
size is within maxHeaderSize, and retain a separate finite hard resource cap for
total raw input. Apply the corresponding changes to the fallback checks and
related header-size validations.

In `@src/js/internal/http1_server_fallback.ts`:
- Around line 179-204: Update the fallback response methods writeHeadAndEnd,
write, and end to honor strictContentLength instead of discarding it: track the
declared Content-Length and cumulative bytes written, validate each write and
final total before socket/body writes, and throw
ERR_HTTP_CONTENT_LENGTH_MISMATCH on any discrepancy. Ensure all fallback and
generic-stream routes enforce the check, then add regression coverage in the
existing Bun-owned HTTP tests.
- Around line 270-315: Complete the parser lifecycle around
onHttp1HeadersComplete by queueing response dispatch so pipelined requests do
not call assignSocket concurrently, and drain the queue after each response
finishes. Route upgrade and CONNECT requests through dedicated handling rather
than ordinary request dispatch. Add the socket end handler to call
parser.finish(), and send parser/finalization failures through
onHttp1SocketError while ensuring every failure completes or closes the
operation with catchable errors.
- Around line 234-246: In connectionListenerHTTP1, explicitly rebind the
accepted socket to the current server by assigning its server reference before
registering it in connections. This must overwrite an existing foreign
socket.server and initialize it for plain Duplex sockets, so req.socket.server
resolves the owning server’s httpValidation and insecureHTTPParser settings.
- Around line 91-125: Update the fallback header-rendering logic around autoBits
to emit Date, Connection, and Keep-Alive only when their corresponding
auto-header bits request them, rather than inferring output from hasDate,
hasConnection, or shouldKeepAlive. Preserve explicitly supplied or removed
headers and close-delimited behavior, and emit the Keep-Alive timeout only when
its timeout bit is set.
🪄 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: 7d2ec92e-759e-4b3b-bc10-82a5412d3d50

📥 Commits

Reviewing files that changed from the base of the PR and between a215285 and 11588aa.

📒 Files selected for processing (19)
  • packages/bun-uws/src/HttpParser.h
  • src/http/lib.rs
  • src/js/internal/cluster/child.ts
  • src/js/internal/http1_server_fallback.ts
  • src/js/node/_http_common.ts
  • src/js/node/_http_server.ts
  • src/js/node/http2.ts
  • src/jsc/bindings/NodeHTTP.cpp
  • src/runtime/cli/Arguments.rs
  • src/runtime/node/node_http_binding.rs
  • test/js/node/http/node-http.test.ts
  • test/js/node/test/parallel/test-http-chunk-extensions-limit.js
  • test/js/node/test/parallel/test-http-generic-streams.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-max-header-size-per-stream.js
  • test/js/node/test/parallel/test-http-max-http-headers.js
  • test/js/node/test/parallel/test-http-server-drop-connections-in-cluster.js
  • test/js/node/test/parallel/test-http2-autoselect-protocol.js

Comment thread packages/bun-uws/src/HttpParser.h Outdated
Comment thread src/js/internal/http1_server_fallback.ts Outdated
Comment thread src/js/internal/http1_server_fallback.ts
Comment thread src/js/internal/http1_server_fallback.ts
Comment thread src/js/internal/http1_server_fallback.ts
The accumulator added the value length after the OWS trims, so a value padded
with trailing spaces was undercharged and a header block Node answers with 431
was accepted. llhttp charges the value as it hands it to on_header_value:
leading OWS skipped, trailing OWS still counted. Measure it there instead.

Verified against the v26.3.0 binary with 400 trailing spaces per header: both
now flip at the same request — 40 headers 200 OK, 41 headers 431.

Also set socket.server in the HTTP/1 listener, which Node's
connectionListenerInternal does so a handler can reach the server through
req.socket.server, and drop the request-line pointer the accumulator replaced.
Comment thread src/js/node/_http_server.ts
Comment thread src/js/node/http2.ts
Comment thread packages/bun-uws/src/HttpParser.h Outdated
@cirospaciari cirospaciari changed the title http/http2: 5 node v26.3.0 compat fixes (+7 upstream tests; test-http 95.2%->96.7%, test-http2 93.4%->93.8%) http/http2: node v26.3.0 compat (+8 upstream tests, +5 red tests fixed; test-http 95.2%->96.9%) Jul 17, 2026
Comment thread src/js/node/_http_common.ts
Comment thread packages/bun-uws/src/HttpParser.h Outdated
Comment thread src/js/node/_http_server.ts Outdated
The server passed native a single "insecure" bool, so httpValidation "relaxed"
and "insecure" were indistinguishable past the JS layer. llhttp separates them:
LENIENT_HEADERS relaxes control bytes in field values, LENIENT_TRANSFER_ENCODING
accepts a chunked coding with another value after it (e.g. a duplicate
Transfer-Encoding: chunked header). Node's "relaxed" enables only the former;
relaxing Transfer-Encoding for it would open request smuggling on a mode whose
contract is header values only.

Thread a two-bit leniency field through setServerCustomOptions/setServerAppFlags
-> Server__setAppFlags -> uws_app_set_flags -> App::setFlags -> HttpContextData
-> the parser: bit 0 is the existing lenient-headers behaviour, set for both
relaxed and insecure; bit 1 is lenient transfer-encoding, set only for the
kLenientAll surface (insecureHTTPParser / httpValidation "insecure" /
--insecure-http-parser). The Transfer-Encoding + Content-Length conflict is
still rejected under both, matching llhttp, which does not relax it.

Adds test-http-header-value-relaxed, which asserts exactly this split: relaxed
still answers a duplicate Transfer-Encoding with 400 while insecure accepts it,
and both accept control bytes in header values.
@cirospaciari cirospaciari changed the title http/http2: node v26.3.0 compat (+8 upstream tests, +5 red tests fixed; test-http 95.2%->96.9%) http/http2: node v26.3.0 compat (+9 upstream tests, +5 red tests fixed; test-http 95.2%->97.2%) Jul 17, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

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

⚠️ Outside diff range comments (1)
src/js/internal/http1_server_fallback.ts (1)

51-58: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Parse Transfer-Encoding codings exactly.

includes("chunked") treats valid non-chunked codings such as x-chunked as chunked, causing chunk framing that contradicts the emitted header. Match parsed coding tokens, with chunked only when it is the final coding.

As per coding guidelines, use real parsers instead of prefix or regex heuristics and preserve legitimate input classes when tightening validation.

🤖 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/http1_server_fallback.ts` around lines 51 - 58, Update the
transfer-encoding handling in the header-name switch to parse coding tokens
using the project’s existing HTTP/token parser rather than String.includes or
regex matching. Set chunked only when the parsed final coding token is exactly
“chunked,” while preserving legitimate non-chunked codings such as “x-chunked”
and the emitted header value.

Source: Coding guidelines

🤖 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.

Outside diff comments:
In `@src/js/internal/http1_server_fallback.ts`:
- Around line 51-58: Update the transfer-encoding handling in the header-name
switch to parse coding tokens using the project’s existing HTTP/token parser
rather than String.includes or regex matching. Set chunked only when the parsed
final coding token is exactly “chunked,” while preserving legitimate non-chunked
codings such as “x-chunked” and the emitted header value.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: aba6241a-aa20-4b8e-953c-08391a1cb2c8

📥 Commits

Reviewing files that changed from the base of the PR and between 11588aa and d0c0354.

📒 Files selected for processing (13)
  • packages/bun-uws/src/App.h
  • packages/bun-uws/src/HttpContext.h
  • packages/bun-uws/src/HttpContextData.h
  • packages/bun-uws/src/HttpParser.h
  • src/js/internal/http.ts
  • src/js/internal/http1_server_fallback.ts
  • src/js/node/_http_server.ts
  • src/jsc/bindings/NodeHTTP.cpp
  • src/runtime/server/mod.rs
  • src/runtime/server/server_body.rs
  • src/uws_sys/App.rs
  • src/uws_sys/libuwsockets.cpp
  • test/js/node/test/parallel/test-http-header-value-relaxed.js

Comment thread src/js/internal/cluster/child.ts
The fallback always wrote a Date line, invented a Connection header when
neither connection bit was set, and emitted Keep-Alive without its bit — so
res.sendDate = false, a removed Date or Connection header, and a suppressed
keep-alive timeout all diverged from the native writeAutoHeaders on this path.
Each line is now written iff its bit is set, and the HTTP/1 listener seeds
res._keepAliveTimeout the way the native dispatcher does, so the timeout bit is
actually produced. A head-less write keeps the old defaults; node:http's
ServerResponse always renders the bits.

Verified over a duplex against the v26.3.0 binary: sendDate = false,
removeHeader("connection"), the keep-alive default and both close variants now
match byte-for-byte.
cirospaciari and others added 4 commits July 17, 2026 14:48
A client that floods pipelined requests while never reading responses made the
server consume and dispatch every one of them: handlers that write()+end()
synchronously complete each exchange before the next dispatch, so the pipelined
branch's existing gate never ran, and the JS gate's response.pause() was a
silent no-op — doPause refuses ENDED responses, and the in-flight response has
always ended by the time the pipeline backs up. Meanwhile one recv buffer can
hold thousands of pipelined requests, and the parse loop dispatched them all in
a single synchronous burst; pausing the socket cannot bound work that has
already been received.

Dispatch is now gated on outgoing backpressure in both branches, pausing stops
the request loop at the next request boundary — the unconsumed remainder is
parked on the parser and replayed, in order, before the socket reads fresh
bytes — and the JS gate pauses through a pauseReads op that skips the
body-flow-control guards. Reads resume only once the outgoing bytes, the
response queue and the parked spill have all drained; an incidental resume
(writeHead re-arming the poll, req.resume()) holds instead of reopening the
flood.

Making the replay correct exposed three pre-existing bugs in pipelined request
handling, each of which could fire without the flood machinery:

- A request body delivered before its reader was armed was dropped outright;
  it is now parked in the same buffer the pause path uses and drained when the
  reader arms.
- Body delivery resolved the JS wrapper through the socket's current response,
  which for a pipelined request is some other response — the armed ondata
  callback lives on this request's wrapper, so delivery read the wrong (empty)
  slot and lost the body. The response now remembers the wrapper that armed it.
- A response queued after its predecessor had already finished and detached had
  nothing in flight to advance the pipeline from, so it sat queued forever. The
  dispatch now kicks the pipeline when it queues onto an idle connection.

Verified against the v26.3.0 binary: test-http-pipeline-flood passes, a
300-request pipelined burst against a slow reader arrives complete and in
order, and 100 pipelined POSTs echo their bodies byte-for-byte in order under
the same backpressure.

Adds test-http-pipeline-flood.
The fallback parsed an Upgrade or CONNECT request and dispatched it as an
ordinary 'request', so a WebSocket handshake over a foreign socket got a
normal HTTP response instead of a protocol switch, and the tunnel bytes that
followed were parsed as HTTP.

Follow Node's parserOnIncoming/onParserExecuteCommon: llhttp's upgrade verdict
only sticks for CONNECT or when someone will handle the 'upgrade' event —
otherwise the request falls through to normal dispatch with req.upgrade
cleared. When it sticks, the headers-complete callback returns 2 so llhttp
stops at the end of the message, the parser is freed, and the connection is
handed to the 'upgrade'/'connect' listener with the first tunnel bytes as
bodyHead — or destroyed when nobody is listening, which is only reachable for
CONNECT.

Also write the derived Content-Length and Transfer-Encoding after the
Date/Connection/Keep-Alive block, where Node's _storeHeader puts them.

Verified over a duplex against the v26.3.0 binary: Upgrade with a listener
(101 + bodyHead + tunnel echo), Upgrade without one (normal dispatch,
req.upgrade false), CONNECT with a listener (200 + tunnel), and CONNECT
without one (destroyed) all match byte-for-byte modulo Date.
Comment thread test/js/node/http/node-http-proxy.js Outdated
Comment on lines +35 to +40
proxyServer.listen(0, "127.0.0.1", async () => {
const address = proxyServer.address();

const options = {
protocol: "http:",
hostname: "localhost",
hostname: address.address,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robobun was this change made due to node compat? if not, can we revert this test to the original?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, not node compat, so reverted to the original in e758733.

For the record, why it had been touched: in the container I run the suite in, listen(0, "localhost") binds ::1 (plain getaddrinfo ordering) while the client's lookup for "localhost" uses AI_ADDRCONFIG and, with no routable IPv6 configured, only yields 127.0.0.1, so the request gets ECONNREFUSED 127.0.0.1:<port>. Node v26.3.0 fails the same script the same way in that environment, and the unmodified test passes on CI and on any host where both sides agree, so it is an environment quirk rather than anything this PR changes. If you want the test hardened against that it can be a separate one-liner PR.

dylan-conway
dylan-conway previously approved these changes Aug 4, 2026
robobun added 3 commits August 4, 2026 21:57
…ector<char> instead of std::string

The replay grows the vector to cover the parser's two fence bytes instead of
relying on reserve(): WTF::Vector poisons [size, capacity) under ASAN.
…as Delivered

pushStream()'s pushResult === -1 branch gives the reserved stream to the
callback, so user code can hold an 'error' listener on it; without the
Delivered bit _destroy() dropped the session error for it. Node v26.3.0 emits
ERR_HTTP2_SESSION_ERROR on that stream (uncaught with no listener); the test
now pins that.

@dylan-conway dylan-conway left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@robobun what about the changes in test/js/node/http/node-http.test.ts? can we revert them an only add the new test?

@robobun

robobun commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

@dylan-conway Not on their own: those three edits are the expected-output side of a behaviour change in this PR (item 4 in the description, writeHead() freezes the body framing like Node's eager _storeHeader), so reverting just the test file makes all three fail here. I tried exactly that against this branch a moment ago (main's node-http.test.ts, this build):

(fail) keep-alive socket reused after a 304 response still frames the next response body [5006ms]  ^ timed out
(fail) flushHeaders on a 204 response carries no chunked framing [5005ms]                           ^ timed out
(fail) HEAD response with explicit writeHead(200) carries no body bytes [5006ms]                    ^ timed out

What each of them has in common: after the 304 / 204 / HEAD response the test exists for, it sends a second GET on the same keep-alive socket whose handler is res.writeHead(200); res.end("hello"), then waits for data.endsWith("hello") and asserts Content-Length: 5. That pinned Bun's old output. Node (checked against v26.3.0) and this branch send that response chunked:

node v26.3.0 : HTTP/1.1 200 OK ... Transfer-Encoding: chunked\r\n\r\n5\r\nhello\r\n0\r\n\r\n
bun main     : HTTP/1.1 200 OK ... Content-Length: 5\r\n\r\nhello
this branch  : HTTP/1.1 200 OK ... Transfer-Encoding: chunked\r\n\r\n5\r\nhello\r\n0\r\n\r\n

so the old wait condition never fires. The edits only touch that follow-up response (wait for the 0\r\n\r\n terminator, expect chunked framing); the 304 / 204 / HEAD assertions are untouched. The same byte shape is what the vendored upstream test-http-chunk-extensions-limit.js asserts (writeHead(200, {...}); end('bye') must produce Transfer-Encoding: chunked ... 3\r\nbye\r\n0\r\n\r\n), which is why item 4 is in this PR at all.

So the choice is really about item 4, not the test file:

  1. keep item 4: the three expectation updates have to stay (current state), or
  2. drop item 4 from this PR (the kFramingFrozenChunked bits in _http_server.ts, its row in the description, and test-http-chunk-extensions-limit.js which depends on it), restore the three tests verbatim, and land that piece as its own PR.

Happy to do 2 right away if you would rather keep this PR's test diff additive; otherwise 1 is what is pushed. The new connectionListener hands off Upgrade and CONNECT like Node test is separate from all this and stays either way.

@cirospaciari

Copy link
Copy Markdown
Member Author

@robobun test/js/node/http/node-http.test.ts should pass in node and on bun if the changes are intended and pass on both is ok to keep, confirm this

@robobun

robobun commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

@cirospaciari Confirmed: the edited node-http.test.ts tests pass on both Node v26.3.0 and this branch, and the pre-edit expectations fail on Node itself.

node-http.test.ts cannot run under Node as a whole (bun:test + harness), so I transcribed the four changed tests 1:1 onto node:test/node:assert (same servers, same raw-socket clients, same assertions; file below) and ran that one file under each runtime:

304 keep-alive reuse 204 flushHeaders HEAD + writeHead(200) connectionListener Upgrade/CONNECT (new)
node v26.3.0 (node --test) pass pass pass pass
this branch (bun-debug test) pass pass pass pass
bun canary / main (contrast) timeout timeout timeout timeout

And the reverse check: the same three tests with main's original expectations restored (data.endsWith("hello"), Content-Length: 5) time out under node --test too, because Node answers the follow-up writeHead(200); end("hello") with Transfer-Encoding: chunked / 5\r\nhello\r\n0\r\n\r\n, which is what the edits assert. So the edits are the intended Node behaviour, not a Bun-specific expectation.

node-http-edits.test.mjs (runs under both node --test and bun test)
// The four node-http.test.ts changes in #34432, transcribed 1:1 onto node:test +
// node:assert so the same file runs under `node --test` and `bun test`.
import assert from "node:assert";
import { once } from "node:events";
import { createServer } from "node:http";
import { connect } from "node:net";
import { duplexPair } from "node:stream";
import { test } from "node:test";

test("keep-alive socket reused after a 304 response still frames the next response body", async () => {
  const server = createServer((req, res) => {
    if (req.url === "/cached") {
      res.writeHead(304);
      res.end();
    } else {
      res.writeHead(200);
      res.end("hello");
    }
  });
  try {
    server.listen(0, "127.0.0.1");
    await once(server, "listening");
    const { port } = server.address();

    const out = await new Promise((resolve, reject) => {
      const socket = connect(port, "127.0.0.1");
      let data = "";
      let sentSecond = false;
      socket.on("data", chunk => {
        data += chunk;
        if (!sentSecond && data.includes("\r\n\r\n")) {
          sentSecond = true;
          socket.write("GET /fresh HTTP/1.1\r\nHost: localhost\r\n\r\n");
        }
        if (sentSecond && data.endsWith("0\r\n\r\n")) {
          socket.end();
          resolve(data);
        }
      });
      socket.on("error", reject);
      socket.write("GET /cached HTTP/1.1\r\nHost: localhost\r\n\r\n");
    });

    assert.ok(out.includes("HTTP/1.1 304"));
    const second = out.slice(out.indexOf("HTTP/1.1 200"));
    assert.ok(second.includes("HTTP/1.1 200"));
    assert.ok(second.includes("Transfer-Encoding: chunked"));
    assert.ok(second.endsWith("\r\n\r\n5\r\nhello\r\n0\r\n\r\n"));
  } finally {
    server.close();
  }
});

test("flushHeaders on a 204 response carries no chunked framing", async () => {
  const server = createServer((req, res) => {
    if (req.url === "/nobody") {
      res.writeHead(204);
      res.flushHeaders();
      res.end();
    } else {
      res.writeHead(200);
      res.end("hello");
    }
  });
  try {
    server.listen(0, "127.0.0.1");
    await once(server, "listening");
    const { port } = server.address();

    const out = await new Promise((resolve, reject) => {
      const socket = connect(port, "127.0.0.1");
      let data = "";
      let sentSecond = false;
      socket.on("data", chunk => {
        data += chunk;
        if (!sentSecond && data.includes("\r\n\r\n")) {
          sentSecond = true;
          socket.write("GET /second HTTP/1.1\r\nHost: localhost\r\n\r\n");
        }
        if (sentSecond && data.endsWith("0\r\n\r\n")) {
          socket.end();
          resolve(data);
        }
      });
      socket.on("error", reject);
      socket.write("GET /nobody HTTP/1.1\r\nHost: localhost\r\n\r\n");
    });

    const first = out.slice(0, out.indexOf("HTTP/1.1 200"));
    assert.ok(first.includes("HTTP/1.1 204"));
    assert.ok(!first.includes("Transfer-Encoding"));
    assert.ok(!first.includes("0\r\n\r\n"));
    const second = out.slice(out.indexOf("HTTP/1.1 200"));
    assert.ok(second.includes("Transfer-Encoding: chunked"));
    assert.ok(second.endsWith("\r\n\r\n5\r\nhello\r\n0\r\n\r\n"));
  } finally {
    server.close();
  }
});

test("HEAD response with explicit writeHead(200) carries no body bytes", async () => {
  const server = createServer((req, res) => {
    res.writeHead(200);
    res.end("hello");
  });
  try {
    server.listen(0, "127.0.0.1");
    await once(server, "listening");
    const { port } = server.address();

    const out = await new Promise((resolve, reject) => {
      const socket = connect(port, "127.0.0.1");
      let data = "";
      let sentSecond = false;
      socket.on("data", chunk => {
        data += chunk;
        if (!sentSecond && data.includes("\r\n\r\n")) {
          sentSecond = true;
          socket.write("GET / HTTP/1.1\r\nHost: x\r\n\r\n");
        }
        if (sentSecond && data.endsWith("0\r\n\r\n")) {
          socket.end();
          resolve(data);
        }
      });
      socket.on("error", reject);
      socket.write("HEAD / HTTP/1.1\r\nHost: x\r\n\r\n");
    });

    const first = out.slice(0, out.indexOf("HTTP/1.1 200", 10));
    assert.ok(first.startsWith("HTTP/1.1 200"));
    assert.ok(first.endsWith("\r\n\r\n"));
    assert.ok(out.endsWith("\r\n\r\n5\r\nhello\r\n0\r\n\r\n"));
  } finally {
    server.close();
  }
});

test("connectionListener hands off Upgrade and CONNECT like Node", async () => {
  {
    const unexpectedRequest = Promise.withResolvers();
    const server = createServer(() =>
      unexpectedRequest.reject(new Error("request handler must not run for a handled upgrade")),
    );
    server.on("upgrade", (req, socket, head) => {
      socket.write("HTTP/1.1 101 Switching Protocols\r\n\r\nHEAD:" + head.toString());
      socket.on("data", d => socket.write("TUNNEL:" + d));
    });
    const [clientSide, serverSide] = duplexPair();
    server.emit("connection", serverSide);
    const out = await Promise.race([
      unexpectedRequest.promise,
      new Promise((resolve, reject) => {
        let buf = "";
        let sentMore = false;
        clientSide.on("data", d => {
          buf += d;
          if (!sentMore && buf.includes("HEAD:early")) {
            sentMore = true;
            clientSide.write("more");
          }
          if (buf.includes("TUNNEL:more")) resolve(buf);
        });
        clientSide.on("error", reject);
        clientSide.on("close", () => reject(new Error("closed before expected output: " + buf)));
        clientSide.write("GET /ws HTTP/1.1\r\nHost: x\r\nUpgrade: ws\r\nConnection: Upgrade\r\n\r\nearly");
      }),
    ]);
    assert.ok(out.startsWith("HTTP/1.1 101 Switching Protocols"));
    assert.ok(out.includes("HEAD:early"));
    assert.ok(out.includes("TUNNEL:more"));
    clientSide.destroy();
    serverSide.destroy();
  }

  {
    const server = createServer((req, res) => {
      res.end("normal:" + req.upgrade);
    });
    const [clientSide, serverSide] = duplexPair();
    server.emit("connection", serverSide);
    const out = await new Promise((resolve, reject) => {
      let buf = "";
      clientSide.on("data", d => {
        buf += d;
        if (buf.includes("normal:")) resolve(buf);
      });
      clientSide.on("error", reject);
      clientSide.on("close", () => reject(new Error("closed before expected output: " + buf)));
      clientSide.write("GET / HTTP/1.1\r\nHost: x\r\nUpgrade: ws\r\nConnection: Upgrade\r\n\r\n");
    });
    assert.ok(out.includes("HTTP/1.1 200"));
    assert.ok(out.endsWith("normal:false"));
    clientSide.destroy();
    serverSide.destroy();
  }

  {
    let requestHandlerRan = false;
    const server = createServer(() => {
      requestHandlerRan = true;
    });
    const [clientSide, serverSide] = duplexPair();
    server.emit("connection", serverSide);
    const closed = new Promise(resolve => serverSide.on("close", () => resolve()));
    clientSide.write("CONNECT example.com:443 HTTP/1.1\r\nHost: example.com\r\n\r\n");
    await closed;
    assert.strictEqual(requestHandlerRan, false);
    assert.strictEqual(serverSide.destroyed, true);
  }
});

@cirospaciari

cirospaciari commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

@robobun are the asan tests failing in another branchs/PRs?

@robobun

robobun commented Aug 4, 2026

Copy link
Copy Markdown
Collaborator

@cirospaciari Yes. Both reds in #88971 show up on other branches with the same signature:

So neither is specific to this branch; both are already reported for triage. Also, this PR's earlier build #88278 passed the asan lane with the same http/http2 changes.

@cirospaciari
cirospaciari merged commit a07354a into main Aug 4, 2026
53 of 55 checks passed
@cirospaciari
cirospaciari deleted the ciro/node-http-v26-compat branch August 4, 2026 23:14
springmin pushed a commit to springmin/bun that referenced this pull request Aug 5, 2026
…http2 session errors, perf_hooks and frame framing (+11 upstream tests) (oven-sh#34432)

Compat fixes against the Node v26.3.0 test suite, covering both
`node:http` (the HTTP/1 server, its JS fallback, and parser bounds) and
`node:http2` (session error scope, `perf_hooks` instrumentation, frame
framing, and a silent client hang). Every behavioural claim below was
measured against the `v26.3.0` binary, not inferred.

| module | before | after |
|---|---|---|
| `test-http-*` | 373/392 (95.2%) | **382/392 (97.4%)** &nbsp;+9 |
| `test-http2-*` | 254/272 (93.4%) | **256/272 (94.1%)** &nbsp;+2 |
| already-vendored | — | **+5 tests fixed** that were red on `main` |

---

### 1. `http.Server` ignored any socket it did not accept itself

`server.emit('connection', socket)` was a **no-op**. Node registers
`connectionListener` in the Server constructor; we registered nothing —
Node reports 1 `'connection'` listener, Bun reported 0.

**The parse path already existed.** `node:http2` has served arbitrary
sockets this way since it needed an HTTP/1 fallback for ALPN:
`connectionListenerHTTP1` drives llhttp from JS over any Duplex, and
`createHttp1FallbackResponseHandle` stands in for the native response
handle. Both move to `internal/http1_server_fallback` so `node:http` can
register the same listener, skipping sockets its native listener already
owns.

Unblocks `test-http-generic-streams`,
`test-http-insecure-parser-per-stream`,
`test-http-max-header-size-per-stream`, and
`test-http2-autoselect-protocol` (which sniffs the client preface on a
raw socket and routes to `h1Server.emit('connection', …)` or
`h2Server.emit('connection', …)` — the h2 branch already worked; the
HTTP/1 branch is what `http.Server` now answers).

### 2. http2's HTTP/1 fallback ignored the server's parser options

It called `parser.initialize(HTTPParser.REQUEST, {})` where Node passes
`maxHeaderSize`, the lenient flags from `calculateLenientFlags`, and
`maxHeadersCount`. An **http2 server with `allowHTTP1` silently dropped
`maxHeaderSize`, `insecureHTTPParser` and `httpValidation`** on every
fallback connection.

### 3. `--insecure-http-parser` was parsed by nothing

`_http_common` hardcoded `const insecureHTTPParser = false` behind a
TODO. Adds the flag, backed by a process-wide atomic set during CLI
parsing and read through a binding — Node's
`getOptionValue('--insecure-http-parser')`. Leniency now resolves
through the shared `calculateLenientFlags` behind one `serverIsLenient`
helper, so `httpValidation` keeps precedence over the flag. The server's
`httpValidation` list was `["default","insecure","relaxed"]`, rejecting
Node's `'strict'` and accepting a `'default'` Node rejects; the client
already had Node's list, and the server now matches.

### 4. `writeHead()` must freeze the body framing, not just the status
line

Node's `_storeHeader` runs eagerly in `writeHead()` and picks chunked
while `_contentLength` is still `null`; a later `end(chunk)` can't add a
Content-Length once `_header` exists. We render headers lazily, so
`end(chunk)` still had the body and derived a length Node never sends.
`writeHead()` already snapshots the status line to emulate the freeze —
this extends it to the framing decision.

It deliberately does **not** apply to the implicit `writeHead` that
`end(chunk)` drives: Node assigns `_contentLength` first, so that path
still sends Content-Length. Freezing it there would switch every
response from an app whose `writeHead` is patched (`on-headers`, so
`compression`/`morgan`/`serve-static`) to chunked.

| case | before | after | node |
|---|---|---|---|
| `end('bye')` | Content-Length | Content-Length | Content-Length |
| **`writeHead(200, {…}); end('bye')`** | **Content-Length** |
**chunked** | **chunked** |
| `writeHead(200); end()` | Content-Length | chunked | chunked |
| `writeHead; write; end` | chunked | chunked | chunked |
| `setHeader; end` | Content-Length | Content-Length | Content-Length |
| patched `writeHead` + `end('bye')` | Content-Length | Content-Length |
Content-Length |

### 5. The chunked `Transfer-Encoding` has to render after `Connection`

Node's `_storeHeader` writes Connection (and Keep-Alive) first, then the
chunked Transfer-Encoding. We pushed it into the flat header array,
which `writeHead` sends before `writeAutoHeaders` renders
Date/Connection/Keep-Alive:

```
ours: Content-Type | Transfer-Encoding | Date | Connection | Keep-Alive
node: Content-Type | Date | Connection | Keep-Alive | Transfer-Encoding
```

It's carried as an auto-header bit and rendered last instead. **This
fixes five tests that were already red before this branch** —
`test-http-1.0`, `test-http-keep-alive-max-requests`,
`test-http-keep-alive-pipeline-max-requests`,
`test-http-server-keep-alive-defaults`,
`test-http-server-keep-alive-max-requests-null`. Four of them read like
a truncated response (`"...Connection: close"` with no trailing CRLF
against a `/Connection: close\r\n/m` regex) but nothing was truncated;
the header was simply last in the block.

### 6. Request headers are bounded the way llhttp bounds them

`maxHeaderSize` is Node's llhttp budget, and llhttp only charges what it
hands to its callbacks: `on_url`, then each field name and value. It
never charges the method, `" HTTP/1.1\r\n"`, the `": "` separators or
the `"\r\n"` line endings. uWS compared a raw offset from the request
line against the same number, so it rejected header blocks Node accepts
— Node took a 16376-byte value where we stopped at 16325.

Verified on the **basis**, not just the boundary: the largest accepted
value now matches exactly (16376), and adding 10 filler headers shifts
that boundary by 60 — 10 × name+value — where counting raw bytes would
shift it by 100. Trailing OWS is charged like llhttp too (40 headers ×
400 trailing spaces → 200 OK, 41 → 431, same as Node).

Two bounds still guard raw bytes (the fallback buffer, and a value whose
terminator hasn't arrived); those get the framing back as explicit
slack, since the framing llhttp ignores is finite — at most 4 bytes per
header, and headers are capped at 200.

### 7. `maxConnections` / `'drop'` did nothing on `http.Server`

Both were implemented only in `net.Server`'s JS accept path;
`http.Server` is served by the native listener and never consulted them.
Applies the same gate on accept, with `net.Server`'s `'drop'` payload
shape. `cluster`'s child was also missing Node's `&&
!self.dropMaxConnection`.

### 8. Pipelined dispatch is bounded like Node's flood prevention

A client that flooded pipelined requests while never reading responses
made the server consume and dispatch every one of them — the existing
native gate never ran for `write()+end()` handlers (each exchange
completes before the next dispatch, so no dispatch was ever
"pipelined"), and the JS gate's `response.pause()` was a silent no-op
(`doPause` refuses ENDED responses, and the in-flight response has
always ended by then). One recv buffer holds thousands of pipelined
requests and the parse loop dispatched them all in a single synchronous
burst.

Dispatch is now gated on outgoing backpressure in both branches; pausing
stops the request loop at the next request boundary, parking the
unconsumed remainder on the parser and replaying it, in order, before
the socket reads fresh bytes. Reads resume only once the outgoing bytes,
the response queue and the parked spill have all drained.

Making the replay correct exposed three pre-existing pipelined-request
bugs, each fixed: a body delivered before its reader was armed was
dropped outright (now parked and drained); body delivery resolved the JS
wrapper through the socket's *current* response — the wrong response
entirely while pipelining — and read an empty callback slot (the
response now remembers the wrapper that armed it); and a response queued
after its predecessor had already detached had nothing to advance the
pipeline (the dispatch now kicks it when queueing onto an idle
connection).

Verified against the v26.3.0 binary: `test-http-pipeline-flood` passes,
a 300-request pipelined burst against a slow reader arrives complete and
in order, and 100 pipelined POSTs (plus 60 over TLS) echo their bodies
byte-for-byte in order under the same backpressure.

### 9. The JS fallback hands off Upgrade and CONNECT

The fallback parsed an Upgrade or CONNECT request and dispatched it as
an ordinary `'request'`, so a WebSocket handshake over a foreign socket
got a normal HTTP response and the tunnel bytes that followed were
parsed as HTTP. It now follows Node's `parserOnIncoming`: the upgrade
verdict only sticks for CONNECT or when an `'upgrade'` listener exists
(otherwise the request falls through with `req.upgrade` cleared); when
it sticks, llhttp stops at the end of the message, the parser is freed,
and the socket is handed to `'upgrade'`/`'connect'` with the first
tunnel bytes as `bodyHead` — or destroyed when nobody is listening. All
four dispositions verified byte-for-byte against the v26.3.0 binary, and
the fallback now honors the auto-header bits exactly (a CodeRabbit find:
`sendDate = false` and removed Date/Connection headers used to diverge
on this path).

### 10. `httpValidation: 'relaxed'` and `'insecure'` are now distinct
past the JS layer

The server passed native a single "insecure" bool, so the two modes were
indistinguishable once the option crossed into the parser. llhttp
separates them: `LENIENT_HEADERS` relaxes control bytes in field values,
`LENIENT_TRANSFER_ENCODING` accepts a chunked coding with another value
after it (e.g. a duplicate `Transfer-Encoding: chunked`). Node's
`'relaxed'` enables only the former — relaxing Transfer-Encoding for it
would open request smuggling on a mode whose contract is header values
only.

A two-bit leniency field now threads
`setServerCustomOptions`/`setServerAppFlags` → `Server__setAppFlags` →
`uws_app_set_flags` → `App::setFlags` → `HttpContextData` → the parser:
bit 0 is the existing lenient-headers behaviour (relaxed + insecure),
bit 1 is lenient transfer-encoding (`kLenientAll` surface only). The
Transfer-Encoding + Content-Length conflict is still rejected under
both, matching llhttp.

Unblocks `test-http-header-value-relaxed`, which asserts exactly this
split: relaxed still answers a duplicate Transfer-Encoding with 400
while insecure accepts it.

---


### 11. An unencodable header block failed the stream, not the session

A header block the HPACK encoder cannot emit is a `COMPRESSION_ERROR`
(9) against the **session** in nghttp2, so Node reports
`ERR_HTTP2_SESSION_ERROR` on the session *and* on the in-flight request;
we reset the stream instead. Three parts: the encode-failure paths now
schedule a session error and leave the stream open (the teardown is what
errors the request, which is how Node's request-side error appears);
`ERR_HTTP2_SESSION_ERROR` carries the numeric code again (Node passes
the raw code at `core.js:760`/`:1611` and uses `nameForErrorCode`
**only** for stream errors at `:2487`, so the named variant was a pure
deviation); and the dispatch waits for the deferred tick, because it is
detected inside the caller's own `submit()` and Node delivers session
errors from the event loop.

`pushStream()` with an unencodable block follows the same rule: the
callback receives the reserved stream so the caller can attach handlers,
and the session then fails — matching Node down to the uncaught-error
behaviour when no handlers are attached.

### 12. `perf_hooks` recorded nothing for http2

`PerformanceObserver({ type: "http2" })` was accepted silently and never
fired. It now receives `Http2Session` and `Http2Stream` entries with
Node's detail shape (`framesReceived`, `framesSent`, `streamCount`,
`maxConcurrentStreams`, `pingRTT`, `streamAverageDuration`, `type`, and
per-stream `id`, `timeToFirstByte`, `timeToFirstByteSent`,
`timeToFirstHeader`, `bytesRead`, `bytesWritten`).

Frame counts come from the engine rather than a JS approximation: a
`&Cell<u64>` parameter on `FrameHeader::write` makes the compiler
enforce counting at every outbound site, and the engine pushes inbound
counts to the embedder through a new `Sink` method — reading them lazily
does not work, because the engine is mutably borrowed during dispatch.
Counters are snapshotted where Node's own accounting stops, and GOAWAY
is excluded since it terminates the session.

Unblocks `test-http2-perf_hooks` (vendored byte-identical; its
`framesReceived === 7` assertion is exact, and passes 25/25 across
runs).

### 13. END_STREAM now rides the final DATA frame

Every response body was followed by a separate empty DATA frame carrying
END_STREAM. Node flags the last DATA frame instead. This covers
`end(chunk)` — the common case — which needed bridging the window where
`Writable` marks the stream `ending` only *after* `end()`'s own
synchronous write has dispatched. Client frame counts for
`stream.end(data)` and `write(data, cb => end())` now match Node exactly
(4 and 5).

### 14. A `'data'` listener attached before `connect()` hung the client

Attaching `'data'` to a Duplex before `http2.connect()` put it in
flowing mode, so the peer's first frames arrived before the connect
callback ran — and the client then never wrote its connection preface. A
hang with no error. Three compounding bugs:

- `flush()`'s JS-socket arm cleared the write buffer after the `onWrite`
dispatch **regardless of the result**, re-latching backpressure only on
a boolean `false`. The handlers return `-1/0/1`, so that check was dead
and a refusal destroyed the preface.
- With the bytes retained, a synchronous transport re-enters `flush()`
from inside its own dispatch, sending them twice — the peer saw a
duplicate preface and answered GOAWAY. Fixed with a consumed-prefix
offset plus a re-entrancy guard.
- The client constructor queued `onConnect` on nextTick **before**
assigning `#parser`; the parser's construction re-enters JS, which can
drain the tick queue, so `onConnect` could run against a half-built
session.

### 15. Smaller http2 parity fixes

- `close()` extends the unACKed-SETTINGS grace to outstanding pings —
Node always delivers a ping callback its RTT on a healthy session, never
a cancel error.
- `session.socket` reads `undefined` once the socket has detached, even
if the proxy was handed out earlier.

### Testing

- **All 11 added tests pass on Bun and on the Node v26.3.0 oracle.**
- `test-http-*` + `test-https-*` + `test-http2-*`: the only remaining
failures are ones that fail identically with this branch's changes
reverted to `origin/main` on the same tree — i.e. pre-existing.
- Smuggling defences re-checked: `test-http-chunked-smuggling`,
`test-http-transfer-encoding-smuggling`, `test-http-header-overflow` all
pass.
- On the current head: all **261** vendored `test-http2-*` files pass (0
failures), and every vendored `test-http-*` file passes individually.
`node-http2.test.js` 311 pass / 0 fail, `node-http.test.ts` 133 pass / 0
fail, grpc `test-server` 45 pass / 0 fail.

### Not addressed here

Root-caused while tracing the above; each needs its own PR, and each is
a real refactor rather than a tweak:

- **`socket.destroy()` synchronously flushes buffered reads** where Node
defers them, so a `once('clientError')` handler is re-entered between
its listener being removed and the next being registered
(`test-http-server-multiple-client-error`). Deferring `us_socket_close`
is *not* the fix — Node's `uv_close` stops reads immediately and defers
only the callback. The fix is deferring `on_close`'s JS dispatch across
a tick, which means carrying `CloseTeardown` (an RAII that runs
`mark_inactive` and branches on reconnect), the `handlers.enter()` guard
and a `Strong` root, plus a task slot on every socket.
- **`req.rawTrailers` is empty when `res.end()` precedes the trailers**
— uWS stops delivering the request body once the response ends, so a
JS-level fix just trades empty trailers for no `'end'` event.
- **Unportable**: `test-http-same-map` (V8 `%HaveSameMap`),
`test-http-client-immediate-error` (fakes `sock._handle.connect()`
returning a UV errno — Bun's `net.Socket.connect` never calls it),
`test-http-agent-domain-reused-gc` (`internal/js_stream_socket`).
- **http2:** `write(x); end()` still emits a trailing empty DATA frame.
Packing it needs same-tick frame coalescing, i.e. rewriting a frame
already serialised into the cork buffer — the hottest path in the h2
engine, and not worth that risk for one frame.
- **http2:** `socket._handle.hasRef()` is still missing (Node exposes
it; `internal/dgram` has the precedent). It is net-scope, so it belongs
with a net/tls blast-radius run rather than here.

<!-- robobun:evidence:begin -->

---

**no test proof** · iteration 22 · Platform-specific test(s) that do not
run on this machine. Deferring to CI, which covers all platforms:
test/js/bun/spawn/spawn-maxbuf.test.ts

<!-- robobun:evidence:end -->

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: robobun <117481402+robobun@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants