Skip to content

node:net: make resetAndDestroy() throw ERR_INVALID_HANDLE_TYPE on TLS and pipe sockets - #38232

Open
robobun wants to merge 6 commits into
mainfrom
farm/5b1b34d9/net-reset-and-destroy-handle-type
Open

robobun wants to merge 6 commits into
mainfrom
farm/5b1b34d9/net-reset-and-destroy-handle-type

Conversation

@robobun

@robobun robobun commented Aug 13, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • socket.resetAndDestroy() on a tls.TLSSocket or on a unix socket / named pipe connection resets and destroys the socket. Node throws TypeError [ERR_INVALID_HANDLE_TYPE]: This handle type cannot be sent synchronously for both (client or server side, connected or still connecting) and only resets plain TCP sockets.
  • On a new tls.TLSSocket(socket) that has not started yet, the call went even further off: the reset ran and failed inside _destroy with this._handle.terminate is not a function, delivered as the socket's 'error'.
  • Cause: Socket.prototype.resetAndDestroy in src/js/node/net.ts has no equivalent of node's if (!(this._handle instanceof TCP)) throw new ERR_INVALID_HANDLE_TYPE() (lib/net.js, v26.3.0 L801-L815). Bun cannot use the handle's class for this: the native handle is the same TCPSocket class for a TCP and for a unix / named-pipe connection.

Fix

  • resetAndDestroy() throws $ERR_INVALID_HANDLE_TYPE() when the socket has a handle and is either a tls.TLSSocket (recognized by its class, the same check Ipc.ts uses to refuse sending a TLS socket) or a pipe connection. The check sits before the connecting branch, like node's.
  • Pipe connections are recorded in a per-socket flag (kIsPipe) at the points where net.ts decides the transport, which is where node would construct a Pipe instead of a TCP handle:
    • Socket.prototype.connect(): the path option; the fd branch sets it to false. Every connect() call decides it, so a reused socket follows its current connection;
    • onconnection: the listener's bound path (handle.unix), or kClusterUnixPath for a cluster worker accepting on the primary's shared fd (the two sources Server.prototype.address() already reads);
    • onClusterConnection (round-robin cluster worker): the path kClusterFauxListen stores on the faux handle, set after the connect({ fd }) that cleared it.
  • Why this is correct: it matches node. Every TLSSocket with a handle has a TLS wrap there, so the class is the exact predicate; for pipes, node's handle type is itself derived from the same path / listen-path decisions, so recording that decision reproduces it. The plain net.Socket underneath tls.connect({ socket }) keeps a TCP handle in node and stays resettable here too.
  • Not covered, on purpose: connections adopted from a bare descriptor (connect({ fd }), listen({ fd }), IPC-received sockets) count as TCP, where node inspects the descriptor type; and a TLSSocket that has no handle at all yet (bare new tls.TLSSocket(), tls.connect({ socket }) over a still-connecting socket, or a server-side new tls.TLSSocket(socket, { isServer: true }) in the tick before its upgrade attaches) takes the existing no-handle branch (destroy(ERR_SOCKET_CLOSED)) where node, which wraps eagerly, throws. Both follow from how bun attaches handles, not from this check, and nothing in the repo calls resetAndDestroy() on either.
  • Verified (every new test fails on the released build and passes with this branch):
    • test/js/node/net/node-net.test.ts, "resetAndDestroy() handle type": unix socket / named pipe while connecting, connected, and on the accepting side throw; TCP still resets in all three states; a socket reused for TCP after a pipe connection resets again.
    • test/js/node/tls/node-tls-connect.test.ts, "TLSSocket.resetAndDestroy()": tls.connect() before and after the handshake, the server's socket, tls.connect({ socket }) over a net.Socket and over a Duplex, and a not-yet-started new TLSSocket(stream) all throw; the wrapped net.Socket still resets.
    • test/js/node/cluster.test.ts: SCHED_RR and SCHED_NONE workers; the unix-path server's accepted socket throws, the TCP server's resets.
    • Full node-net.test.ts: the same 11 failures as without the change in this container (connect(port) localhost resolution and unref tests), nothing new. node-tls-connect.test.ts, cluster.test.ts, child_process_ipc_handle.test.ts, and the proxy tests whose fixtures call resetAndDestroy() on a TLS server socket inside a try/catch (proxy-stress-lifecycle.test.ts, proxy-stress-adversarial.test.ts, the handshake-reset cases in proxy.test.ts; on the TLS legs they now take their destroy() fallback, since bun does not expose a _parent to reset instead): all pass.
    • Vendored test/js/node/test/parallel/test-net-*reset*.js (8 files) pass; the full test-net-* / test-tls-* set has no failures attributable to the change.

Background

  • resetAndDestroy() (added in node 18.3) closes a TCP connection with an RST instead of a FIN; in node it calls TCP.prototype.reset, which only TCP handles have, hence the handle-type guard. Bun implements the RST with the native terminate(), which exists on every native handle, so nothing stopped the call from going through.
  • Node handle types: a net.Socket wraps a TCP handle (TCP connection), a Pipe handle (unix domain socket, or named pipe on Windows), or for tls.TLSSocket a TLSWrap around one of those. this._handle instanceof TCP is how node tells them apart; bun's native handle is a TCPSocket or TLSSocket object regardless of the address family underneath.
  • Symbol.for("::buntls::") (bunTlsSymbol): a method defined on tls.TLSSocket.prototype (and on tls.Server) that net.ts uses throughout to detect TLS sockets; new tls.TLSSocket, tls.connect() and server-accepted TLS sockets are all instances of that class.
  • Cluster workers accept connections in two ways: under SCHED_RR the primary accepts and hands each connection's fd to a worker, which wraps it via a faux listen handle (onClusterConnection); under SCHED_NONE the worker listens on the primary's shared fd, so the native listener has no path and the server records it in kClusterUnixPath.
Probe output (node v26.3.0 vs bun 1.4.0 vs this branch)
case                                          node 26.3.0                 bun 1.4.0 (release)                          this branch
tcp connecting                                no throw, destroyed=false   no throw, destroyed=false                    no throw, destroyed=false
tcp connected                                 no throw, destroyed=true    no throw, destroyed=true                     no throw, destroyed=true
unix client connecting                        ERR_INVALID_HANDLE_TYPE     no throw, destroyed=false                    ERR_INVALID_HANDLE_TYPE
unix client connected                         ERR_INVALID_HANDLE_TYPE     no throw, destroyed=true                     ERR_INVALID_HANDLE_TYPE
unix server-side accepted                     ERR_INVALID_HANDLE_TYPE     no throw, destroyed=true                     ERR_INVALID_HANDLE_TYPE
tls.connect() connecting                      ERR_INVALID_HANDLE_TYPE     no throw, destroyed=false                    ERR_INVALID_HANDLE_TYPE
tls.connect() after secureConnect             ERR_INVALID_HANDLE_TYPE     no throw, destroyed=true                     ERR_INVALID_HANDLE_TYPE
tls server-side accepted                      ERR_INVALID_HANDLE_TYPE     no throw, destroyed=true                     ERR_INVALID_HANDLE_TYPE
tls.connect({ socket }) wrapper               ERR_INVALID_HANDLE_TYPE     no throw, destroyed=true                     ERR_INVALID_HANDLE_TYPE
new tls.TLSSocket(connectedSocket), unstarted ERR_INVALID_HANDLE_TYPE     'error' "_handle.terminate is not a function" ERR_INVALID_HANDLE_TYPE
net.Socket wrapped by tls.connect({ socket }) no throw, destroyed=true    no throw, destroyed=true                     no throw, destroyed=true
new net.Socket() (no handle)                  'error' ERR_SOCKET_CLOSED   'error' ERR_SOCKET_CLOSED                    'error' ERR_SOCKET_CLOSED
new tls.TLSSocket() (no socket, no handle)    ERR_INVALID_HANDLE_TYPE     'error' ERR_SOCKET_CLOSED                    'error' ERR_SOCKET_CLOSED (see "not covered")

no test proof · iteration 1 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/node/cluster.test.ts test/js/node/net/node-net.test.ts

… and pipe sockets

Node only resets a TCP handle: resetAndDestroy() on a TLSSocket or on a
unix socket / named pipe connection throws ERR_INVALID_HANDLE_TYPE
synchronously, before the connecting check. Bun reset and destroyed
every socket.

Bun's native handle is the same class for TCP and pipe connections, so
net.ts now records which one a socket is where the connection is made:
connect() (from the path option) and the server accept paths (the
listener's path, or the one a cluster worker recorded). TLS sockets are
recognized by their class, the way the IPC handle serializer already
does for the same error.
@coderabbitai

coderabbitai Bot commented Aug 13, 2026 •

Copy link
Copy Markdown
Contributor

Review 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: Pro

Run ID: 564d83d5-59a2-49f7-8501-d170d6bec9b2

📥 Commits

Reviewing files that changed from the base of the PR and between b5afcac and 9fae45f.

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

Walkthrough

Summary

net.ts now tracks pipe handles and limits resetAndDestroy() to TCP sockets. Tests cover Unix sockets, Windows named pipes, clustered connections, TCP connections, and TLS wrappers.

Changes

Handle-aware socket reset

Layer / File(s) Summary
Track handle types and guard reset behavior
src/js/node/net.ts
Socket paths record pipe status. resetAndDestroy() throws ERR_INVALID_HANDLE_TYPE for TLS and pipe sockets.
Validate TCP, pipe, and cluster connections
test/js/node/net/node-net.test.ts, test/js/node/cluster.test.ts
Tests cover connection state, server-side sockets, reused sockets, and both cluster scheduling policies.
Validate TLS wrapper behavior
test/js/node/tls/node-tls-connect.test.ts
Tests cover TLS sockets before and after handshakes, transport wrappers, and uninitialized wrappers.

Possibly related PRs

  • oven-sh/bun#35417: Both changes update socket connection and handle-state tracking in src/js/node/net.ts.

Suggested reviewers: cirospaciari, jarred-sumner

🚥 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 primary change to make resetAndDestroy() reject TLS and pipe sockets.
Description check ✅ Passed The description explains the problem, fix, scope, implementation details, and verification results, including relevant test coverage.

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

@robobun

robobun commented Aug 13, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: ready to merge on review; the diff is green, CI is blocked on infrastructure.

Reproduced with the released bun (1.4.0) and node v26.3.0 side by side: resetAndDestroy() on a tls.connect() socket, a server-side TLS socket, a tls.connect({ socket }) wrapper, a not-yet-started new tls.TLSSocket(stream), and on unix-socket connections (connecting, connected, and accepted, plus cluster workers under both scheduling policies) returns normally in bun and throws ERR_INVALID_HANDLE_TYPE in node; plain TCP behaves the same in both. Every new test in test/js/node/net/node-net.test.ts, test/js/node/tls/node-tls-connect.test.ts and test/js/node/cluster.test.ts fails on the released build and passes with this branch.

CI: the last code change is 9fae45f (4528b72 is an empty retrigger). Both builds for it, 95304 and 95829, have 177 of 179 jobs passed with no failing test (the only annotations are tests that passed on retry, none in net/tls/cluster); the remaining two jobs are the two darwin 14 aarch64 - test-bun shards, which expired without an agent in 95304 and have been waiting for one in 95829. Those two shards are the only macOS test lane in the matrix, so the new tests have passed on every Linux lane and on both Windows lanes (which exercise the named-pipe branch) but have not yet run on macOS in CI; the macOS-specific part of the change is only the unix socket path length in the cluster test, which was shortened to fit sun_path with room to spare. I am not going to keep retriggering; if a maintainer wants the two shards re-run, a retry of those jobs is enough.

Related: #34507 has a test that calls resetAndDestroy() on a tls.connect({ socket }) wrapper and expects it to go through; whichever of the two lands second needs that test adjusted (node throws there too).

@robobun

robobun commented Aug 13, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 8:04 AM PT - Aug 14th, 2026

✅ @robobun, your commit 4528b72de61ba9447c1103591010339d49d15c10 passed in Build #95829! 🎉


🧪   To try this PR locally:

bunx bun-pr 38232

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

bun-38232 --bun

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

LGTM — clean Node compat fix with thorough coverage.

What was reviewed:

  • kIsPipe is initialized in the constructor and set at every point where net.ts decides the transport (connect(), onconnection, onClusterConnection); the onClusterConnection assignment correctly follows connect({fd}) so it isn't clobbered.
  • The TLS check reuses the existing typeof this[bunTlsSymbol] === "function" idiom already used at four other sites in net.ts, and ERR_INVALID_HANDLE_TYPE is already registered in ErrorCode.ts.
  • Checked the one internal resetAndDestroy() caller that hits TLS sockets (test/js/bun/http/proxy-stress-helpers.ts) — it unwraps to the raw TCP socket and already has a catch { client.destroy() } fallback, so it isn't broken by the new throw.
Extended reasoning...

Overview

This PR makes net.Socket.prototype.resetAndDestroy() throw ERR_INVALID_HANDLE_TYPE for TLS sockets and unix/named-pipe sockets, matching Node's if (!(this._handle instanceof TCP)) throw guard. Since Bun's native handle is the same class regardless of transport, a new per-socket kIsPipe symbol records the pipe/TCP decision at the four points where net.ts makes it: the Socket constructor (init to false), connect() (from options.path), onconnection (from handle.unix || self[kClusterUnixPath], the same two sources Server.prototype.address() reads), and onClusterConnection (from the faux handle's unix). TLS is detected with the existing bunTlsSymbol prototype-method check.

Security risks

None. This tightens an API to throw where it previously succeeded; it does not relax any check, touch credential handling, or affect trust decisions.

Level of scrutiny

Moderate — a Node-compat behavioral change in src/js/node/net.ts that turns a previously-succeeding call into a synchronous throw, and threads new per-socket state through four code paths including two cluster-worker accept paths. That said, the change is small (~30 production lines), follows established idioms in the file, and the check placement (inside if (this._handle), before the connecting branch) mirrors Node's exactly.

Other factors

  • Test coverage is comprehensive across the variant matrix the review guidelines call for: pipe vs TCP × connecting/connected/accepted, TLS via tls.connect() / server-side / tls.connect({socket}) over both net.Socket and Duplex, cluster SCHED_RR and SCHED_NONE, and a reused socket switching pipe→TCP. Each asserts the exact error name/code/message and destroyed state.
  • The PR description includes a probe table diffed against Node v26.3.0, and confirms the vendored test-net-*reset*.js parallel tests and the full node-net/node-tls/cluster/proxy-stress-lifecycle suites still pass.
  • The one subtlety I checked — that socket[kIsPipe] = !!this.unix in onClusterConnection runs after socket.connect({fd}) (which resets kIsPipe to false since no path is passed) — is correctly ordered.
  • The acknowledged gap (fd-adopted sockets are treated as TCP where Node inspects the descriptor type) is stated in the PR body and does not regress anything.

@robobun

robobun commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator Author

One correction to the review's reasoning: in the pushed revision connect({ fd }) returns before the kIsPipe assignment, so it did not reset the flag; the onClusterConnection ordering only mattered defensively. A reused socket going pipe -> connect({ fd }) would have kept a stale flag. I have a follow-up commit that sets the flag to false on the fd branch too, so every connect() call decides it; pushing it together with anything the remaining review turns up.

Comment thread src/js/node/net.ts Outdated
Comment thread src/js/node/net.ts Outdated
Comment thread src/js/node/net.ts Outdated
Comment thread src/js/node/net.ts Outdated
Comment thread src/js/node/net.ts Outdated

@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. The kIsPipe bookkeeping and the guard in resetAndDestroy() look correct and are thoroughly tested against Node v26.3.0. Because it's a user-visible behavioral change (calls that previously succeeded now throw) in the core node:net compat layer, and there are outstanding comment-cop findings plus an acknowledged interaction with #34507, a human look would still be worthwhile.

What was reviewed:

  • kIsPipe is set on every path that decides the transport (connect() path/fd branches, onconnection, onClusterConnection) and initialized in the constructor — no stale-flag window on socket reuse.
  • The TLS check via bunTlsSymbol matches how the rest of net.ts detects TLSSocket; the underlying net.Socket in a tls.connect({ socket }) wrap correctly stays resettable.
  • Existing in-tree resetAndDestroy() callers on TLS sockets (proxy-stress-helpers.ts) already fall back to destroy() in a try/catch; tls-low-prio-queue-fixture.ts uses plain net.connect TCP sockets, so it's unaffected.
Extended reasoning...

Overview

Adds a Node-compat guard to Socket.prototype.resetAndDestroy() in src/js/node/net.ts: it now throws ERR_INVALID_HANDLE_TYPE synchronously for TLS sockets and unix-socket / named-pipe connections, matching Node's if (!(this._handle instanceof TCP)) check. Since Bun's native handle class is the same for TCP and pipe connections, the transport is recorded in a new per-socket kIsPipe symbol at each site that decides it (client connect(), server onconnection, cluster onClusterConnection, and reset on the connect({ fd }) early-return branch). Three test files gain coverage for pipe vs TCP × connecting/connected/accepted, socket reuse, TLS wrappers over both a net.Socket and a Duplex, and both cluster scheduling policies.

Security risks

None. This narrows an API's accepted inputs (fails closed on non-TCP handles) rather than widening it, and touches no auth/crypto/trust decisions.

Level of scrutiny

Medium-high. src/js/node/net.ts is core Node compat surface used by http, tls, cluster, and user code; the change is small but flips previously-succeeding calls to a synchronous throw. That's the correct Node behavior, and the failure mode for anyone relying on the old behavior is loud (a TypeError with a specific code), but it is still a user-facing behavioral change that a maintainer should sign off on.

Other factors

  • Five outstanding comment-cop inline findings from github-actions[bot] flag the new explanatory comments as too long; those haven't been addressed yet.
  • The author flagged an interaction with #34507, whose test expects resetAndDestroy() on a tls.connect({ socket }) wrapper to succeed — whichever lands second needs adjusting.
  • I checked the other in-tree resetAndDestroy() callers: proxy-stress-helpers.ts already wraps the call in try/catch with a destroy() fallback (and unwraps to the raw TCP socket first), tls-low-prio-queue-fixture.ts calls it on plain TCP net.connect sockets, and _http_server.ts's is a no-op stub on FakeSocket — none regress.
  • Test coverage is strong: the new tests assert the exact error class/code/message, cover both connection states and both sides, verify the flag is decided per-connect(), and were confirmed to fail on the released build.

Comment thread src/js/node/net.ts Outdated
Comment thread src/js/node/net.ts Outdated
@robobun

robobun commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed the follow-ups (head is now 9fae45f):

  • connect({ fd }) sets the flag to false as well, so every connect() call decides it (7ede0a1).
  • One more TLS shape covered: new tls.TLSSocket(stream).resetAndDestroy() before the handshake starts. On the released build that call went through and failed inside _destroy with this._handle.terminate is not a function (surfaced as the socket's 'error'); it now throws ERR_INVALID_HANDLE_TYPE like node (93d1862).
  • Comments trimmed to one line each; the inline threads are resolved (39ea98d, 9fae45f).
  • Cluster test uses shorter names so the absolute socket path stays well inside sun_path with a default macOS TMPDIR (39ea98d).
  • PR body updated: the not-covered list now also names listen({ fd }) and the server-side new TLSSocket(socket, { isServer: true }) pre-upgrade tick, and the verified list includes the proxy tests whose fixtures reset a TLS server socket inside a try/catch (they take their destroy() fallback on the TLS legs now).

No logic changes beyond the first bullet.

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

LGTM — a focused Node.js compat fix with thorough coverage.

What was reviewed:

  • The kIsPipe flag is set at every transport-decision point (connect() path/fd branches, onconnection via handle.unix/kClusterUnixPath, onClusterConnection via the faux handle's unix) and read only in resetAndDestroy(); the bunTlsSymbol check matches the pattern used elsewhere in net.ts and Ipc.ts.
  • $ERR_INVALID_HANDLE_TYPE() produces the exact TypeError/code/message the tests assert (via ErrorCode.cpp).
  • In-repo resetAndDestroy() callers on TLS sockets (proxy-stress-helpers.ts rstClient) already wrap in try/catch with a destroy() fallback, so no existing tests regress.
  • Tests await real events, clean up in try/finally, and cover the connecting/connected/server-side/reused/cluster matrix plus the TLS wrap shapes.
Extended reasoning...

Overview

Adds a private kIsPipe symbol on net.Socket to record whether the underlying transport is a unix socket / named pipe (vs TCP), and guards Socket.prototype.resetAndDestroy() to throw ERR_INVALID_HANDLE_TYPE for TLS sockets (detected via bunTlsSymbol, the same predicate Ipc.ts uses) and for pipe sockets. This mirrors Node's if (!(this._handle instanceof TCP)) guard, which Bun cannot reproduce directly because its native handle class doesn't distinguish TCP from pipe. The flag is set at each point where net.ts decides the transport: connect() (both the path and fd branches), onconnection (server accept, reading handle.unix || self[kClusterUnixPath]), and onClusterConnection (round-robin cluster worker, reading the faux handle's unix set by kClusterFauxListen). Tests are added in three files covering unix/TCP × connecting/connected/server-side, socket reuse, both cluster scheduling policies, and every TLS wrap shape (tls.connect(), server-side, tls.connect({ socket }) over a net.Socket and a Duplex, and an unstarted new TLSSocket(stream)).

Security risks

None. This is a Node compat behavioral change (throwing where Bun previously silently reset). No auth, crypto, permissions, or untrusted-input parsing is touched. The added flag is a private symbol only read by resetAndDestroy().

Level of scrutiny

Medium. It's a Node compat fix in the well-trodden src/js/node/net.ts layer with a narrow surface: one new symbol, five one-line flag assignments at existing transport-decision points, and a two-line guard in resetAndDestroy(). The behavior change is user-visible (a throw where there wasn't one), but it aligns Bun with Node's documented contract, and the only in-repo TLS callers (proxy-stress-helpers.ts rstClient) already wrap the call in try/catch with a destroy() fallback — the PR description confirms those tests pass. No native code, memory management, or hot-path perf concerns.

Other factors

  • The bunTlsSymbol check (typeof this[bunTlsSymbol] === "function") is the same idiom used throughout net.ts and in Ipc.ts for the same purpose.
  • $ERR_INVALID_HANDLE_TYPE() is defined in ErrorCode.ts/ErrorCode.cpp and produces TypeError with code ERR_INVALID_HANDLE_TYPE and message "This handle type cannot be sent", matching the tests' exact-string assertions.
  • The author self-identified and fixed the stale-flag case where connect({ fd }) returned before the flag assignment (7ede0a1), so every connect() call now decides the flag.
  • All comment-cop threads were addressed (comments trimmed to one line each; all threads resolved).
  • Tests follow the repo's conventions: once() for events, try/finally cleanup, port: 0, exact error class/code/message assertions, and a per-connect() reuse test to prove the flag isn't stale. The PR description documents verification against node v26.3.0, USE_SYSTEM_BUN=1 failure, and the vendored test-net-*reset*.js suite.
  • Not covered by CODEOWNERS.

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

Automated review ran and found no bugs. Deferring to a human because this is a user-visible behavior change to resetAndDestroy() (previously-working calls on TLS and pipe sockets now throw) and introduces per-socket kIsPipe state that must stay in sync at every handle-attach point in net.ts.

What was reviewed:

  • The kIsPipe flag is derived from the same sources Server.prototype.address() already reads (handle.unix / kClusterUnixPath), and every connect() path (fd, path, host/port) decides it.
  • The TLS check reuses the bunTlsSymbol pattern already used in Ipc.ts for the same error code; $ERR_INVALID_HANDLE_TYPE() produces the exact node message the tests assert.
  • The onClusterConnection assignment after connect({ fd }) and the fd-branch reset — checked that the ordering is right so the RR worker's flag reflects the faux handle's unix.
  • Tests cover the variant matrix (connecting/connected/server-side, pipe→TCP reuse, both cluster policies, all TLS shapes) and assert TCP still resets.
Extended reasoning...

Overview

The PR adds a handle-type guard to Socket.prototype.resetAndDestroy() in src/js/node/net.ts so it throws ERR_INVALID_HANDLE_TYPE for TLS sockets and unix/named-pipe connections, matching node's if (!(this._handle instanceof TCP)) check. Since bun's native handle class does not distinguish TCP from pipe, a new per-socket kIsPipe symbol records the transport at each point net.ts decides it: the Socket constructor (default false), connect({ fd }) (false), connect({ path }) vs host/port, onconnection (from the listener's handle.unix / kClusterUnixPath), and onClusterConnection (from the faux handle's unix, after the fd-connect that cleared it). TLS is detected via typeof this[bunTlsSymbol] === 'function', the same pattern Ipc.ts uses for this error. Three test files gain thorough coverage.

Security risks

None identified. The change makes an operation stricter (rejects instead of proceeding) and does not touch verification, credentials, or trust decisions. The TLS check is a class-level predicate, not a security gate.

Level of scrutiny

Moderate-to-high. src/js/node/net.ts is a large hot-path built-in and this is a user-visible behavior change: code that previously called resetAndDestroy() on a TLS or unix socket in bun and had it go through will now get a synchronous throw. The PR verified the in-tree proxy fixtures (which do this inside try/catch) still pass on their fallback path, and the vendored test-net-*reset*.js suite passes, but this is still the kind of compat-vs-existing-users trade-off a maintainer should sign off on. The PR also flags a known conflict with #34507's test.

Other factors

  • The runtime diff is small (~15 lines) and follows existing idioms in the file (bunTlsSymbol check, symbol-keyed private state, $ERR_* machinery).
  • The kIsPipe derivation reuses exactly the two sources Server.prototype.address() already reads to answer the same question, so it should stay consistent with what the server reports.
  • The PR explicitly documents its non-coverage (connect({ fd }), listen({ fd }), IPC-received sockets treated as TCP; a bare new tls.TLSSocket() with no handle takes the ERR_SOCKET_CLOSED branch) — these follow from how bun attaches handles rather than from this check, but they are a documented divergence from node.
  • All comment-cop threads are resolved (comments trimmed to one line each in 39ea98d/9fae45f).

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>

This branch has not been deployed

No deployments
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.

1 participant