[robobun]

Problem

• https.createServer() does not see a connection until its TLS handshake ends: no 'connection', getConnections() is 0, maxConnections and blockList act after a full handshake. Node emits 'connection' at TCP accept (tls.md).
• Cause: the filter thunk in src/runtime/server/server_body.rs drops the accept event (if opened != 1 { return; }).

Fix

• The thunk forwards a TLS accept to JS, and a handshake only when it succeeded.
• JS emits 'connection' at accept and 'secureConnection' after the handshake. As in Node, the parser and server.timeout start at 'secureConnection'.
• 'connection' carries the socket object of req.socket, as tls.createServer() in Bun does. Node passes the raw net.Socket.
• Verified: test/js/node/http/node-https-server.test.ts (26 new tests: 23 fail on main, 24 pass on Node v26.3.0), node-http.test.ts, 450 vendored tests. Self-reviewed: 15 concerns raised, 12 addressed (rest in Notes).

Background

• A uWS connection filter gets socket events as integers: 2 accept, 1 open. For TLS, open is the handshake end, failed or not. FilterEvent names them.
• Considered one uWS accept event for both transports. It edits headers that Bun.serve shares and costs 3 operations per https connection, not 1.

Downsides

• Each https connection makes one more native-to-JS call: about +1,000 user instructions of 7.96 M.
• Every node:http server socket grows by 32 bytes. A pending handshake holds 13 JS cells (1,620 bytes). A burst of 500 TLS connects runs 500 accept dispatches in one loop iteration (main: 5).
• No handshakeTimeout or 'tlsClientError'. socket.encrypted is true in 'connection'. test-https-timeout.js hangs in 10 of 750 runs (main: 4).

Notes

No issue reports this. It was found by comparison with Node.

What changes for a program. One TCP peer sends the first byte of a TLS record and stalls. Linux x64, release builds.

	Node v26.3.0	main ad4089b	this PR
'connection', getConnections()	fired, 1	not fired, 0	fired, 1
the peer leaves, or its handshake fails	nothing more	'connection' now	nothing more
server-destroy 1.0.1: destroy() to 'close'	1 ms	11,702 ms	2 ms
server.close() in a 'connection' listener, then the request	served	ECONNRESET	served
closeIdleConnections() in a 'connection' listener, then the request	served	ECONNRESET	served
socket.destroy() in 'connection'	closed, no handshake	stays open	closed, no handshake
socket.setTimeout(n, destroy) in 'connection'	closed after n	stays open	closed after n
maxConnections reached, next TLS client	'drop', ECONNRESET, 0 bytes	handshake completes (2,576 bytes), then closed	'drop', ECONNRESET, 0 bytes
blockList has the peer	closed at accept	handshake completes, then closed	closed at accept
a 'connection' listener throws and the process handles it	'secureConnection' follows	no 'secureConnection'	'secureConnection' follows
close() + closeAllConnections()	the peer stays	the peer stays	the peer stays
server.timeout, headersTimeout before the handshake	do not apply	do not apply	do not apply

• The slow shutdown and the ECONNRESET rows are in Bun 1.4.3 and not in 1.4.2: since 1.4.3 the native connection count runs from accept, and a handshake marks its socket idle right before 'connection'.
• The same holds for new https.Server(), http.createServer({ key, cert }), http.createServer().listen({ tls }) and a cluster worker.
• closeAllConnections() leaves a socket that has not finished its handshake, as Node does. The uWS idle timer closes a stalled handshake after 8 to 12 s, as on main. Node waits for handshakeTimeout (120 s).

Which socket 'connection' carries. One NodeHTTPServerSocket per connection, for 'connection', 'secureConnection' and req.socket. Node has a raw net.Socket for 'connection' and a TLSSocket with _parent for the rest. destroy(), setTimeout(), remoteAddress and 'close' work on the one object. Before the handshake it differs from Node in this:

• socket.encrypted is true, socket.authorized is false, socket.servername is false (Node: undefined).
• socket.write() is held and sent as TLS data after the handshake (Node writes cleartext). socket.end(data) waits for that write. socket.end() sends a FIN and the socket stays until the peer closes.
• socket.resetAndDestroy() throws ERR_INVALID_HANDLE_TYPE, as it does on this socket after the handshake.
• One timer. Node has one on the raw socket and one on the TLSSocket. server.timeout is armed at 'secureConnection'. It does not replace a shorter timeout that a 'connection' listener armed: that one starts again there.
• After the handshake, the 'timeout' of a timer that a 'connection' listener armed also runs the http timeout handler. With no 'timeout' listener on the request, the response or the server, that handler destroys the socket. Node runs only the callback of the listener, because its http handler listens on the TLSSocket. Probe: socket.setTimeout(300, cb) in 'connection', and a request 900 ms after the handshake. Node v26.3.0 calls cb and answers. Bun 1.4.3 and this PR call cb and destroy the socket. A callback that destroys the socket gives the same result on all three.
• destroy(err), emit('error') and a peer reset reach 'clientError' through the http error listener. A peer FIN, non-TLS bytes and a stalled handshake emit no error event, and 'tlsClientError' is never emitted. Node emits both events for all of them.

Transport and phase. The listener generation carries the TLS options of its listen() call (the tls entry that #45659 also adds) and JS takes the transport from there. Native passes only the phase. So listen({ tls }) sockets get encrypted: true and 'secureConnection'. Their authorized and authorizationError still read the constructor's options: #45659 fixes those getters. The handshake phase of a socket is a private symbol, so a plain-http program that sets req.socket.encrypted = true is unaffected.

Other open PRs on these lines.

• node:http, node:net: a plain socket has none of TLSSocket's property names #45659 (same file, 1 conflicting region): it moves encrypted and _secureEstablished into subclass field initialisers, which run after the base constructor. After either rebase, handshaking in the constructor reads listenerGeneration.tls && !handle.secureEstablished. The tests here fail if http state is attached at accept.
• node:http: set the parser options of a connection in the server's own listener #46046 reads the parser options between 'connection' and 'secureConnection' in one call. With this PR that point is the secured arm, before 'secureConnection' is emitted, which is where Node's own listener runs. The PR that lands second takes that move.
• node:http: honor server.maxHeadersCount above 198 fields #45532 copies server.maxHeadersCount behind the filters in onHandshake: still after both events. A 'connection' listener of an https server now runs before the handshake, and the connection takes a limit it assigns, as in Node.
• node:http: close the connection when a 'connection' or 'secureConnection' listener throws and nothing handles it #45876 wraps the thunk's call: it needs on_connection_event in place of on_connection_callback.
• node: fix the genuine-hang class — MessagePort loop starvation, pinned sockets, https handshakeTimeout, TLS alert errors (+2 tests) #35538 (draft): this PR lands the accept-time half of its "Fix 3". Its handshakeTimeout, 'tlsClientError' listener and test-https-timeout-server.js are not here.
• usockets: bound the accepts of one listener readiness event #44198 bounds the accepts of one listener readiness event at 32. The accept dispatches of one loop iteration then have the same bound. That is not measured here.
• fix(node:https): wrap injected raw connections with TLS #42594 (emit('connection', rawSocket) on an https server) and node:https: make req.socket a tls.TLSSocket with getPeerCertificate() #37255, node: tls/https v26 compat wave 2 — allowHalfOpen close_notify, fetch setDefaultCACertificates, https.Server setSecureContext/keylog/TLSSocket (+6 tests) #35535, node: net/tls/http/http2 compat fixes #41400 (the server socket becomes a tls.TLSSocket) are independent. With the latter, 'connection' carries a TLSSocket before its handshake.

Not in this PR.

• options.handshakeTimeout, 'tlsClientError' and its 'clientError' bridge.
• The headersTimeout clock of a TLS connection still starts at accept, as on main. With headersTimeout 1,500 ms and a handshake 3 s after connect, Node answers 408 1,530 ms after the handshake and main 30 ms after. The stamp is in HttpContext::onOpen, which node:http: honor server.maxHeadersCount above 198 fields #45532 edits.
• The 1 s TCP_DEFER_ACCEPT delay for a client that sends nothing (node:http: skip TCP_DEFER_ACCEPT so server.setTimeout fires on time for silent clients #36074).
• The two other places that build a socket (the request dispatcher and clientError, for a connection with no socket yet) do not run the gates.

Existing tests changed. Three blockList tests checked the refusal with https.get(). The refusal is now at accept, and reaches a client as a FIN or as a reset. Bun's https client reports a FIN with a request queued as ERR_SOCKET_CLOSED (Node: ECONNRESET), so they now use a TLS client that sends nothing. They fail on main, where the blocked peer completes its handshake. The whole file passes with TCP_DEFER_ACCEPT refused by an LD_PRELOAD shim, which is the listener mode of macOS and Windows.

Self-review. Done:

• The transport comes from the listener generation.
• The handshake phase of a socket is a private symbol.
• The timer starts again at 'secureConnection'.
• New tests: close() in a listener, the shutdown helpers, a handled throw, end() at accept, destroy(err), both server.timeout orders, and plain sockets that set encrypted.
• The tests do not depend on TCP_DEFER_ACCEPT.
• Reworded comments, and paired measurements.
• From the review of the PR: the handshake slot is a class field with a default.

Not done:

• The headersTimeout clock (above).
• The listen({ tls }) getters (node:http, node:net: a plain socket has none of TLSSocket's property names #45659).
• test-https-timeout.js. It needs the client fix in net: fire socket.setTimeout() when the write queue stalls #42404. Its 10 ms client timeout is lost while the request is queued behind the handshake (socket.setTimeout() never fires when the peer stops reading and the write queue stalls #42403), and the first 'connection' dispatch now runs before the handshake.

Measurements. Release builds (bun-profile) of main ad4089b and of this change on that commit, Linux x64. Counts are gdb breakpoints with ignore counts, 200 sequential connections, one run. Instructions are user-mode perf_event counts of the server's main thread, read at the entry and the ret of the filter thunk. Each cell is the median of the last 200 of 1,200 connections.

per connection	plain http	https	failed handshake	refused peer
on-connection JS calls	1 -> 1	1 -> 2	1 -> 1	1 -> 1
microtask checkpoints from the thunk	1 -> 1	1 -> 2	1 -> 1	1 -> 1
handle getter crossings	2 -> 1	3 -> 2	3 -> 1	2 -> 1
timers armed	0 -> 0	0 -> 0	0 -> 0	0 -> 0

• https, 3 paired runs: 12,060 -> 13,001, 12,059 -> 13,004, 12,003 -> 13,002 instructions in the thunk. Whole connection, minimum of each run: 7,962,181 -> 7,962,688, 7,962,212 -> 7,963,392, 7,962,045 -> 7,963,070.
• Plain http, 4 paired runs: 11,840 -> 11,839, 11,824 -> 11,847, 11,873 -> 11,896, 11,858 -> 11,857 in the thunk. Whole connection, minimum of each run: 48,288 -> 48,282, 48,283 -> 48,272, 48,174 -> 48,190, 48,211 -> 48,265. objdump of the plain thunk: 1 more instruction on the dispatch path (the store of the new argument), none for the other filter events. Bun.serve does not register this thunk, and no uWS code changed.
• Refused peer (blockList, 1 run, last 200 of 400 TLS attempts): 2,576 -> 0 bytes received, 200 -> 0 handshakes completed, 7,751,487 -> 43,532 instructions per peer.
• Failed handshake (1 run): 17,730 -> 16,327 instructions in the thunk.
• Pending handshake (1,000 held, bun:jsc heapStats() after a full GC, 1 run): +13.02 cells and +1,588 bytes each (main: 0). The cells are back to the baseline after the clients leave.
• Class field [kHandshaking] = false, added in review after the numbers above: the cell of every node:http server socket goes from 224 to 256 bytes (inline capacity 26 -> 30 in describe() of bun:jsc, out-of-line capacity 64 -> 64). heapStats() on the debug build, 3 paired runs of 1,000 held handshakes: +27 to +35 bytes each. So a pending handshake holds 1,620 bytes. Plain and TLS sockets now have the same 70 properties (before: 69 and 70). The instruction counts above are from before this field and were not measured again.
• Burst of 500 TLS connects (1 run): at most 5 -> 500 on-connection dispatches in one loop iteration. The 500 are the accept calls, about 5.5 K to 10.6 K instructions each. The 'secureConnection' calls stay at 5 per iteration, behind the handshake throttle.
• Release binary: .text +0 bytes, total +761 bytes (size), which is the growth of the builtin _http_server module. No new symbol: the one new extern "C" function is inlined at link time. bloaty and bpftrace are not installed here, so size, nm and gdb were used.
• Vendored test-http-*.js and test-https-*.js: 450 of 450 pass on both release builds, one run each. test-https-timeout.js over 750 runs each: 4 hangs on main, 10 on this branch.

---

no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/node/http/node-https-server.test.ts

[robobun]

^{Updated 5:58 PM PT - Oct 22nd, 2026}

@robobun, your commit 3560d13 is building: #131044

[robobun]

Status: reproduced on main ad4089b (release build, Linux x64) with a TCP client that connects to an https.createServer() server and sends no TLS handshake. Node.js v26.3.0 emits 'connection' and getConnections() reports 1. Bun emits nothing and reports 0. When the handshake ends, also in failure, Bun emits 'connection'.

Fix: #46167 (this PR). test/js/node/http/node-https-server.test.ts has 26 new tests and 3 changed tests. 26 of these 29 fail on a release build of main. All 73 tests of the file pass on the debug build of this branch.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Source mode: Auto
• Repository: oven-sh/bun
• Branch: robobun/d95e8457/https-connection-at-accept
• File: .coderabbit.yaml
• Commit: 3560d13
• Layers: Repository YAML
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 03b70c46-02de-46e0-8e66-ca3b952f7732

📥 Commits

Reviewing files that changed from the base of the PR and between c3079e7 and 3560d13.

📒 Files selected for processing: 6

• src/js/node/_http_server.ts
• src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp
• src/runtime/server/mod.rs
• src/runtime/server/server_body.rs
• src/uws_sys/App.rs
• test/js/node/http/node-https-server.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.

Walkthrough

The change separates TCP acceptance from TLS handshake completion in Node HTTP servers. It adds named connection-filter events and passes secured status through the native callback bridge. HTTP parser setup and timeout behavior are deferred until TLS negotiation succeeds.

Changes

TLS connection lifecycle

Layer / File(s)	Summary
Connection event decoding and dispatch src/uws_sys/App.rs, src/runtime/server/*, src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp, src/js/internal/http.ts	The runtime decodes named filter events and routes socket and secured-status data through the Node HTTP callback bridge. Connection and tunnel accounting uses the named event variants.
TLS handshake state and HTTP setup src/js/node/_http_server.ts, test/js/node/http/node-https-server.test.ts	TLS sockets remain in a handshaking state until negotiation succeeds. Parser setup, HTTP timeout behavior, and secureConnection are deferred until then. Tests cover handshake states, connection handling, timeouts, cleanup, and listener exceptions.

Possibly related issues

• oven-sh/bun#43381 — The changes test HTTPS connection-listener exceptions, which address part of the issue’s stated objective. They do not cover the remaining node:net and node:tls teardown differences.

Suggested reviewers: jarred-sumner

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 3560d

HTTPS servers now emit 'connection' at TCP accept and 'secureConnection' after the handshake, matching Node. No concrete merge-blocking defect was found in the reviewed change. The author's reported edge-case differences from Node are worth keeping in mind.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title clearly and concisely identifies the main change: emitting the HTTPS "connection" event at TCP accept before the TLS handshake.
Description check		The description provides detailed problem, fix, verification, background, limitations, and performance information. It does not use the template headings exactly, but it includes the required content …
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description provides detailed problem, fix, verification, background, limitations, and performance information. It does not use the template headings exactly, but it includes the required content and is substantially complete.

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

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

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[claude]

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

Beyond the inline findings, I also checked the FilterEvent refactor for behavior drift: the plain-TCP path of the node:http thunk still forwards only Opened (unknown values still return early), and every arm of on_connection_filter in src/runtime/server/mod.rs maps one-to-one onto the old integer values. Bun__NodeHTTP__securedServerSocket null-checks a detached socket and gates on isAuthorized(), and the Rust side checks the empty JSValue before calling into JS, so a failed or torn-down handshake is not reported.

Extended reasoning...

The change moves the node:https 'connection' event from handshake end to TCP accept across the Rust uWS filter thunk (src/runtime/server/server_body.rs), a new C++ export (src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp), a new FilterEvent enum (src/uws_sys/App.rs) and a two-phase onServerConnection in src/js/node/_http_server.ts, with 656 lines of new tests. It touches the TLS server connection lifecycle and the point where user JS runs re-entrantly inside the uWS accept callback, so it is not a candidate for automated approval. Two confirmed findings on the JS timer and hidden-class handling are posted inline; the native refactor was checked and found behavior-preserving for the non-SSL path.

[robobun]

One row of this PR (a 'connection' listener throws, the process handles it, 'secureConnection' follows) was also the target of another branch: robobun/69904b85/https-secure-connection-after-handled-throw. There is no PR for that branch. This PR removes the cause, so the branch is not needed.

What the branch did: it kept the one dispatch, and the error arm of the filter thunk called the connection callback a second time to emit only 'secureConnection'. It has a defect. Its one-shot native getter is consumed when a listener prints the socket, and 'secureConnection' is then lost.

The test of that branch can move here. It is a fixture that runs unchanged under Node.js, and a table of rows. I ran the fixture on a release build of 6a5fec1:

• The 14 rows that are Node.js v26.3.0 output: 14 of 14 equal Node.js. On main, 8 of them differ.
• Two more cases with no throw equal Node.js on this PR and differ on main. The ticks of a 'connection' listener run before 'secureConnection' (handled-quiet-ticks-keeps). A listener that writes 4 MiB and destroys the socket gets no 'secureConnection' (handled-quiet-writes-destroys).

Two edge rows of the table show a difference from Node.js on this PR's build:

• https.createServer({ highWaterMark: 1.5 }) (handled-fractionalHighWaterMark): the constructor of the socket throws at accept. The handshake call then finds no socket and throws TypeError: null is not an object. One connection reports 3 errors. main reports 2, Node.js reports 1.
• server.timeout = -1 (handled-badTimeout): the handshake call throws ERR_OUT_OF_RANGE before it emits 'secureConnection', and the request is then answered. Node.js emits the same events and never answers that peer. main answers it too.

If 'connection' at TCP accept is not wanted, the branch is the other shape for this one row. Its latch then has to be a field of the socket, not a getter.

[claude]

I re-reviewed the two pushed commits and found no bugs; the hidden-class nit from the earlier pass is addressed by the class-body [kHandshaking] = false in c3079e7. Because this reorders the TLS accept/handshake lifecycle across uWS, the Rust filter thunk, the C++ socket binding and node:http's socket state, a human look is still worthwhile.

What was reviewed:

• The Opened → secured path: usockets sets HANDSHAKE_COMPLETED and isAuthorized before onHandshake runs the filter, so Bun__NodeHTTP__securedServerSocket reads the real outcome and a failed handshake yields no second JS call.
• The secured arm's reliance on socketHandle.duplex: the only producer of a pre-handshake JSNodeHTTPServerSocket is the accept dispatch, whose constructor sets duplex before anything can throw; dropped/blocked sockets are closed so onHandshake skips them.
• FilterEvent consumers: both switches (mod.rs counting, server_body.rs forwarding) are exhaustive and the old _ => return fallthrough is preserved as None.
• listenerGeneration.tls is the same value that selects the native SSL app, so encrypted/'secureConnection' now track listen({ tls }) too.

Extended reasoning...

The change spans the uWS filter event model (new strum FromRepr enum in src/uws_sys/App.rs), the Rust filter thunk and on_connection dispatch in src/runtime/server/server_body.rs and mod.rs, a new C++ export in JSNodeHTTPServerSocket.cpp, and the node:http socket lifecycle in src/js/node/_http_server.ts, plus ~650 lines of new tests. It touches TLS connection lifecycle (handshake gating, maxConnections/blockList now applied at accept) but no auth/crypto decisions themselves; rejectUnauthorized handling in onHandshake is unchanged. The code is consistent with usockets ordering and the new tests cover each entry point and the failure/destroy/timeout cases, which is why no finding was raised, but the complexity and the cross-layer lifecycle semantics are why a human should still weigh in rather than auto-approve.

Still open from earlier reviews (1):

• Unresolved: 1 minor or pre-existing.

[claude]

I re-reviewed the latest three commits and didn't find any bugs; the [kHandshaking] = false class-body default from the earlier nit is now in place. Because this changes connection-lifecycle ordering across the uWS filter, Rust FFI, C++ and node:https in one go, a human look would still be worthwhile.

What was reviewed:

• The accept/handshake split: HttpContext::onHandshake closes a rejected handshake before firing filter 1, and Bun__NodeHTTP__securedServerSocket returns empty when isAuthorized() is false, so a failed handshake never reaches the secured arm.
• The $assert(!socket[kHandshaking]) on the request path: ssl_on_data in usockets fires on_handshake before dispatching any decrypted bytes, so the assumption holds.
• The socket created at accept is rooted by strongThis until close, and a socket.destroy() in a 'connection' listener closes the uWS socket synchronously, so no 'secureConnection' can follow it.
• The FilterEvent match in on_connection_filter and the thunk preserve the previous integer handling (unknown codes and Opened/Closed still return early).

Extended reasoning...

The PR touches src/runtime/server/server_body.rs and mod.rs (connection filter decoding and the JS onConnection dispatch), src/uws_sys/App.rs (new FilterEvent enum), src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp (new securedServerSocket export), and src/js/node/_http_server.ts (kHandshaking gating of timeouts, sweeps and closeAllConnections), plus ~650 lines of new tests. It touches the TLS server accept path, which is security-adjacent (rejectUnauthorized handling stays in uWS and is unchanged). The bug hunt ran dry with no findings and the earlier nit was addressed, but the change is large, alters event ordering and per-connection state for every https server, and spans four languages, so it is not a candidate for bot-only approval.

[claude]

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