Skip to content

node:tls: emit 'connect' on a TLSSocket that wraps a connecting socket - #42340

Closed
robobun wants to merge 1 commit into
mainfrom
robobun/d3510055/tls-wrap-connect-event
Closed

robobun wants to merge 1 commit into
mainfrom
robobun/d3510055/tls-wrap-connect-event

Conversation

@robobun

@robobun robobun commented Sep 11, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • tls.connect({ socket }) over a still-connecting net.Socket never emits 'connect' on the TLSSocket, only 'secureConnect', so a 'connect' listener that starts the conversation never runs. Node v26.3.0 emits it when the TCP connect completes.
  • Socket.prototype.connect (src/js/node/net.ts:2082) starts TLS from the wrapped socket's 'connect' and does not re-emit it.
  • end() before that connect found no handle: 'finish' fired at once and no FIN was sent.

Fix

  • Emit 'connect' from the wrapped socket's 'connect', after the TLS handle is attached, as node's TLSSocket._init does (wrap.js#L964-L973). The branch that runs TLS over the stream at once (connecting named pipe, parked write, TLSSocket) gets the same listener.
  • drain in the open handler already sends a write parked while connecting. That write's 'connect' listener now skips a chunk drain took. Without the guard it goes out twice.
  • TLSSocket._final leaves the no-handle case to net.Socket._final, which waits for 'connect' while connecting.
  • Verified: 8 new tests in test/js/node/tls/node-tls-connect.test.ts (5 fail on main), one Windows test in node-tls-namedpipes.test.ts. Self-reviewed: 4 concerns raised, 4 addressed. Suites and exclusions are in the notes.

Background

  • tls.connect({ socket }) wraps an existing stream. For a connected TCP socket, Bun moves the fd into a native TLS socket (upgradeTLSDeferred) and the native open callback runs inside that call. Otherwise a TLS engine runs over the stream (upgradeDuplexToTLS) and open runs a tick later.
  • While connecting is true, _write parks one chunk (_pendingData). _write, _final, _read, ref() and unref() wait on once('connect').
  • A direct tls.connect(port) emits 'connect' through afterConnect. SocketHandlers2.open skips that for a wrapped socket and calls drain, which sends the parked write.
Notes

Repro (from the report), raw still connecting when it is wrapped:

const raw = net.connect(port, "127.0.0.1");
const c = tls.connect({ socket: raw, rejectUnauthorized: false });
c.once("connect", () => c.write("first;"));
c.once("secureConnect", () => { c.write("second;"); c.end(); });
client events server received
node v26.3.0 connect, secureConnect, write callbacks, finish, close first;second;
bun 1.4.3, main secureConnect, second write callback, finish, close second;
this branch same as node first;second;

What matches node, and where. Event matrix for the fd branch (upgradeTLSDeferred), {wrapped while connecting, wrapped after connect, direct} x first action at {same tick, TLSSocket 'connect', wrapped socket's 'connect'} x {write, write+end, end}, against a TLS server that ends when the client does. Before: the three "TLSSocket 'connect'" cells of a socket wrapped while connecting do nothing, and "same tick, end" emits 'finish' and then hangs. After: every cell gives node's event list, except that a bare end() before the handshake gives no 'secureConnect'. That difference is the same for a direct tls.connect(port) and is described in #42181. The eight new tests were also run as a plain script on node v26.3.0 and hold there.

Each added clause has a test that fails without it (checked by deleting the clause): the emit in the wait branch (tests 1, 2), the listener in the stream branch ("a preamble written before the wrap"), self.connecting = false ("a socket wrapped before it dials", where open has not run yet), a deferred emit such as process.nextTick (test 1 pins that the event runs inside the wrapped socket's own emit), the _pendingData guard (test 2 receives before connect;before connect;from connect; without it), the _final change (the end() test), self.destroyed and connection instanceof Socket (the last two tests). The self[kupgraded] !== connection half of the guard mirrors onUpgradedClose from #39066. It is reachable by calling connect({ socket }) again on the same TLSSocket before the first socket connects, and has no test, as in #39066.

Windows. Run on a Windows x64 debug build: tls.connect({ socket: net.connect(pipeName) }) emitted no 'connect' before and does now (the new test in node-tls-namedpipes.test.ts fails without the change and passes with it). node-tls-connect.test.ts, node-tls-upgrade.test.ts and node-tls-socket-allow-half-open-option.test.ts pass there. should work with named pipes and tls times out in a debug build with and without the change.

Not node's behaviour after this change, all of it also true on main:

  • The end() claim ("sends the FIN") holds for the fd branch only. In the stream branch (parked write, named pipe) an end() before the connect now gives 'connect', 'finish', and the peer still sees no FIN. main gives nothing at all there. The engine does not end the underlying stream when it is shut down before the handshake. It reproduces on main with a plain Duplex transport and is reported separately.
  • end() before a wrapped connect that is then refused: main emits a spurious 'finish', this branch emits nothing (node: 'error', 'close'). Neither emits 'error' or 'close' until node:tls: report a wrapped socket's connect failure on the TLSSocket #38122 lands.
  • A zero-length write parked before the connect (write(""), end("")): its callback runs from drain inside the native open, before 'connect' and before the handle is assigned, so end("") still finishes with no FIN. node keeps that write pending until the handshake completes. Same root as the next item.
  • Excluded on purpose from the _final change: new tls.TLSSocket(raw, { isServer: true }).end() in the wrap tick (named as out of scope in tls: end() and destroySoon() before the handshake completes now send the FIN #42181). It needs a wait on the handle attach (kUpgradeAttached), not on 'connect'. node:tls: send the FIN when end() runs before the TLS handle is attached #42339 adds that wait.
  • On Windows, end() in the same tick as wrapping an already connected named pipe never emits 'finish': the TLSSocket reports connecting for one tick and nothing emits 'connect' for it. Unchanged here, reported separately.

One behaviour change outside tls. The _pendingData guard sits in the shared _write. A plain net.Socket that is reused after a failed connect() with a parked write keeps the stale 'connect' listener of that write. node, bun 1.4.3 and main then send the stale chunk again on the next connect and emit ERR_MULTIPLE_CALLBACK. With the guard the stale listener does nothing and only the current write goes out. Plain net.connect and direct tls.connect dials with a parked write are unchanged.

Effect outside tls. http.ClientRequest#setTimeout and _deferToConnect (setNoDelay, setSocketKeepAlive) wait for 'connect' on a connecting socket. With a createConnection that returns such a TLSSocket they never ran.

Adjacent open PRs, none of which emits the event: #38122 (the wrapped socket's connect failure, same callback), #42176, #36534. Two open PRs edit TLSSocket.prototype._final too and will need a textual merge with this one. #42330 covers a new TLSSocket(stream) wrap that was never connected. #42339 (opened while this was in review) makes _final wait for the handle attach, which fixes the same end() symptom from the other side and also the server-side wrap. It emits no 'connect'. Both changes send the no-handle case to net.Socket._final, so they compose: with both, _final waits for the attach and then finds connecting already false.

Suites run with the change (Linux debug build): test/js/node/tls/, test/js/node/net/, test/js/node/http2/, test/js/bun/net/, and the vendored test-tls-*, test-https-*, test-net-*, test-http2-* files. Local failures that are the same with and without this change: SNICallback runs even when the requested servername matches the bind hostname, socket > should not call drain before handshake, net.Socket read > ... in node-net.test.ts, and the vendored test-https-timeout.js.


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/tls/node-tls-namedpipes.test.ts

tls.connect({ socket }) over a net.Socket that has not connected yet never
emitted 'connect' on the TLSSocket, so a 'connect' listener on it never ran
and whatever a connecting socket defers to 'connect' never resumed. Node
re-emits the event from the wrapped socket's 'connect'
(lib/internal/tls/wrap.js _init).

- Emit 'connect' on the TLSSocket once the wrapped socket has connected, in
  the branch that waits for it before starting TLS and in the branch that
  runs the TLS engine over the stream right away (a named pipe, a socket
  with a parked write, a TLSSocket that is still connecting).
- The open handler's drain already sends a write parked while connecting, so
  the write's own 'connect' listener skips a chunk that drain took.
- TLSSocket._final leaves the no-handle case to net.Socket's _final, which
  waits for 'connect' while connecting, instead of finishing with no handle
  to shut down.
@robobun

robobun commented Sep 11, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 9:50 AM PT - Sep 11th, 2026

✅ @robobun, your commit 5d81c67da813e19e860738d10669ee664e98a9da passed in Build #114336! 🎉


🧪   To try this PR locally:

bunx bun-pr 42340

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

bun-42340 --bun

@robobun

robobun commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator Author

Status

Reproduced with the script from the report (raw = net.connect(port) wrapped by tls.connect({ socket: raw }) before it connects, first write issued from the TLSSocket's 'connect' listener):

  • node v26.3.0: connect, secureConnect, both write callbacks, finish, close. Server receives first;second;.
  • bun 1.4.3 and main: no connect. Server receives second;.
  • this branch: same as node.

To check locally:

bun bd test test/js/node/tls/node-tls-connect.test.ts -t "still connecting"

8 tests. 5 fail on main, 3 pin the cases where no 'connect' is due. The Windows named pipe case is test/js/node/tls/node-tls-namedpipes.test.ts, run on a Windows x64 debug build: it fails without the change and passes with it.

@coderabbitai

coderabbitai Bot commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 82abf4b9-071e-43c1-b10a-e22023c0e9b5

📥 Commits

Reviewing files that changed from the base of the PR and between 6a92015 and 5d81c67.

📒 Files selected for processing (4)
  • src/js/node/net.ts
  • src/js/node/tls.ts
  • test/js/node/tls/node-tls-connect.test.ts
  • test/js/node/tls/node-tls-namedpipes.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review.


Walkthrough

TLS upgrades over connecting sockets now emit connect after the underlying connection completes. Pending writes avoid duplication, finalization handles handle-less sockets, and tests cover event ordering, writes, FIN behavior, destruction, custom Duplex transports, and named pipes.

Changes

TLS upgrade connection handling

Layer / File(s) Summary
Complete upgraded socket connections
src/js/node/net.ts
Connecting sockets now re-emit connect after the underlying connection completes. Deferred upgrades use the same handler, and pending writes are not duplicated.
Finalize wrapped TLS sockets
src/js/node/tls.ts
TLSSocket._final waits for the handshake only when the required handle, connection, and pending-write conditions exist.
Validate wrapped-socket behavior
test/js/node/tls/node-tls-connect.test.ts, test/js/node/tls/node-tls-namedpipes.test.ts
Tests cover event ordering, write delivery, stream preambles, deferred FIN, destruction, custom Duplex transports, and named pipes.

Suggested reviewers: cirospaciari

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 5d81c

No actionable merge-blocking risk remains in the reviewed TLS connection changes.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the main change: emitting 'connect' on a TLSSocket that wraps a connecting socket.
Description check ✅ Passed The description clearly explains the problem, implementation, scope, verification results, and known limitations. It does not use the exact template headings, but it provides the required information …

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

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I reviewed this PR and didn't find any bugs. Because it changes shared Socket.prototype._write behavior for all net.Socket instances and reworks TLSSocket._final in a way that interacts with two other open PRs on the same method, a human look at those cross-cutting effects would still be worthwhile.

What was reviewed:

  • onUpgradedConnect placement on both the immediate-duplex-upgrade and deferred-'connect' paths — both set this[kupgraded] and this._handle before the emit, so listeners see consistent state.
  • The _pendingData !== chunk guard against the drain call in the open handler — the closure-captured chunk identity check correctly short-circuits only the already-flushed write, and the PR notes the intentional side-effect on stale parked writes after a failed reconnect.
  • TLSSocket._final no-handle fallthrough — NetSocket.prototype._final handles the connecting wait, so end() before connect no longer finishes early.
  • Tests: port: 0/127.0.0.1, Promise.withResolvers, await using, no sleeps, error events wired to reject, negative cases (already-connected, destroyed-before-connect, plain Duplex) all covered.
Extended reasoning...

Overview

The PR makes tls.connect({ socket }) re-emit the wrapped net.Socket's 'connect' event on the resulting TLSSocket, matching Node's internal/tls/wrap.js behavior. It touches src/js/node/net.ts (new onUpgradedConnect helper wired into two upgrade branches, plus a _pendingData !== chunk guard in the shared Socket.prototype._write) and src/js/node/tls.ts (TLSSocket.prototype._final now falls through to the base _final when _handle is absent instead of returning early). ~246 lines of tests are added to node-tls-connect.test.ts covering event ordering, pre-connect write delivery, TLS-over-stream paths, and end() before connect; a Windows named-pipe test is added to node-tls-namedpipes.test.ts.

Security risks

None identified. The change is purely event-ordering and lifecycle plumbing on the client side of a TLS wrap; it does not touch certificate validation, rejectUnauthorized handling, TLS options merging, or any credential path. The instanceof Socket check gates on a module-local class (not a user-overridable global), matching the existing pattern.

Level of scrutiny

High. src/js/node/net.ts and src/js/node/tls.ts are hot-path Node-compat modules where event ordering and state-flag values relative to emits are load-bearing for the ecosystem. The _pendingData !== chunk guard sits in the shared _write used by every net.Socket, and the PR description explicitly documents a behavior change outside TLS (a stale parked write on a reused socket after a failed connect no longer double-sends / no longer emits ERR_MULTIPLE_CALLBACK). The _final change is also noted to require a textual merge with two other open PRs (#42330, #42339) that edit the same method. These are exactly the interactions a maintainer should sign off on.

Other factors

The implementation follows local conventions closely: onUpgradedConnect mirrors the neighboring onUpgradedClose (named function, .bind(null, self, connection), self[kupgraded] !== connection guard), the Node source permalink is cited at the definition, and the deferred-branch call runs after _handle assignment so pending reads false in the listener as asserted. Tests follow harness conventions (combined-object .toEqual, await using disposal, error events wired to reject the awaited promise, no timing waits) and include negative-contract cases. The PR description is unusually thorough — it names which added clauses each test proves live, enumerates remaining non-Node behaviors, and cross-references adjacent open PRs — which itself argues for a human confirming those stated interactions rather than an automated approval.

Jarred-Sumner pushed a commit that referenced this pull request Oct 3, 2026
…etes (#42350)

### Problem
- Regression from #42181, not released. `tls.connect({ socket: duplex
})`, then `end()` before the handshake completes: `'finish'` fires, but
the Duplex's `final()` never runs, so the peer sees no FIN. A healthy
server then completes the handshake and the session stays open forever.
Node v26.3.0 ends the Duplex at once.
- `UpgradedDuplex::shutdown`
(`src/runtime/socket/UpgradedDuplex.rs:551`) ran `SSL_shutdown` and
nothing else. Only the close callback ended the transport, which needs
the peer's close_notify or a `destroy()`. Mid-handshake, BoringSSL's
`SSL_shutdown` returns 1 and does nothing.

### Fix
- `shutdown()` sends the close_notify (none mid-handshake), then ends
the write side of the transport. `us_internal_ssl_shutdown` does this on
an fd, node's `TLSWrap::DoShutdown` on a stream.
- `end()` in the turn that created the socket arrives before the engine
exists and was dropped. It now sets `pending_shutdown`. `drain_pending`
replays it after the staged input: first flight, then `end()`.
- Correct because the engine keeps reading, and the `writableEnded`
probe in `call_write_or_end` drops handshake output that follows.
- Verified: `test/js/node/tls/node-tls-connect.test.ts`. 7 new cells
fail on main, 6 assert the same report on node v26.3.0. Self-reviewed:
16 concerns raised, 13 addressed, 3 need no change. Remaining gaps:
notes.

### Background
- Bun has three TLS engines. `openssl.c` drives a socket on its fd.
`UpgradedDuplex` runs an `SSLWrapper` over a `stream.Duplex` through
`duplex.write()` and `duplex.end()`. `WindowsNamedPipe` does the same
over a pipe. Only `UpgradedDuplex` changes.
- node:net's `_final`, the shutdown hook of the writable side, calls
`handle.shutdown()`: `UpgradedDuplex::shutdown` here. `'finish'` follows
its callback.
- The engine for a Duplex starts on a later event-loop turn.
`drain_pending` replays what arrived before.

<details><summary>Notes</summary>

**Reports.** The fixture
`test/js/node/tls/tls-shutdown-before-handshake-fixture.mjs` runs on
both runtimes. "same turn" means the call is made in the turn that
created the socket, before bun's engine exists. "after first flight"
means the engine wrote its ClientHello and waits.

| cell | node v26.3.0 | main | this branch |
| --- | --- | --- | --- |
| `end()`, stalled peer, same turn | ClientHello, `final()`, FIN,
`finish` | `finish`, no FIN (times out) | same as node |
| `end()`, stalled peer, after first flight | same | `finish`, no FIN
(times out) | same as node |
| `destroySoon()`, after first flight | `final()`, FIN, `finish`,
`close` | transport destroyed, no `final()` | same as node |
| `destroySoon()`, same turn | ClientHello, `final()`, FIN, `finish`,
`close` | transport destroyed, no ClientHello, no `final()` | unchanged,
see below |
| `end()`, healthy TLS server, same turn | server: `tlsClientError
ECONNRESET`, client closes | server: `secureConnection`, session left
open | same as node |
| server-side `TLSSocket` over a Duplex, `end()` same turn | `final()`,
FIN | no FIN (times out) | same as node |
| server-side, ClientHello already buffered | aborts:
`ERR_INTERNAL_ASSERTION` in `JSStreamSocket.doWrite` | no FIN | server
flight, then FIN (bun-only cell) |
| `end()` after the handshake, server does not answer the close_notify |
`final()` at once | `final()` never runs | same as node |

The healthy-server cell on bun 1.4.3 (before #42181): `_final` waited
for the handshake, so the log is `secureConnect`, `finish`,
close_notify, `final()`, `close`. Late, but closed. #42181 removed that
wait for the case with no queued write, which is right for the fd
engine, where `us_internal_ssl_shutdown` sends the FIN. The stream
engine had no equivalent.

**Behaviour change after the handshake.** `end()` now ends the transport
right after the close_notify. Before, the transport was ended only when
the peer's close_notify arrived. With a peer that does not answer (a
server with `allowHalfOpen`), the transport was never ended. Node ends
it at once. Cell: `duplex-end-established`.

**Against a healthy server, client side.** Node's engine also completes
its half of the handshake after the FIN and writes its last flight into
the ended stream, so node's client emits `secureConnect` and then
`ERR_STREAM_WRITE_AFTER_END`. Bun drops that write (the `writableEnded`
probe) and emits neither. The cell does not assert those two events. The
server side is identical.

**Not covered here.**
- `destroySoon()` in the creating turn over a Duplex. Committed as an
`it.failing` cell for bun. `endNT` (`src/js/node/net.ts:245`) calls the
`_final` callback right after `shutdown()`, so `'finish'` fires before
the engine exists, `destroySoon()`'s `destroy()` runs, and `_destroy`
destroys the transport. Node completes the shutdown in the
`stream.end()` callback, so `'finish'` comes after the transport's
`final()`. Same on 1.4.3. The same cause puts `'finish'` ahead of the
transport's `'finish'` when the transport's write is asynchronous.
Gating `'finish'` on the transport is a separate change in `net.ts`.
- `WindowsNamedPipe::shutdown` with TLS (`tls.connect({ path: pipe })`)
has the same shape: `SSL_shutdown` only. `writer.end()` there closes the
whole pipe, read side included, so it needs the `uv_shutdown` half-close
from #39727 first.
- `tlsSocketFinal` in `src/js/node/_http2_upgrade.ts` ends its engine
with `handle.end()`, a full close, not `shutdown()`. Not changed.
- `end()` in the creating turn when the transport is a `net.Socket`
(unflushed writes, named pipe): `Socket.prototype._final` parks on
`'connect'`, which an upgraded socket never emits, so `shutdown()` is
never reached. #42343 clears `connecting` for such a transport and
#42340 emits the event. `end()` before the fd handle is attached is
#42339. #42343 names the missing FIN on this engine as its open
follow-up: that is this change. With the engine started, those
transports get the FIN from this change (checked by hand with a
net.Socket that has 4 MB of unflushed writes).

**Replay order.** The pending shutdown runs after the staged bytes and
EOF. A server-side socket that was handed a buffered ClientHello answers
it first, so the client gets the server's flight and then the FIN. The
first version replayed the shutdown first and dropped that flight.

**Suites run with the debug build:** all of `test/js/node/tls/`,
`test/js/node/net/`, `test/js/node/http2/`,
`test/js/bun/net/socket.test.ts`, `socket-retention.test.ts`,
`node-http-connect.test.ts`, `ws.test.ts`, `websocket-proxy.test.ts`,
and 223 vendored `test-tls-*`, `test-https-*`,
`test-http2-generic-streams*` scripts (222 exit 0). Every failure also
fails on a debug build of main: tests that need public DNS or an IPv4
`localhost`, `test-https-timeout.js` (hangs on debug builds with and
without this change), and `test-tls-client-allow-partial-trust-chain.js`
(a `node:test` file).

</details>

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

---

**[human-review]** gate passed · iteration 1 · 3 files touched

<details><summary>fails on main (without fix)</summary>

```console
ASAN without fix: 7 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
bun test v1.4.3 (4ff9193)

test/js/node/tls/node-tls-connect.test.ts:
(pass) should have checkServerIdentity [2.90ms]
(pass) should thow ECONNRESET if FIN is received before handshake [362.75ms]
(pass) initializes authorizationError to null in the TLSSocket constructor [12.06ms]
(pass) setMaxSendFragment mirrors OpenSSL's [512, 16384] acceptance without throwing [173.06ms]
(pass) should be able to grab the JSStreamSocket constructor [21.30ms]
(skip) tls.connect > should work with alpnProtocols
(pass) tls.connect > Bun.serve() should work with tls and Bun.file() [120.40ms]
(pass) tls.connect > should have peer certificate when using self asign certificate [140.18ms]
(skip) tls.connect > should have peer certificate
(skip) tls.connect > getCipher, getProtocol, getEphemeralKeyInfo, getSharedSigalgs, getSession, exportKeyingMaterial and isSessionReused should work
(skip) tls.connect > should process options correctly when connect is called with only options
(skip) tls.connect > should process port
... (truncated)

release without fix: 11 failed, 18 skipped
bun test v1.4.3-canary.1 (4ff9193)

test/js/node/tls/node-tls-connect.test.ts:
(pass) should have checkServerIdentity [0.04ms]
(pass) should thow ECONNRESET if FIN is received before handshake [6.73ms]
(pass) initializes authorizationError to null in the TLSSocket constructor [0.18ms]
(pass) setMaxSendFragment mirrors OpenSSL's [512, 16384] acceptance without throwing [4.02ms]
(pass) should be able to grab the JSStreamSocket constructor [0.22ms]
(skip) tls.connect > should work with alpnProtocols
(pass) tls.connect > Bun.serve() should work with tls and Bun.file() [6.15ms]
(pass) tls.connect > should have peer certificate when using self asign certificate [4.22ms]
(skip) tls.connect > should have peer certificate
(skip) tls.connect > getCipher, getProtocol, getEphemeralKeyInfo, getSharedSigalgs, getSession, exportKeyingMaterial and isSessionReused should work
(skip) tls.connect > should process options correctly when connect is called with only options
(skip) tls.connect > should process port and host correctly
(skip) tls.connect > should process port, host, and callback correctly
(skip) tls.connect > should handle the absence of a callback gracefully
(skip) tls.c
... (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
bun test v1.4.3 (4ff9193)

test/js/node/tls/node-tls-connect.test.ts:
(pass) should have checkServerIdentity [3.28ms]
(pass) should thow ECONNRESET if FIN is received before handshake [318.81ms]
(pass) initializes authorizationError to null in the TLSSocket constructor [8.05ms]
(pass) setMaxSendFragment mirrors OpenSSL's [512, 16384] acceptance without throwing [104.40ms]
(pass) should be able to grab the JSStreamSocket constructor [13.33ms]
(skip) tls.connect > should work with alpnProtocols
(pass) tls.connect > Bun.serve() should work with tls and Bun.file() [79.45ms]
(pass) tls.connect > should have peer certificate when using self asign certificate [88.70ms]
(skip) tls.connect > should have peer certificate
(skip) tls.connect > getCipher, getProtocol, getEphemeralKeyInfo, getSharedSigalgs, getSession, exportKeyingMaterial and isSessionReused should work
(skip) tls.connect > should process options correctly when connect is called with only options
(skip) tls.connect > should process port an
... (truncated)

release with fix: 18 skipped
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 612ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/22] gen generated_host_exports.rs
generated_host_exports.rs: 122 exports (host=5, lazy=10, generic=107, rust=0); 243 extern-C blocks audited
[2/22] gen JS modules (bundle-modules)
Preprocess modules (7686ms)
Bundle modules (44ms)
Postprocesss modules (18ms)
Bundle Functions (516ms)
Generate Code (28ms)

[8.30s] Bundled "src/js" for production
  2599 kb
  197 internal modules
  13 native modules
  50 internal functions across 16 files
[2/6] cargo bun_runtime → libbun_runtime.a
�[1m�[92m   Compiling�[0m bun_runtime v0.0.0 (/workspace/bun/src/runtime)
�[1m�[92m    Finished�[0m `release` profile [optimized + debuginfo] target(s) in 4m 38s
[3/6] link bun-profile
[5/6] strip bun
[5/6] bun-profile --revision
1.4.3-canary.1+75fa7a27d
[build] done
bun test v1.4.3-canary.1 (75fa7a2)

test/js/node/tls/node-tls-connect.test.ts:
(pass) should have checkServerIdentity [0.03ms]
(pass) should thow ECONNRESET if FIN is received before handshake [6.16ms]
(pass) initializes authorizationError to null in the TLSSocket 
... (truncated)
```

</details>

<details><summary>diff hotspot</summary>

```
src/runtime/socket/UpgradedDuplex.rs               |  23 ++-
 test/js/node/tls/node-tls-connect.test.ts          | 129 +++++++++++--
 .../tls/tls-shutdown-before-handshake-fixture.mjs  | 202 ++++++++++++++++++++-
 3 files changed, 335 insertions(+), 19 deletions(-)
```

</details>

**gate history** · 1 passed · 0 rejected · iteration 1

<details><summary>evidence per changed file</summary>

```
file                                                      reads  edits  tests
src/runtime/socket/UpgradedDuplex.rs                          2      8     20
test/js/node/tls/node-tls-connect.test.ts                     3      5     18
…t/js/node/tls/tls-shutdown-before-handshake-fixture.mjs      3      3     23
```

</details>

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

---------

Co-authored-by: Ciro Spaciari MacBook <ciro@anthropic.com>
@Jarred-Sumner

Copy link
Copy Markdown
Collaborator

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.

Jarred-Sumner added a commit that referenced this pull request Oct 6, 2026
…Socket that wraps a socket (#42340, #42343, #42339, #42453)

Node's whole rule is `connecting = socket.connecting || !socket._handle`, and
the wrapped socket's 'connect' clears it and is re-emitted. _write, _final,
_read, ref and unref all wait on that pair, in Bun too. Bun broke it both ways.
A wrap of a connected socket on the stream-level engine said connecting with
no 'connect' to come, so end() never finished. A wrap of a connecting socket
never emitted 'connect', so a second resume path grew (open -> drain) and
TLSSocket._final called back at once with no handle: 'finish' with no FIN, and
the connection stayed open.

The rule is now an assignment in linkUpgraded() and 'connect' is emitted after
the engine is attached, so the drain in SocketHandlers2.open and the early
return in TLSSocket._final are gone. endNT picks the handle on the tick that
sends the FIN: a server-side wrap attaches its handle on the tick its
constructor queued, and a wrap made around a socket's own end() left the fd to
the raw half. An engine that fails before it opens has no connect pending, so
connectError reports it itself.
Jarred-Sumner added a commit that referenced this pull request Oct 7, 2026
…Socket that wraps a socket (#42340, #42343, #42339, #42453)

Node's whole rule is `connecting = socket.connecting || !socket._handle`, and
the wrapped socket's 'connect' clears it and is re-emitted. _write, _final,
_read, ref and unref all wait on that pair, in Bun too. Bun broke it both ways.
A wrap of a connected socket on the stream-level engine said connecting with
no 'connect' to come, so end() never finished. A wrap of a connecting socket
never emitted 'connect', so a second resume path grew (open -> drain) and
TLSSocket._final called back at once with no handle: 'finish' with no FIN, and
the connection stayed open.

The rule is now an assignment in linkUpgraded() and 'connect' is emitted after
the engine is attached, so the drain in SocketHandlers2.open and the early
return in TLSSocket._final are gone. endNT picks the handle on the tick that
sends the FIN: a server-side wrap attaches its handle on the tick its
constructor queued, and a wrap made around a socket's own end() left the fd to
the raw half. An engine that fails before it opens has no connect pending, so
connectError reports it itself.
Jarred-Sumner added a commit that referenced this pull request Oct 8, 2026
…Socket that wraps a socket (#42340, #42343, #42339, #42453)

Node's whole rule is `connecting = socket.connecting || !socket._handle`, and
the wrapped socket's 'connect' clears it and is re-emitted. _write, _final,
_read, ref and unref all wait on that pair, in Bun too. Bun broke it both ways.
A wrap of a connected socket on the stream-level engine said connecting with
no 'connect' to come, so end() never finished. A wrap of a connecting socket
never emitted 'connect', so a second resume path grew (open -> drain) and
TLSSocket._final called back at once with no handle: 'finish' with no FIN, and
the connection stayed open.

The rule is now an assignment in linkUpgraded() and 'connect' is emitted after
the engine is attached, so the drain in SocketHandlers2.open and the early
return in TLSSocket._final are gone. endNT picks the handle on the tick that
sends the FIN: a server-side wrap attaches its handle on the tick its
constructor queued, and a wrap made around a socket's own end() left the fd to
the raw half. An engine that fails before it opens has no connect pending, so
connectError reports it itself.
Jarred-Sumner added a commit that referenced this pull request Oct 10, 2026
…Socket that wraps a socket (#42340, #42343, #42339, #42453)

Node's whole rule is `connecting = socket.connecting || !socket._handle`, and
the wrapped socket's 'connect' clears it and is re-emitted. _write, _final,
_read, ref and unref all wait on that pair, in Bun too. Bun broke it both ways.
A wrap of a connected socket on the stream-level engine said connecting with
no 'connect' to come, so end() never finished. A wrap of a connecting socket
never emitted 'connect', so a second resume path grew (open -> drain) and
TLSSocket._final called back at once with no handle: 'finish' with no FIN, and
the connection stayed open.

The rule is now an assignment in linkUpgraded() and 'connect' is emitted after
the engine is attached, so the drain in SocketHandlers2.open and the early
return in TLSSocket._final are gone. endNT picks the handle on the tick that
sends the FIN: a server-side wrap attaches its handle on the tick its
constructor queued, and a wrap made around a socket's own end() left the fd to
the raw half. An engine that fails before it opens has no connect pending, so
connectError reports it itself.
Jarred-Sumner added a commit that referenced this pull request Oct 10, 2026
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants