Skip to content

tls: consolidate the open TLS fixes (both engines, node:tls, node:https, WebSocket, SQL) - #44618

Merged
Jarred-Sumner merged 174 commits into
mainfrom
claude/tls-consolidated
Oct 10, 2026
Merged

tls: consolidate the open TLS fixes (both engines, node:tls, node:https, WebSocket, SQL)#44618
Jarred-Sumner merged 174 commits into
mainfrom
claude/tls-consolidated

Conversation

@Jarred-Sumner

@Jarred-Sumner Jarred-Sumner commented Oct 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

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

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. tls: queue unsent ciphertext per connection so write() counts every sealed record #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. tls: trust a pinned self-signed certificate whose keyUsage lacks keyCertSign #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. node:tls: accept Ed25519 server certificates during client verification #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 tls: over a Duplex, the peer's close_notify waits for the writes in flight #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 (node:tls: destroy the wrapped net.Socket when a TLS wrap is destroyed #38028 + node:tls: report a wrapped socket's connect failure on the TLSSocket #38122 + node:tls: use the stream engine for server-side wraps the native upgrade cannot adopt #38076, node:http2: destroy the accepted socket when an injected connection's TLS proxy is destroyed #38154) is safe only if every write has really completed (node:tls: complete write(cb) and end(cb) over a Duplex when the stream completes the ciphertext #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:

    • node:tls: complete write(cb) and end(cb) over a Duplex when the stream completes the ciphertext #43877 + tls: remove the 'drain' listener on a wrapped stream #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 node:http2: destroy the accepted socket when an injected connection's TLS proxy is destroyed #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 (node:tls: complete write(cb) and end(cb) over a Duplex when the stream completes the ciphertext #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 (tls: pause and resume a wrapped Duplex transport with the TLS socket's reads #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.

@Jarred-Sumner
Jarred-Sumner requested a review from alii as a code owner October 6, 2026 06:16
This was referenced Oct 6, 2026
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Open
Open
Closed
Closed
Closed
Closed
Closed
Closed
robobun added a commit that referenced this pull request Oct 20, 2026
@robobun
wip: PR body and status drafts
d33fbb6 
### Problem

- `setKeyCert(ctx)` in an `ALPNCallback` does not change the served certificate. On TLS 1.2 a session of one ALPN protocol resumes under another, with `authorized: true` for a client that only the other context trusts.
- Cause: BoringSSL runs the ALPN selection callback (`select_alpn_callback`, `socket_body.rs`) after it picked the certificate and, on TLS 1.2, the session (`handshake_server.cc:781`, `:803`, `:881`).

### Fix

- usockets copies the ClientHello's ALPN list and hands it over from BoringSSL's certificate callback, before those picks. `ALPNCallback` runs there.
- A server-side `setKeyCert()` uses the SNI switch, `us_ssl_apply_selected_ctx`. That switch does nothing once BoringSSL has begun to pick.
- Verified: `node-tls-context.test.ts` (77 new tests, 57 fail on main), Node's `test-tls-server-setkeycert.js` (added, fails on main).
- Self-reviewed: 37 concerns raised, 30 addressed. The notes list the rest.

### Background

- Session id context: bytes BoringSSL copies into each new session. A session resumes only under the same id (#42998).
- Certificate callback (`SSL_set_cert_cb`): runs once per server handshake, after version and SNI, before cipher, certificate and session.
- Considered recomputing the verdict after a late switch: a second verdict path, wrong certificate still served. Considered Node's in-place identity: it copies BoringSSL's credential and chain logic.

### Downsides

- `setKeyCert(ctx)` in `ALPNCallback` now applies the `ciphers` and `sigalgs` of `ctx`, as SNI contexts already do. Node keeps the server's.
- On TLS 1.2, `getCipher()`, `getSession()`, `isSessionReused()` and `getPeerCertificate()` report nothing inside `ALPNCallback`. On TLS 1.3 `setKeyCert()` beats an `SNICallback` context. Node does the reverse.
- One allocation per handshake on `ALPNCallback` servers, about 10 branches on SNI servers. Release text: -544 bytes.

<details><summary>Notes</summary>

**Outcome table.** The default context trusts ca1 and serves agent2. ALPN `b` selects a context that trusts ca2 and serves agent3. TLS 1.2 and TLS 1.3 give the same result with this PR.

| connection | main | this PR |
|---|---|---|
| full handshake, ALPN `b` | serves agent2 | serves agent3 |
| TLS 1.2, ca1 client, session of `a` offered with `b` | resumed, `authorized: true` | full handshake, `UNABLE_TO_VERIFY_LEAF_SIGNATURE` |
| TLS 1.2, ca2 client, session of `b` offered with `a` | resumed, `authorized: true` | full handshake, `UNABLE_TO_VERIFY_LEAF_SIGNATURE` |
| a session offered with its own protocol | resumed | resumed |
| ca2 client, `setImmediate(() => this.setKeyCert(ctxB))` in `ALPNCallback` | `authorized: true`, and the session resumes under the default context with `authorized: true` | `authorized: false` on both connections |
| `setKeyCert()` before the ClientHello (in a `'connection'` listener) | `SNICallback` and ALPN are dropped | both run |
| `setKeyCert()` in the `SNICallback` of a server-side `TLSSocket` | ALPN is dropped | ALPN is negotiated |

**Decisions that need a maintainer.**

1. Ciphers and signature algorithms. A context switch in Bun is `SSL_set_SSL_CTX`, so the `ciphers` and `sigalgs` of the selected context apply. That is already so for `SNICallback` and `addContext()` on main. With this PR it is also so for `setKeyCert()` in `ALPNCallback`, because the switch now comes before BoringSSL picks the cipher. Node applies the identity and the CA store only, and keeps the server's `ciphers` and `sigalgs` on all routes. Probe (server `ciphers: "ECDHE-RSA-AES256-GCM-SHA384"`, TLS 1.2 client that offers only `ECDHE-RSA-AES128-GCM-SHA256`, `setKeyCert({ key, cert })` in `ALPNCallback`): this PR connects with the AES128 suite, Node v26.3.0 and main answer `ERR_SSL_NO_SHARED_CIPHER`. A context that repeats the server's `ciphers` keeps the refusal. The test `setKeyCert() in ALPNCallback applies the ciphers of the selected context` pins this. I did not carry the server's settings across the switch: BoringSSL has no getter for the signing preferences, so it needs the configured strings kept beside each `SSL_CTX`, and it changes the `SNICallback` and `addContext()` routes of main too.
2. TLS 1.3 order. Bun runs `SNICallback` before `ALPNCallback` on both versions. Node runs `ALPNCallback` first on TLS 1.3, because OpenSSL negotiates ALPN before its certificate callback there. So when an `SNICallback` answers a context and `ALPNCallback` calls `setKeyCert()`, Node v26.3.0 serves the `setKeyCert()` context on TLS 1.2 and the `SNICallback` context on TLS 1.3. This PR serves the `setKeyCert()` context on both.
3. A late `setKeyCert()` on a server is a silent no-op. Node does not throw there either. "Late" means after the certificate callback returned. A call that `ALPNCallback` defers with `process.nextTick` or a promise is still in time on an fd socket, because that queue drains before the callback returns, and the switch is then complete (certificate, verdict and session). Node serves the selected certificate for such a call too. On the stream engine the queue drains later and the call changes nothing.

**Why the certificate callback and not the select-certificate callback.** The select-certificate callback runs before BoringSSL negotiates the version, so `this.getProtocol()` inside `ALPNCallback` would report `TLSv1.2` for a TLS 1.3 handshake. It also runs before the server name callback that applies `addContext()` entries, which would then replace the context that `ALPNCallback` selected. The certificate callback runs after both. `SSL_set_SSL_CTX` drops it, so `us_ssl_apply_selected_ctx` arms it again while a list waits.

**What `ALPNCallback` sees.** Values inside the callback, on the second of two connections of one client (the second resumes):

| | main, TLS 1.2 | this PR, TLS 1.2 | Node v26.3.0, TLS 1.2 |
|---|---|---|---|
| `getCipher()` | the suite | no suite | the suite |
| `isSessionReused()` | `true` | `false` | `true` |
| `getSession()` | a Buffer | `undefined` | a Buffer |
| `getPeerCertificate()` | the peer | `{}` | the peer |

On a full TLS 1.2 handshake main reports the suite and a session there, and Node reports neither, like this PR. TLS 1.3 is as on main. `ALPNCallback` also runs for a ClientHello that then fails for lack of a shared cipher (TLS 1.2) or signature algorithm (TLS 1.3). Node runs it after those checks on TLS 1.2.

**The ALPN list.** `us_select_cert_alpn` checks the extension with the same CBS calls as `ssl_negotiate_alpn` and keeps the measured length with the copy. A list that BoringSSL is going to reject is not handed over. The copy lives in its own `SSL` ex_data slot, so `us_ssl_rare_t` stays at 64 bytes.

**Sizes.** `sizeof(struct us_ssl_rare_t)`: 64 before, 64 after (gdb, debug build). `NewSocket<true>`: 504 bytes before and after (gdb, debug build. The new `alpn_refused` flag sits in padding). Release build, linux-x64, LTO, `size build/release-size/bun`: text 107068349 before, 107067805 after. data and bss are the same. The file is 107243232 bytes in both.

**Cost per handshake.** Counted from the code. A server with an `ALPNCallback`: one `malloc` and one `free` for the list copy, and one ex_data slot. A server with an SNI resolver and no `ALPNCallback`: 4 branches in `us_select_cert_cb` to read the per-connection flag, and for each context switch 4 branches in `us_ssl_apply_selected_ctx` plus the null checks inside `SSL_in_init` and `SSL_get_pending_cipher`. No allocation, no system call. A server with neither runs no new code.

**Not consolidated.** The two callbacks that BoringSSL reads off the current `SSL_CTX` are still installed where a connection asks for them (`us_ssl_on_alpn_offer`, `us_ssl_set_server_ctx`, `install_sni_alpn_selector`). One install where the `SSL_CTX` is built would remove those sites. It would also run both callbacks on every server handshake, Bun.serve included, because Bun.serve registers its own resolver on contexts from the same builder.

**Relation to other work.** #44618 listed this PR as open for a decision. #45471 (server name on the stream TLS engine) rewrites the same arm of `us_select_cert_cb`, and the two conflict in `openssl.c`. This one is the smaller change. Whichever lands second has to keep the ALPN stage behind the name resolution of the stream engine.

**Not covered.**
- `https.createServer({ ALPNCallback })` does not reach this code: `https.ts` stores the option and nothing reads it.
- `setKeyCert()` in the same tick as `new tls.TLSSocket(socket, { isServer: true })` is dropped, because the native handle does not exist yet. The tests wait one `setImmediate`.
- On a `listen()` server Bun calls `SNICallback` with the `tls.Server` as `this`, so `this.setKeyCert()` there throws. The tests use the `SNICallback` of a server-side `TLSSocket`.
- `setKeyCert()` in `ALPNCallback` is also dropped when the socket already holds the ClientHello at the time it is wrapped (`server.emit("connection", socket)` after a read): `upgradeTLS` feeds the bytes before `net.ts` assigns `_handle`.
- The `ciphers` and `sigalgs` of a server do not survive a context switch on any route (decision 1).

**Found on the way, same on main, not changed.** Both are in the code that this PR moves from `select_alpn_callback` to `us_dispatch_alpn_offer`.
- A Worker that is terminated inside `ALPNCallback` aborts the process: the termination exception reaches `to_utf8()` as the callback's answer.
- A protocol name with a byte above 0x7f that `ALPNCallback` selects is refused: `net.ts` decodes the names as latin1 and the answer is encoded as UTF-8.

**Self-review, rejected concerns.**
- One install of the callbacks at context build (3 concerns): see "Not consolidated".
- Apply the identity, the CA store and the session id context to the `SSL` in place, as Node does (1 concern): BoringSSL has no getter for the credential list of a context, and it completes a lone leaf from the store of the current `SSL_CTX`. Both need a copy of its logic.
- Fold in the same-tick drop, the `SNICallback` receiver and `https.Server` (3 concerns): other mechanisms. They are listed above.

A second pass over the reworked diff (the guard, the lifetime of the list copy, the parser, the new tests) found no defect in the product code and two in the tests. Both are fixed: the raw ClientHello rows now wait for the server's first record, and one test has the name of what it asserts.

**Removed.** `set_key_cert` called `SSL_use_certificate`, `SSL_use_PrivateKey` and `SSL_set1_chain` after `SSL_set_SSL_CTX`. It had no effect: `SSL_set_SSL_CTX` already installs the certificate of the context when it runs in time, and nothing helps when it does not.

**Tests that also pass on main (20 of the 77).** They cover code that this PR adds: the 11 raw ClientHello rows of `ALPNCallback` (valid, boundary and malformed ALPN lists), the refusal alert after `setKeyCert()` (4), a handshake with no shared protocol version (2), and three TLS 1.3 rows where main was already correct (`keeps a refused client out of the handler` on two transports, `sees no cipher, session or peer certificate yet`).

**Suites run on the debug build.** `node-tls-context.test.ts` (160 pass), `node-tls-server.test.ts` (217 of 218. `reports the reset that arrives while the socket is paused` fails in some runs of the whole file, also with the `src/` of main: 1 of 4 runs there), `node-tls-connect.test.ts` (4 failures, all need `bun.sh:443`), `node-tls-upgrade.test.ts`, `node-tls-cert.test.ts`, `node-tls-internals.test.ts`, `renegotiation.test.ts`, `test/js/bun/net/socket.test.ts` (2 failures that need `www.example.com`), `tls-fatal-error-closes.test.ts`, `tls-reject-before-client-cert.test.ts`, `bun-serve-ssl.test.ts`, `serve-http2.test.ts`, `serve-http2-protocol.test.ts`, `node-http2.test.js`. Vendored Node tests: `test-tls-server-setkeycert`, `test-tls-alpn-server-client`, `test-tls-psk-alpn-callback-exception-handling`, `test-tls-socket-constructor-alpn-options-parsing`, `test-http2-alpn`, `test-tls-client-resume`, `test-tls-client-resume-12`, `test-tls-ticket`, `test-tls-session-cache`, `test-tls-secure-session`, `test-tls-sni-option`, `test-tls-sni-server-client`, `test-tls-sni-servername`, `test-tls-snicallback-error`, `test-tls-empty-sni-context`, `test-tls-add-context`, `test-tls-client-verify`, `test-tls-server-verify`, `test-tls-peer-certificate`, `test-https-client-resume`.

</details>

=====STATUS=====
**Status**

Reproduction (fixtures in `test/js/node/tls/fixtures`): a `tls.Server` whose default context trusts ca1 and serves agent2, with an `ALPNCallback` that calls `this.setKeyCert(ctxB)` for protocol `b`. `ctxB` trusts ca2 and serves agent3.

1. Connect with ALPN `b`. On main the client gets agent2. With this PR it gets agent3.
2. On TLS 1.2, connect with the agent3 client certificate (issued by ca2) and ALPN `b`. Then offer that session with ALPN `a`. On main the handler sees `resumed: true, authorized: true` under the default context, which does not trust ca2. With this PR it is a full handshake with `UNABLE_TO_VERIFY_LEAF_SIGNATURE`.
3. Call `setKeyCert(ctxB)` from a `setImmediate` inside `ALPNCallback`. On main the handler sees `authorized: true` on TLS 1.2 and TLS 1.3, and the session resumes under the default context. With this PR the call changes nothing.

Tests: `bun bd test test/js/node/tls/node-tls-context.test.ts` (77 new tests, 57 fail with the `src/` and `packages/` of main), and `bun bd test/js/node/test/parallel/test-tls-server-setkeycert.js`.

PR: #43216
This was referenced Sep 18, 2026
Open
Open
Luligu added a commit to Luligu/matterbridge that referenced this pull request Oct 20, 2026
@Luligu @claude
docs: update Bun known issues and matter.js 0.18.0 release notes
22b2d02 
README-BUN.md: mark the ws TLS and coverage known issues as fixed upstream (oven-sh/bun#44618, oven-sh/bun#45677), merged but not yet released. Matter.js-0.18.md: reference the released matter.js 0.18.0 and keep the dev snapshot audit history.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
robobun added a commit that referenced this pull request Oct 20, 2026
@robobun @Jarred-Sumner
tls: remove the 'drain' listener on a wrapped stream
8cd7257 
A TLS socket over a stream listened for 'drain' on that stream. The
listener ran the socket's flush and its drain callback. Node's
JSStreamSocket has no such listener.

A stream that emits 'drain' inside its own write() ran that listener
inside the send that called write(). NewSocket::internal_flush lends its
queue to that send and settles it afterwards, so the nested flush sent
the queue again: 524288 bytes for 262144, and a bad record MAC for
HTTP/2 over such a stream. Bun 1.4.2 and 1.4.3 do this too.

Since #44618 a TLS socket over a net.Socket that is still dialing is
`connecting`, and node:net parks a write made then. The 'connect' of the
socket and a drain of its handle each send that parked chunk. The
listener delivered such a drain before the 'connect' when the dialing
socket had a backlog of its own, so "hello" arrived as "hellohello".
Without the listener no stock transport delivers a drain there. A
net.Socket subclass that emits 'connect' after the handshake still does.

The listener was also what sent the rest of one chunk above 2^31 - 1
bytes, because SSLWrapper::write_data takes at most that in one call.
Socket.prototype._write now fails such a write with EPROTO on this
engine, as Node does on every TLS socket. The HTTP/2 upgrade path sends
it in slices, because its writer joins frames into one chunk that the
user did not write.

The two comments that said a send cannot reach the pending-write queue
now say what can reach it over a stream.

The deletion is the one of d7b42e1, without the write-done callback
that commit put in the freed slot. When write(cb) completes does not
change.

Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
robobun added a commit that referenced this pull request Oct 21, 2026
@robobun @Jarred-Sumner
tls: remove the 'drain' listener on a wrapped stream
1c49c40 
A TLS socket over a stream listened for 'drain' on that stream. The
listener ran the socket's flush and its drain callback. Node's
JSStreamSocket has no such listener.

A stream that emits 'drain' inside its own write() ran that listener
inside the send that called write(). NewSocket::internal_flush lends its
queue to that send and settles it afterwards, so the nested flush sent
the queue again: 524288 bytes for 262144, and a bad record MAC for
HTTP/2 over such a stream. Bun 1.4.2 and 1.4.3 do this too.

Since #44618 a TLS socket over a net.Socket that is still dialing is
`connecting`, and node:net parks a write made then. The 'connect' of the
socket and a drain of its handle each send that parked chunk. The
listener delivered such a drain before the 'connect' when the dialing
socket had a backlog of its own, so "hello" arrived as "hellohello".
Without the listener no stock transport delivers a drain there. A
net.Socket subclass that emits 'connect' after the handshake still does.

The listener was also what sent the rest of one chunk above 2^31 - 1
bytes, because SSLWrapper::write_data takes at most that in one call.
Socket.prototype._write now fails such a write with EPROTO on this
engine, as Node does on every TLS socket. The HTTP/2 upgrade path sends
it in slices, because its writer joins frames into one chunk that the
user did not write.

The two comments that said a send cannot reach the pending-write queue
now say what can reach it over a stream.

The deletion is the one of d7b42e1, without the write-done callback
that commit put in the freed slot. When write(cb) completes does not
change.

Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
robobun added a commit that referenced this pull request Oct 21, 2026
@robobun @Jarred-Sumner
tls: remove the 'drain' listener on a wrapped stream
8ab1223 
A TLS socket over a stream listened for 'drain' on that stream. The
listener ran the socket's flush and its drain callback. Node's
JSStreamSocket has no such listener.

A stream that emits 'drain' inside its own write() ran that listener
inside the send that called write(). NewSocket::internal_flush lends its
queue to that send and settles it afterwards, so the nested flush sent
the queue again: 524288 bytes for 262144, and a bad record MAC for
HTTP/2 over such a stream. Bun 1.4.2 and 1.4.3 do this too.

Since #44618 a TLS socket over a net.Socket that is still dialing is
`connecting`, and node:net parks a write made then. The 'connect' of the
socket and a drain of its handle each send that parked chunk. The
listener delivered such a drain before the 'connect' when the dialing
socket had a backlog of its own, so "hello" arrived as "hellohello".
Without the listener no stock transport delivers a drain there. A
net.Socket subclass that emits 'connect' after the handshake still does.

The listener was also what sent the rest of one chunk above 2^31 - 1
bytes, because SSLWrapper::write_data takes at most that in one call.
Socket.prototype._write now fails such a write with EPROTO on this
engine, as Node does on every TLS socket. The HTTP/2 upgrade path sends
it in slices, because its writer joins frames into one chunk that the
user did not write.

The two comments that said a send cannot reach the pending-write queue
now state the condition: no JS that the send runs reaches the queue.

The deletion is the one of d7b42e1, without the write-done callback
that commit put in the freed slot. When write(cb) completes does not
change.

Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
@robobun robobun mentioned this pull request Oct 21, 2026
Open
@Jarred-Sumner Jarred-Sumner mentioned this pull request Oct 21, 2026
Merged
This was referenced Oct 21, 2026
Open
Open
Merged
Draft
robobun added a commit that referenced this pull request Oct 21, 2026
@robobun
tls: send a close_notify that the kernel refused from the writable event
d1dec4b 
A graceful close of a TLS socket closed the fd at once when the kernel
refused the close_notify. With input of the peer unread, that close is
a reset, and the kernel drops what it has not sent. On Windows a
write() followed by end() gets there: the kernel takes the write and
refuses the next send.

ssl_handle_shutdown no longer reports a spilled close_notify as a
finished shutdown. The alert stays in the spill like any other record,
the loop keeps the poll for writable of a shut-down TLS socket that
still holds a spill, and the writable event sends it. That event is
not dispatched to an owner whose close waits for the peer: the owner
let go, and fetch answers such an event with a reset. A forced close
does not wait for the alert.

This reverses "close now without it" from #30368, which #44618 kept.
The reason was that nothing sent the alert later.
Jarred-Sumner added a commit that referenced this pull request Oct 21, 2026
@Jarred-Sumner
Hardening follow-up: fix regressions from #46021 and more crashes (#4…
a950fc5 
…6270)

### What does this PR do?

Hardening follow-up to #46021. It fixes regressions from #46021 and
#44618 that were never released, includes more of the open pull requests
that fix a crash, and fixes a few crashes that had no pull request.

#### Regressions fixed (none was released)

- `spyOn(Bun, "$")`, and the same for `"sql"`, `"postgres"` and `"SQL"`,
threw "does not support accessor properties yet".
- Async `fs.read`, `fs.write` and `zlib.brotliCompress` on a view over a
`WebAssembly.Memory` could lose data or copy far more than they needed.
- `node:vm` with `vm.constants.DONT_CONTEXTIFY`: `this` in host
accessors, prototype handling, `util.types.isProxy`, `Bun.deepEquals`,
`assert.deepEqual` and `console.log` of a context.
- `zlib` `stream.flush()` threw for arguments it used to accept, and
streams had a new enumerable property.
- `Bun.Transpiler` `exports.replace` with the name `"default"`.
- `process.env.X = 3000` could store the number once the function was
JIT-compiled.
- `node:sqlite` and `bun:sqlite` could lose rows at exit.
- Dev server: stylesheets shared between pages after a build error.
- Shell `cp` of a file onto itself exited 1.
- `error.stack`: the frame of `require()` reads `at anonymous (unknown)`
again.
- `import * as process from "process"` followed by `process.on(...)`
threw at load. The EventEmitter methods that the module exports now act
on `process`.
- `delete res.statusCode; res.end()` in a `node:http` handler threw. It
sends 200, as in Node. (from #46128)
- `node:tls`, `node:https` and `node:http2`: connection teardown and
write callbacks behave as in 1.4.3 and Node again.

#### Crash fixes

Bundler, parser and CSS: #42286, #46092, #46097, #46177, #46239, #36334,
#43024, #40769.
`bun test`, `--hot` and `--watch`: #30776, #38206, #38974.
Shell: #43183, #37743, #46168, #46207, #34900.
`node:http`, `node:http2`, `Bun.serve`, streams: #43449, #45380, #44457,
#42197, #42485, #38082, #38124, #46074, #45999, #46045.
`node:vm`, native addons, workers: #46129, #46198, #45615, #33412,
#38031.
Dev server: #42790, #42850, #39488.
Other: #35991, #38030, #38586, #40526, #41830, #46029, #46078, #46114,
and the bug of #42215.

#### Behavior changes

- `crypto.randomFill(view, cb)` reports `ERR_CRYPTO_OPERATION_FAILED` if
it could not fill the view. It used to report success.
- A function registered twice on `process` runs twice, as in Node.
(#41830)
- Under `--hot`, a reload after an uncaught exception cancels the timers
that the failed run created. The new run's timers fire, and so do timers
and `fs.watchFile` watchers that earlier runs kept on `globalThis`. The
reload also resets the exit code to 0. (#38206)
- The parent of a Worker no longer gets an `'error'` event for a failed
`Bun.serve` request in that Worker. The error is printed, as on the main
thread.
- `hash.write()` after `digest()` drops the chunk, as in Node. (#35991)
- `http2` over TLS with `server.emit("connection", socket)`:
`session.socket` is a `TLSSocket` and a `'connection'` listener fires,
as in Node. (#38124)
- A `slice()` with no type of a body `Blob` has `type === ""`, as in
Node. (#46114)
- `util.types.isProxy(globalThis)` is false, as in Node.
- A write on a `node:net` or `node:tls` socket that is still pending
when the connection closes calls back with `ECANCELED`, as in Node.
- `HTMLRewriter.on()` throws for a selector that is nested too deeply.
- `Bun.Image`: `maxPixels` also bounds PNG metadata, `metadata()` of a
GIF reports the size that gets decoded, and a path with a NUL byte
throws. (#40526)
- `bun build --no-bundle` with the `base64` or `dataurl` loader prints
an error. (#36334)
- Bundled output for a default-exported function or class expression
changes to `var file_default = function name() {}`, as in esbuild.
`.name` is unchanged. (#46097)
- A Proxy in the prototype chain of a `node:vm` global is allowed. jsdom
30 with `runScripts` needs this.
- `bun:sqlite` commits a write that `iterate()` left open when the main
thread exits. An explicit `BEGIN` is still rolled back.
- When a Worker stops while a native addon's async work is still
running, the addon's finalizers run after that work is done, as in Node.
- Shell: `mv -n` and `cp -n` don't overwrite, and `cp` reads `R`, `v`
and `n` anywhere in a flag group.

#### Performance

- Ordinary `node:vm` contexts run at the same speed. With
`DONT_CONTEXTIFY`, stores to globals and reads of inherited names in a
hot loop cost 10 to 20 % more than on `main`.
- A `process.env` write costs 20 to 35 % less than after #46021.
- An async read into a `WebAssembly.Memory` view looks through the live
memories: about 1.5 µs more with one memory.

#### Not included

- Taken in part, so these should stay open for the rest: #36334, #38974,
#39488, #40526, #40769, #42485, #45615, #45999, #46029, #46092, #46114,
#46168, #46207, #46239.
- Needs a WebKit change: #34655 #39204 #40170 #40253 #41587 #41900
#42699 #42897 #42903 #43084 #43813 #44518 #44949 #45770 #45804 #45827.
- #44723 (covered by #46097), #41675, #42753, #38762, #41475, #46016.

### How did you verify your code works?

- Every item was reproduced first on release builds of `main`, of its
parent and of 1.4.3, and compared with Node v26 where Node has the API.
- Every fix has a test that fails on `main` and passes on a debug + ASAN
build of this branch, except where that isn't practical (huge inputs,
code that only runs on Windows or macOS).
- The regressions were checked on grids of calls against the build
before #46021 and against 1.4.3.
- Bun's own test suites and Node's `test-vm-*`, `test-http2-*` and
`test-zlib-*` files for the areas touched.
Jarred-Sumner pushed a commit that referenced this pull request Oct 21, 2026
@robobun
tls: keep a stream TLS engine's context alive while its entry points …
ebcd487 
…run (#46335)

### Problem
- A callback of a TLS socket over a stream turns the event loop
synchronously while the socket closes. Debug builds report
`AddressSanitizer: heap-use-after-free` in `us_ssl_wrapper_keylog` under
`SSL_do_handshake`, or in `SSLWrapper::flush_pending_events`. Release
builds end with `panic: RefCell already borrowed` or stay silent.
- `DuplexUpgradeContext::on_close` (`src/runtime/socket/socket_body.rs`)
queued the task that frees the context at once. The nested turn ran it
under the TLS engine.
- Found by audit. No issue or crash report names it.

### Fix
- `DuplexUpgradeContext` is refcounted. The open connection holds one
ref, and each `UpgradedDuplex` entry point holds one while it runs. The
free task is queued at zero.
- No free task exists while an entry point runs. A debug assertion
checks that handlers run under a ref.
- Verified: `test/js/node/tls/node-tls-duplex-close-throw-uaf.test.ts`.
Ten cases abort on the base build and pass with the fix. Also twelve
TLS, http2 and module-graph suites (Notes).
- Self-reviewed: 12 concerns raised, 11 addressed. One exit has no test
(Notes).

### Background
- `DuplexUpgradeContext` owns `UpgradedDuplex`, which owns the TLS
engine (`SSLWrapper`) behind `tls.connect({socket})`.
- `expect(promise).resolves` and `Bun.build` with an async plugin
`setup()` run queued tasks while they wait.
- Considered a check in the free task. It parks the free and polls.
`WindowsNamedPipe` (#39983) holds a ref at its entry points.

### Downsides
- Duplex TLS pays 9 instructions per guarded entry. An echo measures
+0.11% instructions, and a control with no guard +0.06%.
- A transport error before the engine starts takes 2 queue entries, not
1.
- The binary grows by 8,192 bytes and one test-only host function.

<details><summary>Notes</summary>

#### What turns the loop, and what closes the socket

- The wait is `EventLoop::wait_for_promise` (`src/jsc/event_loop.rs`).
It calls `tick()`, which runs the task queue. The fixture uses two
callers: a `bun:test` promise matcher in a plain script, and `Bun.build`
with an async plugin `setup()` (`src/runtime/api/JSBundler.rs`).
- The callback does not have to close the socket. Three of the ten cases
close it from elsewhere: a `destroy()` in a later turn (the callback is
the transport's `final()`), a transport that closes during the turn, and
a `Bun.ModuleGraph` that is disposed.
- #44618 covered an ALPNCallback that destroys its socket with no loop
turn (it kept the `SSL` until the pass ends). The owner was still freed
by the queued task.

#### The ten cases on the base build (debug, ASAN)

Each one ends in `heap-use-after-free`. The free is
`DuplexUpgradeContext::deinit` from `run_event`, or from `release_unrun`
for the disposed graph.

| Case | Top frames of the read |
|---|---|
| ALPNCallback | `us_ssl_wrapper_keylog` < `us_ssl_keylog_cb` <
`SSL_do_handshake` |
| server 'keylog' | `SSLWrapper::flush_pending_events` < `traffic_pass`
|
| client 'secureConnect' | `UpgradedDuplex::is_shutdown` <
`UpgradedDuplex::on_handshake` |
| server 'data' | `SSLWrapper::dispatch_read` < `handle_reading` |
| server 'data', turned by `Bun.build()` | `SSLWrapper::dispatch_read` <
`handle_reading` |
| client 'data' | `SSLWrapper::dispatch_read` < `handle_reading` |
| the transport's `write()` | `SSLWrapper::handle_writing` <
`write_data` |
| the transport's `final()`, `destroy()` in a later turn |
`UpgradedDuplex::finish_close` < `on_close` |
| server 'data', the transport closes during the turn |
`SSLWrapper::dispatch_read` < `handle_reading` |
| server 'data', its `Bun.ModuleGraph` is disposed |
`SSLWrapper::dispatch_read` < `handle_reading` |

Release build of the base, original repro (ALPNCallback destroys the
socket, then `expect().resolves`): `panic: RefCell already borrowed`, 5
of 5 runs. The 'keylog' variant printed OK 5 of 5 (silent). Sentry was
not queried.

#### How the tests decide

- The fixture counts live contexts through
`duplexTLSInternals.liveCount()`, so the tests do not need ASAN to fail.
- Each callback case has a third context that nothing holds. The turn
frees it. That shows that the turn ran the queued tasks: the report is
`{ before: 3, afterTurn: 2, afterClose: 0 }`. With no turn it is 3, and
with the fault it is 1.
- The turn is a promise that a microtask settles, so one `tick()` runs.
One case turns with `Bun.build()` instead, so the test does not depend
on the matcher alone.
- The fourth test ends a connection in seven ways and expects zero
contexts left. It does not abort on the base build. There it fails
because the counter does not exist.
- Each guard clause was removed once on the debug build (by a jump in
gdb) to confirm that a test notices:
- no `teardown()` in `release_unrun`: a string from the transport
reaches the disposed graph as `ERR_STREAM_WRAP`.
- no `abort_handle.leave()`: `DuplexUpgradeContext entered after its
last ref was released`.
- the count check in `release_unrun`: the graph case reaches it with a
count of 1, so an unconditional free trips the assertion in `deinit`.

#### Direction

- #33261 asks that nothing re-enters the event loop from a dispatch
callback. #32233 was closed for that reason: it made the dispatcher
tolerate re-entry. #33289 moves the `bun:test` matchers off the nested
wait.
- This change does not touch the dispatcher or the loop. It fixes a
memory fault at one owner, in the shape of `REVIEW.md` ("bracket entry
points that can reach synchronous teardown with ref()/defer deref()")
and of #39983.
- The trigger stays reachable after #33289: `Bun.build` still waits for
an async `setup()` (#33271 was closed without a merge).

#### Sequencing

- This lands first. Two open PRs add entry points to `UpgradedDuplex`
and need the guard after it:
- #45471: `UpgradedDuplex::resolve_server_name` (an SNICallback that
answers in a later turn) needs the `keep_alive()` guard as its first
statement, and `DuplexUpgradeContext::on_server_name` needs the
`debug_assert_kept_alive()` call. Without them the debug build aborts on
the assertion.
  - #43877: the `on_write_done` thunk needs the guard.
- #40139 rewrites the same lines and keeps the unconditional free in the
`Close` arm. A rebase must keep the rule: the `Close` hop is queued only
at zero.

#### Not in this change

- `WindowsNamedPipe`: #39983 guards `on_read`, `on_read_error`,
`on_error`, `on_handshake` and `get_accepted_by`. Seven entries that
dispatch have no guard: `on_writable`, `flush`, `on_timeout`,
`encode_and_write`, `raw_write`, `close`, `shutdown`. No fault is known
there: its transport is libuv and runs no script in write, end, pause or
resume. Read from the code, not run on Windows. Do not copy `keep_alive`
to those entries as is: there, a ref taken at zero queues the free
twice.
- `ProxyTunnel` (`src/http/ProxyTunnel.rs`) runs on the HTTP thread,
where no script runs.
- The start-failure exit of the `StartTLS` arm now releases the
connection's ref. It has no test. `node:tls` and `node:http2` pass a
prebuilt `SSL_CTX`, so the start fails only if `SSL_new` fails.

#### Measurements (linux x64, base 71bc818)

- Instructions: `perf` is not installed here. The counts are
`perf_event_open` (`PERF_COUNT_HW_INSTRUCTIONS`, user only) around the
release binaries. One TLS echo over an in-process Duplex pair, 100,000
round trips of 1 KiB, `BUN_JSC_useJIT=0`, 10 interleaved runs each.
- base 32,160,640,513, this PR 32,197,252,048: +366 per round trip of
321,600 (+0.11%). Run-to-run spread: 0.09% to 0.13%.
- The same echo over loopback TCP (no guard on the path): base
20,199,900,220, this PR 20,211,412,604 (+0.06%). Spread: 0.07% to 0.11%.
- Disassembly of `UpgradedDuplex__flush`: 4 instructions at entry and 5
at exit. A round trip takes 4 guards (gdb count on the debug build).
- `sizeof(DuplexUpgradeContext)`: 568 B -> 568 B (gdb on the debug
builds). The two fields fit in padding.
- Queue entries per duplex TLS socket (gdb hit counts on `run_event`, 50
cycles): normal close 2 -> 2, `destroy()` before the engine starts 2 ->
2, transport error before the engine starts 1 -> 2.
- Live contexts: 2 of 3 inside the callback after the turn, 0 after
unwinding. 0 after 1,000 cycles of each of three orders on the release
build (4,000 sockets).
- Release binary (`llvm-size`): `.text` 81,904,741 -> 81,909,861
(+5,120), `.rodata` +4,096, file 107,497,184 -> 107,505,376 (+8,192).
Host functions: +1, testing only.

#### Suites (debug build, `--timeout 90000`)

Same results as the base build: `node-tls-duplex-end-verify` (115),
`node-tls-duplex-write-throw-error-value` (1),
`node-tls-wrapped-socket-close` (113), `node-tls-server` (218),
`node-tls-upgrade` (21), `node-http2-upgrade` (23), `renegotiation`
(58), `node-tls-context` (83), `tls-fatal-error-closes` (38),
`tls-reject-before-client-cert` (126), `module-graph-io` (28),
`module-graph-isolation` (413). `node-tls-connect` has the same 4
failures on both builds: those tests contact bun.sh, which this machine
cannot reach. `cargo clippy -p bun_runtime` and
`test/internal/source-lints/` are clean.

</details>
@Jarred-Sumner Jarred-Sumner mentioned this pull request Oct 21, 2026
Merged
@dylan-conway dylan-conway mentioned this pull request Oct 21, 2026
Merged
dylan-conway added a commit that referenced this pull request Oct 22, 2026
@dylan-conway @claude
fetch, RedisClient: a TLS session that breaks after its handshake is …
4a9c341 
…a lost connection, not a failed handshake (#46409)

### What does this PR do?

Restores two recoveries that 1.4.3 has and `main` lost: `fetch` sending
a request again after a reused https connection breaks, and
`Bun.RedisClient` reconnecting after a `rediss://` connection breaks.

#### What was wrong

When a TLS session that is already up gets bytes it cannot accept (bytes
that are not a TLS record, a record that does not authenticate) and the
peer closes:

| | 1.4.3 | `main` |
|---|---|---|
| `fetch`, GET on a reused https connection | sent again on a new
connection, resolves | rejects with `EPROTO: The TLS handshake failed` |
| `fetch`, GET through a reused CONNECT tunnel, the session with the
**https proxy** breaks | sent again, resolves | rejects with `EPROTO` |
| `RedisClient` over `rediss://`, a command in flight or the connection
idle | reconnects | never reconnects, every later command rejects with
`Connection has failed` |

A FIN, an RST and a close_notify at the same moment are recovered from
on both builds.

#### Cause

Since #44618, `ssl_fail_established_session`
(`packages/bun-usockets/src/crypto/openssl.c`) reports a fatal error on
an established session to the socket's owner through
`on_handshake(success = 0)`, and then closes the socket itself. An owner
that takes every such report for a refused handshake fails for good,
before the close event that carries its recovery policy arrives:

- `HTTPContext::Handler::on_handshake` called `close_and_fail`, so
`HTTPClient::on_close`, which holds the resend rule, found a client that
had already failed.
- `JSValkeyClient`'s `on_handshake` called `fail_with_js_value`, which
sets `flags.failed`, and `ValkeyClient::on_close` skips the retry policy
for a failed client.

The WebSocket client got its guard for this in #45720.

#### Fix

**fetch** (`src/http/HTTPContext.rs`, the fixing lines): a failed report
for a request that `on_close` would send again returns without failing
the client. uSockets closes the socket right after the report, and
`on_close` does what it does for a FIN. `allow_retry` is only ever set
on a reused connection, which is past its handshake, so a real handshake
failure never takes this path. To ask the same question in both places,
the condition moves out of `on_close` into
`HTTPClient::retries_on_close()` unchanged (`src/http/lib.rs`).

Everything that is not sent again keeps `main`'s behavior and its
`EPROTO`: a POST, a stream body, a response whose body has started,
`keepalive: false`.

**RedisClient** (`src/runtime/valkey_jsc/`): a failed report on a
connection that is `Connected` (its `HELLO` was answered), with
`autoReconnect` on, is a lost connection. The commands in flight are
rejected with the TLS reason, as on `main`, `failed` stays unset, and
the close that follows runs the usual retry policy. The rejections are
made without entering an event loop scope, so they are observed after
`on_close` has run: a command issued from a `catch` waits for the new
connection, as it does after a FIN, instead of going out on the dead
socket.

Two cases keep `main`'s behavior, the client fails and every pending
command is rejected with the TLS reason:

- **Before the reply to `HELLO`.** That is where a TLS 1.3 server
refuses the client's certificate, after the client's side of the
handshake is over, so a failure there is a refusal and not a lost
connection. A client with no certificate gets `EPROTO:
...TLSV1_ALERT_CERTIFICATE_REQUIRED` from its first dial. (1.4.3 keeps
dialing here: 9 connections in 8 s, with the command still pending.)
- **`autoReconnect: false`.** There is no policy to hand the close to,
and failing the client tells the commands that are still queued why, not
only the ones in flight.

#### Checked and left as they are

- **The session inside a CONNECT tunnel** (origin side, `ProxyTunnel`).
A request on a reused tunnel whose inner session ends is not sent again
on `main`, but it is not on 1.4.3 either, and not for a FIN or a
close_notify either. It is a different cause (`ProxyTunnel::on_close`
never consults the resend rule) and older than #44618, so it is not part
of this PR.
- **HTTP/2 sessions**: `fail_streams` sends nothing again for any kind
of close, so only the error code depends on the report.
- **`Bun.SQL`**: the pool does not retry the close of an established
connection whatever the error is, and dials again for the next query.
- A finished TLS 1.2 renegotiation makes `RedisClient` send `HELLO`
again. That is #45413.

### How did you verify your code works?

Reproduced first on a debug build of `main` and on the 1.4.3 release
binary with the same scripts (the table above), then on this branch.

New tests, in the existing files for each area:

`test/js/web/fetch/fetch-keepalive.test.ts`
- a GET that a reused https connection answers with: bytes that are not
a TLS record, a record that does not authenticate, half a response head
and then such bytes, close_notify, FIN, RST. Each is sent again, and the
origin sees exactly `connection 1 request 1`, `connection 1 request 2`,
`connection 2 request 1`.
- a POST, and a GET whose response body has started, are **not** sent
again and reject with `EPROTO`.
- a GET through a reused tunnel when the session with the https proxy
breaks is sent again over a second tunnel.

`test/js/valkey/reliability/connection-failures.test.ts`
- the client reconnects when the session breaks in place of a reply
(with the next command issued straight from the rejection) and while
idle. `onclose` never fires, and the command in flight still rejects
with `EPROTO: ...WRONG_VERSION_NUMBER`.
- with `autoReconnect: false`, a `GET` in flight and an `INFO` queued
behind it both reject with the TLS reason, and `onclose` fires once.
- a TLS 1.3 server that requires a client certificate fails a client
that has none at its first dial, with
`TLSV1_ALERT_CERTIFICATE_REQUIRED`.

| | without the `src/` change | with it |
|---|---|---|
| `fetch-keepalive.test.ts` | 4 fail (the three TLS breaks, the https
proxy) | 58 pass |
| `connection-failures.test.ts`, the 4 new tests | 2 fail (the two
reconnects) | 4 pass |

The existing tests that pin the errors of a broken session pass with the
change: `test/js/bun/net/tls-fatal-error-closes.test.ts` (every owner
still closes, same errors) 38 pass,
`test/js/node/tls/renegotiation.test.ts` (a refused renegotiation,
`fetch` and `RedisClient`) 57 pass.

The close_notify / FIN / RST rows, the two "not sent again" rows and the
last two Redis tests pass on both builds on purpose: they pin what must
not change.

The bug is newer than the latest release, so `USE_SYSTEM_BUN=1` passes
these tests. "Fails without the fix" was shown by building `main`'s
`src/` with the new tests.

Each condition on the Redis side was removed in turn to see a test fail:
without `status == Connected` the server that refuses the certificate is
dialed again, without the `autoReconnect` condition the queued `INFO`
gets `Connection closed`, and with an event loop scope around the
rejections the command issued from the rejection goes out on the dead
socket. The new Redis tests also pass under
`BUN_JSC_validateExceptionChecks=1`.

The files next to these that go through the same two `on_handshake`
handlers, for real handshake failures (wrong name, untrusted chain,
`checkServerIdentity`), also pass:
`test/js/valkey/valkey-tls-verify.test.ts` 14 pass,
`test/js/web/fetch/fetch.tls.test.ts` 64 pass. My machine has no IPv6,
so for that run their servers listened on `127.0.0.1`. The one failure
there, `trusts a self-issued certificate without keyCertSign only when
ca holds that very certificate` (`CERT_SIGNATURE_FAILURE` where
`UNABLE_TO_VERIFY_LEAF_SIGNATURE` is expected), is the same on `main`
without this change.

`connection-failures.test.ts` has one failure on my machine that is not
from this PR: `close() over rediss:// while the peer has stopped reading
closes the socket at once` fails the same way on `main` without this
change. The tests there that need a Redis server were skipped (no
Docker).


---
_Generated by [Claude Code](https://claude.ai/code)_

---------

Co-authored-by: Claude <noreply@anthropic.com>
Jarred-Sumner added a commit that referenced this pull request Oct 22, 2026
@Jarred-Sumner @claude
Fix regressions since 1.4.3 in bun install, bunx, node:fs and node:ht…
2a5cbbc 
…tp2, and update brotli (#46392)

### What does this PR do?

Fixes regressions since 1.4.3 that are still on `main` (none of them was
released), and updates brotli.

Fixes #46352

- **`bun install --frozen-lockfile` and `bun ci` refused lockfiles that
1.4.3 wrote**, so CI went red on the upgrade with nothing else changed:
- a project with `overrides` whose lockfile has a version that an
override doesn't allow. A frozen install installs what the lockfile says
again. A plain `bun install` still applies the override. (from #45307)
- a monorepo whose root names one of its own workspaces a second time:
through an `npm:` alias, as an optional peer, by a dist-tag with
`bun.lockb`, in two dependency groups, or by `file:` under another name.
In one of these shapes a plain `bun install` also exited 1, or swapped
the package.
- **`bun update --interactive` could panic** (`index out of bounds`)
after it had rewritten `package.json` and before it saved the lockfile,
most often in a workspace with a `catalog`. (from #45307)
- **`npx --quiet`, `-q`, `--no` and npx's other flags in a
`package.json` script exited 1** under `bun run`. `bunx` skips the flags
of `npx` and `npm` again. (from #44968)
- **`fs.realpathSync.native(".")`, `fs.realpath.native(".")` and
`fs.promises.realpath(".")` in a working directory that was removed**
returned `"<dir> (deleted)"`. They throw `ENOENT` again, as in Node.
(from #45423)
- **`node:http2`**: connection teardown follows Node and 1.4.3 in one
more case. (from #44618)

- **HTTP/3** (`Bun.serve({ http3: true })` and the `fetch` HTTP/3
client): header size limits are now enforced, counted as in RFC 9114
§4.2.2. The server enforces the limit it advertises
(`--max-http-header-size`, 16 KB by default) and answers `431` to a
request over it. The client advertises 64 KB as before, accepts up to
256 KB like the HTTP/2 client, and rejects a larger response with
`ResponseHeadersTooLarge` without disturbing other requests on the
connection. A request may have 200 fields, as in HTTP/1.1 and HTTP/2. A
header name or value over 65,535 bytes was sent cut short, with no
error. Now `fetch` rejects with `HTTP3HeaderEncodingError`, and a
`Bun.serve` response whose headers can't be sent resets its stream,
where a response with more than 64 KB of encoded headers left the
request hanging. `node:quic` reports such a failure too. `fetch` no
longer sends a POST or PATCH a second time once it was written and its
stream closed before the response headers: it rejects with
`HTTP3StreamReset`. Idempotent methods, requests that weren't written
yet and requests the server rejected with `H3_REQUEST_REJECTED` are
still retried once. (from #42579)
- **brotli is updated from v1.2.0 to v1.2.1**, which has the fix for
CVE-2026-111497 in the decoder. The decoder's new error code is in
`zlib.constants` as `BROTLI_DECODER_ERROR_FORMAT_BLOCK_SWITCH`, and
`node:zlib` reports it as `ERR__ERROR_FORMAT_BLOCK_SWITCH`, as in Node.
The error code changes and Node's regression test are from #46351 by
@fredluz, which backports the same upstream commit as a patch. Thank
you!

#### Behavior changes in `bunx`

- `bunx --registry <url>`, `--ca` and `--cafile` are new: they apply to
the install that `bunx` runs. `--reg` and pnpm's `--config.registry=`
mean `--registry`.
- `--ignore-scripts` applies to that install too.
- `--no`, `--no-yes`, `--yes=false`, `--yes false`, `-n` and `--offline`
mean `--no-install`: a tool that is installed runs, and one that isn't
is an error. 1.4.3 ignored them.
- The other flags that choose a registry or its credentials
(`--userconfig`, `--globalconfig`, `--npmrc-auth-file`,
`--replace-registry-host`, ...) are errors. 1.4.3 ignored them.
- `bun install`, `add`, `remove` and `update` parse their flags exactly
as before.

#### Not included

- With the hoisted linker, the first install in a checkout without
`node_modules` can leave out a workspace's `file:` dependency if that
workspace is also reached under a second name. #45956 has that one.
- A tag in one dependency group next to a version of the same
workspace's name in another: some combinations behave as on `main`.

### How did you verify your code works?

- Each item was reproduced first on release builds of `main` and of
1.4.3, and compared with Node v26 where Node has the API.
- Each fix has a test that fails on `main` and passes on 1.4.3 and on a
debug + ASAN build of this branch.
- `overrides`: 6,398 generated lockfiles (both linkers, `bun.lock` and
`bun.lockb`). `main` refuses 992 that 1.4.3 accepts, this branch none.
Everything that `main` accepted has the same exit code, lockfile bytes,
tree and output.
- Workspaces: 3,011 generated projects with lockfiles from 1.4.0, 1.4.2,
1.4.3 and `main`, 12 steps each (frozen, plain, `bun ci`,
`--production`, `--omit`, a workspace added later). Outside the shapes
above, the only difference from `main` is a frozen install right after a
workspace was added next to an alias or a tag of its name. It exits 0,
as on 1.4.3.
- `bunx`: 222 flag spellings called four ways on 1.4.3, `main` and this
branch, and two local registries that log their requests. 1,584
`install` / `add` / `remove` / `update` calls give the same result as on
`main`.
- brotli: Node's regression test fails on `main` and passes here. The
`node:zlib`, `CompressionStream` and compressed `fetch` tests and Node's
own brotli tests pass.

---------

Co-authored-by: Claude <noreply@anthropic.com>
@dylan-conway dylan-conway mentioned this pull request Oct 22, 2026
Merged
dylan-conway added a commit that referenced this pull request Oct 22, 2026
@dylan-conway @autofix-ci
usockets: fail the connection when send() reports too many bytes (#46452
7382b38 
)

### What does this PR do?

On macOS, `send()` can return a number larger than the length it was
given. Bun trusted that number: a TLS write crashes in `ssl_emit` and a
plain socket write panics. Such a send now fails with `ECONNABORTED` and
the connection is closed.

**Cause.** XNU leaves the result register untouched when a system call
ends in `EJUSTRETURN`
([x86_64](https://github.com/apple-oss-distributions/xnu/blob/main/bsd/dev/i386/systemcalls.c),
[arm64](https://github.com/apple-oss-distributions/xnu/blob/main/bsd/dev/arm/systemcalls.c)),
which
[`ip6_output()`](https://github.com/apple-oss-distributions/xnu/blob/main/bsd/netinet6/ip6_output.c)
returns for a packet pf consumed. x86_64 reads back the system call
number, arm64 the descriptor. golang/go#61060 reports the same.

**Fix.** How much the kernel took is unknown, so the stream cannot
continue. `us_internal_reject_excess_send()` in `socket.c` checks the
five calls to `bsd_send`, `bsd_writev` and `bsd_write2`. On such a count
it:

- fails the send;
- shuts the write side down, because four of the writers take any failed
send for backpressure and would send again;
- asks for a writable event, because no write may close under its
caller. The TLS layer already closes on that event. For a socket without
TLS, `loop.c` now closes with `ECONNRESET` after the event's callbacks.

The `ssl_emit` crash came with #44618.

Not covered:
- On arm64 the descriptor is usually below the length, so the count
still passes as a short write. So does a write of 32 MiB or more on
x86_64.
- `bsd_sendmsg` (unix sockets), UDP and QUIC sends are unchanged.

### How did you verify your code works?

A new fault injection action, `excess`, makes `send` or `writev` report
more than it was given. Debug build on Linux x64, one part of the fix
undone per column:

| Test | No check | No shutdown | No close | This PR |
| --- | --- | --- | --- | --- |
| `node:tls`, `fetch` over https, `wss:` | ASAN error in `ssl_emit` |
pass | pass | pass |
| `node:net` write | panic in the write path | pass | pass | pass |
| `Bun.connect`, peer never answers the FIN | `write` returns 6 | the
next write is sent | times out | pass |
| `Bun.serve` response (`send`, `writev`) | times out | pass | pass |
pass |
| WebSocket server frame, reads paused | times out | pass | times out |
pass |

The seven touched test files pass. Not run on macOS.

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
@VenkatKwest VenkatKwest mentioned this pull request Oct 22, 2026
Open
@bender-ristone bender-ristone mentioned this pull request Oct 22, 2026
Open
robobun added a commit that referenced this pull request Oct 22, 2026
@robobun @Jarred-Sumner
tls: remove the 'drain' listener on a wrapped stream
6a5875e 
A TLS socket over a stream listened for 'drain' on that stream. The
listener ran the socket's flush and its drain callback. Node's
JSStreamSocket has no such listener.

A stream that emits 'drain' inside its own write() ran that listener
inside the send that called write(). NewSocket::internal_flush lends its
queue to that send and settles it afterwards, so the nested flush sent
the queue again: 524288 bytes for 262144, and a bad record MAC for
HTTP/2 over such a stream. Bun 1.4.3 does this too.

Since #44618 a TLS socket over a net.Socket that is still dialing is
`connecting`, and node:net parks a write made then. The 'connect' of the
socket and a drain of its handle each send that parked chunk. The
listener delivered such a drain before the 'connect' when the dialing
socket had a backlog of its own, so "hello" arrived as "hellohello".
Without the listener no stock transport delivers a drain there. A
net.Socket subclass that emits 'connect' after the handshake still does.

The listener was also what sent the rest of one chunk above 2^31 - 1
bytes, because SSLWrapper::write_data takes at most that in one call.
Socket.prototype._write now fails such a write with EPROTO on this
engine, as Node does on every TLS socket. An HTTP/2 session writes
through the handle: there the rest of such a batch now waits for the
next write of the session.

The two comments that said a send cannot reach the pending-write queue
now state the condition: no JS that the send runs reaches the queue.

The deletion is the one of d7b42e1, without the write-done callback
that commit put in the freed slot. When write(cb) completes does not
change.

Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
@Meldiron Meldiron mentioned this pull request Oct 22, 2026
Open
@Jarred-Sumner Jarred-Sumner mentioned this pull request Oct 22, 2026
Open
This was referenced Oct 23, 2026
Open
Closed
@steipete steipete mentioned this pull request Oct 24, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment