node:net: fix tls.connect({ socket }) and onread on a readable: false socket

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");

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<string>();
+    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 });

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<string>();
+      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<string>();
+  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();
+  }
+});