Skip to content
Closed
Show file tree
Hide file tree
Changes from 3 commits
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: 23 additions & 2 deletions src/js/node/net.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,8 @@ function onUpgradeWriteClose(callback) {
callback($ERR_SOCKET_CLOSED());
}
const kUpgradeAttached = Symbol("kUpgradeAttached");
// Set while a TLS socket waits to adopt its transport's handle; kUpgradeAttached is emitted when it has it.
const kUpgradePending = Symbol("kUpgradePending");
const kOnreadTail = Symbol("kOnreadTail");
const kOnreadDraining = Symbol("kOnreadDraining");
const kOnreadBuffer = Symbol("kOnreadBuffer");
Expand Down Expand Up @@ -2003,6 +2005,7 @@ Socket.prototype.connect = function connect(...args) {
this.authorized = false;
this.secureConnecting = true;
this[kPreHandshakeWrite] = false;
this[kUpgradePending] = false;
this._secureEstablished = false;
this._securePending = true;
this[kConnectOptions] = options;
Expand Down Expand Up @@ -2070,7 +2073,9 @@ Socket.prototype.connect = function connect(...args) {
}
} else {
// wait to be connected
connection.once("connect", () => {
this[kUpgradePending] = true;
const onConnect = () => {
connection.removeListener("close", onClose);
// 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.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
Expand Down Expand Up @@ -2118,7 +2123,16 @@ Socket.prototype.connect = function connect(...args) {
throw new Error("Invalid socket");
}
}
});
this[kUpgradePending] = false;
this.emit(kUpgradeAttached);
};
// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L739-L741
const onClose = () => {
connection.removeListener("connect", onConnect);
this.destroy();
};
connection.once("connect", onConnect);
connection.once("close", onClose);
}
}
} catch (error) {
Expand Down Expand Up @@ -2276,6 +2290,10 @@ Socket.prototype._destroy = function _destroy(err, callback) {

Socket.prototype._final = function _final(callback) {
$debug("Socket.prototype._final");
// No TLS handle to shut down yet. Node has one from the constructor and waits on `connecting`: https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L964-L973
if (this[kUpgradePending]) {
return this.once(kUpgradeAttached, this._final.bind(this, callback));
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
if (this.connecting) {
return this.once("connect", this._final.bind(this, callback));
}
Expand Down Expand Up @@ -2406,6 +2424,7 @@ Socket.prototype[Symbol.for("::bunUpgradeServerTLS::")] = function (connection,
return;
}
this[kupgraded] = connection;
this[kUpgradePending] = true;
process.nextTick(() => {
if (this.destroyed || connection.destroyed) {
this.destroy();
Expand All @@ -2432,6 +2451,7 @@ Socket.prototype[Symbol.for("::bunUpgradeServerTLS::")] = function (connection,
connection.on("close", events[3]);
destroyWhenUpgradedCloses(this, connection);
this._handle = result;
this[kUpgradePending] = false;
this.emit(kUpgradeAttached);
return;
}
Expand All @@ -2458,6 +2478,7 @@ Socket.prototype[Symbol.for("::bunUpgradeServerTLS::")] = function (connection,
this.once("end", this[kCloseRawConnection]);
raw.connecting = false;
this._handle = tlsHandle;
this[kUpgradePending] = false;
this.emit(kUpgradeAttached);
});
};
Expand Down
3 changes: 2 additions & 1 deletion src/js/node/tls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -897,7 +897,8 @@ TLSSocket.prototype._start = function _start() {
};

TLSSocket.prototype._final = function _final(callback) {
if (!this._handle) return callback();
// net.Socket's _final waits for a handle that is still on its way, and finishes at once when none is.
if (!this._handle) return NetSocket.prototype._final.$call(this, callback);
// https://github.com/nodejs/node/blob/v26.3.0/src/crypto/crypto_tls.cc#L1119-L1133
if (this.secureConnecting && this[kPreHandshakeWrite]) {
return this.once(kSecureConnectDone, NetSocket.prototype._final.bind(this, callback));
Expand Down
49 changes: 49 additions & 0 deletions test/js/node/tls/node-tls-connect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1986,6 +1986,55 @@ describe.each([
});
});

// Both shapes shut the socket down before it has its TLS handle. The FIN waits
// for the transport, as node's _final does on `connecting`:
// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L964-L973
describe.concurrent("before the TLS handle is attached", () => {
it.skipIf(!exe)("end() and destroySoon() wait for a tls.connect({ socket }) transport to connect", async () => {
const ended = {
log: ["transport connect", "finish"],
peerSawFin: true,
writableFinished: true,
readyState: "readOnly",
destroyed: false,
};
const destroyed = {
log: ["transport connect", "finish", "close"],
peerSawFin: true,
writableFinished: true,
readyState: "closed",
destroyed: true,
};
expect(await run("pending-transport")).toEqual({
"end connecting": ended,
"end unconnected": ended,
"destroySoon connecting": destroyed,
"destroySoon unconnected": destroyed,
});
});

// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L739-L741
it.skipIf(!exe)(
"a tls.connect({ socket }) transport that closes before it connects closes the socket",
async () => {
const closed = { log: ["close"], writableFinished: false, readyState: "closed", destroyed: true };
expect(await run("closed-transport")).toEqual({
"end refused": closed,
"end destroyed": closed,
"destroySoon refused": closed,
"destroySoon destroyed": closed,
});
},
);

it.skipIf(!exe)("a server-side TLSSocket end()s and destroySoon()s in the tick that wraps the socket", async () => {
expect(await run("server-same-tick")).toEqual({
end: { log: ["finish"], clientSawFin: true, writableFinished: true, destroyed: false },
destroySoon: { log: ["finish", "close"], clientSawFin: true, writableFinished: true, destroyed: true },
});
});
});

it.skipIf(!exe)("a server-side TLSSocket end()s while it waits for the client's first flight", async () => {
expect(await run("server-end")).toEqual({
log: ["end secureConnecting=true", "finish"],
Expand Down
118 changes: 118 additions & 0 deletions test/js/node/tls/tls-shutdown-before-handshake-fixture.mjs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.