Repository navigation
Conversation
On a tls.Server that listens, the user's SNICallback ran with the tls.Server as `this`. Node calls it as a method of the server-side TLSSocket of the connection (loadSNI, lib/internal/tls/wrap.js). ServerHandlers.serverName still reads the callback from the `data` that the native dispatch passes. It now takes the receiver from the connection's handle, whose `data` is the TLSSocket. The same handler serves http2.createSecureServer and every form of listen(). The test file runs on node:test, so the same file passes on Node.js.
Collaborator
Author
|
Updated 6:45 PM PT - Oct 19th, 2026
❌ @robobun, your commit 340efb0 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 46099That installs a local version of the PR into your bun-46099 --bun |
Collaborator
Author
|
Status: draft until #46258 merges. Reproduced with the script in the description. Node v26.3.0 prints #46258 is the fix for |
This was referenced Oct 21, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
tls.Serverorhttp2.createSecureServer(), Bun runs theSNICallbackwiththisset to the server. Node v26.3.0 passes the connection'sTLSSocket.this.destroy()) throws aTypeError: the client getsECONNRESET, the server prints nothing. No affected user is known.ServerHandlers.serverName(src/js/node/net.ts:1222) callscb.$call(server, ...), andserveris thetls.Server.Fix
serverNametakes the receiver from the connection's handle:socketHandle.datais theTLSSocket.loadSNIcallsowner._SNICallback(...)on that socket, which'secureConnection'passes later.test/js/node/tls/node-tls-sni-callback-receiver.test.ts(node:test). 7 of 10 tests fail on main. All pass with the fix and on Node.setKeyCert()defects below.Background
serverNameis an internalBun.listensocket handler, called when a ClientHello has a server name.datais the JS value on a native socket. An accepted socket starts with its listener'sdata, thenonconnectionstores theTLSSocketbefore any read.Bun.listenusers can reach.Downsides
this === servernow gets theTLSSocket. The server isthis.server.this.setKeyCert(ctx)in the callback now runs on a listening server, where it loses ALPN and ignores a later refusal. node:tls: setKeyCert() keeps the connection on its own SSL_CTX #46258 fixes that and must merge first.serverNameexecutes 38 bytecodes per ClientHello (36 before), with no new allocation. The binary grows 12 bytes.Notes
Who is affected. No affected user is known. The difference was found by a read of the code. #44140 reported it, and a maintainer closed that issue with this comment: "Use handoff tool. GitHub issues are for user-facing issues and a user did not run into this issue". This change comes through that handoff. Related to #44140.
Two published packages have an
SNICallbackthat needs its socket. Their callbacks were read in the npm tarballs. The packages were not run.acme-bot@1.4.2,lib/acme_challenge_server.js:131:const ctx = this.server.ctx_map.get(servername). With this changethis.serveris the server, as on Node.@vampaz/vite-plugin-local-tls@0.2.0,dist/index.js:2392:this.setKeyCert(selectContext(hostname)), and its'secureConnection'listener readssocket.alpnProtocol === "h2". With this change the handshake completes with the selected certificate, but ALPN is not negotiated. See "Not changed here".What an operator sees on main.
TypeErrordrops the handshake. The client reportsECONNRESET("Client network socket disconnected before secure TLS connection was established"). The server goes on and prints nothing.asynccallback uses the socket:TypeError: this.destroy is not a function. (In 'this.destroy()', 'this.destroy' is undefined)is an unhandled rejection, and the process exits.this.remoteAddress: the value isundefinedand nothing throws.Repro. Run with
nodeand withbun:Where the callback runs, and what
thisis.tls.createServer().listen(port)tls.createServer().listen(path)http2.createSecureServer().listen(port)server.emit("connection", netSocket)new TLSSocket(netSocket, { isServer: true, SNICallback })listen({ fd })and a cluster worker use the same handler. They are not in the tests.ALPNCallbackalready gets theTLSSocketon main, throughsocket.datainServerHandlers.alpnCallback.Tests. The new file runs on
node:test, sobun bd testandnode --testrun the same text. Its last test runs the file on Node whennodeis on the PATH.thismust be the socket that'secureConnection'(or'secure') passes for that server name, withthis.serverthe server.server.close()still gets its socket.this.destroy()drops the connection, and'tlsClientError'passes the same socket.'tlsClientError'with the same socket.this.servername is the requested nameis atodoon Bun and a plain test on Node.src/of main 91e7e3b: 3 pass, 7 fail, 1 todo. The 3 are the two rows that main has right and the Node run.node --test test/js/node/tls/node-tls-sni-callback-receiver.test.ts, Node v26.3.0: 10 pass.test/js/node/tls/node-tls-context.test.ts: 83 pass.test/js/bun/net/socket.test.ts -t "selection callback": 2 pass (the rawserverNamehandler still gets thedataof the listener).test-tls-sni-option.js,test-tls-sni-servername.js,test-tls-snicallback-error.js,test-tls-starttls-server.js,test-tls-empty-sni-context.js,test-tls-psk-alpn-callback-exception-handling.js,test-tls-secure-context-usage-order.js,test-tls-sni-server-client.js,test-tls-alpn-server-client.js.tscforsrc/jsand for the test file, prettier and oxlint are clean.listen(path)row dials a named pipe and is atodo: the TLS listener on a named pipe does not ask for the server name (src/runtime/socket/Listener.rs, theTODOin the named pipe arm). The Node run of the file exits 0 on Windows. Whether that row passes on Node there is not known, because atodocannot fail the run.Measurements (main 5900354 against this change, taken before the rebase to 91e7e3b, which does not touch
net.ts).serverNamebuiltin: 57 -> 60 instructions (+11 bytes).onconnection,open,initAcceptedTLSSocket,handshake,serverHandlersFor,onSNIResolutionandconsumeSNIResultare identical (BUN_JSC_dumpGeneratedBytecodes, debug builds).serverNamefor each SNI ClientHello: 36 -> 38 (+2). For each accept,onconnection85,open23,initAcceptedTLSSocket56 andhandshake87 on both (JSC profiler at the Baseline tier, difference of runs with 100 and 300 handshakes, clients in a Node process).serverNamehas onenew_objectand onebindcall on both builds, each executed once for each ClientHello. After 2,000 handshakes and a GC the heap holds 227 and 262 objects fewer than before on main, 206 and 234 fewer with this change (heapStats(), two runs each).node/net.js102,552 -> 102,564 bytes, and the embedded module blobInternalModuleRegistryConstants.bin2,704,147 -> 2,704,159 bytes (release codegen).SNICallback: the handler table thatnet.Server,tls.Server, atls.Serverwith anALPNCallbackandhttp2.createSecureServer()give toBun.listenhas noserverName. 0 dispatches in 20 accepts for each.this.destroy(),this.setKeyCert(),cb(err)in the call and later, a ClientHello afterclose(), a connection dropped bymaxConnections): 10 rows equal Node v26.3.0, 3 on main. The 4listen()rows differ intypeof this._SNICallbackandthis.servernameonly.Not changed here. These are about what the socket is and does inside the callback. The items that compare with Node were run on Node v26.3.0 and on this branch. They are the same on the rows that main has right.
this.setKeyCert(ctx)in the callback. The nativeset_key_cert(src/runtime/socket/tls_socket_functions.rs) switches the connection to the user'sSSL_CTX, which has neither the ALPN callback nor the callback that waits for the SNICallback's answer. After it, ALPN is not negotiated (Node negotiates it). An answer that comes later is not awaited: withcb(new Error("denied"))50 ms aftersetKeyCert()the client reads the server's data, where Node refuses. If the context also went throughaddContext(), the server has anALPNCallbackand the answer comes from a microtask, a debug build stops at the assertionvendor/boringssl/ssl/s3_both.cc:537intls_next_message. On main these need the socket ofemit("connection")or of the'connection'event. SettingSNICallbacksilently disables ALPN negotiation on a TLS server #41061 was the user report for ALPN after an SNI answer, and tls: consolidate the open TLS fixes (both engines, node:tls, node:https, WebSocket, SQL) #44618 fixed the three answer paths. Open node:tls: run ALPNCallback before BoringSSL picks the certificate and the session #43216 rewrites this function for another purpose. The fix is a pull request of its own, which must merge before this one.this.servernameisundefineduntil the handshake ends. Node assigns it for every server socket when the ClientHello arrives, also without anSNICallback, sothis.servernamein anALPNCallbackdiffers in the same way. A store in this handler would cover only servers with anSNICallback. It would also hold the Latin-1 form that the handler gets, where the handshake handler later assigns the UTF-8 form ofgetServername(). A check that a callback makes onthis.servernamedoes not see the name._SNICallbackof its own. The callback stays on the server.this.end()in the callback lets the handshake complete. Node resets the connection.this.getProtocol()in the callback isTLSv1.2for a TLS 1.3 handshake.ALPNCallbackbefore theSNICallback. Bun runs theSNICallbackfirst on every version. So what theSNICallbackstores onthisis there in theALPNCallbackon Bun, and not on Node with TLS 1.3.answer(err); this.destroy()reportsECONNRESETwhere Node reportserr.answer()is ignored. Node destroys the socket withERR_MULTIPLE_CALLBACK.server._handle.onconnectionhas attached noTLSSocketwhen the ClientHello arrives, thedataof the handle is still the server, and the callback gets the server.https.createServer()(Node's https server doesn't have addContext function #12157, and open node:https: thread SNICallback through to the TLS listener #36707 adds it with the https server asthis), and forhttp2SecureServer.emit("connection", netSocket). From the code, it does not run on a Windows named pipe.Other callbacks whose
thisdiffers from Node. They have other causes and are not changed here:onread.callback(#42420), and the callback ofres.end(callback), which Node runs with the response asthis.Other designs.
_SNICallback, as Node does. Every accepted TLS connection then pays a store that only servers with anSNICallbackneed.servernameon the socket before the call. Left out for the reasons in thethis.servernameitem above.