Repository: websockets/ws
Pull request #1240 (2017-11-17 12:32:54)

Rewrite the parser of the Sec-WebSocket-Extensions header
This rewrites the parser of the `Sec-WebSocket-Extensions` header to make it spec-compliant.
The implementation is not based on regular expressions to avoid possible ReDoS attacks. The added benefit is that it is faster than a [regex based parser](https://github.com/faye/websocket-extensions-node/blob/2af2c182515de125938430a82d1fe2c85a71c88b/lib/parser.js#L14-L51).

```
regex-parser x 154,349 ops/sec ±0.94% (84 runs sampled)
parser x 261,251 ops/sec ±0.92% (84 runs sampled)
```

There is probably also room for improvements and some duplicated code could be removed but I would prefer to leave this as a future exercise.


Patch:
diff --git a/lib/Extensions.js b/lib/Extensions.js
index 9cb55549..14c38ac2 100644
--- a/lib/Extensions.js
+++ b/lib/Extensions.js
@@ -1,67 +1,203 @@
 'use strict';
 
+//
+// Allowed token characters:
+//
+// '!', '#', '$', '%', '&', ''', '*', '+', '-',
+// '.', 0-9, A-Z, '^', '_', '`', a-z, '|', '~'
+//
+// tokenChars[32] === 0 // ' '
+// tokenChars[33] === 1 // '!'
+// tokenChars[34] === 0 // '"'
+// ...
+//
+const tokenChars = [
+  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 0 - 15
+  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 16 - 31
+  0, 1, 0, 1, 1, 1, 1, 1, 0, 0, 1, 1, 0, 1, 1, 0, // 32 - 47
+  1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, // 48 - 63
+  0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 64 - 79
+  1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 1, 1, // 80 - 95
+  1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 96 - 111
+  1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 1, 0, 1, 0 // 112 - 127
+];
+
 /**
- * Parse the `Sec-WebSocket-Extensions` header into an object.
+ * Adds an offer to the map of extension offers.
  *
- * @param {String} value field value of the header
+ * @param {Object} offers The map of extension offers
+ * @param {String} name The extension name
+ * @param {Object} params The extension parameters
+ * @private
+ */
+function pushOffer (offers, name, params) {
+  if (Object.hasOwnProperty.call(offers, name)) offers[name].push(params);
+  else offers[name] = [params];
+}
+
+/**
+ * Adds a parameter to the map of parameters.
+ *
+ * @param {Object} params The map of parameters
+ * @param {String} name The parameter name
+ * @param {Object} value The parameter value
+ * @private
+ */
+function pushParam (params, name, value) {
+  if (Object.hasOwnProperty.call(params, name)) params[name].push(value);
+  else params[name] = [value];
+}
+
+/**
+ * Parses the `Sec-WebSocket-Extensions` header into an object.
+ *
+ * @param {String} header The field value of the header
  * @return {Object} The parsed object
  * @public
  */
-const parse = (value) => {
-  value = value || '';
+function parse (header) {
+  const offers = {};
 
-  const extensions = {};
+  if (header === undefined || header === '') return offers;
 
-  value.split(',').forEach((v) => {
-    const params = v.split(';');
-    const token = params.shift().trim();
+  var params = {};
+  var mustUnescape = false;
+  var isEscaping = false;
+  var inQuotes = false;
+  var extensionName;
+  var paramName;
+  var start = -1;
+  var end = -1;
 
-    if (extensions[token] === undefined) {
-      extensions[token] = [];
-    } else if (!extensions.hasOwnProperty(token)) {
-      return;
-    }
+  for (var i = 0; i < header.length; i++) {
+    const code = header.charCodeAt(i);
 
-    const parsedParams = {};
+    if (extensionName === undefined) {
+      if (end === -1 && tokenChars[code] === 1) {
+        if (start === -1) start = i;
+      } else if (code === 0x20/* ' ' */|| code === 0x09/* '\t' */) {
+        if (end === -1 && start !== -1) end = i;
+      } else if (code === 0x3b/* ';' */ || code === 0x2c/* ',' */) {
+        if (start === -1) throw new Error(`unexpected character at index ${i}`);
 
-    params.forEach((param) => {
-      const parts = param.trim().split('=');
-      const key = parts[0];
-      var value = parts[1];
+        if (end === -1) end = i;
+        const name = header.slice(start, end);
+        if (code === 0x2c) {
+          pushOffer(offers, name, params);
+          params = {};
+        } else {
+          extensionName = name;
+        }
 
-      if (value === undefined) {
-        value = true;
+        start = end = -1;
       } else {
-        // unquote value
-        if (value[0] === '"') {
-          value = value.slice(1);
-        }
-        if (value[value.length - 1] === '"') {
-          value = value.slice(0, value.length - 1);
+        throw new Error(`unexpected character at index ${i}`);
+      }
+    } else if (paramName === undefined) {
+      if (end === -1 && tokenChars[code] === 1) {
+        if (start === -1) start = i;
+      } else if (code === 0x20 || code === 0x09) {
+        if (end === -1 && start !== -1) end = i;
+      } else if (code === 0x3b || code === 0x2c) {
+        if (start === -1) throw new Error(`unexpected character at index ${i}`);
+
+        if (end === -1) end = i;
+        pushParam(params, header.slice(start, end), true);
+        if (code === 0x2c) {
+          pushOffer(offers, extensionName, params);
+          params = {};
+          extensionName = undefined;
         }
+
+        start = end = -1;
+      } else if (code === 0x3d/* '=' */&& start !== -1 && end === -1) {
+        paramName = header.slice(start, i);
+        start = end = -1;
+      } else {
+        throw new Error(`unexpected character at index ${i}`);
       }
+    } else {
+      //
+      // The value of a quoted-string after unescaping must conform to the
+      // token ABNF, so only token characters are valid.
+      // Ref: https://tools.ietf.org/html/rfc6455#section-9.1
+      //
+      if (isEscaping) {
+        if (tokenChars[code] !== 1) {
+          throw new Error(`unexpected character at index ${i}`);
+        }
+        if (start === -1) start = i;
+        else if (!mustUnescape) mustUnescape = true;
+        isEscaping = false;
+      } else if (inQuotes) {
+        if (tokenChars[code] === 1) {
+          if (start === -1) start = i;
+        } else if (code === 0x22/* '"' */ && start !== -1) {
+          inQuotes = false;
+          end = i;
+        } else if (code === 0x5c/* '\' */) {
+          isEscaping = true;
+        } else {
+          throw new Error(`unexpected character at index ${i}`);
+        }
+      } else if (code === 0x22 && header.charCodeAt(i - 1) === 0x3d) {
+        inQuotes = true;
+      } else if (end === -1 && tokenChars[code] === 1) {
+        if (start === -1) start = i;
+      } else if (start !== -1 && (code === 0x20 || code === 0x09)) {
+        if (end === -1) end = i;
+      } else if (code === 0x3b || code === 0x2c) {
+        if (start === -1) throw new Error(`unexpected character at index ${i}`);
+
+        if (end === -1) end = i;
+        var value = header.slice(start, end);
+        if (mustUnescape) {
+          value = value.replace(/\\/g, '');
+          mustUnescape = false;
+        }
+        pushParam(params, paramName, value);
+        if (code === 0x2c) {
+          pushOffer(offers, extensionName, params);
+          params = {};
+          extensionName = undefined;
+        }
 
-      if (parsedParams[key] === undefined) {
-        parsedParams[key] = [value];
-      } else if (parsedParams.hasOwnProperty(key)) {
-        parsedParams[key].push(value);
+        paramName = undefined;
+        start = end = -1;
+      } else {
+        throw new Error(`unexpected character at index ${i}`);
       }
-    });
+    }
+  }
+
+  if (start === -1 || inQuotes) throw new Error('unexpected end of input');
 
-    extensions[token].push(parsedParams);
-  });
+  if (end === -1) end = i;
+  const token = header.slice(start, end);
+  if (extensionName === undefined) {
+    pushOffer(offers, token, {});
+  } else {
+    if (paramName === undefined) {
+      pushParam(params, token, true);
+    } else if (mustUnescape) {
+      pushParam(params, paramName, token.replace(/\\/g, ''));
+    } else {
+      pushParam(params, paramName, token);
+    }
+    pushOffer(offers, extensionName, params);
+  }
 
-  return extensions;
-};
+  return offers;
+}
 
 /**
- * Serialize a parsed `Sec-WebSocket-Extensions` header to a string.
+ * Serializes a parsed `Sec-WebSocket-Extensions` header to a string.
  *
  * @param {Object} value The object to format
  * @return {String} A string representing the given value
  * @public
  */
-const format = (value) => {
+function format (value) {
   return Object.keys(value).map((token) => {
     var paramsList = value[token];
     if (!Array.isArray(paramsList)) paramsList = [paramsList];
@@ -73,6 +209,6 @@ const format = (value) => {
       })).join('; ');
     }).join(', ');
   }).join(', ');
-};
+}
 
 module.exports = { format, parse };
diff --git a/lib/WebSocket.js b/lib/WebSocket.js
index ce183128..76e42c07 100644
--- a/lib/WebSocket.js
+++ b/lib/WebSocket.js
@@ -527,21 +527,8 @@ function initAsClient (address, protocols, options) {
   const isSecure = serverUrl.protocol === 'wss:' || serverUrl.protocol === 'https:';
   const key = crypto.randomBytes(16).toString('base64');
   const httpObj = isSecure ? https : http;
-
-  //
-  // Prepare extensions.
-  //
-  const extensionsOffer = {};
   var perMessageDeflate;
 
-  if (options.perMessageDeflate) {
-    perMessageDeflate = new PerMessageDeflate(
-      options.perMessageDeflate !== true ? options.perMessageDeflate : {},
-      false
-    );
-    extensionsOffer[PerMessageDeflate.extensionName] = perMessageDeflate.offer();
-  }
-
   const requestOptions = {
     port: serverUrl.port || (isSecure ? 443 : 80),
     host: serverUrl.hostname,
@@ -555,8 +542,14 @@ function initAsClient (address, protocols, options) {
   };
 
   if (options.headers) Object.assign(requestOptions.headers, options.headers);
-  if (Object.keys(extensionsOffer).length) {
-    requestOptions.headers['Sec-WebSocket-Extensions'] = Extensions.format(extensionsOffer);
+  if (options.perMessageDeflate) {
+    perMessageDeflate = new PerMessageDeflate(
+      options.perMessageDeflate !== true ? options.perMessageDeflate : {},
+      false
+    );
+    requestOptions.headers['Sec-WebSocket-Extensions'] = Extensions.format({
+      [PerMessageDeflate.extensionName]: perMessageDeflate.offer()
+    });
   }
   if (options.protocol) {
     requestOptions.headers['Sec-WebSocket-Protocol'] = options.protocol;
@@ -692,18 +685,23 @@ function initAsClient (address, protocols, options) {
 
     if (serverProt) this.protocol = serverProt;
 
-    const serverExtensions = Extensions.parse(res.headers['sec-websocket-extensions']);
-
-    if (perMessageDeflate && serverExtensions[PerMessageDeflate.extensionName]) {
+    if (perMessageDeflate) {
       try {
-        perMessageDeflate.accept(serverExtensions[PerMessageDeflate.extensionName]);
+        const serverExtensions = Extensions.parse(
+          res.headers['sec-websocket-extensions']
+        );
+
+        if (serverExtensions[PerMessageDeflate.extensionName]) {
+          perMessageDeflate.accept(
+            serverExtensions[PerMessageDeflate.extensionName]
+          );
+          this.extensions[PerMessageDeflate.extensionName] = perMessageDeflate;
+        }
       } catch (err) {
         socket.destroy();
-        this.emit('error', new Error('invalid extension parameter'));
+        this.emit('error', new Error('invalid Sec-WebSocket-Extensions header'));
         return this.finalize(true);
       }
-
-      this.extensions[PerMessageDeflate.extensionName] = perMessageDeflate;
     }
 
     this.setSocket(socket, head);
diff --git a/lib/WebSocketServer.js b/lib/WebSocketServer.js
index 790add82..571078f8 100644
--- a/lib/WebSocketServer.js
+++ b/lib/WebSocketServer.js
@@ -90,6 +90,7 @@ class WebSocketServer extends EventEmitter {
       });
     }
 
+    if (options.perMessageDeflate === true) options.perMessageDeflate = {};
     if (options.clientTracking) this.clients = new Set();
     this.options = options;
   }
@@ -151,6 +152,7 @@ class WebSocketServer extends EventEmitter {
     socket.on('error', socketError);
 
     const version = +req.headers['sec-websocket-version'];
+    const extensions = {};
 
     if (
       req.method !== 'GET' || req.headers.upgrade.toLowerCase() !== 'websocket' ||
@@ -160,6 +162,27 @@ class WebSocketServer extends EventEmitter {
       return abortConnection(socket, 400);
     }
 
+    if (this.options.perMessageDeflate) {
+      const perMessageDeflate = new PerMessageDeflate(
+        this.options.perMessageDeflate,
+        true,
+        this.options.maxPayload
+      );
+
+      try {
+        const offers = Extensions.parse(
+          req.headers['sec-websocket-extensions']
+        );
+
+        if (offers[PerMessageDeflate.extensionName]) {
+          perMessageDeflate.accept(offers[PerMessageDeflate.extensionName]);
+          extensions[PerMessageDeflate.extensionName] = perMessageDeflate;
+        }
+      } catch (err) {
+        return abortConnection(socket, 400);
+      }
+    }
+
     var protocol = (req.headers['sec-websocket-protocol'] || '').split(/, */);
 
     //
@@ -186,21 +209,30 @@ class WebSocketServer extends EventEmitter {
         this.options.verifyClient(info, (verified, code, message) => {
           if (!verified) return abortConnection(socket, code || 401, message);
 
-          this.completeUpgrade(protocol, version, req, socket, head, cb);
+          this.completeUpgrade(
+            protocol,
+            extensions,
+            version,
+            req,
+            socket,
+            head,
+            cb
+          );
         });
         return;
-      } else if (!this.options.verifyClient(info)) {
-        return abortConnection(socket, 401);
       }
+
+      if (!this.options.verifyClient(info)) return abortConnection(socket, 401);
     }
 
-    this.completeUpgrade(protocol, version, req, socket, head, cb);
+    this.completeUpgrade(protocol, extensions, version, req, socket, head, cb);
   }
 
   /**
    * Upgrade the connection to WebSocket.
    *
    * @param {String} protocol The chosen subprotocol
+   * @param {Object} extensions The accepted extensions
    * @param {Number} version The WebSocket protocol version
    * @param {http.IncomingMessage} req The request object
    * @param {net.Socket} socket The network socket between the server and client
@@ -208,7 +240,7 @@ class WebSocketServer extends EventEmitter {
    * @param {Function} cb Callback
    * @private
    */
-  completeUpgrade (protocol, version, req, socket, head, cb) {
+  completeUpgrade (protocol, extensions, version, req, socket, head, cb) {
     //
     // Destroy the socket if the client has already sent a FIN packet.
     //
@@ -226,25 +258,12 @@ class WebSocketServer extends EventEmitter {
     ];
 
     if (protocol) headers.push(`Sec-WebSocket-Protocol: ${protocol}`);
-
-    const offer = Extensions.parse(req.headers['sec-websocket-extensions']);
-    var extensions;
-
-    try {
-      extensions = acceptExtensions(this.options, offer);
-    } catch (err) {
-      return abortConnection(socket, 400);
-    }
-
-    const props = Object.keys(extensions);
-
-    if (props.length) {
-      const serverExtensions = props.reduce((obj, key) => {
-        obj[key] = [extensions[key].params];
-        return obj;
-      }, {});
-
-      headers.push(`Sec-WebSocket-Extensions: ${Extensions.format(serverExtensions)}`);
+    if (extensions[PerMessageDeflate.extensionName]) {
+      const params = extensions[PerMessageDeflate.extensionName].params;
+      const value = Extensions.format({
+        [PerMessageDeflate.extensionName]: [params]
+      });
+      headers.push(`Sec-WebSocket-Extensions: ${value}`);
     }
 
     //
@@ -252,7 +271,7 @@ class WebSocketServer extends EventEmitter {
     //
     this.emit('headers', headers, req);
 
-    socket.write(headers.concat('', '').join('\r\n'));
+    socket.write(headers.concat('\r\n').join('\r\n'));
 
     const client = new WebSocket([socket, head], null, {
       maxPayload: this.options.maxPayload,
@@ -282,32 +301,6 @@ function socketError () {
   this.destroy();
 }
 
-/**
- * Accept WebSocket extensions.
- *
- * @param {Object} options The `WebSocketServer` configuration options
- * @param {Object} offer The parsed value of the `sec-websocket-extensions` header
- * @return {Object} Accepted extensions
- * @private
- */
-function acceptExtensions (options, offer) {
-  const pmd = options.perMessageDeflate;
-  const extensions = {};
-
-  if (pmd && offer[PerMessageDeflate.extensionName]) {
-    const perMessageDeflate = new PerMessageDeflate(
-      pmd !== true ? pmd : {},
-      true,
-      options.maxPayload
-    );
-
-    perMessageDeflate.accept(offer[PerMessageDeflate.extensionName]);
-    extensions[PerMessageDeflate.extensionName] = perMessageDeflate;
-  }
-
-  return extensions;
-}
-
 /**
  * Close the connection when preconditions are not fulfilled.
  *


Test patch:
diff --git a/test/Extensions.test.js b/test/Extensions.test.js
index 169f68ec..f08ab085 100644
--- a/test/Extensions.test.js
+++ b/test/Extensions.test.js
@@ -6,6 +6,11 @@ const Extensions = require('../lib/Extensions');
 
 describe('Extensions', function () {
   describe('parse', function () {
+    it('returns an empty object if the argument is undefined', function () {
+      assert.deepStrictEqual(Extensions.parse(), {});
+      assert.deepStrictEqual(Extensions.parse(''), {});
+    });
+
     it('parses a single extension', function () {
       const extensions = Extensions.parse('foo');
 
@@ -13,15 +18,15 @@ describe('Extensions', function () {
     });
 
     it('parses params', function () {
-      const extensions = Extensions.parse('foo; bar; baz=1; bar=2');
+      const extensions = Extensions.parse('foo;bar;baz=1;bar=2');
 
       assert.deepStrictEqual(extensions, {
         foo: [{ bar: [true, '2'], baz: ['1'] }]
       });
     });
 
-    it('parse multiple extensions', function () {
-      const extensions = Extensions.parse('foo, bar; baz, foo; baz');
+    it('parses multiple extensions', function () {
+      const extensions = Extensions.parse('foo,bar;baz,foo;baz');
 
       assert.deepStrictEqual(extensions, {
         foo: [{}, { baz: [true] }],
@@ -30,18 +35,121 @@ describe('Extensions', function () {
     });
 
     it('parses quoted params', function () {
-      const extensions = Extensions.parse('foo; bar="hi"');
-
-      assert.deepStrictEqual(extensions, {
+      assert.deepStrictEqual(Extensions.parse('foo;bar="hi"'), {
         foo: [{ bar: ['hi'] }]
       });
+      assert.deepStrictEqual(Extensions.parse('foo;bar="\\0"'), {
+        foo: [{ bar: ['0'] }]
+      });
+      assert.deepStrictEqual(Extensions.parse('foo;bar="b\\a\\z"'), {
+        foo: [{ bar: ['baz'] }]
+      });
+      assert.deepStrictEqual(Extensions.parse('foo;bar="b\\az";bar'), {
+        foo: [{ bar: ['baz', true] }]
+      });
+      assert.throws(
+        () => Extensions.parse('foo;bar="baz"qux'),
+        /^Error: unexpected character at index 13$/
+      );
+      assert.throws(
+        () => Extensions.parse('foo;bar="baz" qux'),
+        /^Error: unexpected character at index 14$/
+      );
     });
 
-    it('ignores names that match Object.prototype properties', function () {
+    it('works with names that match Object.prototype property names', function () {
       const parse = Extensions.parse;
 
-      assert.deepStrictEqual(parse('hasOwnProperty, toString'), {});
-      assert.deepStrictEqual(parse('foo; constructor'), { foo: [{}] });
+      assert.deepStrictEqual(parse('hasOwnProperty, toString'), {
+        hasOwnProperty: [{}],
+        toString: [{}]
+      });
+      assert.deepStrictEqual(parse('foo;constructor'), {
+        foo: [{ constructor: [true] }]
+      });
+    });
+
+    it('ignores the optional white spaces', function () {
+      const header = 'foo; bar\t; \tbaz=1\t ;  bar="1"\t\t, \tqux\t ;norf ';
+
+      assert.deepStrictEqual(Extensions.parse(header), {
+        foo: [{ bar: [true, '1'], baz: ['1'] }],
+        qux: [{ norf: [true] }]
+      });
+    });
+
+    it('throws an error if a name is empty', function () {
+      [
+        [',', 0],
+        ['foo,,', 4],
+        ['foo,  ,', 6],
+        ['foo;=', 4],
+        ['foo; =', 5],
+        ['foo;;', 4],
+        ['foo; ;', 5],
+        ['foo;bar=,', 8],
+        ['foo;bar=""', 9]
+      ].forEach((element) => {
+        assert.throws(
+          () => Extensions.parse(element[0]),
+          new RegExp(`^Error: unexpected character at index ${element[1]}$`)
+        );
+      });
+    });
+
+    it('throws an error if a white space is misplaced', function () {
+      [
+        ['f oo', 2],
+        ['foo;ba r', 7],
+        ['foo;bar =', 8],
+        ['foo;bar= ', 8]
+      ].forEach((element) => {
+        assert.throws(
+          () => Extensions.parse(element[0]),
+          new RegExp(`^Error: unexpected character at index ${element[1]}$`)
+        );
+      });
+    });
+
+    it('throws an error if a token contains invalid characters', function () {
+      [
+        ['f@o', 1],
+        ['f\\oo', 1],
+        ['"foo"', 0],
+        ['f"oo"', 1],
+        ['foo;b@r', 5],
+        ['foo;b\\ar', 5],
+        ['foo;"bar"', 4],
+        ['foo;b"ar"', 5],
+        ['foo;bar=b@z', 9],
+        ['foo;bar=b\\az ', 9],
+        ['foo;bar="b@z"', 10],
+        ['foo;bar="baz;"', 12],
+        ['foo;bar=b"az"', 9],
+        ['foo;bar="\\\\"', 10]
+      ].forEach((element) => {
+        assert.throws(
+          () => Extensions.parse(element[0]),
+          new RegExp(`^Error: unexpected character at index ${element[1]}$`)
+        );
+      });
+    });
+
+    it('throws an error if the header value ends prematurely', function () {
+      [
+        'foo, ',
+        'foo;',
+        'foo;bar,',
+        'foo;bar; ',
+        'foo;bar=',
+        'foo;bar="baz',
+        'foo;bar="1\\'
+      ].forEach((header) => {
+        assert.throws(
+          () => Extensions.parse(header),
+          /^Error: unexpected end of input$/
+        );
+      });
     });
   });
 
diff --git a/test/PerMessageDeflate.test.js b/test/PerMessageDeflate.test.js
index b66ddd08..bfe25acb 100644
--- a/test/PerMessageDeflate.test.js
+++ b/test/PerMessageDeflate.test.js
@@ -174,13 +174,6 @@ describe('PerMessageDeflate', function () {
         assert.throws(() => perMessageDeflate.accept(extensions['permessage-deflate']));
       });
 
-      it('should throw an error if a parameter is undefined', function () {
-        const perMessageDeflate = new PerMessageDeflate();
-        const extensions = Extensions.parse('permessage-deflate; foo;');
-
-        assert.throws(() => perMessageDeflate.accept(extensions['permessage-deflate']));
-      });
-
       it('should throw an error if server_no_context_takeover has a value', function () {
         const perMessageDeflate = new PerMessageDeflate();
         const extensions = Extensions.parse('permessage-deflate; server_no_context_takeover=10');
diff --git a/test/WebSocket.test.js b/test/WebSocket.test.js
index 99cd7551..84e20baf 100644
--- a/test/WebSocket.test.js
+++ b/test/WebSocket.test.js
@@ -352,7 +352,7 @@ describe('WebSocket', function () {
 
     it('invalid server key is denied', function (done) {
       server.once('upgrade', (req, socket) => {
-        socket.on('end', () => socket.end());
+        socket.on('end', socket.end);
         socket.write(
           'HTTP/1.1 101 Switching Protocols\r\n' +
           'Upgrade: websocket\r\n' +
@@ -408,7 +408,7 @@ describe('WebSocket', function () {
 
       const ws = new WebSocket(`ws://localhost:${port}`);
 
-      ws.on('open', () => assert.fail(null, null, 'connect shouldnt be raised here'));
+      ws.on('open', () => done(new Error("unexpected 'open' event")));
       ws.on('error', (err) => {
         assert.ok(err instanceof Error);
         assert.strictEqual(err.message, 'unexpected server response (401)');
@@ -430,8 +430,8 @@ describe('WebSocket', function () {
 
       const ws = new WebSocket(`ws://localhost:${port}`);
 
-      ws.on('open', () => assert.fail(null, null, 'connect shouldnt be raised here'));
-      ws.on('error', () => assert.fail(null, null, 'error shouldnt be raised here'));
+      ws.on('open', () => done(new Error("unexpected 'open' event")));
+      ws.on('error', () => done(new Error("unexpected 'error' event")));
       ws.on('unexpected-response', (req, res) => {
         assert.strictEqual(res.statusCode, 401);
 
@@ -462,8 +462,8 @@ describe('WebSocket', function () {
 
       const ws = new WebSocket(`ws://localhost:${port}`);
 
-      ws.on('open', () => assert.fail(null, null, 'connect shouldnt be raised here'));
-      ws.on('error', () => assert.fail(null, null, 'error shouldnt be raised here'));
+      ws.on('open', () => done(new Error("unexpected 'open' event")));
+      ws.on('error', () => done(new Error("unexpected 'error' event")));
       ws.on('unexpected-response', (req, res) => {
         assert.strictEqual(res.statusCode, 401);
 
@@ -472,20 +472,75 @@ describe('WebSocket', function () {
       });
     });
 
-    it('emits an error if the opening handshake timeout expires', function (done) {
+    it('fails if the opening handshake timeout expires', function (done) {
       server.once('upgrade', (req, socket) => socket.on('end', socket.end));
 
       const ws = new WebSocket(`ws://localhost:${port}`, null, {
         handshakeTimeout: 100
       });
 
-      ws.on('open', () => assert.fail(null, null, 'connect shouldn\'t be raised here'));
+      ws.on('open', () => done(new Error("unexpected 'open' event")));
       ws.on('error', (err) => {
         assert.ok(err instanceof Error);
         assert.strictEqual(err.message, 'opening handshake has timed out');
         done();
       });
     });
+
+    it('fails if the Sec-WebSocket-Extensions response header is invalid', function (done) {
+      server.once('upgrade', (req, socket) => {
+        const key = crypto.createHash('sha1')
+          .update(req.headers['sec-websocket-key'] + constants.GUID, 'binary')
+          .digest('base64');
+
+        socket.end(
+          'HTTP/1.1 101 Switching Protocols\r\n' +
+          'Upgrade: websocket\r\n' +
+          'Connection: Upgrade\r\n' +
+          `Sec-WebSocket-Accept: ${key}\r\n` +
+          'Sec-WebSocket-Extensions: foo;=\r\n' +
+          '\r\n'
+        );
+      });
+
+      const ws = new WebSocket(`ws://localhost:${port}`);
+
+      ws.on('open', () => done(new Error("unexpected 'open' event")));
+      ws.on('error', (err) => {
+        assert.ok(err instanceof Error);
+        assert.strictEqual(err.message, 'invalid Sec-WebSocket-Extensions header');
+        ws.on('close', () => done());
+      });
+    });
+
+    it('fails if server sends a subprotocol when none was requested', function (done) {
+      server.once('upgrade', (req, socket) => {
+        const key = crypto.createHash('sha1')
+          .update(req.headers['sec-websocket-key'] + constants.GUID, 'binary')
+          .digest('base64');
+
+        socket.end(
+          'HTTP/1.1 101 Switching Protocols\r\n' +
+          'Upgrade: websocket\r\n' +
+          'Connection: Upgrade\r\n' +
+          `Sec-WebSocket-Accept: ${key}\r\n` +
+          'Sec-WebSocket-Protocol: foo\r\n' +
+          '\r\n'
+        );
+      });
+
+      const ws = new WebSocket(`ws://localhost:${port}`);
+
+      ws.on('open', () => done(new Error("unexpected 'open' event")));
+      ws.on('error', (err) => {
+        assert.ok(err instanceof Error);
+        assert.strictEqual(
+          err.message,
+          'server sent a subprotocol even though none requested'
+        );
+        ws.on('close', () => done());
+      });
+    });
   });
 
   describe('connection with query string', function () {
@@ -968,7 +1023,7 @@ describe('WebSocket', function () {
       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('open', () => done(new Error("unexpected 'open' event")));
         ws.on('error', (err) => {
           assert.ok(err instanceof Error);
           assert.strictEqual(err.message, 'closed before the connection is established');
@@ -985,7 +1040,7 @@ describe('WebSocket', function () {
       }, () => {
         const ws = new WebSocket(`ws://localhost:${port}`);
 
-        ws.on('open', () => assert.fail(null, null, 'connect shouldnt be raised here'));
+        ws.on('open', () => done(new Error("unexpected 'open' event")));
         ws.on('error', (err) => {
           assert.ok(err instanceof Error);
           assert.strictEqual(err.message, 'closed before the connection is established');
@@ -998,7 +1053,7 @@ describe('WebSocket', function () {
     it('can be called from an error listener while connecting', function (done) {
       const ws = new WebSocket(`ws://localhost:${++port}`);
 
-      ws.on('open', () => assert.fail(null, null, 'connect shouldnt be raised here'));
+      ws.on('open', () => done(new Error("unexpected 'open' event")));
       ws.on('error', (err) => {
         assert.ok(err instanceof Error);
         assert.strictEqual(err.code, 'ECONNREFUSED');
@@ -1011,7 +1066,7 @@ describe('WebSocket', function () {
       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('open', () => done(new Error("unexpected 'open' event")));
         ws.on('error', (err) => {
           assert.ok(err instanceof Error);
           assert.strictEqual(err.message, 'closed before the connection is established');
@@ -1192,7 +1247,7 @@ describe('WebSocket', function () {
       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('open', () => done(new Error("unexpected 'open' event")));
         ws.on('error', (err) => {
           assert.ok(err instanceof Error);
           assert.strictEqual(err.message, 'closed before the connection is established');
@@ -1209,7 +1264,7 @@ describe('WebSocket', function () {
       }, () => {
         const ws = new WebSocket(`ws://localhost:${port}`);
 
-        ws.on('open', () => assert.fail(null, null, 'connect shouldnt be raised here'));
+        ws.on('open', () => done(new Error("unexpected 'open' event")));
         ws.on('error', (err) => {
           assert.ok(err instanceof Error);
           assert.strictEqual(err.message, 'closed before the connection is established');
@@ -1222,7 +1277,7 @@ describe('WebSocket', function () {
     it('can be called from an error listener while connecting', function (done) {
       const ws = new WebSocket(`ws://localhost:${++port}`);
 
-      ws.on('open', () => assert.fail(null, null, 'connect shouldnt be raised here'));
+      ws.on('open', () => done(new Error("unexpected 'open' event")));
       ws.on('error', (err) => {
         assert.ok(err instanceof Error);
         assert.strictEqual(err.code, 'ECONNREFUSED');
@@ -1235,7 +1290,7 @@ describe('WebSocket', function () {
       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('open', () => done(new Error("unexpected 'open' event")));
         ws.on('error', (err) => {
           assert.ok(err instanceof Error);
           assert.strictEqual(err.message, 'closed before the connection is established');