[major] Change WebSocket#{p{i,o}ng,send}() behavior

- If the `readyState` attribute is `CONNECTING`, throw an exception. - If the `readyState` attribute is `CLOSING` or `CLOSED` - Increase the `bufferedAmount` attribute by the length of the `data` argument in bytes. - If specified, call the `callback` function with an error. Fixes #1515
websockets · Mar 19, 2019 · 17a8e7b · 17a8e7b
1 parent 7f5025d
commit 17a8e7b
Show file tree

Hide file tree

Showing 2 changed files with 325 additions and 100 deletions.
diff --git a/lib/websocket.js b/lib/websocket.js
@@ -21,6 +21,7 @@ const {
   kWebSocket,
   NOOP
 } = require('./constants');
+const { toBuffer } = require('./buffer-util');
 
 const readyStates = ['CONNECTING', 'OPEN', 'CLOSING', 'CLOSED'];
 const protocolVersions = [8, 13];
@@ -57,6 +58,7 @@ class WebSocket extends EventEmitter {
     this._socket = null;
 
     if (address !== null) {
+      this._bufferedAmount = 0;
       this._isServer = false;
       this._redirects = 0;
 
@@ -111,7 +113,7 @@ class WebSocket extends EventEmitter {
    * @type {Number}
    */
   get bufferedAmount() {
-    if (!this._socket) return 0;
+    if (!this._socket) return this._bufferedAmount;
 
     //
     // `socket.bufferSize` is `undefined` if the socket is closed.
@@ -255,6 +257,10 @@ class WebSocket extends EventEmitter {
    * @public
    */
   ping(data, mask, cb) {
+    if (this.readyState === WebSocket.CONNECTING) {
+      throw new Error('WebSocket is not open: readyState 0 (CONNECTING)');
+    }
+
     if (typeof data === 'function') {
       cb = data;
       data = mask = undefined;
@@ -263,17 +269,13 @@ class WebSocket extends EventEmitter {
       mask = undefined;
     }
 
-    if (this.readyState !== WebSocket.OPEN) {
-      const err = new Error(
-        `WebSocket is not open: readyState ${this.readyState} ` +
-          `(${readyStates[this.readyState]})`
-      );
+    if (typeof data === 'number') data = data.toString();
 
-      if (cb) return cb(err);
-      throw err;
+    if (this.readyState !== WebSocket.OPEN) {
+      sendAfterClose(this, data, cb);
+      return;
     }
 
-    if (typeof data === 'number') data = data.toString();
     if (mask === undefined) mask = !this._isServer;
     this._sender.ping(data || EMPTY_BUFFER, mask, cb);
   }
@@ -287,6 +289,10 @@ class WebSocket extends EventEmitter {
    * @public
    */
   pong(data, mask, cb) {
+    if (this.readyState === WebSocket.CONNECTING) {
+      throw new Error('WebSocket is not open: readyState 0 (CONNECTING)');
+    }
+
     if (typeof data === 'function') {
       cb = data;
       data = mask = undefined;
@@ -295,17 +301,13 @@ class WebSocket extends EventEmitter {
       mask = undefined;
     }
 
-    if (this.readyState !== WebSocket.OPEN) {
-      const err = new Error(
-        `WebSocket is not open: readyState ${this.readyState} ` +
-          `(${readyStates[this.readyState]})`
-      );
+    if (typeof data === 'number') data = data.toString();
 
-      if (cb) return cb(err);
-      throw err;
+    if (this.readyState !== WebSocket.OPEN) {
+      sendAfterClose(this, data, cb);
+      return;
     }
 
-    if (typeof data === 'number') data = data.toString();
     if (mask === undefined) mask = !this._isServer;
     this._sender.pong(data || EMPTY_BUFFER, mask, cb);
   }
@@ -315,31 +317,31 @@ class WebSocket extends EventEmitter {
    *
    * @param {*} data The message to send
    * @param {Object} options Options object
-   * @param {Boolean} options.compress Specifies whether or not to compress `data`
+   * @param {Boolean} options.compress Specifies whether or not to compress
+   *     `data`
    * @param {Boolean} options.binary Specifies whether `data` is binary or text
    * @param {Boolean} options.fin Specifies whether the fragment is the last one
    * @param {Boolean} options.mask Specifies whether or not to mask `data`
    * @param {Function} cb Callback which is executed when data is written out
    * @public
    */
   send(data, options, cb) {
+    if (this.readyState === WebSocket.CONNECTING) {
+      throw new Error('WebSocket is not open: readyState 0 (CONNECTING)');
+    }
+
     if (typeof options === 'function') {
       cb = options;
       options = {};
     }
 
-    if (this.readyState !== WebSocket.OPEN) {
-      const err = new Error(
-        `WebSocket is not open: readyState ${this.readyState} ` +
-          `(${readyStates[this.readyState]})`
-      );
+    if (typeof data === 'number') data = data.toString();
 
-      if (cb) return cb(err);
-      throw err;
+    if (this.readyState !== WebSocket.OPEN) {
+      sendAfterClose(this, data, cb);
+      return;
     }
 
-    if (typeof data === 'number') data = data.toString();
-
     const opts = Object.assign(
       {
         binary: typeof data !== 'string',
@@ -726,6 +728,38 @@ function abortHandshake(websocket, stream, message) {
   }
 }
 
+/**
+ * Handle cases where the `ping()`, `pong()`, or `send()` methods are called
+ * when the `readyState` attribute is `CLOSING` or `CLOSED`.
+ *
+ * @param {WebSocket} websocket The WebSocket instance
+ * @param {*} data The data to send
+ * @param {Function} cb Callback
+ * @private
+ */
+function sendAfterClose(websocket, data, cb) {
+  if (data) {
+    const length = toBuffer(data).length;
+
+    //
+    // The `_bufferedAmount` property is used only when the peer is a client and
+    // the opening handshake fails. Under these circumstances, in fact, the
+    // `setSocket()` method is not called, so the `_socket` and `_sender`
+    // properties are set to `null`.
+    //
+    if (websocket._socket) websocket._sender._bufferedBytes += length;
+    else websocket._bufferedAmount += length;
+  }
+
+  if (cb) {
+    const err = new Error(
+      `WebSocket is not open: readyState ${websocket.readyState} ` +
+        `(${readyStates[websocket.readyState]})`
+    );
+    cb(err);
+  }
+}
+
 /**
  * The listener of the `Receiver` `'conclude'` event.
  *