[fix] Do not throw if the redirect URL is invalid (#1980)

If the redirect URL is invalid, then emit the error instead of throwing it, otherwise there is no way to handle it. Closes #1975
websockets · Nov 22, 2021 · b8186dd · b8186dd
1 parent 5a905e4
commit b8186dd
Show file tree

Hide file tree

Showing 2 changed files with 85 additions and 12 deletions.
diff --git a/lib/websocket.js b/lib/websocket.js
@@ -630,19 +630,26 @@ function initAsClient(websocket, address, protocols, options) {
 
   const isSecure = parsedUrl.protocol === 'wss:';
   const isUnixSocket = parsedUrl.protocol === 'ws+unix:';
+  let invalidURLMessage;
 
   if (parsedUrl.protocol !== 'ws:' && !isSecure && !isUnixSocket) {
-    throw new SyntaxError(
-      'The URL\'s protocol must be one of "ws:", "wss:", or "ws+unix:"'
-    );
+    invalidURLMessage =
+      'The URL\'s protocol must be one of "ws:", "wss:", or "ws+unix:"';
+  } else if (isUnixSocket && !parsedUrl.pathname) {
+    invalidURLMessage = "The URL's pathname is empty";
+  } else if (parsedUrl.hash) {
+    invalidURLMessage = 'The URL contains a fragment identifier';
   }
 
-  if (isUnixSocket && !parsedUrl.pathname) {
-    throw new SyntaxError("The URL's pathname is empty");
-  }
+  if (invalidURLMessage) {
+    const err = new SyntaxError(invalidURLMessage);
 
-  if (parsedUrl.hash) {
-    throw new SyntaxError('The URL contains a fragment identifier');
+    if (websocket._redirects === 0) {
+      throw err;
+    } else {
+      emitErrorAndClose(websocket, err);
+      return;
+    }
   }
 
   const defaultPort = isSecure ? 443 : 80;
@@ -724,9 +731,7 @@ function initAsClient(websocket, address, protocols, options) {
     if (req === null || req.aborted) return;
 
     req = websocket._req = null;
-    websocket._readyState = WebSocket.CLOSING;
-    websocket.emit('error', err);
-    websocket.emitClose();
+    emitErrorAndClose(websocket, err);
   });
 
   req.on('response', (res) => {
@@ -746,7 +751,15 @@ function initAsClient(websocket, address, protocols, options) {
 
       req.abort();
 
-      const addr = new URL(location, address);
+      let addr;
+
+      try {
+        addr = new URL(location, address);
+      } catch (e) {
+        const err = new SyntaxError(`Invalid URL: ${location}`);
+        emitErrorAndClose(websocket, err);
+        return;
+      }
 
       initAsClient(websocket, addr, protocols, options);
     } else if (!websocket.emit('unexpected-response', req, res)) {
@@ -849,6 +862,19 @@ function initAsClient(websocket, address, protocols, options) {
   });
 }
 
+/**
+ * Emit the `'error'` and `'close'` event.
+ *
+ * @param {WebSocket} websocket The WebSocket instance
+ * @param {Error} The error to emit
+ * @private
+ */
+function emitErrorAndClose(websocket, err) {
+  websocket._readyState = WebSocket.CLOSING;
+  websocket.emit('error', err);
+  websocket.emitClose();
+}
+
 /**
  * Create a `net.Socket` and initiate a connection.
  *

diff --git a/test/websocket.test.js b/test/websocket.test.js
@@ -1028,6 +1028,53 @@ describe('WebSocket', () => {
         ws.on('close', () => done());
       });
     });
+
+    it('emits an error if the redirect URL is invalid (1/2)', (done) => {
+      const onUpgrade = (req, socket) => {
+        socket.end('HTTP/1.1 302 Found\r\nLocation: ws://\r\n\r\n');
+      };
+
+      server.on('upgrade', onUpgrade);
+
+      const ws = new WebSocket(`ws://localhost:${server.address().port}`, {
+        followRedirects: true
+      });
+
+      ws.on('open', () => done(new Error("Unexpected 'open' event")));
+      ws.on('error', (err) => {
+        assert.ok(err instanceof SyntaxError);
+        assert.strictEqual(err.message, 'Invalid URL: ws://');
+        assert.strictEqual(ws._redirects, 1);
+
+        server.removeListener('upgrade', onUpgrade);
+        ws.on('close', () => done());
+      });
+    });
+
+    it('emits an error if the redirect URL is invalid (2/2)', (done) => {
+      const onUpgrade = (req, socket) => {
+        socket.end('HTTP/1.1 302 Found\r\nLocation: http://localhost\r\n\r\n');
+      };
+
+      server.on('upgrade', onUpgrade);
+
+      const ws = new WebSocket(`ws://localhost:${server.address().port}`, {
+        followRedirects: true
+      });
+
+      ws.on('open', () => done(new Error("Unexpected 'open' event")));
+      ws.on('error', (err) => {
+        assert.ok(err instanceof SyntaxError);
+        assert.strictEqual(
+          err.message,
+          'The URL\'s protocol must be one of "ws:", "wss:", or "ws+unix:"'
+        );
+        assert.strictEqual(ws._redirects, 1);
+
+        server.removeListener('upgrade', onUpgrade);
+        ws.on('close', () => done());
+      });
+    });
   });
 
   describe('Connection with query string', () => {