diff --git a/src/js/node/net.ts b/src/js/node/net.ts index 40d65a0f8843..6bb4299aa962 100644 --- a/src/js/node/net.ts +++ b/src/js/node/net.ts @@ -441,6 +441,8 @@ function tlsHandshakeError(verifyError) { // Node reports a throwing 'data' listener as uncaughtException and keeps reading. function pushDataToSocket(self, socket, buffer) { + // TLS took over the fd; the wrapped socket reads nothing: https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L723-L727 + if (socket[kAdoptedTLSRaw]) return; if (self[kDestroyOnRead]) { $debug("DATA on a socket that must receive nothing - destroying it"); self.destroy(); @@ -1768,6 +1770,7 @@ function Socket(options?) { const { self } = socket.data; if (!self) return; self._unrefTimer(); + if (socket[kAdoptedTLSRaw]) return; const tail = self[kOnreadTail]; if (tail !== undefined) { self[kOnreadTail] = Buffer.concat([tail, buffer]); @@ -2345,7 +2348,8 @@ Socket.prototype.resume = function resume() { // override sets handle.reading synchronously for the same reason. const ret = Duplex.prototype.resume.$call(this); // An ended readable side (EOF emitted, or `readable: false`) never restarts the handle: node reaches readStart only from _read. - if (this.readableEnded) return ret; + // An onread socket is the exception: https://github.com/nodejs/node/blob/v26.3.0/lib/net.js#L830-L845 + if (this.readableEnded && this[kOnreadBuffer] === undefined) return ret; if (!this.connecting && !drainOnreadTail(this)) { this._handle?.resume?.(); } @@ -2446,7 +2450,7 @@ Socket.prototype[Symbol.for("::bunUpgradeServerTLS::")] = function (connection, Socket.prototype.read = function read(size) { // See resume(): an ended readable side never restarts the handle. - if (!this.readableEnded && !this.connecting && !drainOnreadTail(this, true)) { + if ((!this.readableEnded || this[kOnreadBuffer] !== undefined) && !this.connecting && !drainOnreadTail(this, true)) { this._handle?.resume?.(); restorePausedHold(this, this._handle); } @@ -3395,7 +3399,7 @@ function afterConnect(status, handle, req, readable, writable) { // Ours already reads, Node's starts at read(): stop a paused plain socket now, and after the listeners unless one asked for a read. // A socket built with `readable: false` never reads: read() cannot reach _read once the readable side has ended. - const pausedBeforeConnect = self.isPaused() || self.readableEnded; + const pausedBeforeConnect = self.isPaused() || (self.readableEnded && self[kOnreadBuffer] === undefined); if (pausedBeforeConnect && !self.encrypted) readStop(self, self._handle); self.emit("connect"); diff --git a/test/js/node/net/node-net.test.ts b/test/js/node/net/node-net.test.ts index acf6472644af..cd2163a30499 100644 --- a/test/js/node/net/node-net.test.ts +++ b/test/js/node/net/node-net.test.ts @@ -1317,6 +1317,36 @@ it("a client dialed with readable: false never reads and keeps writing", async ( } }); +// https://github.com/nodejs/node/blob/v26.3.0/lib/net.js#L830-L845 +it("an onread client dialed with readable: false still reads into its buffer", async () => { + const server = createServer(socket => socket.end("banner")); + await once(server.listen(0, "127.0.0.1"), "listening"); + try { + const done = Promise.withResolvers(); + let got = ""; + const client = connect({ + port: (server.address() as import("node:net").AddressInfo).port, + host: "127.0.0.1", + readable: false, + onread: { + buffer: Buffer.alloc(64), + callback(n: number, buf: Buffer) { + got += buf.toString("latin1", 0, n); + if (got === "banner") done.resolve(got); + }, + }, + }); + const closed = once(client, "close"); + client.on("error", done.reject); + client.on("close", () => done.reject(new Error(`closed after ${JSON.stringify(got)}`))); + expect(await done.promise).toBe("banner"); + client.destroy(); + await closed; + } finally { + server.close(); + } +}); + it("passes readable / writable through to the Duplex like node (a TLSSocket is always a full duplex)", () => { // Values observed under node v26.3.0. const a = new Socket({ readable: false }); diff --git a/test/js/node/tls/node-tls-upgrade.test.ts b/test/js/node/tls/node-tls-upgrade.test.ts index b374abd6ea22..409028ddbea5 100644 --- a/test/js/node/tls/node-tls-upgrade.test.ts +++ b/test/js/node/tls/node-tls-upgrade.test.ts @@ -60,3 +60,91 @@ test("should be able to upgrade a paused socket and also have backpressure on it expect().pass(); }); + +// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L723-L727 +test.each([ + ["readable: false", () => ({ readable: false })], + [ + "an onread buffer", + (saw: string[]) => ({ onread: { buffer: Buffer.alloc(64), callback: (n: number) => saw.push(`onread ${n}`) } }), + ], + ["no reader", () => ({})], +])( + "tls.connect({ socket }) over a net.Socket with %s keeps the TLS bytes off the wrapped socket", + async (_, options) => { + const server = tls.createServer(certs, socket => { + socket.on("error", () => {}); + socket.write("banner"); + socket.on("data", data => socket.write("echo:" + data)); + }); + await once(server.listen(0, "127.0.0.1"), "listening"); + try { + const saw: string[] = []; + const raw = net.connect({ + port: (server.address() as net.AddressInfo).port, + host: "127.0.0.1", + ...options(saw), + }); + const { promise, resolve, reject } = Promise.withResolvers(); + raw.on("error", reject); + await once(raw, "connect"); + const tlsSocket = tls.connect({ socket: raw, ca: certs.cert, servername: "localhost" }); + const closed = once(tlsSocket, "close"); + let got = ""; + tlsSocket.on("error", reject); + tlsSocket.on("close", () => reject(new Error(`closed after ${JSON.stringify(got)}`))); + tlsSocket.on("secureConnect", () => tlsSocket.write("hi")); + tlsSocket.on("data", data => { + got += data; + if (got.endsWith("echo:hi")) resolve(got); + }); + expect(await promise).toBe("bannerecho:hi"); + expect(saw).toEqual([]); + expect(raw.readableLength).toBe(0); + tlsSocket.destroy(); + await closed; + } finally { + 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[] = []; + const { promise, resolve, reject } = Promise.withResolvers(); + const server = net.createServer(socket => { + socket.on("error", reject); + let wrapped = false; + socket.on("data", data => { + if (wrapped) return void saw.push(`server data ${data.length}`); + wrapped = true; + socket.write("GO", () => { + const tlsSocket = new tls.TLSSocket(socket, { isServer: true, secureContext: tls.createSecureContext(certs) }); + tlsSocket.on("error", reject); + tlsSocket.on("data", data => tlsSocket.write("echo:" + data)); + }); + }); + }); + 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" }); + raw.on("error", reject); + let tlsSocket: tls.TLSSocket | undefined; + raw.on("data", data => { + if (tlsSocket) return void saw.push(`client data ${data.length}`); + tlsSocket = tls.connect({ socket: raw, ca: certs.cert, servername: "localhost" }); + tlsSocket.on("error", reject); + tlsSocket.on("secureConnect", () => tlsSocket!.write("hi")); + tlsSocket.on("data", data => resolve(String(data))); + }); + raw.write("STARTTLS"); + expect(await promise).toBe("echo:hi"); + expect(saw).toEqual([]); + const closed = once(tlsSocket!, "close"); + tlsSocket!.destroy(); + await closed; + } finally { + server.close(); + } +});