Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
25 changes: 25 additions & 0 deletions src/js/node/net.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@
const kConnectOptions = Symbol("connect-options");
const kAttach = Symbol("kAttach");
const kCloseRawConnection = Symbol("kCloseRawConnection");
const kOnUpgradedClose = Symbol("kOnUpgradedClose");
const kupgraded = Symbol("kupgraded");
const kAdoptedTLSRaw = Symbol("kAdoptedTLSRaw");
const ksocket = Symbol("ksocket");
Expand Down Expand Up @@ -256,6 +257,18 @@
function destroyNT(self, err) {
self.destroy(err);
}
// 'close' listener armed on the connection a TLS socket is upgraded over: the
// connection going away (its owner destroyed it, or the transport under a
// wrapped duplex closed) takes the TLS socket with it, like node's wrap 'close'
// listener. The connection's EOF alone does not; that is what an allowHalfOpen
// TLS socket outlives.
// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L739-L741
Comment thread
robobun marked this conversation as resolved.
Outdated
function onUpgradedClose(self, connection) {
if (self[kupgraded] === connection) self.destroy();
}
function destroyWhenUpgradedCloses(self, connection) {
connection.once("close", (self[kOnUpgradedClose] = onUpgradedClose.bind(null, self, connection)));

Check warning on line 270 in src/js/node/net.ts

View check run for this annotation

Claude / Claude Code Review

kOnUpgradedClose instance field not initialized in Socket constructor

The new `this[kOnUpgradedClose]` field is written here and read in `kCloseRawConnection` but never initialized in the Socket constructor, unlike the neighboring `this[kupgraded] = null` (net.ts:1597) and other symbol fields. REVIEW.md's built-in JS rules for `src/js/` require every instance field to be declared with a default in the class body for object-shape stability — consider adding `this[kOnUpgradedClose] = undefined;` next to the `kupgraded` init.
Comment thread
claude[bot] marked this conversation as resolved.
}
let addAbortListener;
function destroyWhenAborted(err) {
if (!this.destroyed) {
Expand Down Expand Up @@ -1886,8 +1899,13 @@
SocketHandlers.drain(socket);
};

// 'end' listener of an fd-upgraded TLS socket: retires the net.Socket whose fd
// this socket took over.
Comment thread
robobun marked this conversation as resolved.
Outdated
Socket.prototype[kCloseRawConnection] = function () {
const connection = this[kupgraded];
// The 'close' this retirement emits is not the connection going away under
// this socket; one its owner already started (connection.destroyed) is.
Comment thread
robobun marked this conversation as resolved.
Outdated
if (!connection.destroyed) connection.removeListener("close", this[kOnUpgradedClose]);
connection.connecting = false;
connection._handle = null;
connection.unref();
Expand Down Expand Up @@ -2023,6 +2041,7 @@
connection.on("end", events[1]);
connection.on("drain", events[2]);
connection.on("close", events[3]);
destroyWhenUpgradedCloses(this, connection);
this._handle = result;
} else {
// upgradeTLS requires an established socket; a socket that is still
Expand All @@ -2042,6 +2061,7 @@
connection._handle = raw;
raw[kAdoptedTLSRaw] = true;
this.once("end", this[kCloseRawConnection]);
destroyWhenUpgradedCloses(this, connection);
raw.connecting = false;
this._handle = tls;
} else {
Expand Down Expand Up @@ -2074,6 +2094,7 @@
connection.on("end", events[1]);
connection.on("drain", events[2]);
connection.on("close", events[3]);
destroyWhenUpgradedCloses(this, connection);
this._handle = result;
} else {
this[kupgraded] = connection;
Expand All @@ -2089,6 +2110,7 @@
connection._handle = raw;
raw[kAdoptedTLSRaw] = true;
this.once("end", this[kCloseRawConnection]);
destroyWhenUpgradedCloses(this, connection);
raw.connecting = false;
this._handle = tls;
} else {
Expand Down Expand Up @@ -2384,6 +2406,7 @@
connection.on("end", events[1]);
connection.on("drain", events[2]);
connection.on("close", events[3]);
destroyWhenUpgradedCloses(this, connection);
this[kupgraded] = connection;
this._handle = result;
return;
Expand Down Expand Up @@ -2413,6 +2436,7 @@
connection.on("end", events[1]);
connection.on("drain", events[2]);
connection.on("close", events[3]);
destroyWhenUpgradedCloses(this, connection);
this._handle = result;
this.emit(kUpgradeAttached);
return;
Expand All @@ -2437,6 +2461,7 @@
connection._handle = raw;
raw[kAdoptedTLSRaw] = true;
this.once("end", this[kCloseRawConnection]);
destroyWhenUpgradedCloses(this, connection);
raw.connecting = false;
this._handle = tlsHandle;
this.emit(kUpgradeAttached);
Expand Down
16 changes: 15 additions & 1 deletion src/js/node/tls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -735,7 +735,21 @@ function TLSSocket(socket?, options?) {
throw $ERR_INVALID_ARG_TYPE("socket", "Duplex", socket);
}

options = isNetSocketOrDuplex ? { ...options, allowHalfOpen: false } : options || socket || {};
// A wrapped socket decides allowHalfOpen; the option only applies when this
// socket opens its own connection. The wrapped socket is the first argument
// here, or `options.socket` on the tls.connect({ socket }) path (node's
// connect() forwards it as the first argument).
// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L592
// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L1756-L1757
Comment thread
robobun marked this conversation as resolved.
Outdated
if (isNetSocketOrDuplex) {
options = { ...options, allowHalfOpen: socket.allowHalfOpen };
} else {
options = options || socket || {};
const wrapped = options.socket;
if (wrapped instanceof Duplex) {
options = { ...options, allowHalfOpen: wrapped.allowHalfOpen };
}
}

this._rejectUnauthorized = !!options.rejectUnauthorized;

Expand Down
Loading