Skip to content

node:tls: make client-side new TLSSocket(socket) upgrade the socket it wraps #37664

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
robobun wants to merge 3 commits into
base: main
Choose a base branch
from
Open

node:tls: make client-side new TLSSocket(socket) upgrade the socket it wraps #37664

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
38b89c9
node:tls: make client-side new TLSSocket(socket) upgrade the socket i…
robobun Aug 11, 2026
ba7858c
Keep the UpgradedDuplex take_error hunk identical to #36909
robobun Aug 11, 2026
0b3d4b9
Trim the new comments to one line each
robobun Aug 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/js/internal/net/symbols.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,8 @@ export default {
// 'secureConnect' (node parity), so internal deferrals park on this instead.
kSecureConnectDone: Symbol("kSecureConnectDone"),
kVerifyError: Symbol("kVerifyError"),
// net.Socket.prototype method driving the client-side upgrade behind
// `new tls.TLSSocket(socket)`; lives in net.ts next to the connect() path it
// shares with tls.connect({ socket }).
Comment thread
robobun marked this conversation as resolved.
Outdated
kUpgradeClientTLS: Symbol("kUpgradeClientTLS"),
};
104 changes: 62 additions & 42 deletions src/js/node/net.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ import type { TLSSocket } from "node:tls";
const { kTimeout, getTimerDuration } = require("internal/timers");
const { validateFunction, validateNumber, validateAbortSignal, validatePort, validateBoolean, validateInt32, validateString } = require("internal/validators"); // prettier-ignore
const { isIPv4, isIPv6, isIP } = require("internal/net/isIP");
const { kArmHandshakeTimeout, kSecureConnectDone, kVerifyError } = require("internal/net/symbols");
const { kArmHandshakeTimeout, kSecureConnectDone, kVerifyError, kUpgradeClientTLS } = require("internal/net/symbols");

const ArrayPrototypeIncludes = Array.prototype.includes;
const ArrayPrototypeJoin = Array.prototype.join;
Expand Down Expand Up @@ -144,6 +144,9 @@ const kConnectOptions = Symbol("connect-options");
const kAttach = Symbol("kAttach");
const kCloseRawConnection = Symbol("kCloseRawConnection");
const kupgraded = Symbol("kupgraded");
// Set on a TLSSocket constructed over an existing socket (`new tls.TLSSocket(socket)`
// rather than tls.connect()); see onClientHandshakeComplete.
Comment thread
robobun marked this conversation as resolved.
Outdated
const kStandaloneWrap = Symbol("kStandaloneWrap");
const kAdoptedTLSRaw = Symbol("kAdoptedTLSRaw");
const ksocket = Symbol("ksocket");
const khandlers = Symbol("khandlers");
Expand Down Expand Up @@ -299,14 +302,22 @@ function onClientHandshakeComplete(self, socket, verifyError) {
self._secureEstablished = true;
self[kVerifyError] = verifyError ?? null;
self.alpnProtocol = socket.alpnProtocol;
// Node installs onConnectSecure from tls.connect() only: a TLSSocket
// constructed over an existing socket gets _finishInit alone, so it neither
// checks the certificate against a connect() host it does not have nor emits
// 'secureConnect'. It still applies the chain verdict (authorized /
// rejectUnauthorized), like the standalone server wrap in ServerHandlers.
// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L1081-L1108
// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L1810
Comment thread
robobun marked this conversation as resolved.
Outdated
Comment thread
robobun marked this conversation as resolved.
const connectSecure = !self[kStandaloneWrap];
// Node has no try/catch around these emits; a listener throw reaches
// InternalCallbackScope as uncaughtException. reportError mirrors that
// without changing Bun.connect's handshake-throw-to-error-handler contract.
// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L1107
try {
// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L1662-L1673
const { checkServerIdentity } = self[bunTLSConnectOptions];
if (!verifyError && !self.isSessionReused() && typeof checkServerIdentity === "function") {
if (connectSecure && !verifyError && !self.isSessionReused() && typeof checkServerIdentity === "function") {
const hostname = self.servername || self._host || "localhost";
const cert = self.getPeerCertificate(true);
if (cert) {
Expand All @@ -321,7 +332,10 @@ function onClientHandshakeComplete(self, socket, verifyError) {
if (rejectUnauthorized ?? self._rejectUnauthorized) {
self.destroy(verifyError);
// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L1686-L1688
self.emit("secure", self);
// A wrap reports the rejection once, through the destroy: its
// 'secure' listeners read ssl.verifyError(), which is gone after
// destroy (the mysql driver's STARTTLS does exactly this).
Comment thread
robobun marked this conversation as resolved.
Outdated
if (connectSecure) self.emit("secure", self);
return;
}
} else {
Expand All @@ -333,7 +347,7 @@ function onClientHandshakeComplete(self, socket, verifyError) {
// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L1697-L1698
self.secureConnecting = false;
self.emit(kSecureConnectDone);
self.emit("secureConnect", verifyError);
if (connectSecure) self.emit("secureConnect", verifyError);
const pendingSession = self[kpendingSession];
if (pendingSession) {
self[kpendingSession] = null;
Expand Down Expand Up @@ -1940,10 +1954,6 @@ Socket.prototype.connect = function connect(...args) {
}
tls.checkServerIdentity = checkServerIdentity || tls.checkServerIdentity;
this[bunTLSConnectOptions] = tls;
let tlsSocket;
if (!connection && (tlsSocket = tls.socket)) {
connection = tlsSocket;
}
}
if (connection) {
if (
Expand Down Expand Up @@ -1994,10 +2004,7 @@ Socket.prototype.connect = function connect(...args) {
tls,
socket: this[khandlers],
});
connection.on("data", events[0]);
connection.on("end", events[1]);
connection.on("drain", events[2]);
connection.on("close", events[3]);
listenToUpgradedDuplex(this, connection, events);
this._handle = result;
} else {
// upgradeTLS requires an established socket; a socket that is still
Expand All @@ -2024,8 +2031,15 @@ Socket.prototype.connect = function connect(...args) {
throw new Error("Invalid socket");
}
} else {
// Until the upgrade takes the connection over, destroying this
// socket destroys it too, as closing Node's wrap does; afterwards the
// upgrade's own teardown (kCloseRawConnection / the shared fd) owns it.
// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/js_stream_socket.js#L242-L253
Comment thread
robobun marked this conversation as resolved.
Outdated
Comment thread
robobun marked this conversation as resolved.
const destroyConnection = () => connection.destroy();
this.once("close", destroyConnection);
// wait to be connected
connection.once("connect", () => {
this.removeListener("close", destroyConnection);
// The TLS socket may have been destroyed before the underlying
// socket connected (e.g. tls.connect({ socket }).destroy()); don't
// start a handshake on a dead socket.
Expand All @@ -2045,10 +2059,7 @@ Socket.prototype.connect = function connect(...args) {
tls,
socket: this[khandlers],
});
connection.on("data", events[0]);
connection.on("end", events[1]);
connection.on("drain", events[2]);
connection.on("close", events[3]);
listenToUpgradedDuplex(this, connection, events);
this._handle = result;
} else {
this[kupgraded] = connection;
Expand Down Expand Up @@ -2278,6 +2289,27 @@ function hasUnflushedWrites(connection) {
return connection.writableLength > 0 || connection[kwriteCallback] != null;
}

// Wires the stream-level TLS engine to the stream it runs over. The engine only
// listens for traffic; the stream's 'error' is ours to take, as Node routes it
// to the TLS socket (JSStreamSocket re-emits it, _init forwards it) and tears
// the wrap down on the stream's close. Left unhandled it would be thrown out of
// the stream's destroy, and the 'close' that tears the TLS socket down would
// never be emitted.
// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/js_stream_socket.js#L65
// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L740
// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L977
Comment thread
robobun marked this conversation as resolved.
Outdated
Comment thread
robobun marked this conversation as resolved.
function listenToUpgradedDuplex(self, connection, events) {
connection.on("data", events[0]);
connection.on("end", events[1]);
connection.on("drain", events[2]);
connection.on("close", events[3]);
connection.on("error", onUpgradedDuplexError.bind(self));
}

function onUpgradedDuplexError(err) {
if (!this.destroyed) this.destroy(err);
}

function drainOnreadTail(self, fromRead?) {
if (self[kOnreadTail] === undefined) return false;
if (fromRead) self[kOnreadReadRequested] = true;
Expand Down Expand Up @@ -2360,10 +2392,7 @@ Socket.prototype[Symbol.for("::bunUpgradeServerTLS::")] = function (connection,
socket: serverHandlersFor(this),
isServer: true,
});
connection.on("data", events[0]);
connection.on("end", events[1]);
connection.on("drain", events[2]);
connection.on("close", events[3]);
listenToUpgradedDuplex(this, connection, events);
this[kupgraded] = connection;
this._handle = result;
return;
Expand All @@ -2389,10 +2418,7 @@ Socket.prototype[Symbol.for("::bunUpgradeServerTLS::")] = function (connection,
socket: serverHandlersFor(this),
isServer: true,
});
connection.on("data", events[0]);
connection.on("end", events[1]);
connection.on("drain", events[2]);
connection.on("close", events[3]);
listenToUpgradedDuplex(this, connection, events);
this._handle = result;
this.emit(kUpgradeAttached);
return;
Expand Down Expand Up @@ -2423,6 +2449,18 @@ Socket.prototype[Symbol.for("::bunUpgradeServerTLS::")] = function (connection,
});
};

// Client-side counterpart, for `new tls.TLSSocket(socket)` (a STARTTLS client
// wrapping the connection it already holds): the same upgrade as
// tls.connect({ socket }), so the handshake starts here and _handle is the TLS
// handle that _write/_read/_destroy expect. Node likewise wraps the handle in
// the constructor; only the onConnectSecure half of tls.connect() is left out
// (see onClientHandshakeComplete).
// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L590-L608
Comment thread
robobun marked this conversation as resolved.
Outdated
Socket.prototype[kUpgradeClientTLS] = function (connection) {
this[kStandaloneWrap] = true;
Socket.prototype.connect.$call(this, { socket: connection });
};

Socket.prototype.read = function read(size) {
if (!this.connecting && !drainOnreadTail(this, true)) {
this._handle?.resume?.();
Expand Down Expand Up @@ -3073,11 +3111,6 @@ function internalConnect(self, options, address, port, addressType, localAddress
}

//TLS
let connection = self[ksocket];
const optionsSocket = options.socket;
if (optionsSocket) {
connection = optionsSocket;
}
let tls = undefined;
const bunTLS = self[bunTlsSymbol];
if (typeof bunTLS === "function") {
Expand All @@ -3091,10 +3124,6 @@ function internalConnect(self, options, address, port, addressType, localAddress
self.servername = tls.servername;
tls.checkServerIdentity = checkServerIdentity || tls.checkServerIdentity;
self[bunTLSConnectOptions] = tls;
let tlsSocket;
if (!connection && (tlsSocket = tls.socket)) {
connection = tlsSocket;
}
}
self.authorized = false;
self.secureConnecting = true;
Expand Down Expand Up @@ -3221,11 +3250,6 @@ function internalConnectMultiple(context, canceled?) {
}

//TLS
let connection = self[ksocket];
const contextOptionsSocket = context.options.socket;
if (contextOptionsSocket) {
connection = contextOptionsSocket;
}
let tls = undefined;
const bunTLS = self[bunTlsSymbol];
if (typeof bunTLS === "function") {
Expand All @@ -3239,10 +3263,6 @@ function internalConnectMultiple(context, canceled?) {
self.servername = tls.servername;
tls.checkServerIdentity = checkServerIdentity || tls.checkServerIdentity;
self[bunTLSConnectOptions] = tls;
let tlsSocket;
if (!connection && (tlsSocket = tls.socket)) {
connection = tlsSocket;
}
}
self.authorized = false;
self.secureConnecting = true;
Expand Down
41 changes: 22 additions & 19 deletions src/js/node/tls.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const {
} = require("internal/validators");

const { Server: NetServer, Socket: NetSocket } = net;
const { kArmHandshakeTimeout, kSecureConnectDone, kVerifyError } = require("internal/net/symbols");
const { kArmHandshakeTimeout, kSecureConnectDone, kVerifyError, kUpgradeClientTLS } = require("internal/net/symbols");

const getBundledRootCertificates = $newCppFunction("NodeTLS.cpp", "getBundledRootCertificates", 1);
const getExtraCACertificates = $newCppFunction("NodeTLS.cpp", "getExtraCACertificates", 1);
Expand Down Expand Up @@ -783,15 +783,6 @@ function TLSSocket(socket?, options?) {
if (ALPNProtocols) {
convertALPNProtocols(ALPNProtocols, this);
}

if (isNetSocketOrDuplex && !this.isServer) {
this._handle = socket;
// keep compatibility with http2-wrapper or other places that try to grab JSStreamSocket in node.js, with here is just the TLSSocket
this._handle._parentWrap = this;
}
// For the server wrap, _handle is assigned the upgraded TLS handle by the
// server-upgrade method below; leaving it unset until then means a synchronous
// teardown during upgradeTLS won't call close() on the bare net.Socket.
}
// Internal path: keep the per-digest cache (the user-facing constructors,
// createSecureContext() and new tls.SecureContext(), own theirs exclusively).
Expand All @@ -807,12 +798,22 @@ function TLSSocket(socket?, options?) {
this[kcheckServerIdentity] = checkServerIdentityOption || checkServerIdentity;
this[ksession] = options.session || null;

// `new tls.TLSSocket(socket, { isServer: true })`: drive the server-side TLS
// handshake over the provided socket via net.ts's native upgrade path (reaches
// the module-private kupgraded + the shared ServerHandlers). Client-side wraps
// go through the connect path elsewhere.
if (isNetSocketOrDuplex && this.isServer) {
this[Symbol.for("::bunUpgradeServerTLS::")](socket, this[buntls](null, null));
// `new tls.TLSSocket(socket, ...)`: drive the handshake over the provided
// socket via net.ts's native upgrade paths (they reach the module-private
// kupgraded state and handler tables). Both leave _handle unset until the
// upgrade hands back the TLS handle, so nothing ever treats the wrapped
// net.Socket itself as a handle.
Comment thread
robobun marked this conversation as resolved.
Outdated
if (isNetSocketOrDuplex) {
if (isServer) {
this[Symbol.for("::bunUpgradeServerTLS::")](socket, this[buntls](null, null));
} else {
this[kUpgradeClientTLS](socket);
// http2-wrapper derives its JSStreamSocket from
// `new TLSSocket(new PassThrough())._handle._parentWrap.constructor`;
// here that is the TLSSocket itself.
Comment thread
robobun marked this conversation as resolved.
Outdated
const handle = this._handle;
if (handle) handle._parentWrap = this;
}
}
}
$toClass(TLSSocket, "TLSSocket", NetSocket);
Expand Down Expand Up @@ -871,8 +872,11 @@ TLSSocket.prototype._destroySSL = function _destroySSL() {
};

TLSSocket.prototype._start = function _start() {
// some frameworks uses this _start internal implementation is suposed to start TLS handshake/connect
this.connect();
// In Node this sends the ClientHello of a socket wrapped by the constructor
// (the mysql driver's STARTTLS calls it right after `new TLSSocket(socket)`).
// Here the constructor's upgrade and connect() both start the handshake
// natively, so there is nothing left to kick off.
// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L1110-L1129
Comment thread
robobun marked this conversation as resolved.
Outdated
Comment thread
robobun marked this conversation as resolved.
};

TLSSocket.prototype._final = function _final(callback) {
Expand Down Expand Up @@ -1117,7 +1121,6 @@ TLSSocket.prototype[buntls] = function (port, host) {
servername = host && !net.isIP(host) ? host : "";
}
return {
socket: this._handle,
ALPNProtocols: this.ALPNProtocols,
checkServerIdentity: this[kcheckServerIdentity],
session: this[ksession],
Expand Down
8 changes: 5 additions & 3 deletions src/runtime/socket/UpgradedDuplex.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,22 +241,24 @@ impl UpgradedDuplex {
_ => return,
};

// `on_error` hands the value to the JS error handler, so it must be the
// thrown value (`take_error`), not the `JSC::Exception` cell wrapping it.
if let Some(data) = data {
let buffer = match bun_jsc::array_buffer::BinaryType::Buffer.to_js(data, &global) {
Ok(b) => b,
Err(err) => {
(self.handlers.on_error)(self.handlers.ctx, global.take_exception(err));
(self.handlers.on_error)(self.handlers.ctx, global.take_error(err));
return;
}
};
buffer.ensure_still_alive();

if let Err(err) = write_or_end.call(&global, duplex, &[buffer]) {
(self.handlers.on_error)(self.handlers.ctx, global.take_exception(err));
(self.handlers.on_error)(self.handlers.ctx, global.take_error(err));
}
} else {
if let Err(err) = write_or_end.call(&global, duplex, &[JSValue::NULL]) {
(self.handlers.on_error)(self.handlers.ctx, global.take_exception(err));
(self.handlers.on_error)(self.handlers.ctx, global.take_error(err));
}
}
}
Expand Down
Loading