node:tls: use the stream engine for server-side wraps the native upgrade cannot adopt

src/js/node/net.ts

@@ -2352,78 +2352,102 @@ Socket.prototype.pause = function pause() {
 // state carried via `data` (mirrors tls.createServer's one-handler-for-all model).
 Socket.prototype[Symbol.for("::bunUpgradeServerTLS::")] = function (connection, tls) {
   const socket = connection._handle;
-  if (!socket || connection.encrypted || hasUnflushedWrites(connection)) {
-    // No adoptable fd (generic Duplex / not yet connected), TLS over TLS (the
-    // fd belongs to the outer SSL layer), or pending plain writes that must
-    // flush first: run the TLS engine over the stream itself.
-    const [result, events] = upgradeDuplexToTLS(connection, {
-      data: this,
-      tls,
-      socket: serverHandlersFor(this),
-      isServer: true,
-    });
-    connection.on("data", events[0]);
-    connection.on("end", events[1]);
-    connection.on("drain", events[2]);
-    connection.on("close", events[3]);
+  if (!socket || isNamedPipeSocket(socket) || connection.encrypted || hasUnflushedWrites(connection)) {
+    // No fd to adopt (Duplex, named pipe), TLS over TLS, or plain writes still queued: TLS engine over the stream.
+    attachServerTLSEngine(this, connection, tls);
     this[kupgraded] = connection;
-    this._handle = result;
     return;
   }
   this[kupgraded] = connection;
-  process.nextTick(() => {
-    if (this.destroyed || connection.destroyed) {
-      this.destroy();
-      return;
-    }
-    const handle = connection._handle;
-    if (!handle) {
+  if (connection.connecting) {
+    // upgradeTLS needs an established socket: adopt once connected; a failed connect closes the wrap instead.
+    const onConnect = () => {
+      connection.removeListener("close", onClose);
+      process.nextTick(adoptServerTLS, this, connection, tls);
+    };
+    const onClose = () => {
+      connection.removeListener("connect", onConnect);
       this.destroy();
+    };
+    connection.once("connect", onConnect);
+    connection.once("close", onClose);
+    return;
+  }
+  process.nextTick(adoptServerTLS, this, connection, tls);
+};
+
+function attachServerTLSEngine(self, connection, tls) {
+  const [result, events] = upgradeDuplexToTLS(connection, {
+    data: self,
+    tls,
+    socket: serverHandlersFor(self),
+    isServer: true,
+  });
+  connection.on("data", events[0]);
+  connection.on("end", events[1]);
+  connection.on("drain", events[2]);
+  connection.on("close", events[3]);
+  self._handle = result;
+}
+
+// The wrap reports the error and the connection dies with it: an inline-wrapped connection has no other owner.
+function failServerAdoption(self, connection, err) {
+  self._handle = null;
+  connection.destroy();
+  self.destroy(err);
+}
+
+// Deferred a tick so plain writes made by later 'connection' listeners are seen below.
+function adoptServerTLS(self, connection, tls) {
+  if (self.destroyed || connection.destroyed) {
+    failServerAdoption(self, connection);
+    return;
+  }
+  // Re-read: family autoselection swaps handles while connecting, and the result may be a pipe.
+  const handle = connection._handle;
+  if (!handle) {
+    failServerAdoption(self, connection);
+    return;
+  }
+  if (isNamedPipeSocket(handle) || hasUnflushedWrites(connection)) {
+    try {
+      attachServerTLSEngine(self, connection, tls);
+    } catch (err) {
+      // No caller to throw to here, unlike the synchronous engine branch.
+      failServerAdoption(self, connection, err);
       return;
     }
-    // Writes may have been queued between the wrap and this tick (a user
-    // 'connection' listener runs after the server's): those bytes must flush
-    // before any TLS output, so fall back to the stream-level engine.
-    if (hasUnflushedWrites(connection)) {
-      const [result, events] = upgradeDuplexToTLS(connection, {
-        data: this,
-        tls,
-        socket: serverHandlersFor(this),
-        isServer: true,
-      });
-      connection.on("data", events[0]);
-      connection.on("end", events[1]);
-      connection.on("drain", events[2]);
-      connection.on("close", events[3]);
-      this._handle = result;
-      this.emit(kUpgradeAttached);
-      return;
-    }
-    // Bytes that already arrived before the wrap were pulled off the fd into
-    // the connection's readable buffer; hand them to the TLS engine so the
-    // handshake doesn't stall.
-    const pending = connection.read();
-    const result = handle.upgradeTLS({
-      data: this,
+    self.emit(kUpgradeAttached);
+    return;
+  }
+  // Bytes already pulled off the fd (the ClientHello) go to the TLS engine.
+  const pending = connection.read();
+  let result;
+  try {
+    result = handle.upgradeTLS({
+      data: self,
       tls,
-      socket: serverHandlersFor(this),
+      socket: serverHandlersFor(self),
       isServer: true,
       initialData: pending || undefined,
     });
-    if (!result) {
-      this._handle = null;
-      this.destroy(new Error("Invalid socket"));
-      return;
-    }
-    const [raw, tlsHandle] = result;
-    connection._handle = raw;
-    raw[kAdoptedTLSRaw] = true;
-    this.once("end", this[kCloseRawConnection]);
-    raw.connecting = false;
-    this._handle = tlsHandle;
-    this.emit(kUpgradeAttached);
-  });
-};
+  } catch (err) {
+    // e.g. the peer reset a connection that still had unread bytes: the handle outlives the native socket.
+    failServerAdoption(self, connection, err);
+    return;
+  }
+  if (!result) {
+    failServerAdoption(self, connection, new Error("Invalid socket"));
+    return;
+  }
+  const [raw, tlsHandle] = result;
+  connection._handle = raw;
+  raw[kAdoptedTLSRaw] = true;
+  self.once("end", self[kCloseRawConnection]);
+  raw.connecting = false;
+  self._handle = tlsHandle;
+  self.emit(kUpgradeAttached);
+}
 
 Socket.prototype.read = function read(size) {
   if (!this.connecting && !drainOnreadTail(this, true)) {

test/js/node/tls/node-tls-namedpipes.test.ts

@@ -3,7 +3,7 @@ import { expectMaxObjectTypeCount, isWindows, tls } from "harness";
 import { randomUUID } from "node:crypto";
 import { once } from "node:events";
 import net from "node:net";
-import { connect, createServer } from "node:tls";
+import { connect, createServer, TLSSocket } from "node:tls";
 
 it.if(isWindows)("should work with named pipes and tls", async () => {
   await expectMaxObjectTypeCount(expect, "TLSSocket", 0);
@@ -94,3 +94,60 @@ it.if(isWindows)("should be able to upgrade a named pipe connection to TLS", asy
   await test(`\\\\.\\pipe\\test\\${randomUUID()}`);
   await expectMaxObjectTypeCount(expect, "TLSSocket", 3);
 });
+
+// Server-side wraps of an accepted named-pipe connection. A pipe has no fd for
+// the native upgrade to adopt; the wrap has to run the TLS engine over the
+// stream, the way tls.connect({ socket }) already does for pipes. It used to
+// throw "upgradeTLS requires an established socket" from nextTick, uncaught.
+type ServerWrap = (accepted: net.Socket, echo: (secure: TLSSocket) => void, fail: (err: Error) => void) => void;
+
+async function serverWrapRoundTrip(wrap: ServerWrap) {
+  const pipeName = `\\\\.\\pipe\\test\\${randomUUID()}`;
+  const echoed = Promise.withResolvers<string>();
+  const server = net.createServer(accepted => {
+    accepted.on("error", echoed.reject);
+    wrap(
+      accepted,
+      secure => {
+        secure.on("error", echoed.reject);
+        secure.on("data", chunk => secure.end(`echo:${chunk}`));
+      },
+      echoed.reject,
+    );
+  });
+  let client: TLSSocket | undefined;
+  try {
+    server.listen(pipeName);
+    await once(server, "listening");
+    client = connect({ socket: net.connect(pipeName), rejectUnauthorized: false }, () => client!.write("ping"));
+    client.on("error", echoed.reject);
+    // Read through to the server's close_notify before tearing down: a pipe
+    // write is only complete once the peer has read it, so closing on the
+    // first data chunk would fail the server's still-pending close_notify
+    // write with EPIPE.
+    let received = "";
+    client.on("data", chunk => (received += chunk));
+    client.on("end", () => echoed.resolve(received));
+    expect(await echoed.promise).toBe("echo:ping");
+  } finally {
+    client?.destroy();
+    server.close();
+  }
+}
+
+it.if(isWindows)("new TLSSocket(pipeSocket, { isServer: true }) completes a handshake over a named pipe", async () => {
+  await serverWrapRoundTrip((accepted, echo) => echo(new TLSSocket(accepted, { isServer: true, ...tls })));
+});
+
+it.if(isWindows)("tls.Server wraps a named-pipe connection handed in via emit('connection')", async () => {
+  const tlsServer = createServer(tls);
+  try {
+    await serverWrapRoundTrip((accepted, echo, fail) => {
+      tlsServer.once("secureConnection", echo);
+      tlsServer.once("tlsClientError", fail);
+      tlsServer.emit("connection", accepted);
+    });
+  } finally {
+    tlsServer.close();
+  }
+});