node:https: invoke SNICallback on the server

[robobun]

What

A node:https server never invokes SNICallback. Every client, whatever servername it sends, is served the server's default certificate, so multi-domain ("one server, N domains") HTTPS servers present the wrong certificate for every non-default name and real browsers hard-fail with a name mismatch.

Repro

import https from "node:https";
import tls from "node:tls";

const altCtx = tls.createSecureContext({ key: altKey, cert: altCert }); // CN=alt.example
const sniCalls = [];
const srv = https.createServer({
  key, cert,                                                            // default: CN=127.0.0.1
  SNICallback: (name, cb) => { sniCalls.push(name); cb(null, name === "alt.example" ? altCtx : undefined); },
}, (req, res) => res.end("ok"));

srv.listen(0, "127.0.0.1", async () => {
  const port = srv.address().port;
  // client sending servername "alt.example" should receive CN=alt.example
});

node v26.3.0:   client(servername=alt.example) got cert CN=alt.example
                SNICallback invocations: ["alt.example","localhost"]
bun (before):   client(servername=alt.example) got cert CN=127.0.0.1   <- WRONG cert
                SNICallback invocations: []                            <- never called

Cause

https.Server is http.Server, which builds Bun.serve({ tls }). src/js/node/_http_server.ts forwarded only a fixed subset of TLS options (serverName, key, cert, ca, passphrase, secureOptions, requestCert, rejectUnauthorized) into the config, so SNICallback was dropped on the floor. Bun.serve also had no dynamic per-handshake certificate selector at all: its only SNI mechanism was the static tls: [{ serverName }, ...] array registered up front. tls.createServer does honor SNICallback because it goes through Bun.listen/node:tls, which already has the resolver machinery.

Fix

Expose the per-handshake certificate selector that node:tls already uses to the uWS App that Bun.serve runs on, and forward the node:https SNICallback into it.

• uSockets already supported the dynamic-resolver shape for node:tls/Bun.listen (us_listen_socket_on_server_name + us_socket_sni_resolve, driven from the BoringSSL select-certificate callback). This adds a resolver hook on the uWS App/HttpContextData and routes it through the same us_select_cert_cb path, so a synchronous pick, an asynchronous suspend, and a refusal all behave identically to the node:tls server.
• Asynchronous callbacks park the handshake (BoringSSL select-certificate retry) until resumeServerSNI(token, context, isError) completes it, instead of falling through to the default context. A small per-thread registry keyed by an opaque token tracks suspended handshakes; the entry is dropped when the handshake's SSL is freed (which always precedes the socket's own free), so a resolution that outlives its connection is a safe no-op.
• A callback that errors (or returns a non-context) refuses the connection without a TLS alert and surfaces as tlsClientError, matching Node and the existing node:tls behavior.

Verification

test/js/node/http/node-https-sni.test.ts covers: per-connection certificate selection, serving a request through the selected cert, synchronous refusal (error / invalid context / throw), fall-through to the default, asynchronous suspend-and-resume, asynchronous error, and a connection destroyed while its callback is still pending.

7 pass, 0 fail  (bun bd)

The tests fail against the released binary (USE_SYSTEM_BUN=1) and pass with this build. The existing node:tls server SNI suite (node-tls-server.test.ts) and the node:tls parallel SNI tests still pass.

Related issues

Implements SNICallback / dynamic SNI for Bun.serve and node:https:

Fixes #4053
Fixes #6749
Fixes #14395

Also relevant (not auto-closed here): #17932 covers the node:tls server where SNICallback already fires and adds a separate ALPNCallback request; #31580 tracks the node:tls SNICallback error path from #31155. This PR makes the node:https server behave the same way for both.

[robobun]

Reproduced on stock 1.4.0: a node:https server serves the default cert for every servername and SNICallback never fires. Fixed by plumbing a per-handshake certificate selector through Bun.serve (the same resolver node:tls already uses) and forwarding SNICallback into it; async callbacks suspend the handshake, errors refuse it as tlsClientError. New coverage in test/js/node/http/node-https-sni.test.ts (7 pass, fails on the released binary).

CI: all review feedback addressed and clippy is green. Latest build 69599 on b18ec7c has 281/287 lanes passed; the only hard failure is test/napi/napi.test.ts "napi_wrap has the right lifetime" on Windows 2019 x64 (a GC-timing flake, unrelated to this change). The prior build's failures were also infra only (windows-aarch64 agent creation, darwin artifact-download timeout, puppeteer launch). The new node-https-sni.test.ts passed on every lane. Ready for review/merge.

[robobun]

^{Updated 2:16 AM PT - Jul 7th, 2026}

❌ @robobun, your commit b18ec7c has 2 failures in Build #69599 (All Failures):

• test/js/sql/sql-prepare-false.test.ts - timeout on 🍎 14 x64
• test/napi/napi.test.ts - code 1 on 🪟 2019 x64

---

🧪   To try this PR locally:

bunx bun-pr 33536

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

bun-33536 --bun

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 39 minutes

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

How can I continue?

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

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

How do review limits work?

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

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

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: e7cff8bc-d04f-402a-aebe-7df2c83ee158

📥 Commits

Reviewing files that changed from the base of the PR and between 213caf5 and b18ec7c.

📒 Files selected for processing (2)

• packages/bun-usockets/src/crypto/openssl.c
• test/js/node/http/node-https-sni.test.ts

Walkthrough

This PR adds asynchronous SNICallback support spanning uSockets/OpenSSL handshake suspension bookkeeping, uWS App server-name dispatch, a new Rust ServerSNI module with FFI resume bridging, ServerConfig on_server_name plumbing, and node:https/_http_server.ts SNICallback integration, plus new tests.

Changes

Async SNICallback support

Layer / File(s)	Summary
OpenSSL pending-state ownership and resume API packages/bun-usockets/src/crypto/openssl.c, packages/bun-usockets/src/libusockets.h	Adds owner/owner_free fields to pending SNI state, centralizes pending-state get/free, and adds `us_socket_sni_attach_resume` to attach a resume handle with guaranteed single-invocation cleanup.
uWS App SNI dispatch and resolver wiring packages/bun-uws/src/App.h, packages/bun-uws/src/HttpContextData.h	Adds `ServerNameResolver` type/fields and consolidates missing-server-name/resolver logic into `onServerNameDispatch`, updating listener callback installation conditions.
uws_sys FFI bindings src/uws_sys/_libusockets.h, src/uws_sys/libuwsockets.cpp, src/uws_sys/App.rs, src/uws_sys/us_socket_t.rs	Adds `uws_server_name_resolver` typedef/setter, Rust `App::server_name_resolver`, and `us_socket_t::sni_attach_resume` wrapper with corresponding C bindings.
Rust ServerSNI module, config, and trampoline src/runtime/server/ServerConfig.rs, src/runtime/server/ServerSNI.rs, src/runtime/server/mod.rs	Adds `on_server_name` config hook parsing, a suspension token registry with `Bun__resumeServerSNI` FFI entry point, and wires the `on_server_name` trampoline into `listen()`.
Node http/https SNICallback bridge src/jsc/bindings/NodeHTTP.cpp, src/js/internal/http.ts, src/js/node/_http_server.ts	Exposes `resumeServerSNI` binding and adds `dispatchServerSNI`/SNICallback option handling and `onServerName` wiring in the `Server` class.
node-https-sni test suite test/js/node/http/node-https-sni.test.ts	Adds tests for sync/async SNICallback certificate selection, error handling, default fallback, and connection-destroy robustness.

Possibly related PRs

• oven-sh/bun#31155: Implements overlapping uSockets/OpenSSL dynamic on_server_name resolver behavior and in-handshake suspension state directly related to the async-SNI resume bookkeeping introduced here.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and accurately reflects the main change: wiring SNICallback into node:https.
Description check	✅ Passed	The description covers the problem, fix, and verification well, though it uses custom headings instead of the template.
Linked Issues check	✅ Passed	The change implements SNICallback and dynamic SNI support for Bun.serve and node:https as requested by #4053, #6749, and #14395.
Out of Scope Changes check	✅ Passed	The changed files stay focused on SNI/SNICallback plumbing, async resume handling, and tests, with no clear unrelated additions.

---

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

[github-actions]

Found 5 issues this PR may fix:

1. SNICallback support #4053 - Original enhancement request for SNICallback support in Bun.serve / node:https servers
2. Node SNICallback support #6749 - Reports that SNICallback gives nothing in node:https — this PR implements the callback
3. SNICallback and ALPNCallback not triggered in Bun when creating TLS server #17932 - SNICallback not triggered when creating TLS server — this PR fixes the SNICallback portion (ALPNCallback remains unaddressed)
4. tls: sni_cb should return SSL_TLSEXT_ERR_ALERT_FATAL when SNICallback throws or returns non-SecureContext #31580 - Requests SSL_TLSEXT_ERR_ALERT_FATAL when SNICallback throws or returns non-SecureContext — this PR handles error/refusal paths
5. Supporting dynamic SNI (tls.serverName) for subdomain based routing #14395 - Requests dynamic SNI via SNICallback for subdomain-based routing — this PR implements both sync and async SNICallback

If this is helpful, copy the block below into the PR description to auto-close these issues on merge.

Fixes #4053
Fixes #6749
Fixes #17932
Fixes #31580
Fixes #14395

🤖 Generated with Claude Code

[robobun]

Thanks. Added Fixes for #4053, #6749, and #14395 (all the Bun.serve/node:https SNICallback and dynamic-SNI requests this PR implements). Left #17932 and #31580 as related rather than auto-closed: #17932 also asks for ALPNCallback and is scoped to the node:tls server, and #31580 tracks the node:tls error path from #31155; this PR brings the node:https server to the same behavior but doesn't close those out.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/bun-usockets/src/crypto/openssl.c`:
- Around line 216-227: The helper us_ssl_sni_pending_get currently allocates a
new us_ssl_sni_pending_t and assumes SSL_set_ex_data succeeds, but that call can
fail and leave the allocation leaked. Update the SSL_get_ex_data /
SSL_set_ex_data path in us_ssl_sni_pending_get so that if SSL_set_ex_data
returns failure you free the newly allocated pending state and return NULL
instead of storing a detached object; keep the existing behavior for the
us_ssl_sni_pending_idx guard and the successful retrieval path.

In `@test/js/node/http/node-https-sni.test.ts`:
- Around line 37-211: These independent https.Server/SNICallback tests should
run concurrently instead of sequentially. Update the individual it() cases in
node-https-sni.test.ts to use test.concurrent (or wrap the block in
describe.concurrent) since each case creates its own server on port 0 and does
not share mutable state; keep the same assertions and cleanup logic in each test
while making the concurrency explicit.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: dcb35c8a-8517-4a07-b419-b1dc9ce8a953

📥 Commits

Reviewing files that changed from the base of the PR and between 48ff9eb and 213caf5.

📒 Files selected for processing (15)

• packages/bun-usockets/src/crypto/openssl.c
• packages/bun-usockets/src/libusockets.h
• packages/bun-uws/src/App.h
• packages/bun-uws/src/HttpContextData.h
• src/js/internal/http.ts
• src/js/node/_http_server.ts
• src/jsc/bindings/NodeHTTP.cpp
• src/runtime/server/ServerConfig.rs
• src/runtime/server/ServerSNI.rs
• src/runtime/server/mod.rs
• src/uws_sys/App.rs
• src/uws_sys/_libusockets.h
• src/uws_sys/libuwsockets.cpp
• src/uws_sys/us_socket_t.rs
• test/js/node/http/node-https-sni.test.ts

[robobun]

Superseded by #31155. #6749 is now closed as fixed on main.