diff --git a/src/js/node/tls.ts b/src/js/node/tls.ts index 9bb397a2c3d5..f44d7b3a68b7 100644 --- a/src/js/node/tls.ts +++ b/src/js/node/tls.ts @@ -740,14 +740,14 @@ function TLSSocket(socket?, options?) { throw $ERR_INVALID_ARG_TYPE("socket", "Duplex", socket); } - // The wrapped socket's allowHalfOpen wins: https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L592 + // A wrapped socket keeps its allowHalfOpen and drops onread: https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L592-L596 if (isNetSocketOrDuplex) { - options = { ...options, allowHalfOpen: socket.allowHalfOpen }; + options = { ...options, allowHalfOpen: socket.allowHalfOpen, onread: null }; } else { options = options || socket || {}; const wrapped = options.socket; if (wrapped instanceof Duplex) { - options = { ...options, allowHalfOpen: wrapped.allowHalfOpen }; + options = { ...options, allowHalfOpen: wrapped.allowHalfOpen, onread: null }; } } diff --git a/test/js/node/tls/node-tls-upgrade.test.ts b/test/js/node/tls/node-tls-upgrade.test.ts index 409028ddbea5..d2a603bd1077 100644 --- a/test/js/node/tls/node-tls-upgrade.test.ts +++ b/test/js/node/tls/node-tls-upgrade.test.ts @@ -53,7 +53,8 @@ test("should be able to upgrade a paused socket and also have backpressure on it await promise; socket.off("data", onData); } - for (let i = 0; i < 100; i++) { + // 20 round trips of 128 KiB: 100 took up to 5 s on a debug ASAN build. + for (let i = 0; i < 20; i++) { // upgrade the tlsSocket await doWrite(tlsSocket); } @@ -109,6 +110,69 @@ test.each([ }, ); +// Node only honors onread when the TLS socket dials its own connection: +// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L596 +test("tls.connect({ socket, onread }) ignores onread and emits 'data' (#42419)", async () => { + const server = tls.createServer(certs, socket => { + socket.on("error", () => {}); + socket.end("hello"); + }); + await once(server.listen(0, "127.0.0.1"), "listening"); + try { + const raw = net.connect({ port: (server.address() as net.AddressInfo).port, host: "127.0.0.1" }); + await once(raw, "connect"); + const log: string[] = []; + const tlsSocket = tls.connect({ + socket: raw, + ca: certs.cert, + servername: "localhost", + onread: { buffer: Buffer.alloc(16), callback: (n: number) => log.push(`onread ${n}`) }, + }); + tlsSocket.on("data", data => log.push(`data ${data.length}`)); + tlsSocket.on("error", () => {}); + await once(tlsSocket, "close"); + expect(log).toEqual(["data 5"]); + } finally { + server.close(); + } +}); + +test("new tls.TLSSocket(socket, { isServer: true, onread }) ignores onread and emits 'data' (#42419)", async () => { + const log: string[] = []; + const { promise, resolve, reject } = Promise.withResolvers(); + const server = net.createServer(socket => { + socket.on("error", reject); + const tlsSocket = new tls.TLSSocket(socket, { + isServer: true, + secureContext: tls.createSecureContext(certs), + onread: { buffer: Buffer.alloc(16), callback: (n: number) => log.push(`onread ${n}`) }, + }); + tlsSocket.on("error", reject); + tlsSocket.on("data", data => { + log.push(`data ${data.length}`); + tlsSocket.end(); + }); + tlsSocket.on("close", resolve); + }); + await once(server.listen(0, "127.0.0.1"), "listening"); + let client: tls.TLSSocket | undefined; + try { + client = tls.connect({ + port: (server.address() as net.AddressInfo).port, + host: "127.0.0.1", + ca: certs.cert, + servername: "localhost", + }); + client.on("error", reject); + client.on("secureConnect", () => client!.write("hello")); + await promise; + expect(log).toEqual(["data 5"]); + } finally { + client?.destroy(); + server.close(); + } +}); + // Both peers keep their plaintext 'data' listener across the upgrade. test("a STARTTLS exchange hands no TLS bytes to the 'data' listeners of the wrapped sockets (#32239)", async () => { const saw: string[] = [];