[fix] Make the {noS,s}erver, and port options mutually exclusive

Remove ambiguity and prevent `WebSocketServer.prototype.address()` from throwing an error if the `noServer` option is used along with the `port` and/or `server` options.
websockets · Jul 8, 2021 · 66e58d2 · 66e58d2
1 parent ecb9d9e
commit 66e58d2
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 7 deletions.
diff --git a/doc/ws.md b/doc/ws.md
@@ -80,8 +80,8 @@ This class represents a WebSocket server. It extends the `EventEmitter`.
   - `maxPayload` {Number} The maximum allowed message size in bytes.
 - `callback` {Function}
 
-Create a new server instance. One of `port`, `server` or `noServer` must be
-provided or an error is thrown. An HTTP server is automatically created,
+Create a new server instance. One and only one of `port`, `server` or `noServer`
+must be provided or an error is thrown. An HTTP server is automatically created,
 started, and used if `port` is set. To use an external HTTP/S server instead,
 specify only `server` or `noServer`. In this case the HTTP/S server must be
 started manually. The "noServer" mode allows the WebSocket server to be

diff --git a/lib/websocket-server.js b/lib/websocket-server.js
@@ -62,9 +62,14 @@ class WebSocketServer extends EventEmitter {
       ...options
     };
 
-    if (options.port == null && !options.server && !options.noServer) {
+    if (
+      (options.port == null && !options.server && !options.noServer) ||
+      (options.port != null && (options.server || options.noServer)) ||
+      (options.server && options.noServer)
+    ) {
       throw new TypeError(
-        'One of the "port", "server", or "noServer" options must be specified'
+        'One and only one of the "port", "server", or "noServer" options ' +
+          'must be specified'
       );
     }
 

diff --git a/test/websocket-server.test.js b/test/websocket-server.test.js
@@ -18,12 +18,44 @@ const { NOOP } = require('../lib/constants');
 describe('WebSocketServer', () => {
   describe('#ctor', () => {
     it('throws an error if no option object is passed', () => {
-      assert.throws(() => new WebSocket.Server());
+      assert.throws(
+        () => new WebSocket.Server(),
+        new RegExp(
+          '^TypeError: One and only one of the "port", "server", or ' +
+            '"noServer" options must be specified$'
+        )
+      );
     });
 
     describe('options', () => {
-      it('throws an error if no `port` or `server` option is specified', () => {
-        assert.throws(() => new WebSocket.Server({}));
+      it('throws an error if required options are not specified', () => {
+        assert.throws(
+          () => new WebSocket.Server({}),
+          new RegExp(
+            '^TypeError: One and only one of the "port", "server", or ' +
+              '"noServer" options must be specified$'
+          )
+        );
+      });
+
+      it('throws an error if mutually exclusive options are specified', () => {
+        const server = http.createServer();
+        const variants = [
+          { port: 0, noServer: true, server },
+          { port: 0, noServer: true },
+          { port: 0, server },
+          { noServer: true, server }
+        ];
+
+        for (const options of variants) {
+          assert.throws(
+            () => new WebSocket.Server(options),
+            new RegExp(
+              '^TypeError: One and only one of the "port", "server", or ' +
+                '"noServer" options must be specified$'
+            )
+          );
+        }
       });
 
       it('exposes options passed to constructor', (done) => {