[fix] Use 1005 status code if close frame payload length is 0

Fixes #1257
websockets · Jan 5, 2018 · a31b1f6 · a31b1f6
1 parent f941a40
commit a31b1f6
Show file tree

Hide file tree

Showing 6 changed files with 17 additions and 19 deletions.
diff --git a/doc/ws.md b/doc/ws.md
@@ -308,7 +308,7 @@ not yet transmitted to the network.
 
 Received bytes count.
 
-### websocket.close([code][, reason])
+### websocket.close([code[, reason]])
 
 - `code` {Number} A numeric value indicating the status code explaining why
   the connection is being closed.

diff --git a/lib/Receiver.js b/lib/Receiver.js
@@ -402,7 +402,7 @@ class Receiver {
   controlMessage (data) {
     if (this._opcode === 0x08) {
       if (data.length === 0) {
-        this.onclose(1000, '');
+        this.onclose(1005, '');
         this._loop = false;
         this.cleanup(this._cleanupCallback);
       } else if (data.length === 1) {

diff --git a/lib/Sender.js b/lib/Sender.js
@@ -116,18 +116,12 @@ class Sender {
     var buf;
 
     if (code === undefined) {
-      code = 1000;
+      buf = constants.EMPTY_BUFFER;
     } else if (typeof code !== 'number' || !ErrorCodes.isValidErrorCode(code)) {
       throw new Error('first argument must be a valid error code number');
-    }
-
-    if (data === undefined || data === '') {
-      if (code === 1000) {
-        buf = constants.EMPTY_BUFFER;
-      } else {
-        buf = Buffer.allocUnsafe(2);
-        buf.writeUInt16BE(code, 0, true);
-      }
+    } else if (data === undefined || data === '') {
+      buf = Buffer.allocUnsafe(2);
+      buf.writeUInt16BE(code, 0, true);
     } else {
       buf = Buffer.allocUnsafe(2 + Buffer.byteLength(data));
       buf.writeUInt16BE(code, 0, true);

diff --git a/lib/WebSocket.js b/lib/WebSocket.js
@@ -148,7 +148,11 @@ class WebSocket extends EventEmitter {
       this._closeFrameReceived = true;
       this._closeMessage = reason;
       this._closeCode = code;
-      if (!this._finalized) this.close(code, reason);
+
+      if (this._finalized) return;
+
+      if (code === 1005) this.close();
+      else this.close(code, reason);
     };
     this._receiver.onerror = (error, code) => {
       this._closeMessage = '';

diff --git a/test/Receiver.test.js b/test/Receiver.test.js
@@ -26,7 +26,7 @@ describe('Receiver', function () {
     const p = new Receiver();
 
     p.onclose = function (code, data) {
-      assert.strictEqual(code, 1000);
+      assert.strictEqual(code, 1005);
       assert.strictEqual(data, '');
       done();
     };
@@ -836,7 +836,7 @@ describe('Receiver', function () {
       assert.strictEqual(p._bufferedBytes, textFrame.length + closeFrame.length);
 
       p.cleanup(() => {
-        assert.deepStrictEqual(results, ['Hello', 'Hello', 1000, '']);
+        assert.deepStrictEqual(results, ['Hello', 'Hello', 1005, '']);
         assert.strictEqual(p.onmessage, null);
         done();
       });

diff --git a/test/WebSocket.test.js b/test/WebSocket.test.js
@@ -1222,7 +1222,7 @@ describe('WebSocket', function () {
           assert.ok(sent.equals(Buffer.from([0x88, 0x00])));
 
           ws.on('close', (code, reason) => {
-            assert.strictEqual(code, 1000);
+            assert.strictEqual(code, 1005);
             assert.strictEqual(reason, '');
             wss.close(done);
           });
@@ -1295,7 +1295,7 @@ describe('WebSocket', function () {
 
         ws.on('message', (message) => messages.push(message));
         ws.on('close', (code) => {
-          assert.strictEqual(code, 1000);
+          assert.strictEqual(code, 1005);
           assert.deepStrictEqual(messages, ['foo', 'bar', 'baz']);
           wss.close(done);
         });
@@ -1330,7 +1330,7 @@ describe('WebSocket', function () {
         const ws = new WebSocket(`ws://localhost:${port}`);
 
         ws.on('close', (code) => {
-          assert.strictEqual(code, 1000);
+          assert.strictEqual(code, 1005);
           assert.strictEqual(ws.readyState, WebSocket.CLOSED);
           ws.close();
           wss.close(done);
@@ -2180,7 +2180,7 @@ describe('WebSocket', function () {
           ws.on('message', (message) => {
             assert.strictEqual(message, 'hi');
             ws.on('close', (code) => {
-              assert.strictEqual(code, 1000);
+              assert.strictEqual(code, 1005);
               wss.close(done);
             });
           });