From e35dbb026b76afa6490d367ad6f58716102c58be Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 22 Jul 2026 15:43:59 +0000 Subject: [PATCH 1/6] node:tls: keep the native handle reading through the TLS handshake when paused Socket.prototype.pause() called while a TLS handshake is in flight stopped native reads (us_socket_pause sets the poll write-only), so the TLS engine never saw the ClientHello / server Finished and the handshake wedged forever. In Node the TLSWrap reads the underlying fd independently of the TLSSocket's stream state, so a pre-handshake pause() only affects delivery of decrypted output; the handshake and FIN/close_notify proceed. Gate the native pause()/unref() on !secureConnecting so only the Duplex layer pauses. The data handler applies native backpressure at highWaterMark once application data starts arriving. Also make ServerHandlers.handshake honor a handler's stream state (readableFlowing !== null) instead of unconditionally resume()ing, so a pause() made inside the 'connection' or 'secureConnection' handler survives. Covers: - s.pause() inside a TLS server's 'connection' handler - tls.createServer({ pauseOnConnect: true }) - tls.connect({ pauseOnConnect: true }) --- src/js/node/net.ts | 24 +++- ...t-tls-server-parent-constructor-options.js | 68 +++++++++ test/js/node/tls/node-tls-server.test.ts | 133 ++++++++++++++++++ 3 files changed, 220 insertions(+), 5 deletions(-) create mode 100644 test/js/node/test/parallel/test-tls-server-parent-constructor-options.js diff --git a/src/js/node/net.ts b/src/js/node/net.ts index 2046ed40ae41..b824eae36849 100644 --- a/src/js/node/net.ts +++ b/src/js/node/net.ts @@ -881,10 +881,17 @@ const ServerHandlers: SocketHandler = { // after secureConnection event we emmit secure and secureConnect self.emit("secure", self); self.emit("secureConnect", verifyError); - if (server?.pauseOnConnect) { - self.pause(); - } else { - self.resume(); + // readableFlowing === null: no 'connection' / 'secureConnection' handler + // touched the stream state, so apply the server's default. A handler that + // paused (false) or attached 'data'/'readable' (true/false) is honored - + // the post-emit resume() here must not stomp a pause() made inside either + // handler. + if (self.readableFlowing === null) { + if (server?.pauseOnConnect) { + self.pause(); + } else { + self.resume(); + } } }, error(socket, error) { @@ -2110,7 +2117,14 @@ Socket.prototype.resume = function resume() { }; Socket.prototype.pause = function pause() { - if (!this.destroyed) { + // While the TLS handshake is in flight the native handle must keep reading: + // stopping the poll here would starve the TLS engine of the ClientHello / + // server Finished and wedge the handshake forever. In Node, TLSWrap reads + // the underlying socket independently of the TLSSocket's stream state, so a + // pre-handshake pause() only affects delivery of *decrypted* bytes. Mirror + // that: pause only the Duplex; the data handler applies native backpressure + // at highWaterMark once application data starts arriving. + if (!this.destroyed && !this.secureConnecting) { this._handle?.pause?.(); // libuv only counts a stream handle as active - and therefore as keeping // the event loop alive - while it is reading. A paused socket lets the diff --git a/test/js/node/test/parallel/test-tls-server-parent-constructor-options.js b/test/js/node/test/parallel/test-tls-server-parent-constructor-options.js new file mode 100644 index 000000000000..f8b34e8b0cab --- /dev/null +++ b/test/js/node/test/parallel/test-tls-server-parent-constructor-options.js @@ -0,0 +1,68 @@ +'use strict'; +const common = require('../common'); + +if (!common.hasCrypto) + common.skip('missing crypto'); + +// Test that `tls.Server` constructor options are passed to the parent +// constructor. + +const assert = require('assert'); +const fixtures = require('../common/fixtures'); +const tls = require('tls'); + +const options = { + key: fixtures.readKey('agent1-key.pem'), + cert: fixtures.readKey('agent1-cert.pem'), +}; + +{ + const server = tls.createServer(options, common.mustCall((socket) => { + assert.strictEqual(socket.allowHalfOpen, false); + assert.strictEqual(socket.isPaused(), false); + })); + + assert.strictEqual(server.allowHalfOpen, false); + assert.strictEqual(server.pauseOnConnect, false); + + server.listen(0, common.mustCall(() => { + const socket = tls.connect({ + port: server.address().port, + rejectUnauthorized: false + }, common.mustCall(() => { + socket.end(); + })); + + socket.on('close', () => { + server.close(); + }); + })); +} + +{ + const server = tls.createServer({ + allowHalfOpen: true, + pauseOnConnect: true, + ...options + }, common.mustCall((socket) => { + assert.strictEqual(socket.allowHalfOpen, true); + assert.strictEqual(socket.isPaused(), true); + socket.on('end', socket.end); + })); + + assert.strictEqual(server.allowHalfOpen, true); + assert.strictEqual(server.pauseOnConnect, true); + + server.listen(0, common.mustCall(() => { + const socket = tls.connect({ + port: server.address().port, + rejectUnauthorized: false + }, common.mustCall(() => { + socket.end(); + })); + + socket.on('close', () => { + server.close(); + }); + })); +} diff --git a/test/js/node/tls/node-tls-server.test.ts b/test/js/node/tls/node-tls-server.test.ts index 8042dd49db79..6e94931cc0ad 100644 --- a/test/js/node/tls/node-tls-server.test.ts +++ b/test/js/node/tls/node-tls-server.test.ts @@ -1303,3 +1303,136 @@ it("tls.connect honors secureOptions when negotiating the protocol version", asy } await once(server, "close"); }); + +describe("pausing a TLS socket before the handshake does not stall it", () => { + // Before the fix Socket.prototype.pause() stopped native reads unconditionally, + // so a pre-handshake pause() starved the TLS engine of the ClientHello and the + // handshake never completed. Node's TLSWrap keeps reading the underlying fd + // regardless of the TLSSocket's stream state; these tests match that. + + it("server: s.pause() inside the 'connection' handler", async () => { + const server: Server = createServer(COMMON_CERT); + let connSock: TLSSocket | undefined; + server.on("connection", s => { + connSock = s as TLSSocket; + s.pause(); + }); + const accepted = Promise.withResolvers(); + server.on("secureConnection", s => accepted.resolve(s)); + server.on("tlsClientError", accepted.reject); + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + let cli: TLSSocket | undefined; + let srv: TLSSocket | undefined; + try { + const port = (server.address() as AddressInfo).port; + cli = connect({ port, host: "127.0.0.1", rejectUnauthorized: false }); + cli.on("error", () => {}); + // Before the fix this hung: the native poll was switched to write-only + // and the TLS engine never saw the ClientHello. + await once(cli, "secureConnect"); + srv = await accepted.promise; + expect({ paused: srv.isPaused(), flowing: srv.readableFlowing }).toEqual({ paused: true, flowing: false }); + + let stopped = true; + let got = ""; + srv.on("data", d => { + if (stopped) throw new Error("data event fired while paused"); + got += d; + }); + cli.write("hello"); + // Round-trip the other way so we know the client's write has traversed + // the event loop on the server side while still paused. + srv.write("ack"); + expect((await once(cli, "data"))[0].toString()).toBe("ack"); + expect({ got, flowing: srv.readableFlowing, readableLength: srv.readableLength }).toEqual({ + got: "", + flowing: false, + readableLength: 5, + }); + stopped = false; + const dataP = once(srv, "data"); + srv.resume(); + await dataP; + expect(got).toBe("hello"); + } finally { + cli?.destroy(); + srv?.destroy(); + connSock?.destroy(); + server.close(); + } + await once(server, "close"); + }); + + it("server: pauseOnConnect: true", async () => { + const server: Server = createServer({ ...COMMON_CERT, pauseOnConnect: true }); + const accepted = Promise.withResolvers<{ paused: boolean; flowing: boolean | null; socket: TLSSocket }>(); + server.on("secureConnection", s => + accepted.resolve({ paused: s.isPaused(), flowing: s.readableFlowing, socket: s }), + ); + server.on("tlsClientError", accepted.reject); + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + let cli: TLSSocket | undefined; + let srv: TLSSocket | undefined; + try { + const port = (server.address() as AddressInfo).port; + cli = connect({ port, host: "127.0.0.1", rejectUnauthorized: false }); + cli.on("error", () => {}); + await once(cli, "secureConnect"); + const { paused, flowing, socket } = await accepted.promise; + srv = socket; + expect({ paused, flowing }).toEqual({ paused: true, flowing: false }); + + cli.write("hello"); + srv.write("ack"); + expect((await once(cli, "data"))[0].toString()).toBe("ack"); + expect({ flowing: srv.readableFlowing, readableLength: srv.readableLength }).toEqual({ + flowing: false, + readableLength: 5, + }); + srv.resume(); + expect((await once(srv, "data"))[0].toString()).toBe("hello"); + } finally { + cli?.destroy(); + srv?.destroy(); + server.close(); + } + await once(server, "close"); + }); + + it("client: pauseOnConnect: true", async () => { + const server: Server = createServer(COMMON_CERT); + const accepted = Promise.withResolvers(); + server.on("secureConnection", s => accepted.resolve(s)); + server.on("tlsClientError", accepted.reject); + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + let cli: TLSSocket | undefined; + let srv: TLSSocket | undefined; + try { + const port = (server.address() as AddressInfo).port; + cli = connect({ port, host: "127.0.0.1", rejectUnauthorized: false, pauseOnConnect: true }); + cli.on("error", () => {}); + // Before the fix SocketHandlers2.open called self.pause() -> native + // pause and the handshake never completed. + await once(cli, "secureConnect"); + srv = await accepted.promise; + // Sanity: data flows both ways. The client side is resumed explicitly + // because Node leaves the TLSSocket at readableFlowing === null here + // (pauseOnConnect applies to the underlying net.Socket, which is a + // separate object in Node) while Bun preserves the pause() on the same + // TLSSocket instance; resuming makes the observable match either way. + cli.resume(); + srv.write("from-server"); + expect((await once(cli, "data"))[0].toString()).toBe("from-server"); + cli.write("from-client"); + expect((await once(srv, "data"))[0].toString()).toBe("from-client"); + } finally { + cli?.destroy(); + srv?.destroy(); + server.close(); + } + await once(server, "close"); + }); +}); From 957e354e9b0804c2732d30ae77d2631a5c2ef08d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 22 Jul 2026 16:39:29 +0000 Subject: [PATCH 2/6] address review: poll readableLength directly, add secureConnection-handler coverage, clarify Node-divergence comments --- src/js/node/net.ts | 18 ++---- test/js/node/tls/node-tls-server.test.ts | 76 ++++++++++++++++++------ 2 files changed, 65 insertions(+), 29 deletions(-) diff --git a/src/js/node/net.ts b/src/js/node/net.ts index b824eae36849..121ea2d962a9 100644 --- a/src/js/node/net.ts +++ b/src/js/node/net.ts @@ -881,11 +881,9 @@ const ServerHandlers: SocketHandler = { // after secureConnection event we emmit secure and secureConnect self.emit("secure", self); self.emit("secureConnect", verifyError); - // readableFlowing === null: no 'connection' / 'secureConnection' handler - // touched the stream state, so apply the server's default. A handler that - // paused (false) or attached 'data'/'readable' (true/false) is honored - - // the post-emit resume() here must not stomp a pause() made inside either - // handler. + // null: no 'connection'/'secureConnection' handler touched the stream + // state, so apply the server's default. A handler that paused (false) or + // attached 'data'/'readable' is honored; resume() must not stomp it. if (self.readableFlowing === null) { if (server?.pauseOnConnect) { self.pause(); @@ -2117,13 +2115,9 @@ Socket.prototype.resume = function resume() { }; Socket.prototype.pause = function pause() { - // While the TLS handshake is in flight the native handle must keep reading: - // stopping the poll here would starve the TLS engine of the ClientHello / - // server Finished and wedge the handshake forever. In Node, TLSWrap reads - // the underlying socket independently of the TLSSocket's stream state, so a - // pre-handshake pause() only affects delivery of *decrypted* bytes. Mirror - // that: pause only the Duplex; the data handler applies native backpressure - // at highWaterMark once application data starts arriving. + // secureConnecting: the native handle must keep reading so the TLS engine + // sees the handshake (Node's TLSWrap reads independently of stream state). + // Pause only the Duplex; the data handler applies native backpressure at HWM. if (!this.destroyed && !this.secureConnecting) { this._handle?.pause?.(); // libuv only counts a stream handle as active - and therefore as keeping diff --git a/test/js/node/tls/node-tls-server.test.ts b/test/js/node/tls/node-tls-server.test.ts index 6e94931cc0ad..f617f437ba45 100644 --- a/test/js/node/tls/node-tls-server.test.ts +++ b/test/js/node/tls/node-tls-server.test.ts @@ -1305,10 +1305,15 @@ it("tls.connect honors secureOptions when negotiating the protocol version", asy }); describe("pausing a TLS socket before the handshake does not stall it", () => { - // Before the fix Socket.prototype.pause() stopped native reads unconditionally, - // so a pre-handshake pause() starved the TLS engine of the ClientHello and the - // handshake never completed. Node's TLSWrap keeps reading the underlying fd - // regardless of the TLSSocket's stream state; these tests match that. + // Socket.prototype.pause() previously stopped native reads unconditionally, + // starving the TLS engine of the ClientHello. The Node-matching observable + // is that the handshake completes; where the post-handshake readable state + // diverges from Node (Bun hands the same TLSSocket to 'connection' and + // 'secureConnection', Node delivers separate objects) the test says so. + + async function waitFor(cond: () => boolean) { + for (let i = 0; !cond() && i < 2000; i++) await new Promise(r => setImmediate(r)); + } it("server: s.pause() inside the 'connection' handler", async () => { const server: Server = createServer(COMMON_CERT); @@ -1332,6 +1337,9 @@ describe("pausing a TLS socket before the handshake does not stall it", () => { // and the TLS engine never saw the ClientHello. await once(cli, "secureConnect"); srv = await accepted.promise; + // Bun delivers the same TLSSocket to 'connection' and 'secureConnection', + // so the pause() is visible here; in Node `srv` is a separate object + // with readableFlowing === null and these assertions would not hold. expect({ paused: srv.isPaused(), flowing: srv.readableFlowing }).toEqual({ paused: true, flowing: false }); let stopped = true; @@ -1341,10 +1349,9 @@ describe("pausing a TLS socket before the handshake does not stall it", () => { got += d; }); cli.write("hello"); - // Round-trip the other way so we know the client's write has traversed - // the event loop on the server side while still paused. - srv.write("ack"); - expect((await once(cli, "data"))[0].toString()).toBe("ack"); + // Await the actual observable; a reverse round-trip is not a barrier + // because kqueue/IOCP do not order ready-fd dispatch within a batch. + await waitFor(() => srv!.readableLength >= 5); expect({ got, flowing: srv.readableFlowing, readableLength: srv.readableLength }).toEqual({ got: "", flowing: false, @@ -1382,11 +1389,12 @@ describe("pausing a TLS socket before the handshake does not stall it", () => { await once(cli, "secureConnect"); const { paused, flowing, socket } = await accepted.promise; srv = socket; + // Node also reports paused:true / flowing:false here + // (test-tls-server-parent-constructor-options). expect({ paused, flowing }).toEqual({ paused: true, flowing: false }); cli.write("hello"); - srv.write("ack"); - expect((await once(cli, "data"))[0].toString()).toBe("ack"); + await waitFor(() => srv!.readableLength >= 5); expect({ flowing: srv.readableFlowing, readableLength: srv.readableLength }).toEqual({ flowing: false, readableLength: 5, @@ -1401,6 +1409,41 @@ describe("pausing a TLS socket before the handshake does not stall it", () => { await once(server, "close"); }); + it("server: s.pause() inside the 'secureConnection' handler", async () => { + // Exercises the readableFlowing === null gate in ServerHandlers.handshake: + // the post-emit resume() must not stomp a pause() made inside the handler. + const server: Server = createServer(COMMON_CERT); + const accepted = Promise.withResolvers(); + server.on("secureConnection", s => { + s.pause(); + accepted.resolve(s); + }); + server.on("tlsClientError", accepted.reject); + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + let cli: TLSSocket | undefined; + let srv: TLSSocket | undefined; + try { + const port = (server.address() as AddressInfo).port; + cli = connect({ port, host: "127.0.0.1", rejectUnauthorized: false }); + cli.on("error", () => {}); + await once(cli, "secureConnect"); + srv = await accepted.promise; + // Before the fix the post-emit resume() flipped this back to + // paused:false / flowing:true. + expect({ paused: srv.isPaused(), flowing: srv.readableFlowing }).toEqual({ paused: true, flowing: false }); + + cli.write("hello"); + srv.resume(); + expect((await once(srv, "data"))[0].toString()).toBe("hello"); + } finally { + cli?.destroy(); + srv?.destroy(); + server.close(); + } + await once(server, "close"); + }); + it("client: pauseOnConnect: true", async () => { const server: Server = createServer(COMMON_CERT); const accepted = Promise.withResolvers(); @@ -1414,15 +1457,14 @@ describe("pausing a TLS socket before the handshake does not stall it", () => { const port = (server.address() as AddressInfo).port; cli = connect({ port, host: "127.0.0.1", rejectUnauthorized: false, pauseOnConnect: true }); cli.on("error", () => {}); - // Before the fix SocketHandlers2.open called self.pause() -> native - // pause and the handshake never completed. + // Before the fix SocketHandlers2.open's self.pause() stopped native + // reads and the handshake never completed. await once(cli, "secureConnect"); srv = await accepted.promise; - // Sanity: data flows both ways. The client side is resumed explicitly - // because Node leaves the TLSSocket at readableFlowing === null here - // (pauseOnConnect applies to the underlying net.Socket, which is a - // separate object in Node) while Bun preserves the pause() on the same - // TLSSocket instance; resuming makes the observable match either way. + // Bun pauses the returned TLSSocket here (isPaused()===true); Node leaves + // it at readableFlowing===null (pauseOnConnect applies to the separate + // underlying net.Socket). Pre-existing divergence; this PR fixes only + // the handshake stall, so resume explicitly and assert data flows. cli.resume(); srv.write("from-server"); expect((await once(cli, "data"))[0].toString()).toBe("from-server"); From 125ae76075946f4f6c80c71d12c1b80046783e77 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 22 Jul 2026 16:52:11 +0000 Subject: [PATCH 3/6] move tests to their own file; node-tls-server.test.ts has an unrelated pre-existing failure --- test/js/node/tls/node-tls-server.test.ts | 175 ------------------ test/js/node/tls/tls-pause-handshake.test.ts | 180 +++++++++++++++++++ 2 files changed, 180 insertions(+), 175 deletions(-) create mode 100644 test/js/node/tls/tls-pause-handshake.test.ts diff --git a/test/js/node/tls/node-tls-server.test.ts b/test/js/node/tls/node-tls-server.test.ts index f617f437ba45..8042dd49db79 100644 --- a/test/js/node/tls/node-tls-server.test.ts +++ b/test/js/node/tls/node-tls-server.test.ts @@ -1303,178 +1303,3 @@ it("tls.connect honors secureOptions when negotiating the protocol version", asy } await once(server, "close"); }); - -describe("pausing a TLS socket before the handshake does not stall it", () => { - // Socket.prototype.pause() previously stopped native reads unconditionally, - // starving the TLS engine of the ClientHello. The Node-matching observable - // is that the handshake completes; where the post-handshake readable state - // diverges from Node (Bun hands the same TLSSocket to 'connection' and - // 'secureConnection', Node delivers separate objects) the test says so. - - async function waitFor(cond: () => boolean) { - for (let i = 0; !cond() && i < 2000; i++) await new Promise(r => setImmediate(r)); - } - - it("server: s.pause() inside the 'connection' handler", async () => { - const server: Server = createServer(COMMON_CERT); - let connSock: TLSSocket | undefined; - server.on("connection", s => { - connSock = s as TLSSocket; - s.pause(); - }); - const accepted = Promise.withResolvers(); - server.on("secureConnection", s => accepted.resolve(s)); - server.on("tlsClientError", accepted.reject); - server.listen(0, "127.0.0.1"); - await once(server, "listening"); - let cli: TLSSocket | undefined; - let srv: TLSSocket | undefined; - try { - const port = (server.address() as AddressInfo).port; - cli = connect({ port, host: "127.0.0.1", rejectUnauthorized: false }); - cli.on("error", () => {}); - // Before the fix this hung: the native poll was switched to write-only - // and the TLS engine never saw the ClientHello. - await once(cli, "secureConnect"); - srv = await accepted.promise; - // Bun delivers the same TLSSocket to 'connection' and 'secureConnection', - // so the pause() is visible here; in Node `srv` is a separate object - // with readableFlowing === null and these assertions would not hold. - expect({ paused: srv.isPaused(), flowing: srv.readableFlowing }).toEqual({ paused: true, flowing: false }); - - let stopped = true; - let got = ""; - srv.on("data", d => { - if (stopped) throw new Error("data event fired while paused"); - got += d; - }); - cli.write("hello"); - // Await the actual observable; a reverse round-trip is not a barrier - // because kqueue/IOCP do not order ready-fd dispatch within a batch. - await waitFor(() => srv!.readableLength >= 5); - expect({ got, flowing: srv.readableFlowing, readableLength: srv.readableLength }).toEqual({ - got: "", - flowing: false, - readableLength: 5, - }); - stopped = false; - const dataP = once(srv, "data"); - srv.resume(); - await dataP; - expect(got).toBe("hello"); - } finally { - cli?.destroy(); - srv?.destroy(); - connSock?.destroy(); - server.close(); - } - await once(server, "close"); - }); - - it("server: pauseOnConnect: true", async () => { - const server: Server = createServer({ ...COMMON_CERT, pauseOnConnect: true }); - const accepted = Promise.withResolvers<{ paused: boolean; flowing: boolean | null; socket: TLSSocket }>(); - server.on("secureConnection", s => - accepted.resolve({ paused: s.isPaused(), flowing: s.readableFlowing, socket: s }), - ); - server.on("tlsClientError", accepted.reject); - server.listen(0, "127.0.0.1"); - await once(server, "listening"); - let cli: TLSSocket | undefined; - let srv: TLSSocket | undefined; - try { - const port = (server.address() as AddressInfo).port; - cli = connect({ port, host: "127.0.0.1", rejectUnauthorized: false }); - cli.on("error", () => {}); - await once(cli, "secureConnect"); - const { paused, flowing, socket } = await accepted.promise; - srv = socket; - // Node also reports paused:true / flowing:false here - // (test-tls-server-parent-constructor-options). - expect({ paused, flowing }).toEqual({ paused: true, flowing: false }); - - cli.write("hello"); - await waitFor(() => srv!.readableLength >= 5); - expect({ flowing: srv.readableFlowing, readableLength: srv.readableLength }).toEqual({ - flowing: false, - readableLength: 5, - }); - srv.resume(); - expect((await once(srv, "data"))[0].toString()).toBe("hello"); - } finally { - cli?.destroy(); - srv?.destroy(); - server.close(); - } - await once(server, "close"); - }); - - it("server: s.pause() inside the 'secureConnection' handler", async () => { - // Exercises the readableFlowing === null gate in ServerHandlers.handshake: - // the post-emit resume() must not stomp a pause() made inside the handler. - const server: Server = createServer(COMMON_CERT); - const accepted = Promise.withResolvers(); - server.on("secureConnection", s => { - s.pause(); - accepted.resolve(s); - }); - server.on("tlsClientError", accepted.reject); - server.listen(0, "127.0.0.1"); - await once(server, "listening"); - let cli: TLSSocket | undefined; - let srv: TLSSocket | undefined; - try { - const port = (server.address() as AddressInfo).port; - cli = connect({ port, host: "127.0.0.1", rejectUnauthorized: false }); - cli.on("error", () => {}); - await once(cli, "secureConnect"); - srv = await accepted.promise; - // Before the fix the post-emit resume() flipped this back to - // paused:false / flowing:true. - expect({ paused: srv.isPaused(), flowing: srv.readableFlowing }).toEqual({ paused: true, flowing: false }); - - cli.write("hello"); - srv.resume(); - expect((await once(srv, "data"))[0].toString()).toBe("hello"); - } finally { - cli?.destroy(); - srv?.destroy(); - server.close(); - } - await once(server, "close"); - }); - - it("client: pauseOnConnect: true", async () => { - const server: Server = createServer(COMMON_CERT); - const accepted = Promise.withResolvers(); - server.on("secureConnection", s => accepted.resolve(s)); - server.on("tlsClientError", accepted.reject); - server.listen(0, "127.0.0.1"); - await once(server, "listening"); - let cli: TLSSocket | undefined; - let srv: TLSSocket | undefined; - try { - const port = (server.address() as AddressInfo).port; - cli = connect({ port, host: "127.0.0.1", rejectUnauthorized: false, pauseOnConnect: true }); - cli.on("error", () => {}); - // Before the fix SocketHandlers2.open's self.pause() stopped native - // reads and the handshake never completed. - await once(cli, "secureConnect"); - srv = await accepted.promise; - // Bun pauses the returned TLSSocket here (isPaused()===true); Node leaves - // it at readableFlowing===null (pauseOnConnect applies to the separate - // underlying net.Socket). Pre-existing divergence; this PR fixes only - // the handshake stall, so resume explicitly and assert data flows. - cli.resume(); - srv.write("from-server"); - expect((await once(cli, "data"))[0].toString()).toBe("from-server"); - cli.write("from-client"); - expect((await once(srv, "data"))[0].toString()).toBe("from-client"); - } finally { - cli?.destroy(); - srv?.destroy(); - server.close(); - } - await once(server, "close"); - }); -}); diff --git a/test/js/node/tls/tls-pause-handshake.test.ts b/test/js/node/tls/tls-pause-handshake.test.ts new file mode 100644 index 000000000000..c977877ed9b2 --- /dev/null +++ b/test/js/node/tls/tls-pause-handshake.test.ts @@ -0,0 +1,180 @@ +import { tls as COMMON_CERT } from "harness"; +import type { AddressInfo } from "net"; +import { once } from "node:events"; +import { connect, createServer, Server, TLSSocket } from "tls"; +import { describe, expect, it } from "bun:test"; + +describe("pausing a TLS socket before the handshake does not stall it", () => { + // Socket.prototype.pause() previously stopped native reads unconditionally, + // starving the TLS engine of the ClientHello. The Node-matching observable + // is that the handshake completes; where the post-handshake readable state + // diverges from Node (Bun hands the same TLSSocket to 'connection' and + // 'secureConnection', Node delivers separate objects) the test says so. + + async function waitFor(cond: () => boolean) { + for (let i = 0; !cond() && i < 2000; i++) await new Promise(r => setImmediate(r)); + } + + it("server: s.pause() inside the 'connection' handler", async () => { + const server: Server = createServer(COMMON_CERT); + let connSock: TLSSocket | undefined; + server.on("connection", s => { + connSock = s as TLSSocket; + s.pause(); + }); + const accepted = Promise.withResolvers(); + server.on("secureConnection", s => accepted.resolve(s)); + server.on("tlsClientError", accepted.reject); + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + let cli: TLSSocket | undefined; + let srv: TLSSocket | undefined; + try { + const port = (server.address() as AddressInfo).port; + cli = connect({ port, host: "127.0.0.1", rejectUnauthorized: false }); + cli.on("error", () => {}); + // Before the fix this hung: the native poll was switched to write-only + // and the TLS engine never saw the ClientHello. + await once(cli, "secureConnect"); + srv = await accepted.promise; + // Bun delivers the same TLSSocket to 'connection' and 'secureConnection', + // so the pause() is visible here; in Node `srv` is a separate object + // with readableFlowing === null and these assertions would not hold. + expect({ paused: srv.isPaused(), flowing: srv.readableFlowing }).toEqual({ paused: true, flowing: false }); + + let stopped = true; + let got = ""; + srv.on("data", d => { + if (stopped) throw new Error("data event fired while paused"); + got += d; + }); + cli.write("hello"); + // Await the actual observable; a reverse round-trip is not a barrier + // because kqueue/IOCP do not order ready-fd dispatch within a batch. + await waitFor(() => srv!.readableLength >= 5); + expect({ got, flowing: srv.readableFlowing, readableLength: srv.readableLength }).toEqual({ + got: "", + flowing: false, + readableLength: 5, + }); + stopped = false; + const dataP = once(srv, "data"); + srv.resume(); + await dataP; + expect(got).toBe("hello"); + } finally { + cli?.destroy(); + srv?.destroy(); + connSock?.destroy(); + server.close(); + } + await once(server, "close"); + }); + + it("server: pauseOnConnect: true", async () => { + const server: Server = createServer({ ...COMMON_CERT, pauseOnConnect: true }); + const accepted = Promise.withResolvers<{ paused: boolean; flowing: boolean | null; socket: TLSSocket }>(); + server.on("secureConnection", s => + accepted.resolve({ paused: s.isPaused(), flowing: s.readableFlowing, socket: s }), + ); + server.on("tlsClientError", accepted.reject); + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + let cli: TLSSocket | undefined; + let srv: TLSSocket | undefined; + try { + const port = (server.address() as AddressInfo).port; + cli = connect({ port, host: "127.0.0.1", rejectUnauthorized: false }); + cli.on("error", () => {}); + await once(cli, "secureConnect"); + const { paused, flowing, socket } = await accepted.promise; + srv = socket; + // Node also reports paused:true / flowing:false here + // (test-tls-server-parent-constructor-options). + expect({ paused, flowing }).toEqual({ paused: true, flowing: false }); + + cli.write("hello"); + await waitFor(() => srv!.readableLength >= 5); + expect({ flowing: srv.readableFlowing, readableLength: srv.readableLength }).toEqual({ + flowing: false, + readableLength: 5, + }); + srv.resume(); + expect((await once(srv, "data"))[0].toString()).toBe("hello"); + } finally { + cli?.destroy(); + srv?.destroy(); + server.close(); + } + await once(server, "close"); + }); + + it("server: s.pause() inside the 'secureConnection' handler", async () => { + // Exercises the readableFlowing === null gate in ServerHandlers.handshake: + // the post-emit resume() must not stomp a pause() made inside the handler. + const server: Server = createServer(COMMON_CERT); + const accepted = Promise.withResolvers(); + server.on("secureConnection", s => { + s.pause(); + accepted.resolve(s); + }); + server.on("tlsClientError", accepted.reject); + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + let cli: TLSSocket | undefined; + let srv: TLSSocket | undefined; + try { + const port = (server.address() as AddressInfo).port; + cli = connect({ port, host: "127.0.0.1", rejectUnauthorized: false }); + cli.on("error", () => {}); + await once(cli, "secureConnect"); + srv = await accepted.promise; + // Before the fix the post-emit resume() flipped this back to + // paused:false / flowing:true. + expect({ paused: srv.isPaused(), flowing: srv.readableFlowing }).toEqual({ paused: true, flowing: false }); + + cli.write("hello"); + srv.resume(); + expect((await once(srv, "data"))[0].toString()).toBe("hello"); + } finally { + cli?.destroy(); + srv?.destroy(); + server.close(); + } + await once(server, "close"); + }); + + it("client: pauseOnConnect: true", async () => { + const server: Server = createServer(COMMON_CERT); + const accepted = Promise.withResolvers(); + server.on("secureConnection", s => accepted.resolve(s)); + server.on("tlsClientError", accepted.reject); + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + let cli: TLSSocket | undefined; + let srv: TLSSocket | undefined; + try { + const port = (server.address() as AddressInfo).port; + cli = connect({ port, host: "127.0.0.1", rejectUnauthorized: false, pauseOnConnect: true }); + cli.on("error", () => {}); + // Before the fix SocketHandlers2.open's self.pause() stopped native + // reads and the handshake never completed. + await once(cli, "secureConnect"); + srv = await accepted.promise; + // Bun pauses the returned TLSSocket here (isPaused()===true); Node leaves + // it at readableFlowing===null (pauseOnConnect applies to the separate + // underlying net.Socket). Pre-existing divergence; this PR fixes only + // the handshake stall, so resume explicitly and assert data flows. + cli.resume(); + srv.write("from-server"); + expect((await once(cli, "data"))[0].toString()).toBe("from-server"); + cli.write("from-client"); + expect((await once(srv, "data"))[0].toString()).toBe("from-client"); + } finally { + cli?.destroy(); + srv?.destroy(); + server.close(); + } + await once(server, "close"); + }); +}); From 3757edc24975c8250caecd8018cb81c60f599860 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Wed, 22 Jul 2026 16:54:28 +0000 Subject: [PATCH 4/6] [autofix.ci] apply automated fixes --- test/js/node/tls/tls-pause-handshake.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/js/node/tls/tls-pause-handshake.test.ts b/test/js/node/tls/tls-pause-handshake.test.ts index c977877ed9b2..16b5437384ed 100644 --- a/test/js/node/tls/tls-pause-handshake.test.ts +++ b/test/js/node/tls/tls-pause-handshake.test.ts @@ -1,8 +1,8 @@ +import { describe, expect, it } from "bun:test"; import { tls as COMMON_CERT } from "harness"; import type { AddressInfo } from "net"; import { once } from "node:events"; import { connect, createServer, Server, TLSSocket } from "tls"; -import { describe, expect, it } from "bun:test"; describe("pausing a TLS socket before the handshake does not stall it", () => { // Socket.prototype.pause() previously stopped native reads unconditionally, From 63889213d534bffe9ce7e0794068fc934dde5398 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 22 Jul 2026 16:56:44 +0000 Subject: [PATCH 5/6] test: capture 'data while paused' as a flag instead of throwing from the event callback --- test/js/node/tls/tls-pause-handshake.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/js/node/tls/tls-pause-handshake.test.ts b/test/js/node/tls/tls-pause-handshake.test.ts index 16b5437384ed..6ecfb7b8db52 100644 --- a/test/js/node/tls/tls-pause-handshake.test.ts +++ b/test/js/node/tls/tls-pause-handshake.test.ts @@ -43,16 +43,18 @@ describe("pausing a TLS socket before the handshake does not stall it", () => { expect({ paused: srv.isPaused(), flowing: srv.readableFlowing }).toEqual({ paused: true, flowing: false }); let stopped = true; + let firedWhilePaused = false; let got = ""; srv.on("data", d => { - if (stopped) throw new Error("data event fired while paused"); + if (stopped) firedWhilePaused = true; got += d; }); cli.write("hello"); // Await the actual observable; a reverse round-trip is not a barrier // because kqueue/IOCP do not order ready-fd dispatch within a batch. await waitFor(() => srv!.readableLength >= 5); - expect({ got, flowing: srv.readableFlowing, readableLength: srv.readableLength }).toEqual({ + expect({ firedWhilePaused, got, flowing: srv.readableFlowing, readableLength: srv.readableLength }).toEqual({ + firedWhilePaused: false, got: "", flowing: false, readableLength: 5, From bf584bdc6b31ae5923750ef9bdd2753625cbc134 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 23 Jul 2026 05:16:15 +0000 Subject: [PATCH 6/6] ServerHandlers.handshake: pause only the Duplex for pauseOnConnect so FIN/close_notify still arrive --- src/js/node/net.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/js/node/net.ts b/src/js/node/net.ts index 02aa875bb2c2..491063a3de43 100644 --- a/src/js/node/net.ts +++ b/src/js/node/net.ts @@ -910,7 +910,10 @@ const ServerHandlers: SocketHandler = { } const pauseOnConnect = server && (server.pauseOnConnect ?? server[bunSocketServerOptions]?.pauseOnConnect); if (pauseOnConnect) { - self.pause(); + // Duplex only: the native handle keeps reading so close_notify/FIN still + // arrive (Node buffers decrypted bytes on a paused TLSSocket and emits + // 'end'); ServerHandlers.data applies native backpressure at HWM. + Duplex.prototype.pause.$call(self); } if (server) { const connectionListener = server[bunSocketServerOptions]?.connectionListener;