From cd619122a2af001f879b3a2855989eb323d28420 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 13 Aug 2026 08:30:54 +0000 Subject: [PATCH 1/3] node:tls: report a wrapped socket's connect failure on the TLSSocket tls.connect({ socket }) over a net.Socket that has not connected yet only waited for the socket's 'connect' before upgrading it. If that connect failed, the plain socket emitted 'error' and 'close' but the TLSSocket never emitted anything, so callers waiting on it hung. Node re-emits the wrapped socket's 'error' on the TLSSocket and destroys the TLSSocket when the wrapped socket closes; do the same while waiting for 'connect', and drop the listeners once the upgrade happens. --- src/js/node/net.ts | 26 +++++- test/js/node/tls/node-tls-connect.test.ts | 104 ++++++++++++++++++++++ 2 files changed, 128 insertions(+), 2 deletions(-) diff --git a/src/js/node/net.ts b/src/js/node/net.ts index 58e05eb224ee..c05a8ccd462f 100644 --- a/src/js/node/net.ts +++ b/src/js/node/net.ts @@ -2027,7 +2027,9 @@ Socket.prototype.connect = function connect(...args) { } } else { // wait to be connected - connection.once("connect", () => { + const onConnect = () => { + connection.removeListener("error", onError); + 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. @@ -2073,7 +2075,27 @@ Socket.prototype.connect = function connect(...args) { throw new Error("Invalid socket"); } } - }); + }; + // Until the upgrade above happens nothing ties this socket to the + // connection, so a connect failure would leave it pending forever. + // Node re-emits the wrapped socket's 'error' here and destroys the + // TLS socket when the wrapped socket closes: + // https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L977 + // https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L740 + const onError = error => { + // The connection keeps connecting after this socket is destroyed + // (onConnect tears it down); its failure must not surface as an + // 'error' after this socket's 'close'. + if (!this.destroyed) this._emitTLSError(error); + }; + const onClose = () => { + connection.removeListener("connect", onConnect); + connection.removeListener("error", onError); + this.destroy(); + }; + connection.once("connect", onConnect); + connection.on("error", onError); + connection.once("close", onClose); } } } catch (error) { diff --git a/test/js/node/tls/node-tls-connect.test.ts b/test/js/node/tls/node-tls-connect.test.ts index fa62c2de5ebc..fbddd16b06db 100644 --- a/test/js/node/tls/node-tls-connect.test.ts +++ b/test/js/node/tls/node-tls-connect.test.ts @@ -1426,3 +1426,107 @@ describe("throwing 'secureConnect' listener", () => { expect(exitCode).toBe(0); }); }); + +describe("tls.connect({ socket }) over a net.Socket whose connect has not completed yet", () => { + // The upgrade to TLS only happens once the wrapped socket emits 'connect'. + // Until then node forwards the wrapped socket's 'error' to the TLSSocket and + // destroys the TLSSocket when the wrapped socket closes; the event sequences + // asserted below are the ones node v26.3.0 produces. + // https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L977 + // https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L740 + + // A port on 127.0.0.1 that refuses connections: it is the local port of a + // live connection, so nothing listens on it and, unlike a port that was + // bound and released again, a concurrent listen(0) cannot be handed it. + async function refusedPort(): Promise<{ port: number } & AsyncDisposable> { + const sink = net.createServer(); + await once(sink.listen(0, "127.0.0.1"), "listening"); + const holder = net.connect((sink.address() as AddressInfo).port, "127.0.0.1"); + const [[accepted]] = await Promise.all([once(sink, "connection"), once(holder, "connect")]); + return { + port: (holder.address() as AddressInfo).port, + async [Symbol.asyncDispose]() { + holder.destroy(); + accepted.destroy(); + sink.close(); + await once(sink, "close"); + }, + }; + } + + function observe(name: string, socket: net.Socket, log: string[], errors: Error[] = []) { + socket.on("error", err => { + errors.push(err); + log.push(`${name} error ${(err as NodeJS.ErrnoException).code}`); + }); + socket.on("close", hadError => log.push(`${name} close hadError=${hadError}`)); + } + + // events.once() would reject on the 'error' these tests expect first. + function closed(socket: net.Socket): Promise { + return new Promise(resolve => socket.once("close", () => resolve())); + } + + it("a refused connect is emitted as the TLSSocket's 'error', then its 'close'", async () => { + await using refused = await refusedPort(); + const log: string[] = []; + const errors: Error[] = []; + const raw = net.connect(refused.port, "127.0.0.1"); + observe("raw", raw, log, errors); + const client = tls.connect({ socket: raw, rejectUnauthorized: false }); + observe("tls", client, log, errors); + + await closed(client); + + expect(log).toEqual([ + "raw error ECONNREFUSED", + "tls error ECONNREFUSED", + "raw close hadError=true", + "tls close hadError=false", + ]); + // The TLSSocket re-emits the wrapped socket's error object itself. + expect(errors[1]).toBe(errors[0]); + }); + + it("a socket that starts connecting after being wrapped reports the failure to TLSSocket listeners alone", async () => { + await using refused = await refusedPort(); + const log: string[] = []; + const raw = new net.Socket(); + const client = tls.connect({ socket: raw, rejectUnauthorized: false }); + observe("tls", client, log); + raw.connect(refused.port, "127.0.0.1"); + + await closed(client); + + expect(log).toEqual(["tls error ECONNREFUSED", "tls close hadError=false"]); + }); + + it("destroying the socket before it connects closes the TLSSocket without an error", async () => { + await using refused = await refusedPort(); + const log: string[] = []; + const raw = net.connect(refused.port, "127.0.0.1"); + observe("raw", raw, log); + const client = tls.connect({ socket: raw, rejectUnauthorized: false }); + observe("tls", client, log); + raw.destroy(); + + await closed(client); + + expect(log).toEqual(["raw close hadError=false", "tls close hadError=false"]); + }); + + it("a connect that fails after the TLSSocket was destroyed is neither emitted on it nor left unhandled", async () => { + await using refused = await refusedPort(); + const log: string[] = []; + // No listeners on the wrapped socket: its refused connect used to surface + // as an uncaught exception here. + const raw = net.connect(refused.port, "127.0.0.1"); + const client = tls.connect({ socket: raw, rejectUnauthorized: false }); + observe("tls", client, log); + client.destroy(); + + await closed(raw); + + expect(log).toEqual(["tls close hadError=false"]); + }); +}); From e1cf627aaaa15da3e5f965095eafe863e1e8fe7d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 13 Aug 2026 18:02:26 +0000 Subject: [PATCH 2/3] test: cover reconnecting the wrapped socket after the failed connect --- test/js/node/tls/node-tls-connect.test.ts | 28 +++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test/js/node/tls/node-tls-connect.test.ts b/test/js/node/tls/node-tls-connect.test.ts index fbddd16b06db..852d0bf95196 100644 --- a/test/js/node/tls/node-tls-connect.test.ts +++ b/test/js/node/tls/node-tls-connect.test.ts @@ -1529,4 +1529,32 @@ describe("tls.connect({ socket }) over a net.Socket whose connect has not comple expect(log).toEqual(["tls close hadError=false"]); }); + + it("a socket reconnected after the failure is no longer tied to the closed TLSSocket", async () => { + await using refused = await refusedPort(); + // resume() so whatever the client sends is discarded and the accepted + // socket can reach 'end' -> autoDestroy, letting the disposal close() finish. + await using plain = net.createServer(socket => { + socket.resume(); + socket.end("plain"); + }); + await once(plain.listen(0, "127.0.0.1"), "listening"); + const log: string[] = []; + const raw = net.connect(refused.port, "127.0.0.1"); + raw.on("error", () => {}); + const client = tls.connect({ socket: raw, rejectUnauthorized: false }); + observe("tls", client, log); + await closed(raw); + + // The wrap's 'connect' listener used to stay armed and upgrade whatever + // the socket connected to next, so the plain reply below reached the + // TLSSocket as ERR_SSL_WRONG_VERSION_NUMBER. + raw.connect((plain.address() as AddressInfo).port, "127.0.0.1"); + const received: Buffer[] = []; + raw.on("data", chunk => received.push(chunk)); + await closed(raw); + + expect(Buffer.concat(received).toString()).toBe("plain"); + expect(log).toEqual(["tls error ECONNREFUSED", "tls close hadError=false"]); + }); }); From 9c8e832106d8afac57a1903601520e1227c9946a Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 13 Aug 2026 18:14:07 +0000 Subject: [PATCH 3/3] net: shorten the comments on the tls wrap's wait-for-connect listeners --- src/js/node/net.ts | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/js/node/net.ts b/src/js/node/net.ts index c05a8ccd462f..cfbc14ed226b 100644 --- a/src/js/node/net.ts +++ b/src/js/node/net.ts @@ -2076,16 +2076,9 @@ Socket.prototype.connect = function connect(...args) { } } }; - // Until the upgrade above happens nothing ties this socket to the - // connection, so a connect failure would leave it pending forever. - // Node re-emits the wrapped socket's 'error' here and destroys the - // TLS socket when the wrapped socket closes: - // https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L977 - // https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L740 + // Until then, route the connection's failure like node's wrap (internal/tls/wrap.js _init / _wrapHandle). const onError = error => { - // The connection keeps connecting after this socket is destroyed - // (onConnect tears it down); its failure must not surface as an - // 'error' after this socket's 'close'. + // Not once this socket was destroyed while waiting (onConnect tears the connection down). if (!this.destroyed) this._emitTLSError(error); }; const onClose = () => {