From 1ebb3089f9e592bda151b6a5871ba5a4dca3cb60 Mon Sep 17 00:00:00 2001 From: Sids15 Date: Tue, 16 Jun 2026 05:09:57 +0530 Subject: [PATCH 1/2] net: stop named-pipe tls.connect({ socket }) re-emitting post-upgrade bytes on the original socket On Windows, tls.connect({ socket }) over a named-pipe net.Socket takes the upgradeDuplexToTLS path, which (unlike the TCP upgradeTLS path) does not replace connection._handle. The original native pipe handle keeps firing SocketHandlers2.data, and the TLS layer was fed via connection.on("data", ...). So post-upgrade ciphertext reached both the TLS feeder and any pre-existing user data listener (the STARTTLS re-entry from #32239), re-emitting encrypted bytes as cleartext on the original socket. The #32241 kupgradedToTLS flag cannot be reused here: suppressing self.push() would also starve the feeder, which consumes that same data event. Fix: store the data feeder on the connection and have SocketHandlers2.data call it directly, bypassing connection.push so no data event is emitted post-upgrade. The socket goes quiet after the upgrade. Windows named pipes only. Closes #32242 --- src/js/node/net.ts | 27 ++++++++- test/js/node/tls/node-tls-namedpipes.test.ts | 60 ++++++++++++++++++++ 2 files changed, 85 insertions(+), 2 deletions(-) diff --git a/src/js/node/net.ts b/src/js/node/net.ts index 0f31fd0358b1..ae42a8f92b78 100644 --- a/src/js/node/net.ts +++ b/src/js/node/net.ts @@ -73,6 +73,7 @@ const kAttach = Symbol("kAttach"); const kCloseRawConnection = Symbol("kCloseRawConnection"); const kpendingRead = Symbol("kpendingRead"); const kupgraded = Symbol("kupgraded"); +const kupgradeDuplexFeeder = Symbol("kupgradeDuplexFeeder"); const ksocket = Symbol("ksocket"); const khandlers = Symbol("khandlers"); const kclosed = Symbol("closed"); @@ -516,6 +517,14 @@ const SocketHandlers2: SocketHandler { + const { promise: messageReceived, resolve: resolveMessageReceived } = Promise.withResolvers(); + const { promise: clientReceived, resolve: resolveClientReceived } = Promise.withResolvers(); + const { promise: done, resolve: resolveDone, reject: rejectDone } = Promise.withResolvers(); + + let client: ReturnType | null = null; + let nonTLSClient: ReturnType | null = null; + let server: ReturnType | null = null; + + // Bytes seen by a pre-existing listener on the ORIGINAL (pre-upgrade) socket. + const leakedToOriginal: Buffer[] = []; + + const pipe_name = `\\\\.\\pipe\\test\\${randomUUID()}`; + try { + server = createServer(tls, socket => { + socket.on("error", rejectDone); + socket.on("data", data => { + socket.write("Goodbye World!"); + resolveMessageReceived(data.toString()); + }); + }); + server.on("error", rejectDone); + + server.listen(pipe_name); + await once(server, "listening"); + + nonTLSClient = net.connect(pipe_name); + nonTLSClient.on("error", rejectDone); + // The STARTTLS-style pre-existing listener. After the TLS upgrade it must + // receive nothing — the bytes are encrypted and belong to the TLS layer. + nonTLSClient.on("data", chunk => { + leakedToOriginal.push(chunk); + }); + + client = connect({ socket: nonTLSClient, ca: tls.cert }); + client.on("error", rejectDone); + client.on("data", data => { + resolveClientReceived(data.toString()); + }); + + await once(client, "secureConnect"); + client.write("Hello World!"); + + expect(await messageReceived).toBe("Hello World!"); + expect(await clientReceived).toBe("Goodbye World!"); + + // The original socket's listener must not have seen any TLS bytes. + expect(Buffer.concat(leakedToOriginal).length).toBe(0); + resolveDone(); + await done; + } finally { + client?.destroy(); + nonTLSClient?.destroy(); + server?.close(); + } +}); From 629fd43066a41e3f6054b54d7f446dd1c6028e1d Mon Sep 17 00:00:00 2001 From: Sids15 Date: Tue, 16 Jun 2026 05:41:43 +0530 Subject: [PATCH 2/2] =?UTF-8?q?address=20review=20=E2=80=94=20feed=20onrea?= =?UTF-8?q?d=20handler=20+=20fail-fast=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Route post-upgrade bytes through the TLS feeder in the custom onread data handler too (sibling of SocketHandlers2.data), so onread sockets don't leak ciphertext to onread.callback or starve the feeder. - Restore pure-Duplex data wiring via attachUpgradeDuplexDataFeeder (named-pipe net.Socket -> feeder; pure Duplex -> public data event). - Test: trim comments to the issue URL; race awaits against the error promise." --- src/js/node/net.ts | 8 ++++++++ test/js/node/tls/node-tls-namedpipes.test.ts | 14 ++++---------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/js/node/net.ts b/src/js/node/net.ts index ae42a8f92b78..635b63210702 100644 --- a/src/js/node/net.ts +++ b/src/js/node/net.ts @@ -766,6 +766,14 @@ function Socket(options?) { const { self } = socket.data; if (!self) return; self._unrefTimer(); + // After a named-pipe TLS upgrade, route ciphertext to the TLS feeder + // rather than the user's onread.callback (same re-emission concern as + // SocketHandlers2.data above; #32242). + const feeder = self[kupgradeDuplexFeeder]; + if (feeder !== undefined) { + feeder(buffer); + return; + } try { onread.callback(buffer.length, buffer); } catch (e) { diff --git a/test/js/node/tls/node-tls-namedpipes.test.ts b/test/js/node/tls/node-tls-namedpipes.test.ts index 2e8fef75718e..c40c1eece942 100644 --- a/test/js/node/tls/node-tls-namedpipes.test.ts +++ b/test/js/node/tls/node-tls-namedpipes.test.ts @@ -95,9 +95,7 @@ it.if(isWindows)("should be able to upgrade a named pipe connection to TLS", asy await expectMaxObjectTypeCount(expect, "TLSSocket", 3); }); -// Regression for #32242: after tls.connect({ socket }) upgrades a named-pipe -// net.Socket, post-upgrade ciphertext must NOT re-emit as `data` on the -// original socket's pre-existing listeners. +// https://github.com/oven-sh/bun/issues/32242 it.if(isWindows)("named-pipe TLS upgrade does not re-emit bytes on the original socket", async () => { const { promise: messageReceived, resolve: resolveMessageReceived } = Promise.withResolvers(); const { promise: clientReceived, resolve: resolveClientReceived } = Promise.withResolvers(); @@ -107,7 +105,6 @@ it.if(isWindows)("named-pipe TLS upgrade does not re-emit bytes on the original let nonTLSClient: ReturnType | null = null; let server: ReturnType | null = null; - // Bytes seen by a pre-existing listener on the ORIGINAL (pre-upgrade) socket. const leakedToOriginal: Buffer[] = []; const pipe_name = `\\\\.\\pipe\\test\\${randomUUID()}`; @@ -126,8 +123,6 @@ it.if(isWindows)("named-pipe TLS upgrade does not re-emit bytes on the original nonTLSClient = net.connect(pipe_name); nonTLSClient.on("error", rejectDone); - // The STARTTLS-style pre-existing listener. After the TLS upgrade it must - // receive nothing — the bytes are encrypted and belong to the TLS layer. nonTLSClient.on("data", chunk => { leakedToOriginal.push(chunk); }); @@ -138,13 +133,12 @@ it.if(isWindows)("named-pipe TLS upgrade does not re-emit bytes on the original resolveClientReceived(data.toString()); }); - await once(client, "secureConnect"); + await Promise.race([once(client, "secureConnect"), done]); client.write("Hello World!"); - expect(await messageReceived).toBe("Hello World!"); - expect(await clientReceived).toBe("Goodbye World!"); + expect(await Promise.race([messageReceived, done])).toBe("Hello World!"); + expect(await Promise.race([clientReceived, done])).toBe("Goodbye World!"); - // The original socket's listener must not have seen any TLS bytes. expect(Buffer.concat(leakedToOriginal).length).toBe(0); resolveDone(); await done;