From 9535702e50525621313941f0e1a972fdca403045 Mon Sep 17 00:00:00 2001
From: Luigi Pinca <luigipinca@gmail.com>
Date: Sat, 25 Jan 2020 10:22:22 +0100
Subject: [PATCH] [fix] Make `_final()` a noop if no socket is assigned

Prevent `_final()` from throwing an error if `duplex.end()` is called
while the websocket is closing and no socket is assigned to it.
---
 lib/stream.js                        |  8 +++-
 test/create-websocket-stream.test.js | 57 ++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 1 deletion(-)

diff --git a/lib/stream.js b/lib/stream.js
index 3f85b48f8..3899a399a 100644
--- a/lib/stream.js
+++ b/lib/stream.js
@@ -109,6 +109,12 @@ function createWebSocketStream(ws, options) {
       return;
     }
 
+    // If the value of the `_socket` property is `null` it means that `ws` is a
+    // client websocket and the handshake failed. In fact, when this happens, a
+    // socket is never assigned to the websocket. Wait for the `'error'` event
+    // that will be emitted by the websocket.
+    if (ws._socket === null) return;
+
     if (ws._socket._writableState.finished) {
       if (duplex._readableState.endEmitted) duplex.destroy();
       callback();
@@ -116,7 +122,7 @@ function createWebSocketStream(ws, options) {
       ws._socket.once('finish', function finish() {
         // `duplex` is not destroyed here because the `'end'` event will be
         // emitted on `duplex` after this `'finish'` event. The EOF signaling
-        // `null` chunk is, in fact, pushed when the WebSocket emits `'close'`.
+        // `null` chunk is, in fact, pushed when the websocket emits `'close'`.
         callback();
       });
       ws.close();
diff --git a/test/create-websocket-stream.test.js b/test/create-websocket-stream.test.js
index 65c3b4119..a16a5a2ce 100644
--- a/test/create-websocket-stream.test.js
+++ b/test/create-websocket-stream.test.js
@@ -2,6 +2,7 @@
 
 const assert = require('assert');
 const EventEmitter = require('events');
+const { createServer } = require('http');
 const { Duplex } = require('stream');
 const { randomBytes } = require('crypto');
 
@@ -145,6 +146,62 @@ describe('createWebSocketStream', () => {
       });
     });
 
+    it('makes `_final()` a noop if no socket is assigned', (done) => {
+      const server = createServer();
+
+      server.on('upgrade', (request, socket) => {
+        socket.on('end', socket.end);
+
+        const headers = [
+          'HTTP/1.1 101 Switching Protocols',
+          'Upgrade: websocket',
+          'Connection: Upgrade',
+          'Sec-WebSocket-Accept: foo'
+        ];
+
+        socket.write(headers.concat('\r\n').join('\r\n'));
+      });
+
+      server.listen(() => {
+        const called = [];
+        const ws = new WebSocket(`ws://localhost:${server.address().port}`);
+        const duplex = WebSocket.createWebSocketStream(ws);
+        const final = duplex._final;
+
+        duplex._final = (callback) => {
+          called.push('final');
+          assert.strictEqual(ws.readyState, WebSocket.CLOSING);
+          assert.strictEqual(ws._socket, null);
+
+          final(callback);
+        };
+
+        duplex.on('error', (err) => {
+          called.push('error');
+          assert.ok(err instanceof Error);
+          assert.strictEqual(
+            err.message,
+            'Invalid Sec-WebSocket-Accept header'
+          );
+        });
+
+        duplex.on('finish', () => {
+          called.push('finish');
+        });
+
+        duplex.on('close', () => {
+          assert.deepStrictEqual(called, ['final', 'error']);
+          server.close(done);
+        });
+
+        ws.on('upgrade', () => {
+          process.nextTick(() => {
+            duplex.end();
+          });
+        });
+      });
+    });
+
     it('reemits errors', (done) => {
       const wss = new WebSocket.Server({ port: 0 }, () => {
         const ws = new WebSocket(`ws://localhost:${wss.address().port}`);