[fix] Abort the request if close is called while connecting

Fixes #388

Author: lpinca

lib/WebSocket.js

@@ -262,10 +262,10 @@ class WebSocket extends EventEmitter {
    */
   close (code, data) {
     if (this.readyState === WebSocket.CLOSED) return;
-
     if (this.readyState === WebSocket.CONNECTING) {
-      this.readyState = WebSocket.CLOSED;
-      return;
+      this._req.abort();
+      this.emit('error', new Error('closed while connecting'));
+      return this.finalize(true);
     }
 
     if (this.readyState === WebSocket.CLOSING) {
@@ -374,6 +374,11 @@ class WebSocket extends EventEmitter {
    */
   terminate () {
     if (this.readyState === WebSocket.CLOSED) return;
+    if (this.readyState === WebSocket.CONNECTING) {
+      this._req.abort();
+      this.emit('error', new Error('closed while connecting'));
+      return this.finalize(true);
+    }
 
     if (this._socket) {
       this.readyState = WebSocket.CLOSING;
@@ -385,8 +390,6 @@ class WebSocket extends EventEmitter {
       //
       clearTimeout(this._closeTimer);
       this._closeTimer = setTimeout(this._finalize, closeTimeout, true);
-    } else if (this.readyState === WebSocket.CONNECTING) {
-      this.finalize(true);
     }
   }
 }
@@ -610,43 +613,32 @@ function initAsClient (address, protocols, options) {
 
   if (agent) requestOptions.agent = agent;
 
-  const req = httpObj.get(requestOptions);
+  this._req = httpObj.get(requestOptions);
+
+  this._req.on('error', (error) => {
+    if (this._req.aborted) return;
 
-  req.on('error', (error) => {
     this.emit('error', error);
-    this.finalize(error);
+    this.finalize(true);
   });
 
-  req.on('response', (res) => {
-    var error;
-
-    if (!this.emit('unexpected-response', req, res)) {
-      error = new Error(`unexpected server response (${res.statusCode})`);
-      req.abort();
-      this.emit('error', error);
+  this._req.on('response', (res) => {
+    if (!this.emit('unexpected-response', this._req, res)) {
+      this._req.abort();
+      this.emit('error', new Error(`unexpected server response (${res.statusCode})`));
+      this.finalize(true);
     }
-
-    this.finalize(error);
   });
 
-  req.on('upgrade', (res, socket, head) => {
-    if (this.readyState === WebSocket.CLOSED) {
-      // client closed before server accepted connection
-      this.emit('close');
-      this.removeAllListeners();
-      socket.end();
-      return;
-    }
-
+  this._req.on('upgrade', (res, socket, head) => {
     const digest = crypto.createHash('sha1')
       .update(key + GUID, 'binary')
       .digest('base64');
 
     if (res.headers['sec-websocket-accept'] !== digest) {
+      socket.destroy();
       this.emit('error', new Error('invalid server key'));
-      this.removeAllListeners();
-      socket.end();
-      return;
+      return this.finalize(true);
     }
 
     const serverProt = res.headers['sec-websocket-protocol'];
@@ -662,30 +654,28 @@ function initAsClient (address, protocols, options) {
     }
 
     if (protError) {
+      socket.destroy();
       this.emit('error', new Error(protError));
-      this.removeAllListeners();
-      socket.end();
-      return;
+      return this.finalize(true);
     }
 
     if (serverProt) this.protocol = serverProt;
 
     const serverExtensions = Extensions.parse(res.headers['sec-websocket-extensions']);
+
     if (perMessageDeflate && serverExtensions[PerMessageDeflate.extensionName]) {
       try {
         perMessageDeflate.accept(serverExtensions[PerMessageDeflate.extensionName]);
       } catch (err) {
+        socket.destroy();
         this.emit('error', new Error('invalid extension parameter'));
-        this.removeAllListeners();
-        socket.end();
-        return;
+        return this.finalize(true);
       }
+
       this.extensions[PerMessageDeflate.extensionName] = perMessageDeflate;
     }
 
+    this._req = null;
     this.setSocket(socket, head);
-
-    req.removeAllListeners();
-    agent = null;
   });
 }

test/WebSocket.test.js

@@ -95,39 +95,28 @@ describe('WebSocket', function () {
       wss.on('connection', (ws) => ws.send('foobar'));
     });
 
-    it('#url exposes the server url', function (done) {
-      server.createServer(++port, (srv) => {
-        const url = `ws://localhost:${port}`;
-        const ws = new WebSocket(url);
-
-        assert.strictEqual(ws.url, url);
+    it('#url exposes the server url', function () {
+      const url = `ws://localhost:${port}`;
+      const ws = new WebSocket(url, { agent: new CustomAgent() });
 
-        ws.on('close', () => srv.close(done));
-        ws.close();
-      });
+      assert.strictEqual(ws.url, url);
     });
 
-    it('#protocolVersion exposes the protocol version', function (done) {
-      server.createServer(++port, (srv) => {
-        const ws = new WebSocket(`ws://localhost:${port}`);
-
-        assert.strictEqual(ws.protocolVersion, 13);
-
-        ws.on('close', () => srv.close(done));
-        ws.close();
+    it('#protocolVersion exposes the protocol version', function () {
+      const ws = new WebSocket(`ws://localhost:${port}`, {
+        agent: new CustomAgent()
       });
+
+      assert.strictEqual(ws.protocolVersion, 13);
     });
 
     describe('#bufferedAmount', function () {
-      it('defaults to zero', function (done) {
-        server.createServer(++port, (srv) => {
-          const ws = new WebSocket(`ws://localhost:${port}`);
-
-          assert.strictEqual(ws.bufferedAmount, 0);
-
-          ws.on('close', () => srv.close(done));
-          ws.close();
+      it('defaults to zero', function () {
+        const ws = new WebSocket(`ws://localhost:${port}`, {
+          agent: new CustomAgent()
         });
+
+        assert.strictEqual(ws.bufferedAmount, 0);
       });
 
       it('defaults to zero upon "open"', function (done) {
@@ -203,15 +192,12 @@ describe('WebSocket', function () {
     });
 
     describe('#readyState', function () {
-      it('defaults to connecting', function (done) {
-        server.createServer(++port, (srv) => {
-          const ws = new WebSocket(`ws://localhost:${port}`);
-
-          assert.strictEqual(ws.readyState, WebSocket.CONNECTING);
-
-          ws.on('close', () => srv.close(done));
-          ws.close();
+      it('defaults to connecting', function () {
+        const ws = new WebSocket(`ws://localhost:${port}`, {
+          agent: new CustomAgent()
         });
+
+        assert.strictEqual(ws.readyState, WebSocket.CONNECTING);
       });
 
       it('set to open once connection is established', function (done) {
@@ -236,7 +222,7 @@ describe('WebSocket', function () {
             srv.close(done);
           });
 
-          ws.close(1001);
+          ws.on('open', () => ws.close(1001));
         });
       });
 
@@ -249,7 +235,7 @@ describe('WebSocket', function () {
             srv.close(done);
           });
 
-          ws.terminate();
+          ws.on('open', () => ws.terminate());
         });
       });
     });
@@ -308,26 +294,68 @@ describe('WebSocket', function () {
   });
 
   describe('connection establishing', function () {
-    it('can disconnect before connection is established', function (done) {
-      server.createServer(++port, (srv) => {
+    it('can terminate before connection is established (1/2)', function (done) {
+      const wss = new WebSocketServer({ port: ++port }, () => {
         const ws = new WebSocket(`ws://localhost:${port}`);
 
         ws.on('open', () => assert.fail(null, null, 'connect shouldnt be raised here'));
-        ws.on('close', () => srv.close(done));
+        ws.on('error', (err) => {
+          assert.ok(err instanceof Error);
+          assert.strictEqual(err.message, 'closed while connecting');
+          ws.on('close', () => wss.close(done));
+        });
         ws.terminate();
       });
     });
 
-    it('can close before connection is established', function (done) {
-      server.createServer(++port, (srv) => {
+    it('can terminate before connection is established (2/2)', function (done) {
+      const wss = new WebSocketServer({
+        verifyClient: (info, cb) => setTimeout(cb, 300, true),
+        port: ++port
+      }, () => {
         const ws = new WebSocket(`ws://localhost:${port}`);
 
         ws.on('open', () => assert.fail(null, null, 'connect shouldnt be raised here'));
-        ws.on('close', () => srv.close(done));
+        ws.on('error', (err) => {
+          assert.ok(err instanceof Error);
+          assert.strictEqual(err.message, 'closed while connecting');
+          ws.on('close', () => wss.close(done));
+        });
+        setTimeout(() => ws.terminate(), 150);
+      });
+    });
+
+    it('can close before connection is established (1/2)', function (done) {
+      const wss = new WebSocketServer({ port: ++port }, () => {
+        const ws = new WebSocket(`ws://localhost:${port}`);
+
+        ws.on('open', () => assert.fail(null, null, 'connect shouldnt be raised here'));
+        ws.on('error', (err) => {
+          assert.ok(err instanceof Error);
+          assert.strictEqual(err.message, 'closed while connecting');
+          ws.on('close', () => wss.close(done));
+        });
         ws.close(1001);
       });
     });
 
+    it('can close before connection is established (2/2)', function (done) {
+      const wss = new WebSocketServer({
+        verifyClient: (info, cb) => setTimeout(cb, 300, true),
+        port: ++port
+      }, () => {
+        const ws = new WebSocket(`ws://localhost:${port}`);
+
+        ws.on('open', () => assert.fail(null, null, 'connect shouldnt be raised here'));
+        ws.on('error', (err) => {
+          assert.ok(err instanceof Error);
+          assert.strictEqual(err.message, 'closed while connecting');
+          ws.on('close', () => wss.close(done));
+        });
+        setTimeout(() => ws.close(1001), 150);
+      });
+    });
+
     it('can handle error before request is upgraded', function (done) {
       // Here, we don't create a server, to guarantee that the connection will
       // fail before the request is upgraded
@@ -791,15 +819,17 @@ describe('WebSocket', function () {
     });
 
     it('before connect should pass error through callback, if present', function (done) {
-      server.createServer(++port, (srv) => {
+      const wss = new WebSocketServer({ port: ++port }, () => {
         const ws = new WebSocket(`ws://localhost:${port}`);
 
-        ws.send('hi', (error) => {
-          assert.ok(error instanceof Error);
-          srv.close(done);
-          ws.terminate();
+        ws.send('hi', (err) => {
+          assert.ok(err instanceof Error);
+          assert.strictEqual(err.message, 'not opened');
+          ws.on('close', () => wss.close(done));
         });
       });
+
+      wss.on('connection', (ws) => ws.close());
     });
 
     it('without data should be successful', function (done) {

test/WebSocketServer.test.js

@@ -202,38 +202,37 @@ describe('WebSocketServer', function () {
         const ws = new WebSocket(`ws://localhost:${port}`);
       });
 
-      wss.on('connection', (client) => {
+      wss.on('connection', (ws) => {
         assert.strictEqual(wss.clients.size, 1);
-        wss.close();
-        done();
+        wss.close(done);
       });
     });
 
     it('can be disabled', function (done) {
       const wss = new WebSocketServer({ port: ++port, clientTracking: false }, () => {
         assert.strictEqual(wss.clients, undefined);
         const ws = new WebSocket(`ws://localhost:${port}`);
+
+        ws.on('open', () => ws.close());
       });
 
-      wss.on('connection', (client) => {
+      wss.on('connection', (ws) => {
         assert.strictEqual(wss.clients, undefined);
-        wss.close();
-        done();
+        ws.on('close', () => wss.close(done));
       });
     });
 
     it('is updated when client terminates the connection', function (done) {
       const wss = new WebSocketServer({ port: ++port }, () => {
         const ws = new WebSocket(`ws://localhost:${port}`);
 
-        wss.on('connection', (client) => {
-          client.on('close', () => {
-            assert.strictEqual(wss.clients.size, 0);
-            wss.close();
-            done();
-          });
+        ws.on('open', () => ws.terminate());
+      });
 
-          ws.close();
+      wss.on('connection', (ws) => {
+        ws.on('close', () => {
+          assert.strictEqual(wss.clients.size, 0);
+          wss.close(done);
         });
       });
     });
@@ -242,14 +241,13 @@ describe('WebSocketServer', function () {
       const wss = new WebSocketServer({ port: ++port }, () => {
         const ws = new WebSocket(`ws://localhost:${port}`);
 
-        wss.on('connection', (client) => {
-          client.on('close', () => {
-            assert.strictEqual(wss.clients.size, 0);
-            wss.close();
-            done();
-          });
+        ws.on('open', () => ws.close());
+      });
 
-          ws.close();
+      wss.on('connection', (ws) => {
+        ws.on('close', () => {
+          assert.strictEqual(wss.clients.size, 0);
+          wss.close(done);
         });
       });
     });