Conversation
Node's http.Server registers a 'connection' listener that attaches an
HTTPParser to any duplex passed through the event, so user code can
terminate TLS elsewhere (or bridge any stream) and still receive
'request' events. Bun parsed only natively accepted uWS connections, so
server.emit('connection', socket) registered no parser and 'request'
never fired.
Port Node v26.3.0's connectionListener/connectionListenerInternal and
its socket/parser plumbing (parserOnIncoming, resOnFinish, pipelining
queue, keep-alive timeout, upgrade/CONNECT handoff, clientError,
Expect: 100-continue, maxRequestsPerSocket, requireHostHeader) for
sockets that are not native NodeHTTPServerSocket wrappers, driving the
existing llhttp binding through node:_http_common and the standalone
ServerResponse path (assignSocket + OutgoingMessage framing). Also
export http._connectionListener like Node.
Fixes #35281
|
Updated 1:16 PM PT - Jul 23rd, 2026
❌ @robobun, your commit 42afacf has 1 failures in
🧪 To try this PR locally: bunx bun-pr 35285That installs a local version of the PR into your bun-35285 --bun |
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughBun’s Node HTTP server now parses externally emitted duplex sockets, supports request, upgrade, CONNECT, error, and keep-alive flows, updates standalone response state handling, exposes internal listener symbols, and adds comprehensive adopted-socket tests. ChangesHTTP adopted-socket support
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/node/_http_server.ts`:
- Line 1522: Fix duplicate conditional property accesses in
src/js/node/_http_server.ts at lines 1522, 1532, 1583, 1632, 1634, 1650, 1680,
and 1828 by destructuring each referenced property into a local before its
guard, condition, or use: server.timeout, server.maxHeadersCount, socket.parser,
state.outgoing, socket._httpMessage, parser.incoming, socket.parser, and
req.headers.expect respectively. Reuse those locals throughout each affected
branch without changing behavior.
🪄 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: 01d3826f-d523-47a2-9baf-a6629bd7e84e
📒 Files selected for processing (3)
src/js/node/_http_server.tssrc/js/node/http.tstest/js/node/http/node-http-emit-connection.test.ts
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
|
Good catch: #34432 (item 1) also makes
Both register a 'connection' listener in the Server constructor, so they conflict on merge. Deferring to maintainers: if #34432 lands first, this can be closed or rebased down to the remaining deltas plus the adopted-socket test suite in |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/js/node/_http_server.ts (1)
1825-1833: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSame duplicate-property-access pattern, not yet fixed here.
server.maxRequestsPerSocketis read twice in theisRequestsLimitSetcondition (Line 1825) and again at Lines 1829 and 1833 — the same anti-pattern just fixed at the neighboringexpect/maxHeadersCount/etc. sites in this function. Destructure once and reuse.♻️ Proposed fix
- const isRequestsLimitSet = typeof server.maxRequestsPerSocket === "number" && server.maxRequestsPerSocket > 0; + const { maxRequestsPerSocket } = server; + const isRequestsLimitSet = typeof maxRequestsPerSocket === "number" && maxRequestsPerSocket > 0; if (isRequestsLimitSet) { state.requestsCount++; - res.maxRequestsOnConnectionReached = server.maxRequestsPerSocket <= state.requestsCount; + res.maxRequestsOnConnectionReached = maxRequestsPerSocket <= state.requestsCount; } const { expect } = req.headers; - if (isRequestsLimitSet && server.maxRequestsPerSocket < state.requestsCount) { + if (isRequestsLimitSet && maxRequestsPerSocket < state.requestsCount) {🤖 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 1825 - 1833, Update the request-limit logic around isRequestsLimitSet to destructure server.maxRequestsPerSocket once and reuse the local value in the positivity check, res.maxRequestsOnConnectionReached assignment, and subsequent comparison; preserve the existing behavior.
🤖 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/node/_http_server.ts`:
- Around line 1825-1833: Update the request-limit logic around
isRequestsLimitSet to destructure server.maxRequestsPerSocket once and reuse the
local value in the positivity check, res.maxRequestsOnConnectionReached
assignment, and subsequent comparison; preserve the existing behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1e54384d-ea3f-4df0-a324-97ef4f48f5da
📒 Files selected for processing (1)
src/js/node/_http_server.ts
…ding A standalone ServerResponse (no native handle) whose header was rendered by writeHead() but that had no socket assigned yet (a response queued behind an in-flight pipelined response on an adopted socket) hit an early return in end() that buffered nothing: the 400/503/417 auto-responses never reached the wire and the connection hung. Route the no-handle path through OutgoingMessage.end unconditionally, like Node, so assignSocket() -> _flush() writes the buffered response and 'finish' fires. Also point NodeHTTPServerSocket#setEncoding at the shared socketSetEncoding helper, hoist maxRequestsPerSocket, and reject pending test waiters on socket close/error so regressions fail with the received bytes instead of a timeout.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
test/js/node/http/node-http-emit-connection.test.ts (2)
141-151: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCover adopted
CONNECTseparately.The implementation has a distinct
CONNECT→connectdispatch branch; an Upgrade-only test cannot catch a wrong event selection. Add an adopted-socket CONNECT case that asserts theconnectlistener and forwarded head/data. As per coding guidelines, tests must cover the complete variant matrix, including sibling entry points.🤖 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 `@test/js/node/http/node-http-emit-connection.test.ts` around lines 141 - 151, Extend the HTTP connection event tests with a separate adopted-socket CONNECT case alongside the existing "'upgrade' hands the adopted socket to the listener" test. Register a "connect" listener and assert that CONNECT dispatch selects it, forwards any initial head data, and preserves subsequent socket data handling; do not rely on the upgrade test to cover this sibling branch.Source: Coding guidelines
179-181: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExercise the exported listener behaviorally.
typeof http._connectionListener === "function"permits an unrelated placeholder. Invoke the export against an adopted duplex and assert request parsing/response output. As per coding guidelines, every assertion should assert the strongest invariant.🤖 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 `@test/js/node/http/node-http-emit-connection.test.ts` around lines 179 - 181, Replace the type-only assertion in the “http._connectionListener is exported” test with a behavioral test that invokes http._connectionListener using an adopted duplex, then verifies request parsing and the resulting response output. Assert concrete response invariants rather than only checking that the export is callable.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 `@test/js/node/http/node-http-emit-connection.test.ts`:
- Around line 141-151: Extend the HTTP connection event tests with a separate
adopted-socket CONNECT case alongside the existing "'upgrade' hands the adopted
socket to the listener" test. Register a "connect" listener and assert that
CONNECT dispatch selects it, forwards any initial head data, and preserves
subsequent socket data handling; do not rely on the upgrade test to cover this
sibling branch.
- Around line 179-181: Replace the type-only assertion in the
“http._connectionListener is exported” test with a behavioral test that invokes
http._connectionListener using an adopted duplex, then verifies request parsing
and the resulting response output. Assert concrete response invariants rather
than only checking that the export is callable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9c8e48ec-9a88-4c1c-867c-687f408dcb81
📒 Files selected for processing (2)
src/js/node/_http_server.tstest/js/node/http/node-http-emit-connection.test.ts
…ation on adopted sockets
The upgrade branch freed the parser as soon as req.upgrade was set, but parserOnIncoming returns 0 (no skip-body), so llhttp parses the request body. An upgrade request whose body spans multiple data chunks was handed off mid-body: req never completed and the remaining body bytes leaked into the tunnel stream. Gate the handoff on req.complete, like Node v26 (minus its early-emit UpgradeStream wrapper): llhttp pauses at message end with the upgrade flag, the body stays on req, and the first tunnel bytes arrive in bodyHead. Also gate the adopted-socket 'drain' emit on writableLength === 0 to match Node v26's socketOnDrain.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@test/js/node/http/node-http-emit-connection.test.ts`:
- Line 222: Update the Promise wrapping raw.listen in the test setup to accept
both resolve and reject callbacks, and attach raw.once("error", reject) before
starting the listener. Ensure listen failures reject the await instead of
leaving it pending or producing an unhandled server error.
- Line 176: Remove the duplicate connectEvent const declaration in the test
scope, retaining a single Promise declaration with the existing type and
resolver behavior so the file parses successfully.
🪄 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: 3875717c-1ae6-4410-a240-7b3d1f355cab
📒 Files selected for processing (2)
src/js/node/_http_server.tstest/js/node/http/node-http-emit-connection.test.ts
…th on handle-less responses ServerResponse.prototype.writableLength only counted native-handle bytes, so a queued pipelined response on an adopted socket reported 0 even with outputData populated. OutgoingMessage.end() branches on writableLength to decide whether 'finish' queues behind the buffered output; getting 0 sent it through process.nextTick, firing resOnFinish before the response was assigned its socket (assertion failure in debug, uncaught ERR_HTTP_SOCKET_ASSIGNED in release). Fall through to the OutgoingMessage accounting (outputSize + chunked framing + assigned socket buffer) when there is no native handle. Also make the bodied-upgrade test yield between writes so the two packets arrive as separate 'data' events, and wire the remaining listen promise to reject on error.
…nished on handle-less responses Same class as the writableLength fix: both getters only consulted native-handle state, so on adopted sockets writableNeedDrain stayed false after a buffered write returned false, and writableFinished reported true for a queued pipelined response with bytes still in outputData (which also short-circuited writableLength to 0 after end()). Fall through to the OutgoingMessage accounting when there is no native handle. Verified both against Node v26.3.0.
….socket while queued Returning 0 from parserOnIncoming for upgrades parsed the request body into req, but with no UpgradeStream early-emit nothing drains req: a body larger than the readable high-water mark paused the adopted socket via parserOnBody's backpressure and the connection stalled forever (Node serves it). Return 2 (skip body + upgrade) like Node before its UpgradeStream: llhttp pauses at the end of the headers and body/tunnel bytes reach the listener raw through bodyHead and 'data' events. Also register Node's 'resume' guard so a user resume() on the adopted duplex cannot feed data to a parser paused by the pipelining gate (execute() on a paused llhttp surfaces HPE_PAUSED as a spurious clientError), and make res.socket return null for a standalone response until assignSocket() instead of fabricating a FakeSocket (the deprecated fetch()-style response keeps it).
There was a problem hiding this comment.
No new issues found in this pass, but this is a ~480-line port of Node's connectionListener into _http_server.ts with several subtle interactions against the existing native-handle ServerResponse path — worth a human look before merge.
What was reviewed:
- The adopted-socket parser lifecycle (
socketOnData/onParserExecuteCommon/parserOnIncoming/resOnFinish) against Node v26.3.0'slib/_http_server.js— the upgrade skip-body (return 2),onSocketResumere-pause guard, andabortIncoming/abortOutgoingon close are now in place. - The
!this[kHandle]fallbacks onServerResponse'ssocket/writableLength/writableNeedDrain/writableFinishedgetters — each now mirrors theOutgoingMessagegetter it shadows on the standalone path. - The unconditional
OutgoingMessagePrototype.enddelegation for handle-less responses and the_headerSentterm added tosocketOnError's raw-400 guard — native responses still gate onheaderStateSymbolalone. - Test file: promise rejections wired on all listen/one-shot-event waits,
collect()fails waiters on socket close/error, and the upgrade-body tests now assert the skip-body semantics.
Extended reasoning...
Overview
This PR ports Node.js's connectionListener / connectionListenerInternal (~400 net lines in src/js/node/_http_server.ts) so that sockets fed into an http.Server via server.emit('connection', duplex) get an llhttp parser attached and dispatch 'request' events. It also adds !this[kHandle] fallbacks to four ServerResponse getters (socket, writableLength, writableNeedDrain, writableFinished) so the standalone/adopted-socket path reports OutgoingMessage accounting instead of native-handle sums, makes ServerResponse.prototype.end unconditionally delegate to OutgoingMessage.end when there is no native handle, exports http._connectionListener, and ships a 15-test file covering request dispatch, keep-alive, pipelining, chunked responses, upgrade/CONNECT, 100-continue, clientError, and backpressure.
Security risks
The new path parses untrusted HTTP over an adopted duplex, but it reuses the existing process.binding('http_parser') llhttp binding and _http_common freelist rather than hand-rolling parsing; header-size limits, lenient-flags, maxHeadersCount, requireHostHeader, and maxRequestsPerSocket are wired through. The socketOnError change only adds a && !message._headerSent term to the existing raw-400 guard, so it strictly narrows when a raw error response is written. No new eval/shell/fs surface.
Level of scrutiny
High. This is core node:http server compatibility code with a new JS-side request path that must interoperate with the existing native uWS ServerResponse implementation. Six prior review rounds each surfaced real bugs (a queued-response end() hang, a writableLength misreport that fired 'finish' out of order, an upgrade-body deadlock, missing onSocketResume/abortIncoming, and several native-shaped getter overrides reporting wrong values on the adopted path). All flagged threads are now resolved and the current diff addresses each, but the density of subtle path-interaction bugs found so far argues for a maintainer pass rather than auto-approval.
Other factors
- The
instanceof NodeHTTPServerSocketearly-return keeps natively accepted connections on the uWS fast path, and the getter changes are all gated on!this[kHandle], so the existing hot path should be unaffected — but that invariant is worth a maintainer confirming. - One prior 🟡 (call
resetSocketTimeoutat the top ofonParserExecuteCommon, not only inparserOnIncoming) is marked resolved but not applied; the residual divergence (a >6s idle gap between header bytes on an adopted keep-alive socket) is narrow enough that leaving it may be intentional. - Test coverage is solid and hermetic (local
net.Serverbridge,port: 0, failure events reject waiters,test.concurrent), and the PR body shows the suite failing on 1.4.0 and passing here.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@test/js/node/http/node-http-emit-connection.test.ts`:
- Around line 147-157: Update the test setup callback around res.write and the
"drain" listener: register the drain listener before writing, and ensure
res.end() is also reached when no drain event occurs so the client closes
without relying on a timeout. Preserve the existing state tracking and final
assertions so an incorrect write() result or missing drain is reported as an
assertion failure.
🪄 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: 215733cb-8272-4f16-bf68-f51b11abf0f4
📒 Files selected for processing (3)
src/js/node/_http_outgoing.tssrc/js/node/_http_server.tstest/js/node/http/node-http-emit-connection.test.ts
…never sent 100 res._expect_continue was assigned but nothing consumed it: Node's writeHead clears shouldKeepAlive when the client expected 100 Continue and only a final status was sent, since the client may still put the request body on the wire (which would desync the parser on a kept-alive connection). Add the same gate before the standalone header render; verified against the v26.3.0 binary (Connection: close + socket end, where bun previously advertised keep-alive and left the socket open).
There was a problem hiding this comment.
No new findings this pass, but this needs a human look: it's a ~470-line port of Node's connectionListener/parser plumbing plus changes to shared ServerResponse accessors (socket, writableLength/writableNeedDrain/writableFinished, the no-handle end()/writeHead() paths) that the native uWS path also flows through. There's also an overlapping open PR (#34432) taking a different approach to the same feature.
A few earlier threads were resolved without a code change I can see in the current diff — worth a spot-check by whoever merges: the queued-response res.destroy() deferral, resetSocketTimeout at the top of onParserExecuteCommon, abortIncoming in socketOnEnd, and headersTimeout/requestTimeout enforcement on adopted sockets.
Extended reasoning...
Overview
This PR ports Node v26.3.0's connectionListener / connectionListenerInternal and its socket/parser lifecycle into src/js/node/_http_server.ts (~470 net new lines) so server.emit('connection', duplex) attaches an llhttp parser and dispatches 'request' like Node. It also touches shared ServerResponse surfaces used by the native uWS path: the socket getter, writableLength/writableNeedDrain/writableFinished, the no-handle branch of end(), and the no-handle branch of writeHead() (new _expect_continue consumer). _http_outgoing.ts exports kChunkedLength; http.ts re-exports _connectionListener. A new 341-line test file covers request dispatch, keep-alive, pipelining, chunked, upgrade/CONNECT, 100-continue, clientError/400, backpressure, and the native path.
Security risks
The new path attaches a REQUEST-mode parser to arbitrary user-supplied duplex streams. Parsing goes through the existing hardened process.binding('http_parser') llhttp binding with the server's maxHeaderSize/maxHeadersCount/lenient-flag settings, and error handling routes through the pre-existing socketOnError (raw 400/431/408). I don't see new hand-rolled parsing. The main security-adjacent gap is that headersTimeout/requestTimeout are not enforced on adopted sockets (raised earlier and resolved without a visible change) — a slow-loris on an adopted duplex is not swept by checkConnections. That is a DoS-resilience difference from Node worth a maintainer decision.
Level of scrutiny
High. This is a substantial Node-compat port with many interacting edge cases (pipelining backpressure, upgrade body handling, keep-alive timeout arming, Expect: 100-continue), and it modifies getters/end() that the native fast path also reads — a regression there would affect every http.Server user, not just adopted sockets. Eight prior review rounds each surfaced real bugs that led to fix commits, which is a signal of the surface's complexity.
Other factors
- There is an overlapping open PR (#34432) that implements the same feature by reusing the http2
allowHTTP1fallback listener; a maintainer should decide which approach to land. - Several earlier inline threads are marked resolved but I can't find corresponding code in the current diff (queued
res.destroy()once('socket')deferral /resOnFinishguard;resetSocketTimeoutat first-byte inonParserExecuteCommon;abortIncoming(state.incoming)insocketOnEnd;headersTimeout/requestTimeoutsweep). They may have been intentionally deferred as edge-case follow-ups; flagging so the merger can confirm. - Test coverage is thorough for the happy paths and the specific regressions caught in review; the shared-getter changes on the native path are covered only by the "server sockets accepted natively are unaffected" smoke test.
|
For whoever reviews: the four threads resolved without a code change were deliberate declines, each checked against the v26.3.0 reference rather than assumed. Summary so the rationale is in one place:
On #34432: overlap acknowledged earlier in this thread; both register a connection listener in the Server constructor, so whichever lands first, the other rebases or closes. |
|
CI summary for build 78798: 195 of 196 jobs passed. The one red lane (debian 13 x64) is a native crash in test-https-server-connections-checking-leak.js, a GC-finalizer test with release-only unmapped native frames. This diff is JS-only and the crash does not reproduce locally in 110 runs (100 on the main-based release canary, 10 under ASAN with this diff); it has been reported to main-break triage separately. Every other failure in the run passed on retry. The diff is ready for review. |
|
Re-checked against current main (165dc9f) now that #35281 is closed. The issue's own scenario is covered: #34432 registers a These 4 still fail on main, deterministically, also when run in isolation:
The two pipelining failures are a bug in the adopted-socket path on main: sending |
|
Re-checked on main at 731aa92. This PR stays open. The scenario from #35281 is fixed on main by #34432, which registers a
The two pipelining failures come from The branch now conflicts with main in |
This branch has not been deployed
2 participants
Fixes #35281
Problem
Node's
http.Serverregisters an internalconnectionlistener that attaches anHTTPParserto every socket passed through the event, so user code can feed arbitrary Duplex streams (e.g. a TLS-terminated socket from a proxy tunnel) intohttp.createServer()viaserver.emit('connection', socket). In Bun, HTTP parsing happens natively in uWS and only for sockets the server itself accepted, so emitting a foreign socket attached no parser andrequestnever fired.Fix
Port Node v26.3.0's
connectionListener/connectionListenerInternal(lib/_http_server.js) and its socket/parser plumbing for sockets that are not nativeNodeHTTPServerSocketwrappers:process.binding('http_parser')binding, driven throughnode:_http_common's parser freelist) to the adopted duplexIncomingMessage/ServerResponseover the duplex using the existing standalone response path (assignSocket+ OutgoingMessage header rendering/chunked framing)resOnFinishsocket lifecycle, upgrade/CONNECT handoff,clientError(raw 400/431 replies),Expect: 100-continue/checkContinue/checkExpectation,maxRequestsPerSocket503 +dropRequest,requireHostHeader, server/keep-alive timeoutshttp._connectionListenerlike NodeAlso extends the shared
socketOnErrorraw-reply guard to honor_headerSent, which only standalone (adopted-socket) responses maintain; native responses keep usingheaderStateSymboland are unaffected.Review follow-ups hardened the handle-less
ServerResponsesurfaces the new path exercises:end()always routes throughOutgoingMessage.end, andsocket/writableLength/writableNeedDrain/writableFinishedfall through to the OutgoingMessage accounting when there is no native handle (kChunkedLengthis exported fromnode:_http_outgoingfor exactly that getter). Upgrade requests skip their body at the parser (headers-complete returns 2, like Node before its UpgradeStream), so bodied upgrades cannot stall on readable backpressure; body and tunnel bytes reach the'upgrade'/'connect'listener asbodyHead+ raw'data'events.Verification
PASS — got request), identical behavior on Node v26test/js/node/http/node-http-emit-connection.test.tscover request dispatch, keep-alive, request bodies, chunked responses,clientError, default 400, upgrade, 100-continue, the native path, and the_connectionListenerexport; all 10 fail on bun 1.4.0 and pass with this change (verified against Node for parity)node-http.test.ts, CONNECT, server timeouts, backpressure,test-http2-https-fallback*,test-http-server-unconsume-consume, clientError node tests)[review] gate passed · iteration 1 · 4 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 9 passed · 0 rejected · iteration 1
evidence per changed file
root cause · written by the author bot
When a request arrived with Expect: 100-continue on an adopted socket, the
_expect_continueflag was set but never read, so a handler that rejected the request without sending 100 Continue still produced a keep-alive response and left the socket open. Since the client is allowed to send the request body anyway, those body bytes were then parsed as a fresh request, producing spurious parse errors where Node cleanly closes the connection. The fix ports Node's check into the standalone writeHead path, clearing shouldKeepAlive when a final status is written before any 100 Continue, so the res…