Merge branch 'main' of github.com:oven-sh/bun into ali/test-net-server-listen-options

src/bun.js/bindings/NodeValidator.cpp

@@ -321,7 +321,7 @@ JSC_DEFINE_HOST_FUNCTION(jsFunction_validatePort, (JSC::JSGlobalObject * globalO
     if (port_num > 0xffff) return Bun::ERR::SOCKET_BAD_PORT(scope, globalObject, name, port, allowZero_b);
     if (port_num == 0 && !allowZero_b) return Bun::ERR::SOCKET_BAD_PORT(scope, globalObject, name, port, allowZero_b);
 
-    return JSValue::encode(port);
+    return JSValue::encode(jsNumber(port_num));
 }
 
 JSC_DEFINE_HOST_FUNCTION(jsFunction_validateAbortSignal, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame))

src/js/node/net.ts

@@ -31,10 +31,10 @@ const {
   getBufferedAmount,
 } = require("internal/net");
 const { ExceptionWithHostPort } = require("internal/shared");
-import type { SocketListener, SocketHandler } from "bun";
+import type { SocketHandler, SocketListener } from "bun";
 import type { ServerOpts } from "node:net";
 const { getTimerDuration } = require("internal/timers");
-const { validateFunction, validateNumber, validateAbortSignal } = require("internal/validators");
+const { validateFunction, validateNumber, validateAbortSignal, validatePort } = require("internal/validators");
 
 // IPv4 Segment
 const v4Seg = "(?:25[0-5]|2[0-4][0-9]|1[0-9][0-9]|[1-9][0-9]|[0-9])";
@@ -1327,111 +1327,81 @@ Server.prototype.getConnections = function getConnections(callback) {
   return this;
 };
 
-Server.prototype.listen = function listen(port, hostname, onListen) {
-  let backlog;
-  let path;
-  let exclusive = false;
-  let allowHalfOpen = false;
-  let reusePort = false;
-  let ipv6Only = false;
-  //port is actually path
-  if (typeof port === "string") {
-    if (Number.isSafeInteger(hostname)) {
-      if (hostname > 0) {
-        //hostname is backlog
-        backlog = hostname;
-      }
-    } else if (typeof hostname === "function") {
-      //hostname is callback
-      onListen = hostname;
-    }
+/*
+(options[...][, cb])
+(path[...][, cb])
+([port][, host][...][, cb])
+*/
 
-    path = port;
-    hostname = undefined;
-    port = undefined;
-  } else {
-    if (typeof hostname === "function") {
-      onListen = hostname;
-      hostname = undefined;
-    }
+function toNumber(x) {
+  return (x = Number(x)) >= 0 ? x : false;
+}
+Server.prototype.listen = function listen(...args) {
+  const normalized = normalizeArgs(args) as [
+    {
+      port?: number;
+      host?: string;
+      path?: string;
+      backlog?: number;
+      exclusive?: boolean;
+      allowHalfOpen?: boolean;
+      reusePort?: boolean;
+      ipv6Only?: boolean;
+      writableAll?: boolean;
+      readableAll?: boolean;
+    },
+    Function,
+  ];
 
-    if (typeof port === "function") {
-      onListen = port;
-      port = 0;
-    } else if (typeof port === "object") {
-      const options = port;
-      addServerAbortSignalOption(this, options);
+  let options = normalized[0];
 
-      hostname = options.host;
-      exclusive = options.exclusive;
-      path = options.path;
-      port = options.port;
-      ipv6Only = options.ipv6Only;
-      allowHalfOpen = options.allowHalfOpen;
-      reusePort = options.reusePort;
+  const cb = normalized[1];
 
-      const isLinux = process.platform === "linux";
+  const port = options.port ?? 0;
 
-      if (!Number.isSafeInteger(port) || port < 0) {
-        if (path) {
-          const isAbstractPath = path.startsWith("\0");
-          if (isLinux && isAbstractPath && (options.writableAll || options.readableAll)) {
-            const message = `The argument 'options' can not set readableAll or writableAll to true when path is abstract unix socket. Received ${JSON.stringify(options)}`;
-
-            const error = new TypeError(message);
-            error.code = "ERR_INVALID_ARG_VALUE";
-            throw error;
-          }
-
-          hostname = path;
-          port = undefined;
-        } else {
-          let message = 'The argument \'options\' must have the property "port" or "path"';
-          try {
-            message = `${message}. Received ${JSON.stringify(options)}`;
-          } catch {}
-
-          const error = new TypeError(message);
-          error.code = "ERR_INVALID_ARG_VALUE";
-          throw error;
-        }
-      } else if (port === undefined) {
-        port = 0;
-      }
-
-      // port <number>
-      // host <string>
-      // path <string> Will be ignored if port is specified. See Identifying paths for IPC connections.
-      // backlog <number> Common parameter of server.listen() functions.
-      // exclusive <boolean> Default: false
-      // readableAll <boolean> For IPC servers makes the pipe readable for all users. Default: false.
-      // writableAll <boolean> For IPC servers makes the pipe writable for all users. Default: false.
-      // ipv6Only <boolean> For TCP servers, setting ipv6Only to true will disable dual-stack support, i.e., binding to host :: won't make 0.0.0.0 be bound. Default: false.
-      // signal <AbortSignal> An AbortSignal that may be used to close a listening server.
-
-      if (typeof options.callback === "function") onListen = options?.callback;
-    } else if (!Number.isSafeInteger(port) || port < 0) {
-      port = 0;
-    }
-    hostname = hostname || "::";
-  }
+  addServerAbortSignalOption(this, normalized);
 
   if (this._handle) {
     throw $ERR_SERVER_ALREADY_LISTEN();
   }
 
-  if (onListen != null) {
-    this.once("listening", onListen);
+  const backlogFromArgs = toNumber(args.length > 1 && args[1]) || toNumber(args.length > 2 && args[2]);
+
+  let backlog;
+
+  if (
+    typeof args[0] !== "object" &&
+    typeof args[0] !== "function" &&
+    typeof args[0] !== "undefined" &&
+    typeof args[0] !== "string" &&
+    typeof args[0] !== "number"
+  ) {
+    throw $ERR_INVALID_ARG_VALUE("options", args[0], "is invalid");
+  }
+
+  if (typeof port === "number" || typeof port === "string") {
+    validatePort(port, "options.port");
+    backlog = options.backlog || backlogFromArgs;
+
+    if (options.reusePort === true) {
+      options.exclusive = true;
+    }
+
+    return this;
+  }
+
+  if (cb) {
+    this.once("listening", cb);
   }
 
   try {
-    var tls = undefined;
-    var TLSSocketClass = undefined;
+    let tls = undefined;
+    let TLSSocketClass = undefined;
     const bunTLS = this[bunTlsSymbol];
     const options = this[bunSocketServerOptions];
     let contexts: Map<string, any> | null = null;
     if (typeof bunTLS === "function") {
-      [tls, TLSSocketClass] = bunTLS.$call(this, port, hostname, false);
+      [tls, TLSSocketClass] = bunTLS.$call(this, port, options.host, false);
       options.servername = tls.serverName;
       options[kSocketClass] = TLSSocketClass;
       contexts = tls.contexts;
@@ -1442,6 +1412,8 @@ Server.prototype.listen = function listen(port, hostname, onListen) {
       options[kSocketClass] = Socket;
     }
 
+    backlog = options.backlog || backlogFromArgs;
+
     listenInCluster(
       this,
       null,
@@ -1449,17 +1421,17 @@ Server.prototype.listen = function listen(port, hostname, onListen) {
       4,
       backlog,
       undefined,
-      exclusive,
-      ipv6Only,
-      allowHalfOpen,
-      reusePort,
+      options.exclusive,
+      options.ipv6Only,
+      options.allowHalfOpen,
+      options.reusePort,
       undefined,
       undefined,
-      path,
-      hostname,
+      options.path,
+      options.host,
       tls,
       contexts,
-      onListen,
+      cb,
     );
   } catch (err) {
     setTimeout(emitErrorNextTick, 1, this, err);
@@ -1619,34 +1591,37 @@ function createServer(options, connectionListener) {
   return new Server(options, connectionListener);
 }
 
-function normalizeArgs(args: unknown[]): [options: Record<PropertyKey, any>, cb: Function | null] {
-  while (args.length && args[args.length - 1] == null) args.pop();
-  let arr;
+function normalizeArgs(args: unknown[]) {
+  let arr: [Record<PropertyKey, any>, Function | null];
 
   if (args.length === 0) {
     arr = [{}, null];
-    arr[normalizedArgsSymbol as symbol] = true;
+    arr[normalizedArgsSymbol] = true;
     return arr;
   }
 
-  const arg0 = args[0];
-  let options: any = {};
-  if (typeof arg0 === "object" && arg0 !== null) {
-    options = arg0;
-  } else if (isPipeName(arg0)) {
-    options.path = arg0;
+  const optionsOrPathOrPort = args[0];
+  let options: Record<PropertyKey, any> = {};
+
+  if (typeof optionsOrPathOrPort === "object" && optionsOrPathOrPort !== null) {
+    options = optionsOrPathOrPort;
+  } else if (isPipeName(optionsOrPathOrPort)) {
+    options.path = optionsOrPathOrPort;
   } else {
-    options.port = arg0;
+    options.port = optionsOrPathOrPort;
     if (args.length > 1 && typeof args[1] === "string") {
       options.host = args[1];
     }
   }
 
   const cb = args[args.length - 1];
-  if (typeof cb !== "function") arr = [options, null];
-  else arr = [options, cb];
-  arr[normalizedArgsSymbol as symbol] = true;
+  if (typeof cb !== "function") {
+    arr = [options, null];
+  } else {
+    arr = [options, cb];
+  }
 
+  arr[normalizedArgsSymbol] = true;
   return arr;
 }
 

test/js/node/test/parallel/test-net-server-listen-options.js

@@ -0,0 +1,96 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const net = require('net');
+
+function close() { this.close(); }
+
+{
+  // Test listen()
+  net.createServer().listen().on('listening', common.mustCall(close));
+  // Test listen(cb)
+  net.createServer().listen(common.mustCall(close));
+  // Test listen(port)
+  net.createServer().listen(0).on('listening', common.mustCall(close));
+  // Test listen({port})
+  net.createServer().listen({ port: 0 })
+    .on('listening', common.mustCall(close));
+}
+
+// Test listen(port, cb) and listen({ port }, cb) combinations
+const listenOnPort = [
+  (port, cb) => net.createServer().listen({ port }, cb),
+  (port, cb) => net.createServer().listen(port, cb),
+];
+
+{
+  const assertPort = () => {
+    return common.expectsError({
+      code: 'ERR_SOCKET_BAD_PORT',
+      name: 'RangeError'
+    });
+  };
+
+  for (const listen of listenOnPort) {
+    // Arbitrary unused ports
+    listen('0', common.mustCall(close));
+    listen(0, common.mustCall(close));
+    listen(undefined, common.mustCall(close));
+    listen(null, common.mustCall(close));
+    // Test invalid ports
+    assert.throws(() => listen(-1, common.mustNotCall()), assertPort());
+    assert.throws(() => listen(NaN, common.mustNotCall()), assertPort());
+    assert.throws(() => listen(123.456, common.mustNotCall()), assertPort());
+    assert.throws(() => listen(65536, common.mustNotCall()), assertPort());
+    assert.throws(() => listen(1 / 0, common.mustNotCall()), assertPort());
+    assert.throws(() => listen(-1 / 0, common.mustNotCall()), assertPort());
+  }
+  // In listen(options, cb), port takes precedence over path
+  assert.throws(() => {
+    net.createServer().listen({ port: -1, path: common.PIPE },
+                              common.mustNotCall());
+  }, assertPort());
+}
+
+{
+  function shouldFailToListen(options) {
+    const fn = () => {
+      net.createServer().listen(options, common.mustNotCall());
+    };
+
+    if (typeof options === 'object' &&
+      !(('port' in options) || ('path' in options))) {
+      assert.throws(fn,
+                    {
+                      code: 'ERR_INVALID_ARG_VALUE',
+                      name: 'TypeError',
+                      message: /^The argument 'options' must have the property "port" or "path"\. Received .+$/,
+                    });
+    } else {
+      console.log(options);
+
+      assert.throws(fn,
+                    {
+                      code: 'ERR_INVALID_ARG_VALUE',
+                      name: 'TypeError',
+                      message: /^The argument 'options' is invalid\. Received .+$/,
+                    });
+    }
+  }
+
+  shouldFailToListen(false, { port: false });
+  shouldFailToListen({ port: false });
+  shouldFailToListen(true);
+  shouldFailToListen({ port: true });
+  // Invalid fd as listen(handle)
+  shouldFailToListen({ fd: -1 });
+  // Invalid path in listen(options)
+  shouldFailToListen({ path: -1 });
+
+  // Neither port or path are specified in options
+  shouldFailToListen({});
+  shouldFailToListen({ host: 'localhost' });
+  shouldFailToListen({ host: 'localhost:3000' });
+  shouldFailToListen({ host: { port: 3000 } });
+  shouldFailToListen({ exclusive: true });
+}