[major] Validate subprotocol names

Make the `WebSocket` constructor throw a `SyntaxError` if any of the
subprotocol names are invalid or duplicated.

Author: lpinca

lib/websocket.js

@@ -24,6 +24,7 @@ const { format, parse } = require('./extension');
 const { toBuffer } = require('./buffer-util');
 
 const readyStates = ['CONNECTING', 'OPEN', 'CLOSING', 'CLOSED'];
+const subprotocolRegex = /^[!#$%&'*+\-.0-9A-Z^_`|a-z~]+$/;
 const protocolVersions = [8, 13];
 const closeTimeout = 30 * 1000;
 
@@ -61,11 +62,15 @@ class WebSocket extends EventEmitter {
       this._isServer = false;
       this._redirects = 0;
 
-      if (Array.isArray(protocols)) {
-        protocols = protocols.join(', ');
-      } else if (typeof protocols === 'object' && protocols !== null) {
-        options = protocols;
-        protocols = undefined;
+      if (protocols === undefined) {
+        protocols = [];
+      } else if (!Array.isArray(protocols)) {
+        if (typeof protocols === 'object' && protocols !== null) {
+          options = protocols;
+          protocols = [];
+        } else {
+          protocols = [protocols];
+        }
       }
 
       initAsClient(this, address, protocols, options);
@@ -558,7 +563,7 @@ module.exports = WebSocket;
  *
  * @param {WebSocket} websocket The client to initialize
  * @param {(String|URL)} address The URL to which to connect
- * @param {String} [protocols] The subprotocols
+ * @param {Array} protocols The subprotocols
  * @param {Object} [options] Connection options
  * @param {(Boolean|Object)} [options.perMessageDeflate=true] Enable/disable
  *     permessage-deflate
@@ -623,6 +628,7 @@ function initAsClient(websocket, address, protocols, options) {
   const defaultPort = isSecure ? 443 : 80;
   const key = randomBytes(16).toString('base64');
   const get = isSecure ? https.get : http.get;
+  const protocolSet = new Set();
   let perMessageDeflate;
 
   opts.createConnection = isSecure ? tlsConnect : netConnect;
@@ -651,8 +657,22 @@ function initAsClient(websocket, address, protocols, options) {
       [PerMessageDeflate.extensionName]: perMessageDeflate.offer()
     });
   }
-  if (protocols) {
-    opts.headers['Sec-WebSocket-Protocol'] = protocols;
+  if (protocols.length) {
+    for (const protocol of protocols) {
+      if (
+        typeof protocol !== 'string' ||
+        !subprotocolRegex.test(protocol) ||
+        protocolSet.has(protocol)
+      ) {
+        throw new SyntaxError(
+          'An invalid or duplicated subprotocol was specified'
+        );
+      }
+
+      protocolSet.add(protocol);
+    }
+
+    opts.headers['Sec-WebSocket-Protocol'] = protocols.join(',');
   }
   if (opts.origin) {
     if (opts.protocolVersion < 13) {
@@ -739,15 +759,16 @@ function initAsClient(websocket, address, protocols, options) {
     }
 
     const serverProt = res.headers['sec-websocket-protocol'];
-    const protList = (protocols || '').split(/, */);
     let protError;
 
-    if (!protocols && serverProt) {
-      protError = 'Server sent a subprotocol but none was requested';
-    } else if (protocols && !serverProt) {
+    if (serverProt) {
+      if (!protocolSet.size) {
+        protError = 'Server sent a subprotocol but none was requested';
+      } else if (!protocolSet.has(serverProt)) {
+        protError = 'Server sent an invalid subprotocol';
+      }
+    } else if (protocolSet.size) {
       protError = 'Server sent no subprotocol';
-    } else if (serverProt && !protList.includes(serverProt)) {
-      protError = 'Server sent an invalid subprotocol';
     }
 
     if (protError) {

test/websocket.test.js

@@ -26,6 +26,15 @@ describe('WebSocket', () => {
       );
     });
 
+    it('throws an error if a subprotocol is invalid or duplicated', () => {
+      for (const subprotocol of [null, '', 'a,b', ['a', 'a']]) {
+        assert.throws(
+          () => new WebSocket('ws://localhost', subprotocol),
+          /^SyntaxError: An invalid or duplicated subprotocol was specified$/
+        );
+      }
+    });
+
     it('accepts `url.URL` objects as url', (done) => {
       const agent = new CustomAgent();
 
@@ -44,13 +53,18 @@ describe('WebSocket', () => {
         let count = 0;
         let ws;
 
-        agent.addRequest = () => count++;
+        agent.addRequest = (req) => {
+          assert.strictEqual(
+            req.getHeader('sec-websocket-protocol'),
+            undefined
+          );
+          count++;
+        };
 
         ws = new WebSocket('ws://localhost', undefined, { agent });
-        ws = new WebSocket('ws://localhost', null, { agent });
         ws = new WebSocket('ws://localhost', [], { agent });
 
-        assert.strictEqual(count, 3);
+        assert.strictEqual(count, 2);
       });
 
       it('accepts the `maxPayload` option', (done) => {
@@ -651,7 +665,7 @@ describe('WebSocket', () => {
       server.once('upgrade', (req, socket) => socket.on('end', socket.end));
 
       const port = server.address().port;
-      const ws = new WebSocket(`ws://localhost:${port}`, null, {
+      const ws = new WebSocket(`ws://localhost:${port}`, {
         handshakeTimeout: 100
       });