node:net: emit 'close' on a server TLS wrap whose handshake fails on the stream-level engine by robobun · Pull Request #38058 · oven-sh/bun

robobun · 2026-08-13T05:31:49Z

Problem

A server-side TLS socket whose handshake fails emits 'error' (or 'tlsClientError' on a tls.Server) but never 'close', although socket.destroyed becomes true. Node emits 'error' followed by 'close' with hadError === true.
It happens whenever the failure is reported while JS is already on the stack. Two ways users hit that:
- new tls.TLSSocket(conn, { isServer: true }) running on the stream-level TLS engine (taken when conn still has unflushed plain writes, or is a generic Duplex), and the peer then sends non-TLS bytes or disconnects: the engine is fed from a 'data' listener.
- a tls.Server (or an fd-adopting wrap) whose asynchronous SNICallback rejects the connection, the usual shape of an SNI-routing server turning away unknown hostnames: resumeSNI fails the handshake from inside the user's callback.
Cause: the handshake-failure destroy (_closeAfterHandlingError) makes Socket.prototype._destroy (src/js/node/net.ts:2193) defer closeSocketHandle to a microtask so the 'error' listeners can still see the connection, and closeSocketHandle re-read self._handle when it finally ran. Bun's native close callback (ServerHandlers.close -> detachSocket) nulls _handle, and on the dispatches above it runs synchronously right after the handshake callback, inside the same native call, before any microtask checkpoint (microtasks drain only when the outermost event-loop entry unwinds, src/jsc/event_loop.rs exit()). The deferred close then found no handle and returned without emitting. A failure dispatched straight from the event loop (for example an fd-adopting wrap fed bad bytes) drains the microtask first, which is why that ordering worked.
The error code reported on the engine path (UNABLE_TO_GET_ISSUER_CERT instead of ERR_SSL_WRONG_VERSION_NUMBER) is a separate bug fixed by node:tls: report the fatal TLS alert when a handshake over a Duplex fails #32929; this PR leaves it alone and the tests do not assert the code.

Fix

_destroy passes the handle it found (currentHandle) into closeSocketHandle, which closes and reports that handle instead of re-reading _handle. closeAdoptedTLSRawNowNT was the same close-then-report sequence, so the adopted raw-socket branch now calls closeSocketHandle too.
Why this is correct: a net.Socket emits 'close' exactly once, from whichever teardown branch its _destroy takes. The native close callback deliberately does not emit it (it only detaches the handle and pushes EOF, relying on _destroy), so once _destroy has run the deferred branch is the only emitter left and must not depend on _handle still being attached. The synchronous branch already tolerates the detach (_destroy re-checks this._handle after closeSocketHandle returns, because handle.close() dispatches the native close re-entrantly); this gives the deferred branch the same tolerance. It is also what Node's closeSocketHandle effectively does: nothing in Node detaches _handle behind the stream's back, so it always closes the handle destroy() found.
Closing the captured handle after the native side closed it is a no-op: the close dispatch puts the wrapper into the detached state before calling into JS (src/runtime/socket/socket_body.rs:2057), close() on a detached wrapper does nothing (src/uws_sys/socket.rs:343), and the event-loop unref it performs is idempotent (src/io/keep_alive.rs:43). When nothing detached the handle in between, currentHandle === this._handle and the behaviour is byte-for-byte the old one.
Verified with test/js/node/tls/node-tls-server.test.ts, describe "server-side handshake failure emits 'close'": six cases (fd-adopting wrap fed bad bytes; unflushed-write wrap fed bad bytes; generic Duplex wrap fed bad bytes; unflushed-write wrap whose peer disconnects; fd-adopting wrap rejected by an asynchronous SNICallback; tls.Server rejected by an asynchronous SNICallback, asserting 'close' on the socket handed to tlsClientError). Five wait forever for 'close' on the unfixed build; all six pass with the fix; the expected sequences were checked against Node v26.3.0 with the probes below.
Also run with the fix: the rest of node-tls-server.test.ts, node-tls-connect.test.ts, node-tls-upgrade.test.ts, the test/js/node/net suites, and 89 vendored Node test-tls-* / test-net-* / test-https-* close, destroy, SNI and handshake-failure tests (including test-tls-socket-close for the adopted raw-socket branch and test-tls-sni-option for asynchronous SNI), all passing. The only failures seen were pre-existing in this container and reproduce without the diff (tests binding localhost while connecting to 127.0.0.1, and one 60s leak test).

Background

_closeAfterHandlingError: Node's server-side handshake failure path destroys the socket with the error but keeps _handle attached until the next microtask so 'error' / 'tlsClientError' listeners can still inspect the connection; closeSocketHandle then closes the handle and emits 'close'. Bun mirrors this.
detachSocket: Bun's native close callbacks null socket._handle because the native object is gone. It is the one way _handle can disappear between destroy() and the deferred close, and it never emits 'close' itself.
Nested dispatch: Bun drains microtasks when the outermost native-to-JS entry returns. A socket callback invoked straight from the I/O loop gets a microtask checkpoint as soon as it returns; one invoked from inside a JS-initiated native call (the stream engine's listener thunks, resumeSNI, _handle.close()) does not get one until the outer JS frame finishes, so anything native does right after that callback, such as dispatching the close, runs before the microtask.
Server wrap paths: new TLSSocket(conn, { isServer: true }) normally adopts the connection's fd into a native TLS socket driven by the C TLS code. When the fd cannot be adopted (pending plain bytes must go out first, or there is no fd), upgradeDuplexToTLS runs a BoringSSL engine over the stream instead, fed by the stream's 'data' events.

Event logs from the probes (events on the server-side socket)

case                                            Bun before                              Bun after                                  Node v26.3.0
unflushed-write wrap, non-TLS bytes             error                                   error, close:true                          error, close:true
unflushed-write wrap, peer disconnects          end, error(ECONNRESET)                  end, error(ECONNRESET), close:true         end, close:false
generic Duplex wrap, non-TLS bytes              error                                   error, close:true                          error, close:true
fd-adopting wrap, async SNICallback rejects     error                                   error, close:true                          error, close:true
tls.Server, async SNICallback rejects           tlsClientError                          tlsClientError, close:true                 tlsClientError, close:true
fd-adopting wrap, non-TLS bytes                 error, close:true                       unchanged                                  error, close:true

The 'error' on a mid-handshake disconnect is pre-existing behaviour on both wrap flavours and out of scope; that test only asserts that 'close' arrives and that its hadError agrees with whether an 'error' was emitted.

net.ts debug trace of an unfixed failing case (BUN_DEBUG_JS=net), showing the native close detaching the handle before the deferred close runs:

[net] Socket.prototype._destroy          <- ServerHandlers.handshake(false): destroy(err), close deferred to a microtask
[net] Bun.Server close                   <- native close callback, detachSocket() nulls _handle
[events] TLSSocket.emit error
[net] closeSocketHandle true true false  <- _handle already null: nothing closed, no 'close'

The first revision of this PR described the problem as specific to the stream-level engine; review of the dispatch path showed the condition is the nested dispatch above, which the asynchronous SNICallback cases also reach on the plain fd path. The code change is the same; the description, comments and test matrix were widened to match.

…n the meantime Socket.prototype._destroy defers the handle close to a microtask when _closeAfterHandlingError is set (server-side TLS handshake failures), and closeSocketHandle re-read this._handle when that microtask ran. When the server-side TLS engine runs over a stream (upgradeDuplexToTLS: a wrapped connection with unflushed writes, or a generic Duplex), the engine dispatches its close callback right after the failed handshake, inside the same stream event, and that callback detaches _handle before the microtask runs. The deferred close then found no handle and the wrap never emitted 'close'. Capture the handle in _destroy and pass it to closeSocketHandle, so the deferred close tears down and reports the handle destroy() found regardless of what detached it since. Closing a handle that already closed natively is a no-op.

coderabbitai · 2026-08-13T05:32:00Z

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 14 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8e2066f9-8bd2-4700-9efa-1fed2e112bcf

📥 Commits

Reviewing files that changed from the base of the PR and between 04148c8 and 14f056c.

📒 Files selected for processing (2)

src/js/node/net.ts
test/js/node/tls/node-tls-server.test.ts

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

robobun · 2026-08-13T05:32:05Z

^{Updated 4:56 AM PT - Aug 13th, 2026}

@robobun, your commit 14f056c is building: #94711

robobun · 2026-08-13T05:32:43Z

Status: fix and tests pushed (latest 14f056c), waiting on CI.

Reproduced on the released build (1.4.0-canary.1) and on a debug build of main: a server-side TLS socket whose handshake is failed while JS is on the stack emits 'error' / 'tlsClientError' but never 'close'. Triggers used: new tls.TLSSocket(conn, { isServer: true }) over a corked connection or a generic Duplex that is fed non-TLS bytes (or whose peer disconnects), and a tls.Server or fd-adopting wrap whose asynchronous SNICallback rejects. Node v26.3.0 emits 'close' with hadError === true in every case.

Five of the six cases in test/js/node/tls/node-tls-server.test.ts ("server-side handshake failure emits 'close'") wait forever for 'close' without the src/js/node/net.ts change and pass with it; the sixth (a failure dispatched straight from the event loop) passes both ways and pins the unchanged ordering.

claude

I reviewed this PR and didn't find any bugs. The change is small and the analysis is thorough, but it modifies Socket.prototype._destroy — the teardown path every net/tls socket goes through — and removes the null guard in closeSocketHandle, so a maintainer look would still be worthwhile.

What was reviewed:

Confirmed both closeSocketHandle call sites are inside if (this._handle) with currentHandle captured immediately after, so the removed null guard cannot be reached with a null handle.
Checked the synchronous branch is behavior-identical (currentHandle === this._handle at that point) and closeAdoptedTLSRawNT is a separate path that doesn't call closeSocketHandle.
Looked for double-'close' risk — _destroy runs once per socket and detachSocket only nulls _handle without emitting, so the deferred emit remains the sole emitter.

Extended reasoning...

Overview

This PR fixes a missing 'close' event on server-side TLSSocket wraps that use the stream-level TLS engine (i.e., when the underlying connection has unflushed writes or is a generic Duplex) and the handshake fails. The fix is in src/js/node/net.ts: _destroy now passes the already-captured currentHandle into closeSocketHandle instead of having closeSocketHandle re-read self._handle, which may have been nulled by detachSocket before the deferred microtask runs. Four new tests in test/js/node/tls/node-tls-server.test.ts cover the fd-adopting wrap, unflushed-write wrap, generic Duplex wrap, and mid-handshake disconnect.

Security risks

None identified. This is lifecycle/event-emission ordering; no user input parsing, no auth/crypto/permission logic.

Level of scrutiny

High. Socket.prototype._destroy (net.ts:2177-2225) runs for every net.Socket teardown across net, tls, http, and https. The change removes a defensive if (handle) guard in closeSocketHandle and relies on the invariant that both callers pass a non-null captured handle, and that handle.close() on an already-natively-closed handle is a no-op. The PR description argues both convincingly and reports running the full net/tls suites plus 109 vendored Node tests, but socket lifecycle in this file is one of the most historically delicate areas of the codebase.

Other factors

Verified there are exactly two closeSocketHandle callers (net.ts:2205, 2212), both inside the if (this._handle) block with currentHandle = this._handle captured on line 2184, so the removed null guard is provably unreachable with null.
The synchronous call site (line 2212) is behavior-identical: nothing between capture and call can mutate _handle.
The tests await the actual 'close' event (not sleeps), wire failure paths to reject, clean up in finally, use port: 0, and cover the variant matrix (adopting/unflushed/Duplex/disconnect) — they meet the repo's test-quality bar.
The only claim I can't verify statically is that handle.close() on a wrapper whose native side already dispatched close is a safe no-op; the PR description states it is and cites the detached-state check, but that depends on native handle behavior a maintainer should confirm.

robobun · 2026-08-13T05:40:31Z

For the one point left open in the review, the "second close() is a no-op" claim, here is the native code it rests on (all on main, none of it changed by this PR):

The native close dispatch puts the wrapper into the detached state before it calls into JS: src/runtime/socket/socket_body.rs:2057 (this.socket.set(DETACHED) in on_close; the no-handlers early return at :2043 does the same). This is what has already happened when the deferred closeSocketHandle runs on the engine path.
_handle.close() is the close host function at socket_body.rs:3101. It calls socket.close(FastShutdown) (:3132), which on a detached wrapper is the detached => {} arm of NewSocketHandler::close in src/uws_sys/socket.rs:343; it then sets the already-detached state again and unrefs the loop via KeepAlive::unref (src/io/keep_alive.rs:43), which returns early unless the ref is still active. The is_semi_connect deref at :3122 only triggers for a live connected us_socket_t, and InternalSocket::get() returns None for the detached and duplex arms.

The new engine-path tests exercise exactly this sequence (native close first, then the deferred close() on the captured handle) under the ASAN debug build, which is also where a double release would show up.

The lost 'close' is not specific to the stream-level TLS engine: the native close callback detaches _handle ahead of the deferred close whenever the failed handshake is dispatched with JS already on the stack, which an asynchronous SNICallback rejection (resumeSNI runs from the user's callback) does on a plain tls.Server and on an fd-adopting wrap as well. Describe that condition at the _destroy branch and add both fd-backed cases to the test matrix. closeAdoptedTLSRawNowNT was the same close-then-report sequence as closeSocketHandle now that the latter takes the handle; use it directly.

claude

I reviewed this PR and didn't find any bugs. Because it changes the net.Socket destroy/close-emit path — lifecycle code that every TCP/TLS connection goes through, with correctness resting on the native-side "close() on a detached handle is a no-op" invariant — a human look would still be worthwhile.

What was reviewed:

All closeSocketHandle callers pass a provably non-null handle (captured inside the if (this._handle) guard), so dropping the null check is safe.
closeAdoptedTLSRawNT now delegates to closeSocketHandle with isCleanupPending defaulting to false — behavior-identical to the deleted closeAdoptedTLSRawNowNT, and emitCloseNT remains live (used at net.ts:2218).
Argument reordering of closeAdoptedTLSRawNT is consistent between call site and definition; no other callers exist.

Extended reasoning...

Overview

The PR changes closeSocketHandle in src/js/node/net.ts to accept the handle as an explicit argument (captured by the caller at destroy time) rather than re-reading self._handle, which can have been nulled by detachSocket between _destroy scheduling the deferred close and the microtask actually running. This fixes server-side TLS wraps on the stream-level engine path never emitting 'close' after a handshake failure. It also folds closeAdoptedTLSRawNowNT into closeSocketHandle (deduplication) and adds six regression tests to test/js/node/tls/node-tls-server.test.ts.

Security risks

None identified. The change is confined to teardown-event ordering; no auth, crypto, input parsing, or trust decisions are touched.

Level of scrutiny

High. Socket.prototype._destroy and closeSocketHandle sit on the teardown path of every net/tls/http connection in Bun. The fix's correctness depends on a native-side invariant (a second close() on a detached socket wrapper is a no-op — the author cited socket_body.rs / uws_sys/socket.rs / keep_alive.rs) and on 'close' being emitted exactly once by whichever _destroy branch runs. That reasoning is well laid out and I found no counterexample, but lifecycle code with cross-layer invariants is exactly where a maintainer's eye is most valuable.

Other factors

All three closeSocketHandle call sites and the one closeAdoptedTLSRawNT call site were updated consistently; the argument reorder of closeAdoptedTLSRawNT matches at both ends. No stale callers.
Removing the if (handle) guard is justified: every caller passes currentHandle, captured inside if (this._handle).
closeAdoptedTLSRawNowNT was deleted; emitCloseNT (its former dependency) is still referenced by the no-handle branch of _destroy, so no dead code was introduced.
Tests cover the variant matrix (fd-adopting / unflushed-write / Duplex / peer-disconnect / async-SNI on both wrap and tls.Server), await the actual 'close' event, and clean up in finally. The PR description confirms fail-without-fix on the three engine-path cases.
The comment-cop bot's inline feedback about long comments was addressed in follow-up commits and all threads are resolved.
No CODEOWNERS match the changed files.

robobun · 2026-08-13T17:42:58Z

Follow-up after self-review (c19943c, 26773c6, 14f056c; the net.ts logic is unchanged from the first revision):

The lost 'close' is not specific to the stream-level engine. The condition is a handshake failure dispatched while JS is already on the stack, and an asynchronous SNICallback rejection reaches it on the plain fd path too (resumeSNI fails the handshake from inside the user's callback, and the native close follows before the microtask). Verified on the released build: a tls.Server and an fd-adopting wrap with an async SNICallback that rejects both emit 'tlsClientError' / 'error' and never 'close'; with this change both emit 'close' with hadError === true, as Node does. The PR description, the comment at the _destroy branch and the test matrix now describe that condition; two cases covering the fd path were added (six total, five failing without the fix).
closeAdoptedTLSRawNowNT was the same close-then-report sequence as closeSocketHandle once the latter takes the handle, so the adopted raw-socket branch now calls closeSocketHandle directly (test-tls-socket-close still passes).
The comment-bot threads were addressed by trimming the added comments; the three-line note at the _destroy branch is kept deliberately, see the thread replies.

The CodeRabbit comments above are its rate-limit notices; there is no review content in them to act on.

robobun · 2026-09-11T14:43:34Z

Heads-up on an overlap: #42330 also edits closeSocketHandle in src/js/node/net.ts. It replaces the unconditional handle.close(onSocketHandleClosed) with if (handle instanceof Duplex) handle.destroy(); else handle.close(onSocketHandleClosed); for a client-side new tls.TLSSocket(stream) whose _handle is still the wrapped stream. This PR changes the signature of the same function, so the two conflict in those lines. Whichever lands second needs to carry the instanceof Duplex branch into the new shape. The two changes do not depend on each other.

…ists (#44196) A TLS socket over a `stream.Duplex` (`tls.connect({ socket })`, `new TLSSocket(duplex)`) has no fd. Bun runs a TLS engine over the stream, and a queued task starts that engine after the wrap. "Before the engine" below is the rest of the script, `process.nextTick`, microtasks and promise jobs. The lost EOF needs the close hook of #39066. The freed listener functions need the `pending_close` branch of #42176. No release has either. ### What the TLS socket emits | The transport, before the engine | Node v26.10 | main | This PR | |---|---|---|---| | Client: EOF, close | `end`, ECONNRESET, `close` true | `close` false | as Node | | Server wrap: EOF, close | `end`, `close` false | `close` false | as Node | | Client: bytes, EOF, close | `end`, ECONNRESET, `close` true | `close` false | as Node | | Server wrap: bytes, EOF, close | `end`, `close` false | `close` false | as Node | | Client: non-TLS bytes, EOF | ERR_SSL_HTTP_REQUEST, `close` true | `secureConnect`, `end`, `close` false | `end`, ECONNRESET, `close` true | | Server wrap: a complete ClientHello, EOF | `end`, 1 write | `end`, 1 write | as Node | | Server wrap: a complete ClientHello, EOF, close | `close` false | `close` false | `end`, `close` false | Last row: Node writes its answer to the ended transport, and that failure closes the socket ahead of `end`. The same rows after the engine started do not change. ### What happens to the transport | Case | Node v26.10 | main | This PR | |---|---|---|---| | `tls.connect({ socket: netSocket }).destroy()` in the same tick | raw socket closed, peer gets a FIN | raw socket open, no FIN | as Node | | The socket or the transport is destroyed after the engine started | no `end()`, `writableEnded` false | `end()` on the destroyed transport, `writableEnded` true | as Node | | 16 wraps closed before the engine, a GC, then 8 new wraps | | 7 of 8 never reach `secureConnect` | 8 of 8 answer | Last row: the close left the four listener functions of the context set. The GC freed them, and the task that frees the context then wrote into the freed cells. ### What the transport throws | Case | Node v26.10 | main | This PR | |---|---|---|---| | The `write` getter throws (the ClientHello) | `uncaughtException` | the exception stays pending on the VM (debug build: panic) | `error` on the socket, `close` true | | The `end` getter throws, close after the engine started | `end` is not read | the exception stays pending on the VM (debug build: assertion) | `uncaughtException` | | `end()` throws, close after the engine started | `end()` is not called | dropped | `uncaughtException` | | `end()` or its getter throws, close before the engine | `end()` is not called | `end()` is not called | `uncaughtException` | The `end` rows need a transport that is not ended and not destroyed when the socket closes. ### Behavior changes | Case | Before | After | |---|---|---| | Close before the engine, transport not ended and not destroyed | the transport is left as it is | `end()` is called on it | | `end()` of the transport throws during the close | dropped | uncaught exception | | Close after the engine started, transport destroyed | `end()` is called, `writableEnded` true | no call, `writableEnded` false | | EOF before the engine | held until the engine starts | reported inside the transport's `'end'` | | Client, EOF only, before the engine | 1 write (the ClientHello) | 0 writes (Node: 1) | | Client, bytes ahead of an EOF, before the engine | the engine reads them | not read, so an SSL error in them shows as ECONNRESET | | Server wrap, `socket.end()` in an `'end'` listener | transport `writableEnded` true, `final` not called | `writableEnded` false, `final` not called (Node: true, `final` called, see #42350) | ### Not in this PR - #32929: non-TLS bytes give a client `secureConnect`. - #38058: a server wrap whose handshake fails gets no `'close'`. - #43392: an extra `'end'` ahead of `'close'` when the socket is destroyed after the engine started. - #42350: `socket.end()` ends the transport. - #38028: destroy a wrapped `net.Socket`, as Node does. This PR ends it. ### Tests `node-tls-connect.test.ts` and `node-tls-duplex-close-throw-uaf.test.ts`: 25 fail on main (9f70da0), 0 fail with this PR. `test/js/node/tls` and `test/js/node/http2`: 1077 pass, 0 fail. Debug + ASAN, macOS arm64.  --- **[human-review]** gate passed · iteration 3 · 4 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 15 failed, 18 skipped $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/node/tls/node-tls-connect.test.ts test/js/node/tls/node-tls-duplex-close-throw-uaf.test.ts bun test v1.4.3 (367d939) test/js/node/tls/node-tls-duplex-close-throw-uaf.test.ts: (pass) tls.connect({socket: Duplex}) does not read freed Handlers > when a pre-open duplex error races StartTLS [1332.69ms] (pass) tls.connect({socket: Duplex}) does not read freed Handlers > when duplex.end() throws after close [1443.48ms] 25 | const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); 26 | 27 | // On failure stderr carries the ASAN "use-after-poison" report; include 28 | // it in the assertion so the diff shows the crash rather than just an 29 | // empty stdout. 30 | expect({ stdout: stdout.trim(), stderr, exitCode }).toEqual({ stdout: expected, stderr: "", exitCode: 0 }); ^ error: expect(received).toEqual(expected) { "exitCode": 0, "stderr": "", "stdout": - "the duplex closes, end() calls: 1 - the soc ... (truncated) release without fix: 34 failed, 22 skipped bun test v1.4.3-canary.1 (367d939) test/js/node/tls/node-tls-duplex-close-throw-uaf.test.ts: (skip) tls.connect({socket: Duplex}) does not read freed Handlers > when duplex.end() throws after close (skip) tls.connect({socket: Duplex}) does not read freed Handlers > when a pre-open duplex error races StartTLS (skip) tls.connect({socket: Duplex}) does not read freed Handlers > when duplex.end() throws after a close that comes before StartTLS (skip) tls.connect({socket: Duplex}) does not read freed Handlers > when an EOF listener destroys the socket and throws before StartTLS test/js/node/tls/node-tls-connect.test.ts: (pass) should have checkServerIdentity [0.20ms] (pass) should thow ECONNRESET if FIN is received before handshake [8.07ms] (pass) initializes authorizationError to null in the TLSSocket constructor [0.16ms] (pass) setMaxSendFragment mirrors OpenSSL's [512, 16384] acceptance without throwing [3.07ms] (pass) should be able to grab the JSStreamSocket constructor [0.23ms] (skip) tls.connect > should work with alpnProtocols (pass) tls.connect > Bun.serve() should work with tls and Bun.file() [42.60ms] (pass) tls.connect > should have peer certificate when ... (truncated) ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: 18 skipped $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/node/tls/node-tls-connect.test.ts test/js/node/tls/node-tls-duplex-close-throw-uaf.test.ts bun test v1.4.3 (367d939) test/js/node/tls/node-tls-duplex-close-throw-uaf.test.ts: (pass) tls.connect({socket: Duplex}) does not read freed Handlers > when duplex.end() throws after close [2203.98ms] (pass) tls.connect({socket: Duplex}) does not read freed Handlers > when a pre-open duplex error races StartTLS [1670.45ms] (pass) tls.connect({socket: Duplex}) does not read freed Handlers > when duplex.end() throws after a close that comes before StartTLS [2263.36ms] (pass) tls.connect({socket: Duplex}) does not read freed Handlers > when an EOF listener destroys the socket and throws before StartTLS [2238.29ms] test/js/node/tls/node-tls-connect.test.ts: (pass) should have checkServerIdentity [2.07ms] (pass) should thow ECONNRESET if FIN is received before handshake [276.06ms] (pass) initializes authorizationError to null in the TLSSocket constructor [6.64ms] (pass) setMaxSendFragment mirrors OpenSSL's [512, 16384] acceptance without th ... (truncated) release with fix: 22 skipped $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) target linux-x64-gnu build type Release build dir ./build/release revision 562c022 features lto, baseline 23 deps, 136 codegen, 1176 objects in 4630ms ninja: Entering directory `/workspace/bun/build/release' [1/4] fetch lolhtml [lolhtml] up to date [2/4] fetch rust-argon2 [rust-argon2] up to date [2/4] cargo plan → /workspace/bun/build/release/rust-target/plan.json 244 units: 172 lib, 16 proc-macro (host), 19 custom-build (host), 15 run custom-build, 17 lib (host), 4 run custom-build (host), 1 rlib [3/4] reconfigure [1/1499] mkdir stamps [2/1499] mkdir codegen [3/1499] install /workspace/bun bun install v1.4.3-canary.1 (367d939) Checked 26 installs across 65 packages (no changes) [730.00ms] [4/1499] rustc unicode_ident [5/1499] rustc build_script_build [6/1499] rustc heck [7/1499] install /workspace/bun/packages/bun-error bun install v1.4.3-canary.1 (367d939) Checked 1 install across 2 packages (no changes) [110.00ms] [8/1499] rustc build_script_build [9/1499] rustc unicode_xid [10/1499] rustc build_script_build [11/1499] rustc build_s ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` src/runtime/socket/UpgradedDuplex.rs | 41 +-- src/runtime/socket/socket_body.rs | 2 +- test/js/node/tls/node-tls-connect.test.ts | 332 ++++++++++++++++++++- .../tls/node-tls-duplex-close-throw-uaf.test.ts | 103 ++++++- 4 files changed, 451 insertions(+), 27 deletions(-) ``` </details> **gate history** · 1 passed · 3 rejected · iteration 3 <details><summary>evidence per changed file</summary> ``` file reads edits tests src/runtime/socket/UpgradedDuplex.rs 8 6 68 src/runtime/socket/socket_body.rs 7 2 68 test/js/node/tls/node-tls-connect.test.ts 6 5 54 test/js/node/tls/node-tls-duplex-close-throw-uaf.test.ts 2 1 43 ``` </details>  --------- Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>

Jarred-Sumner · 2026-10-06T06:19:47Z

Superseded by #44618, which consolidates the open TLS pull requests. This fix and its tests are in there, either as written, rewritten smaller, or merged with the other PRs that patched the same cause (see the "By area" list in that PR). Closing in favor of it.

…ed close ran (#38058) A failed server-side handshake destroys the socket with _closeAfterHandlingError, which closes the handle from a microtask. That microtask re-read self._handle. When the failure is reported under a JS call (the stream-level engine fed from a 'data' listener, resumeSNI() from an asynchronous SNICallback) the native close runs first and detaches the handle, so nothing was closed and 'close' was never emitted. _destroy already holds the handle: pass it. closeAdoptedTLSRawNowNT was the same function, so it is gone.

…ps, WebSocket, SQL) (#44618) ### What does this PR do? Consolidates the open TLS pull requests into one. Each was reproduced on `main` and, for `node:*` behavior, on Node v26.3.0 first. About a third are ported as written, the rest are rewritten smaller or merged into one fix where several PRs patched the same cause. One commit per fix, so it can be read commit by commit. Fixes #43520, fixes #31396, fixes #43635, fixes #37193, fixes #43846, fixes #17932, fixes #41061, fixes #36887, fixes #31810, fixes #35240, fixes #32234, fixes #44365, fixes #43807, fixes #42280, fixes #44517. Addresses #41856 (SNI and `servername`; not `checkServerIdentity` for SQL), #24845 (the spin is gone, shown with fault injection on Linux; not run on macOS), #19754 (node-fetch forwards the agent's TLS options; the Kubernetes client itself was not run). #### The ones that matter most | | On `main` | PRs | |---|---|---| | Client certificate disclosure | `https.request()` with a client certificate sends it to a server it then refuses (wrong name, `checkServerIdentity`, `destroy()` in `'secureConnect'`, `terminate()` in `handshake`). A server can force it with a junk record behind its Finished | #43946 | | False `authorized` | Over a Duplex, `secureConnect` with `authorized === true` for a peer that failed the key proof; `secureConnect` for a plaintext peer with `rejectUnauthorized: false` | #44422, #32929 | | Cleartext https | `https.createServer()` without a usable key/cert answers plain HTTP | #41672, #33539 | | Revoked client certificates | An https mTLS server never sees `crl`, so a revoked client is `authorized` | #41641 | | Pooled sockets | Requests with different client certificates or CAs share an `https.Agent` socket and session | #42498 | | Silent plaintext | `tls: [...]` given to `Bun.listen` / `Bun.connect` is plain TCP | #41490 | | Server weakened by a client knob | `NODE_TLS_REJECT_UNAUTHORIZED=0` turns off a server's client-certificate enforcement | #35245 | | Pins never checked | `WebSocket` never calls `tls.checkServerIdentity` and ignores `tls.serverName` | #41648 | | `verify-full` dropped | `PGSSLMODE=verify-*` is lost next to a `TLS_*` URL variable; `tls: true` sends no SNI | #44498 | | Crashes | use-after-free from `destroy()` in `ALPNCallback` over a Duplex; `abort()` on a late `setSession()`; SIGABRT in `fetch` with an https proxy from the environment and a `Bun.file()` body | #44462, #41671, #44458 | | Stream corruption | A TLS `write()` can lose 16 KiB it reported as written while another socket on the loop is stalled | #44529 | | Hangs and spins | 100% CPU on a failing `send()`; a fatal `SSL_write` leaves the socket open forever; `idleTimeout` never sheds a TLS client that ignores `close_notify` | #34510, #38176, #42336 | | Wrong certificate (regression since 1.3.14) | Connections accepted before `stop()` / `close()` get the default certificate and skip their entry's `requestCert` / `ca` | #42355 | | Quadratic Duplex / proxy tunnel | Reading one chunk over a Duplex, CPU: 8 MB 0.88 s → 0.14 s, 16 MB 3.18 s → 0.23 s, 32 MB 11.75 s → 0.39 s; `fetch` upload through CONNECT: 1.7 s → 0.18 s (debug build) | #44464 | #### By area - **fd engine, write path** (`openssl.c`, `socket.c`): #42352, #34510 + #38176 + #42336 as one change, #44529, #44458, #44192. A rejected `send()` ends the write side only and closes at the next writable event unless the peer's bytes are still queued (a 413 sent before a reset is still read). No new per-socket state. Also, on kqueue, **a FIN no longer ends a socket that waits in the low-priority queue** (`loop.c`): with more than 5 TLS handshakes at once, a client that ended right after its handshake could be reset and its server socket report `socket hang up`, because the eof that the sentinel read knote reports was acted on ahead of the unread Finished. That is on `main` too (the macOS entry for `node-tls-server.test.ts` in `test/flaky-tests.txt`: 7 of 48 recent builds of other branches), and this branch made it likelier (6 of 8 builds), since Finished now leaves in one segment with the close_notify. - **Error reporting, both engines**: #44422, #32929, #44516, #37094, #41272 + #42324 + #44223 as one change, #44021, #37472, #43946, #33630. One channel: a fatal error on an established session is reported, then **the engine closes the connection itself**, whatever the owner does with the report. `test/js/bun/net/tls-fatal-error-closes.test.ts` asserts closed-and-nothing-delivered for every owner (node:tls, `Bun.connect`, `Bun.listen`, `fetch` direct and through CONNECT, `Bun.serve`, `WebSocket` direct and through a proxy, Postgres, MySQL, Valkey, Duplex). - **Duplex engine** (`SSLWrapper`, `UpgradedDuplex`): #44462, #43529, #42332, #44464. #43877 + #44394 were in and are **out again**, see "Worth a look" 5. - **node:tls wrap lifecycle** (`net.ts`, `tls.ts`): #38007, #38058, #38028 + #38122 + #38076 as one change (six copies of the attach code become two helpers), #38311, #39008, #38154, #42340 + #42343 + #42339 + #42453 as one change, #43791, #42425, #44085, #42683, #39088, #39040, #40375, and what was still real of #36534. - **SNI, ALPN, server contexts**: #43080, #42050, #37195 + #43849 as one change (**one** SNI matcher for TCP and HTTP/3), #42355, #42285, #33253, part of #37896, part of #37013. A `tls.Server` has one `SSL_CTX`. - **Verification and options**: #44738, #41490, #37005 + the cwd pin of #40984, #31811, #43982, #33483 + #35245, #41810, #32235, #44441, #38092. - **node:tls API and CA store**: #41671, #38145, #32824, #43594, #39997, #41696, #33534, #34748, #42991, #42996, #42970. - **node:https, Agent, `ws`, node-fetch**: #41672 (https half), #41641, #38261, #42498, #44346, #35609, #31397, #42325. - **WebSocket client**: #41648, #37487 + #43048 as one change. - **SQL, Redis**: #33666, #41711, #44498, part of #42054. - **Tests only**: #41426, #40040, #44395, #44016, #37860, #40591, #44440, #41424. Found on the way and fixed here: an upload that a TLS 1.2 server interrupts with a renegotiation never completes on `main` (0 of 32 runs over `https.request`, `fetch`, `node:tls` and `Bun.connect`: the renegotiation ClientHello lands inside an application record that is still unsent, or the socket gets no `drain` again) and completes here, with two tests from robobun; the fix for #40653 (final flight and first write in one segment) stopped working whenever another TLS socket on the loop was stalled, on `main` too; the `tls.Server` prototype pinned the last server constructed and every `SSL_CTX` it owned; `Object.create(process.env).NODE_TLS_REJECT_UNAUTHORIZED = "0"` turned verification off process-wide once a `SHARE_ENV` worker existed; two debug panics when wrapping a shut-down or still-connecting socket; a `fetch` POST through a proxy sent its headers twice when the origin renegotiated; `BlockList` ignored IPv6 zone ids; a test now ties `root_certs.der` to `certdata.txt`. #### Behavior changes - **A server's `ca` without `requestCert` no longer asks for a client certificate** (`Bun.serve`, `Bun.listen`, HTTP/3, node:tls). It matches the docs and Node. On `main` such a server refused clients with no certificate but served any unrelated self-signed one, so it was never authentication. **Set `requestCert: true` to require a certificate.** A matrix test pins that `requestCert: true` still refuses no certificate and an untrusted one on 8 kinds of server, TLS 1.2 and 1.3, with `NODE_TLS_REJECT_UNAUTHORIZED` unset and `0`. - `NODE_TLS_REJECT_UNAUTHORIZED=0` no longer relaxes a server. - `Bun.connect` / `Bun.listen` hear of a fatal TLS error after the handshake through `error(socket, err)`. With no `error` handler the socket just closes. - HTTP/3 server names match like TCP: `*.` covers exactly one label, case is ignored, a trailing dot is ignored, the last registration of a name wins. - `requestCert` on node:https is `=== true`, as in Node. - An array where a generated options dictionary is expected throws (`tls: []`, `jest.useFakeTimers([])`). - `key` / `cert` arrays serve every identity. A client that can use both gets ECDSA, where `main` served whichever pair came last. - `ecdhCurve` is forwarded by node:https, `ws` and node-fetch now, so a group BoringSSL lacks (`X448`) throws there as it already does in `tls.createServer`. - A wrapped socket's error is re-emitted on the TLS socket as in Node, so `raw.destroy(err)` with a listener on `raw` only is uncaught, as in Node. - `sql.options.tls` is always an object, never `true`. `RedisClient` sends SNI. - `tls: { secureContext }` alone asks for TLS on `Bun.listen` / `Bun.connect` (it was plain TCP), and a value that is not a `SecureContext` throws. The context is served as it is: the `requestCert` / `rejectUnauthorized` it was created with hold whatever the options next to it say, and `requestCert` in the options over a context that does not ask throws at `listen()`. - `tls.DEFAULT_CIPHERS` reaches every client once assigned (`fetch`, `WebSocket`, `Bun.connect`, `RedisClient`, `Bun.SQL`, `S3Client`, proxy tunnels) and servers again. A list that selects no cipher throws `ERR_SSL_NO_CIPHER_MATCH` at the assignment. `fetch.preconnect()` dials nothing after an assignment. - The warning for an unreadable `NODE_EXTRA_CA_CERTS` is Node's one line, without the `warn:` prefix. - `BUN_CONFIG_WS_CLOSE_TIMEOUT` (default 30 s): how long a `WebSocket` client waits for the server to close the connection after the closing handshake. #### Worth a look in review 1. **#44529**: the kernel-refused remainder of a TLS write moves from the loop's one slot onto the connection (in the existing rare struct), so the write BIO never refuses a sealed record. Nothing is allocated on an unstalled path (200 writes: 0 appends, same `send()` count as `main`), memory with 16 stalled writers is lower than on `main` (276 KB vs 340 KB, which `main` holds inside BoringSSL's buffers), `us_socket_t` stays 80 bytes. It needs a bound on how long a deferred close waits, or a peer that stops reading pins the fd past `destroy()`: `US_SSL_CLOSE_AFTER_SPILL_TIMEOUT` is a fixed 10 s, not re-armed on progress. Separate commits, but the fix that keeps the client certificate off the wire beside a stalled socket builds on them. 2. **The default name check of node:tls also runs inside the handshake**, so a wrong-name server gets no client certificate on TLS 1.2 either. JS still runs it after every successful handshake, so a difference between the two matchers can only refuse. Error objects are byte-identical. 3. **#44441** widens trust by design: a self-issued leaf whose `keyUsage` lacks `keyCertSign` (`dotnet dev-certs`) is its own anchor when the store holds a byte-identical copy. No BoringSSL change. Expired pin, same subject with another key, wrong EKU and a pinned intermediate are tested to fail. 4. **#32235** only adds Ed25519 and ECDSA P-521 to the verify list. A captured ClientHello shows `main`'s list with the two inserted; `rsa_pkcs1_sha1` stays. 5. **A stream that a TLS socket wraps, when that TLS socket closes.** An earlier state of this branch lost data here while CI was green (found by #44709's report): with the peer closing first, 4 of 8 MiB arrived with TLS in TLS, 4 of 32 MiB on the http2 `emit("connection")` path, and a `write()` with no `'error'` listener ended the process. Three Node-parity changes only hold together: destroying the wrapped stream at the close (#38028 + #38122 + #38076, #38154) is safe only if every write has really completed (#43877), which in turn needs Node's handling of the peer's close_notify, which needs half-open sockets that the GC can collect. So: - #43877 + #44394 are reverted and reopened. A write over a stream completes once the stream has taken the ciphertext, as on `main`. - Until the verdict on the peer lets the session through, the application cannot have written over it. There the wrapped stream is destroyed as in Node, with the sessions below it. That keeps the release of the connection after a failed handshake, a rejected certificate and an early `destroy()`. The same for an http2 socket the application never got, and for `resetAndDestroy()`. - After that it is `main`'s teardown: a `net.Socket` only gets the engine's `end()`, closes at its peer's FIN, keeps its own timeout and reports its own errors. Any other stream is destroyed with the TLS socket. The regular suites cannot see any of this (999 files were green on every broken variant), so it was steered by eleven seeded differential fuzzers run on this build, `main`, Node v26.3.0 and the earlier state: close, `end()`, `destroy()`, `destroySoon()`, resets, hung and half-open peers, paused writers, timeouts, two and three sessions deep, over TCP and over Duplexes, before, at and after the handshake, and http2 requests. See "How did you verify". #### Known limits - `fetch` with a `checkServerIdentity` function still sends the client certificate (not the request) to a server the function refuses. On TLS 1.2 any verdict a JS callback gives is too late, as in Node. - `addContext()` / `SNICallback` still do not apply to a server-side socket on the stream engine (`emit("connection", duplex)`, TLS in TLS, unflushed writes, named pipes), as on `main`. - A CA bundled in a pfx extends an explicit `ca` only, for `ws` / node-fetch / `WebSocket`: the native `ca` can only replace the default store, and that store keeps `SSL_CERT_FILE` / `SSL_CERT_DIR`. - P-521 leaves work on TLS 1.3 only. TLS 1.2 needs secp521r1 in every ClientHello (`it.todo`). - Once `tls.DEFAULT_CIPHERS` is assigned, `fetch(url, { protocol: "http3" })` is `HTTP3Unsupported`, as with an explicit `ciphers`. - `addCACert()` by hand does not extend the chains of a context with several identities. - A throwing `ALPNCallback` sends `no_application_protocol` on both engines. Node sends nothing and its client sees `ECONNRESET`. - TLS in TLS, peer FIN while the outer handshake runs: the inner socket gets one `write EPIPE`, where Node gives `ECONNRESET` (`main` gives it no error at all). - `@SECLEVEL` in `ciphers` is dropped by the `ws` / node-fetch shims, which used to ignore `ciphers`. node:tls keeps throwing `ERR_SSL_INVALID_COMMAND`. - Beside a stalled TLS socket only the first record (16 KiB) of the first write leaves with the handshake flight. The rest goes record by record, which is what bounds the memory of stalled writers. - After a fatal error on an established session the socket emits `'error'` and then `'close'`. Node emits `'error'` and leaves the socket open. - A paused reader whose own write the kernel rejects loses what it had not read yet, with an `EPIPE`, as on Node. `main` reports no error there and delivers it. - On `main` too: a `Bun.listen` socket without `allowHalfOpen` that has unsent ciphertext when the client's `shutdown()` arrives loses that ciphertext (32 KiB), and over plain TCP `end()` with the peer still sending is a close over unread input, so a reset. - Differences from both `main` and Node that the differential runs below found and that stay, all with a peer that aborts: `ECONNRESET` instead of a clean `'end'` after the socket's own `'finish'` when the peer destroyed with unread data; under TLS 1.2, a zero-length `write()` followed by `destroy()` in `'secureConnection'` leaves the client without `'secureConnect'` (a plain `destroy()` there matches Node); a TLS 1.2 client that destroys in `'secureConnect'` gets no `'session'`; a `ClientRequest` whose handshake fails with an alert emits `'error'` and `'close'` but no `'finish'` (`writableFinished` is true). - Once `tls.DEFAULT_CIPHERS` is assigned, `fetch.preconnect()` opens nothing: `fetch()` then uses a context of its own, and a socket warmed under the default one would never be picked up. - A TLS `send()` that the kernel refuses outside a `write()` call (the drain of unsent ciphertext) is reported with the close, as `read EPIPE` / `read ECONNRESET`. Node says `write EPIPE`. `main` does not report it at all. - Once the application has a session over a `net.Socket` (TLS in TLS, http2 `emit("connection")`), a peer that never sends its FIN holds that socket after the TLS socket closed, as on `main`. Node destroys it. Two tests of #38154 are `todo` for this. Closing it any earlier (at its `'finish'`, say) makes the kernel drop what it has not sent yet as soon as the peer's close_notify arrives. - Plaintext that was queued on a socket before it was wrapped (STARTTLS with a backlog) is dropped when the TLS socket is destroyed, or its handshake fails, before the session is accepted. Node drops it too, except on `destroySoon()`. `main` sends it. - Over a stream that is no `net.Socket`, `end()` can still cut what that stream has buffered, and there is no backpressure, both as on `main` (#43877). - `tls.secureContext` (the undocumented door node:tls uses) is not read by a Windows named pipe listener, which builds its context from the options. On `upgradeTLS({ isServer: true })` the options next to it are the policy, as with Node's `SetVerifyMode`. - `selectServerName()` rebuilds the name tree per ClientHello for injected sockets of a server with `addContext()` entries: 0.4 µs for 1 entry, 3.7 µs for 10, 41 µs for 100, against 631–1111 µs for a handshake. #### Not included Left open, because they need a decision or are not TLS: #43877 + #44394 (see "Worth a look" 5; #43874 stays open with them), #38548, #38591 (both shrink who is trusted), #41589 (`verify-full` vs `NODE_TLS_REJECT_UNAUTHORIZED=0`), #37197, #41706, #43216, #33487, #33545, #36707, #32435, #37255, #28691, #40275, #30314 (features), #38120 (needs the BoringSSL fork, as did #33517, which the stale bot has closed since), #38529 (needs a Windows measurement), #34342, #38232, #43089, #44454, #40451, #42710, #44527, #38088, #38093, #41898. #37896, #42054 and #37013 stay open for the halves not taken. `http.createServer({ key, cert })` keeps serving TLS on purpose. One open question: `tls: {}` (an object that names no TLS option) is plain TCP on `Bun.listen` / `Bun.connect`, here and on `main`. It is the same trap as `tls: []`, but changing it changes a Bun default, so it is left alone. ### How did you verify your code works? - Every new test fails on `main` for the stated reason and passes here, except guards that pin existing behavior, each shown to fail when its clause is removed. `node:*` tests also pass on Node v26.3.0; the few that cannot say which Node version has the behavior. - 212 test files that touch TLS, sockets, http, http2, fetch, WebSocket, SQL, Valkey and workers: 6154 pass, 2 fail. Both are seen on `main` too: `serve.test.ts` "root range port" (the box runs as root), and `worker_threads.test.ts` "terminate(): nothing of the worker's runs after the request", which is flaky there and passed in the run below. - 58 of those files the way the ASAN lane runs them (LeakSanitizer + `BUN_JSC_validateExceptionChecks`): 58 files, 48 of them with leak checking, 4132 pass, 3 fail. All three also fail on `main`: `serve.test.ts` "root range port", `node-net.test.ts` "should not leak when connect({path}) fails synchronously on a reused handle" (times out under this environment), `worker_threads.test.ts` "process.exit() with a shell cp in flight" (a `ShellCpTask` leak). - 647 vendored `test-tls-*`, `test-https-*`, `test-net-*`, `test-http2-*`: the only two failures also fail on `main`. - The SNI matcher was diffed against both old matchers: 3 seeds × 1.23 M lookups × 3 registration flavours, every difference in one of the intended classes, TCP and HTTP/3 identical on every lookup. - The headline rows were also driven by hand with scripts against this build, `main` and Node v26.3.0: cleartext https, `crl`, `tls: []` / `{ secureContext }`, the `ca` / `requestCert` matrix, the client certificate on a wrong-name server, late `setSession()`, `destroy()` in `ALPNCallback`, `[rsa, ec]` identities with an intermediate from `ca`, `WebSocket` `checkServerIdentity`, a corrupted record, the Duplex read above, `tls.DEFAULT_CIPHERS`. - The `setSession()` guard was checked against the real `abort()` at 43 handshake states. - `bun run rust:check-all`: 12 of 12 targets. `tsc`, oxlint, source lints, prettier, rustfmt, mordant clean. - usockets' `_Nonnull` is compiled out of debug builds, so 105 of those files were also run on a local release ASAN build with the CI runner's environment (92 with leak checking): 4595 pass, 1 fail, `child_process.test.ts` "spawn reports EPERM after dropping privileges", which cannot pass as root and fails on `main` too. - The close of a TLS socket over another stream ("Worth a look" 5): eleven seeded differential fuzzers, 8,424 scenarios compared, each run on a release ASAN build of this branch, on `main`, on Node v26.3.0 and on the earlier state of the branch. Against `main`: - Data that `main` delivers in full is cut in 5 scenarios, and about 150 that `main` cuts arrive in full. Of the 5, in 2 `main` never notices the peer's close and keeps the socket for good, 2 call `end()` on the middle one of three sessions over an in-memory Duplex, and 1 does the same on Node. - No dead timeout, no silent reset and no uncaught error that `main` does not have (4 uncaught errors fewer). - A socket stays open where `main` closes it in 109, and closes where `main` keeps it in 295. 92 of the 109 do the same on Node or on the earlier state (a `destroy()` that an in-memory Duplex does not show its peer, half-open peers). 14 wait for a peer that paused reading and so does not read the FIN (#42332's backpressure, as in Node); the socket's own timeout fires there. 3 are left: one on a 5 ms timer, two with three sessions over an in-memory Duplex. - The earlier state of the branch cut data in 173 of the 400 scenarios of one of them, where `main` cuts none and this cuts none. - 23 new tests pin what they found. Each earlier attempt at this fix fails the ones that describe it, the earlier state of the branch fails 7, and all pass on Node. - After that change: 999 test files on the release ASAN build (20,246 pass; the 11 files that fail need a database, Docker, DNS or a non-root user, or share a temp directory with a parallel run and pass alone), 61 on the debug build. - TLS over a file descriptor (`openssl.c`, the path of `fetch`, `Bun.serve`, `tls.connect`, `Bun.connect`) got the same treatment after the rebase: seeded differential fuzzers on CI's release build of this branch, on `main` and, for `node:*`, on Node v26.3.0. Every runtime also against itself for the noise floor, injected faults and known bugs of `main` as positive controls, and a difference counts only if it shows in 5 of 5 fresh processes. - `node:tls` over TCP: 11,500 scenarios (one connection with Node as the oracle line by line; 2 to 60 connections beside stalled neighbours; raw peers that break the handshake). HTTPS: about 136,000 runs over `Bun.serve` + `fetch`, `node:https`, `node:http2` and `wss://`, also with the two ends in different runtimes. `Bun.connect` / `Bun.listen` / `upgradeTLS`: 11,500 scenarios and 720 slow connections, with writers driven by what `write()` returns, beside up to 6 stalled, dripping, closing or resetting neighbours, and plain TCP as a second oracle. No crash, hang, duplication, reordering or silent truncation, and no change in time or in connection reuse. - They found six things that `main` does better, none of which any test showed. All are fixed, each with a test that fails on the build before: what the peer sent lost behind a rejected `send()` (23 scenarios, and an early HTTPS response lost with only `EPIPE`), the same silently for a paused reader, `server.close()` never calling back on a half-open server after a ClientHello and a reset (17), `closeAllConnections()` taking 12 s with a stalled client, `end()` losing up to 1.3 of 4 MiB that `write()` had reported while the peer still uploads, and `end()` a little after a stall never closing beside other stalled TLS sockets. The last two fixes also deliver the 1 to 2 MiB that `main` loses there, and close the socket that `main` keeps for good without such neighbours. - All of them again after every fix, on CI's release build of it. That caught one regression of a fix itself (a reader stopped for backpressure lost 86,385 bytes, 1 of 6,000 scenarios), fixed too. On the last build: scenarios that lose data where `main` does not 23 → 2, and Node loses it in both, with the same `EPIPE`; `server.close()` that never calls back 17 → 0; connections held 4 → 0; requests that end in an error only where `main` has a response 6 → 0. With a Node server in another process, a request ends in an error only in 8 and 10 of 1,500 scenarios here, 5 and 3 on `main`, 10 with Node as the client. - `Bun.connect` / `Bun.listen` on the last build against `main`, in scenarios: hangs 0 against 1,031, sockets and fds never released 0 against 965, corrupted data 0 against 345, `abort()` 0 against 26 (`setSession()` after the handshake), writers that never close 0 against 101 of 720 connections. No kind of failure shows here and not on `main`. About a third of the slow connections close later than on `main`, in 1 to 16 s instead of at once, waiting for unsent ciphertext or for the peer's close_notify, and 79 more of them deliver all that `write()` reported. RSS and time with 16 to 256 stalled writers are the same. - What they found that `main` does worse: a `WebSocket` that calls `close()` with sends pending loses messages in 81 of 999 scenarios (0 here), 37 server sockets left open, 10 `server.close()` that never call back, 20 write callbacks that never run. - The kqueue fix cannot be run on Linux. The `connectionListener` count test now says what became of a missing connection, which is how the cause was found (`'tlsClientError'` "socket hang up", then `read ECONNRESET` at the client of the same port, after its `'secureConnect'`). On macOS x64 it failed every attempt of the three builds before the fix and passed at the first attempt of the build with it. - Windows and macOS were only run by CI. Four new tests asserted what only the Linux kernel does (a FIN read ahead of a reset, unread bytes surviving a reset, loopback buffer sizes, `fstat()` on a socket) and now say so per platform. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: robobun <117481402+robobun@users.noreply.github.com>

github-actions Bot added the claude label Aug 13, 2026

claude Bot reviewed Aug 13, 2026

View reviewed changes