tls(Server) fix connectionListener and make alpnProtocol more compatible with node.js (#14695)

Co-authored-by: cirospaciari <cirospaciari@users.noreply.github.com>

Author: cirospaciari

src/bun.js/api/bun/socket.zig

@@ -1295,17 +1295,13 @@ fn selectALPNCallback(
         if (protos.len == 0) {
             return BoringSSL.SSL_TLSEXT_ERR_NOACK;
         }
-
         const status = BoringSSL.SSL_select_next_proto(bun.cast([*c][*c]u8, out), outlen, protos.ptr, @as(c_uint, @intCast(protos.len)), in, inlen);
-
         // Previous versions of Node.js returned SSL_TLSEXT_ERR_NOACK if no protocol
         // match was found. This would neither cause a fatal alert nor would it result
         // in a useful ALPN response as part of the Server Hello message.
         // We now return SSL_TLSEXT_ERR_ALERT_FATAL in that case as per Section 3.2
         // of RFC 7301, which causes a fatal no_application_protocol alert.
-        const expected = if (comptime BoringSSL.OPENSSL_NPN_NEGOTIATED == 1) BoringSSL.SSL_TLSEXT_ERR_OK else BoringSSL.SSL_TLSEXT_ERR_ALERT_FATAL;
-
-        return if (status == expected) 1 else 0;
+        return if (status == BoringSSL.OPENSSL_NPN_NEGOTIATED) BoringSSL.SSL_TLSEXT_ERR_OK else BoringSSL.SSL_TLSEXT_ERR_ALERT_FATAL;
     } else {
         return BoringSSL.SSL_TLSEXT_ERR_NOACK;
     }

src/js/node/net.ts

@@ -84,6 +84,10 @@ function closeNT(callback, err) {
   callback(err);
 }
 
+function detachAfterFinish() {
+  this[bunSocketInternal] = null;
+}
+
 var SocketClass;
 const Socket = (function (InternalSocket) {
   SocketClass = InternalSocket;
@@ -164,7 +168,7 @@ const Socket = (function (InternalSocket) {
         self._secureEstablished = !!success;
 
         self.emit("secure", self);
-
+        self.alpnProtocol = socket.alpnProtocol;
         const { checkServerIdentity } = self[bunTLSConnectOptions];
         if (!verifyError && typeof checkServerIdentity === "function" && self.servername) {
           const cert = self.getPeerCertificate(true);
@@ -291,10 +295,7 @@ const Socket = (function (InternalSocket) {
 
         if (typeof connectionListener == "function") {
           this.pauseOnConnect = pauseOnConnect;
-          if (isTLS) {
-            // add secureConnection event handler
-            self.once("secureConnection", () => connectionListener.$call(self, _socket));
-          } else {
+          if (!isTLS) {
             connectionListener.$call(self, _socket);
           }
         }
@@ -312,6 +313,7 @@ const Socket = (function (InternalSocket) {
         self._secureEstablished = !!success;
         self.servername = socket.getServername();
         const server = self.server;
+        self.alpnProtocol = socket.alpnProtocol;
         if (self._requestCert || self._rejectUnauthorized) {
           if (verifyError) {
             self.authorized = false;
@@ -329,7 +331,11 @@ const Socket = (function (InternalSocket) {
         } else {
           self.authorized = true;
         }
-        self.server.emit("secureConnection", self);
+        const connectionListener = server[bunSocketServerOptions]?.connectionListener;
+        if (typeof connectionListener == "function") {
+          connectionListener.$call(server, self);
+        }
+        server.emit("secureConnection", self);
         // after secureConnection event we emmit secure and secureConnect
         self.emit("secure", self);
         self.emit("secureConnect", verifyError);
@@ -685,14 +691,23 @@ const Socket = (function (InternalSocket) {
     }
 
     _destroy(err, callback) {
-      const socket = this[bunSocketInternal];
-      if (socket) {
+      const { ending } = this._writableState;
+      // lets make sure that the writable side is closed
+      if (!ending) {
+        // at this state destroyed will be true but we need to close the writable side
+        this._writableState.destroyed = false;
+        this.end();
+        // we now restore the destroyed flag
+        this._writableState.destroyed = true;
+      }
+
+      if (this.writableFinished) {
+        // closed we can detach the socket
         this[bunSocketInternal] = null;
-        // we still have a socket, call end before destroy
-        process.nextTick(endNT, socket, callback, err);
-        return;
+      } else {
+        // lets wait for the finish event before detaching the socket
+        this.once("finish", detachAfterFinish);
       }
-      // no socket, just destroy
       process.nextTick(closeNT, callback, err);
     }
 
@@ -706,7 +721,6 @@ const Socket = (function (InternalSocket) {
         this.#final_callback = callback;
       } else {
         // emit FIN not allowing half open
-        this[bunSocketInternal] = null;
         process.nextTick(endNT, socket, callback);
       }
     }

src/js/node/tls.ts

@@ -330,6 +330,7 @@ const TLSSocket = (function (InternalTLSSocket) {
     #socket;
     #checkServerIdentity;
     #session;
+    alpnProtocol = null;
 
     constructor(socket, options) {
       super(socket instanceof InternalTCPSocket ? options : options || socket);
@@ -503,10 +504,6 @@ const TLSSocket = (function (InternalTLSSocket) {
       throw Error("Not implented in Bun yet");
     }
 
-    get alpnProtocol() {
-      return this[bunSocketInternal]?.alpnProtocol;
-    }
-
     [buntls](port, host) {
       return {
         socket: this.#socket,

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

@@ -6,6 +6,7 @@ import { tmpdir } from "os";
 import { join } from "path";
 import type { PeerCertificate } from "tls";
 import tls, { connect, createServer, rootCertificates, Server, TLSSocket } from "tls";
+import { once } from "node:events";
 
 const { describe, expect, it, createCallCheckCtx } = createTest(import.meta.path);
 
@@ -662,3 +663,43 @@ it("tls.rootCertificates should exists", () => {
   expect(rootCertificates.length).toBeGreaterThan(0);
   expect(typeof rootCertificates[0]).toBe("string");
 });
+
+it("connectionListener should emit the right amount of times, and with alpnProtocol available", async () => {
+  let count = 0;
+  const promises = [];
+  const server: Server = createServer(
+    {
+      ...COMMON_CERT,
+      ALPNProtocols: ["bun"],
+    },
+    socket => {
+      count++;
+      expect(socket.alpnProtocol).toBe("bun");
+      socket.end();
+    },
+  );
+  server.setMaxListeners(100);
+
+  server.listen(0);
+  await once(server, "listening");
+  for (let i = 0; i < 50; i++) {
+    const { promise, resolve } = Promise.withResolvers();
+    promises.push(promise);
+    const socket = connect(
+      {
+        ca: COMMON_CERT.cert,
+        rejectUnauthorized: false,
+        port: server.address().port,
+        ALPNProtocols: ["bun"],
+      },
+      () => {
+        socket.on("close", resolve);
+        socket.resume();
+        socket.end();
+      },
+    );
+  }
+
+  await Promise.all(promises);
+  expect(count).toBe(50);
+});