Test on recent versions of Node

There is a regular expression denial of service (ReDoS) vulnerability in
the parser we use to process the `Sec-WebSocket-Extensions` header. It
can be exploited by sending an opening WebSocket handshake to a server
containing a header of the form:

    Sec-WebSocket-Extensions: a;b="\c\c\c\c\c\c\c\c\c\c ...

i.e. a header containing an unclosed string parameter value whose
content is a repeating two-byte sequence of a backslash and some other
character. The parser takes exponential time to reject this header as
invalid, and this can be used to exhaust the server's capacity to
process requests.

This vulnerability has been assigned the identifier CVE-2020-7662 and
was reported by Robert McLaughlin.

We believe this flaw stems from the grammar specified for this header.
[RFC 6455][1] defines the grammar for the header as:

    Sec-WebSocket-Extensions = extension-list

    extension-list    = 1#extension
    extension         = extension-token *( ";" extension-param )
    extension-token   = registered-token
    registered-token  = token
    extension-param   = token [ "=" (token | quoted-string) ]

It refers to [RFC 2616][2] for the definitions of `token` and
`quoted-string`, which are:

    token          = 1*<any CHAR except CTLs or separators>
    separators     = "(" | ")" | "<" | ">" | "@"
                   | "," | ";" | ":" | "\" | <">
                   | "/" | "[" | "]" | "?" | "="
                   | "{" | "}" | SP | HT

    quoted-string  = ( <"> *(qdtext | quoted-pair ) <"> )
    qdtext         = <any TEXT except <">>
    quoted-pair    = "\" CHAR

These rely on the `CHAR`, `CTL` and `TEXT` grammars, which are:

    CHAR           = <any US-ASCII character (octets 0 - 127)>
    CTL            = <any US-ASCII control character (octets 0 - 31) and DEL (127)>
    TEXT           = <any OCTET except CTLs, but including LWS>

Other relevant definitions to support these:

    OCTET          = <any 8-bit sequence of data>
    LWS            = [CRLF] 1*( SP | HT )
    CRLF           = CR LF

    HT             = <US-ASCII HT, horizontal-tab (9)>
    LF             = <US-ASCII LF, linefeed (10)>
    CR             = <US-ASCII CR, carriage return (13)>
    SP             = <US-ASCII SP, space (32)>

To expand some of these terms out and write them as regular expressions:

    OCTET         = [\x00-\xFF]
    CHAR          = [\x00-\x7F]
    TEXT          = [\t \x21-\x7E\x80-\xFF]

The allowable bytes for `token` are [\x00-\x7F], except [\x00-\x1F\x7F]
(leaving [\x20-\x7E]) and `separators`, which leaves the following set
of allowed chars:

    ! # $ % & ' * + - . ^ _ ` | ~ [0-9] [A-Z] [a-z]

`quoted-string` contains a repeated pattern of either `qdtext` or
`quoted-pair`. `qdtext` is any `TEXT` byte except <">, and the <">
character is ASCII 34, or 0x22. The <!> character is 0x21. So `qdtext`
can be written either positively as:

    qdtext        = [\t !\x23-\x7E\x80-\xFF]

or negatively, as:

    qdtext        = [^\x00-\x08\x0A-\x1F\x7F"]

We use the negative definition here. The other alternative in the
`quoted-string` pattern is:

    quoted-pair   = \\[\x00-\x7F]

The problem is that the set of bytes matched by `qdtext` includes <\>,
and intersects with the second element of `quoted-pair`. That means the
sequence \c can be matched as either two `qdtext` bytes, or as a single
`quoted-pair`. When the regex engine fails to find a trailing <"> to
close the string, it back-tracks and tries every alternate parse for the
string, which doubles with each pair of bytes in the input.

To fix the ReDoS flaw we need to rewrite the repeating pattern so that
none of its alternate branches can match the same text. For example, we
could try dividing the set of bytes [\x00-\xFF] into those that must not
follow a <\>, those that may follow a <\>, and those that must be
preceded by <\>, and thereby construct a pattern of the form:

    (A|\?B|\C)*

where A, B and C have no characters in common. In our case the three
branch patterns would be:

    A   =   qdtext - CHAR   =   [\x80-\xFF]
    B   =   qdtext & CHAR   =   [\t !\x23-\x7E]
    C   =   CHAR - qdtext   =   [\x00-\x08\x0A-\x1F\x7F"]

These sets do not intersect, and notice <"> appears in set C so must be
preceded by <\>. But we still have a problem: <\> (0x5C) and all the
alphabetic characters are in set B, so the pattern \?B can match all
these:

    c
    \
    \c

So the sequence \c\c\c... still produces exponential back-tracking. It
also fails to parse input like this correctly:

    Sec-WebSocket-Extensions: a; b="c\", d"

Because the grammar allows a single backslash to appear by itself, this
is arguably a syntax error where the parameter `b` has value `c\` and
then a new extension `d` begins with a <"> appearing where it should
not.

So the core problem is with the grammar itself: `qdtext` matches a
single backslash <\>, and `quoted-pair` matches a pair <\\>. So given a
sequence of backslashes there's no canonical parse and the grammar is
ambiguous.

[RFC 7230][3] remedies this problem and makes the grammar clearer.
First, it defines `token` explicitly rather than implicitly:

    token          = 1*tchar

    tchar          = "!" / "#" / "$" / "%" / "&" / "'" / "*"
                   / "+" / "-" / "." / "^" / "_" / "`" / "|" / "~"
                   / DIGIT / ALPHA

And second, it defines `quoted-string` so that backslashes cannot appear
on their own:

     quoted-string  = DQUOTE *( qdtext / quoted-pair ) DQUOTE
     qdtext         = HTAB / SP /%x21 / %x23-5B / %x5D-7E / obs-text
     obs-text       = %x80-FF
     quoted-pair    = "\" ( HTAB / SP / VCHAR / obs-text )

where VCHAR is any printing ASCII character 0x21-0x7E. Notice `qdtext`
is just our previous definition but with 5C excluded, so it cannot
accept a single backslash.

This commit makes this modification to our matching patterns, and
thereby removes the ReDoS vector. Technically this means it does not
match the grammar of RFC 6455, but we expect this to have little or no
practical impact, especially since the one main protocol extension,
`permessage-deflate` ([RFC 7692][4]), does not have any string-valued
parameters.

[1]: https://tools.ietf.org/html/rfc6455#section-9.1
[2]: https://tools.ietf.org/html/rfc2616#section-2.2
[3]: https://tools.ietf.org/html/rfc7230#section-3.2.6
[4]: https://tools.ietf.org/html/rfc7692

.github/workflows/test.yml

@@ -0,0 +1,41 @@
+on:
+  - push
+  - pull_request
+
+jobs:
+  test:
+    strategy:
+      fail-fast: false
+      matrix:
+        node:
+          - '0.8'
+          - '0.10'
+          - '0.12'
+          - '4'
+          - '6'
+          - '8'
+          - '10'
+          - '12'
+          - '14'
+          - '16'
+          - '18'
+          - '20'
+    name: node.js v${{ matrix.node }}
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+
+      - uses: actions/setup-node@v2
+        with:
+          node-version: ${{ matrix.node }}
+
+      - if: matrix.node == '0.8'
+        run: npm conf set strict-ssl false
+
+      - run: node --version
+      - run: npm install
+
+      - run: npm install 'nopt@5'
+      - run: rm -rf node_modules/jstest/node_modules/nopt
+
+      - run: npm test

.gitignore

@@ -1 +1,2 @@
 node_modules
+package-lock.json

.npmignore

@@ -1,6 +0,0 @@
-.git
-.gitignore
-.npmignore
-.travis.yml
-node_modules
-spec

.travis.yml

@@ -1,11 +0,0 @@
-language: node_js
-
-node_js:
-  - "0.6"
-  - "0.8"
-  - "0.10"
-  - "0.12"
-  - "iojs"
-
-before_install:
-  - '[ "${TRAVIS_NODE_VERSION}" = "0.6" ] && npm conf set strict-ssl false || true'

CHANGELOG.md

@@ -1,8 +1,28 @@
+### 0.1.4 / 2020-06-02
+
+- Remove a ReDoS vulnerability in the header parser (CVE-2020-7662, reported by
+  Robert McLaughlin)
+- Change license from MIT to Apache 2.0
+
+### 0.1.3 / 2017-11-11
+
+- Accept extension names and parameters including uppercase letters
+- Handle extension names that clash with `Object.prototype` properties
+
+### 0.1.2 / 2017-09-10
+
+- Catch synchronous exceptions thrown when calling an extension
+- Fix race condition caused when a message is pushed after a cell has stopped
+  due to an error
+- Fix failure of `close()` to return if a message that's queued after one that
+  produces an error never finishes being processed
+
 ### 0.1.1 / 2015-02-19
 
-* Prevent sessions being closed before they have finished processing messages
-* Add a callback to `Extensions.close()` so the caller can tell when it's safe to close the socket
+- Prevent sessions being closed before they have finished processing messages
+- Add a callback to `Extensions.close()` so the caller can tell when it's safe
+  to close the socket
 
 ### 0.1.0 / 2014-12-12
 
-* Initial release
+- Initial release

CODE_OF_CONDUCT.md

@@ -0,0 +1,4 @@
+# Code of Conduct
+
+All projects under the [Faye](https://github.com/faye) umbrella are covered by
+the [Code of Conduct](https://github.com/faye/code-of-conduct).

LICENSE.md

@@ -0,0 +1,12 @@
+Copyright 2014-2020 James Coglan
+
+Licensed under the Apache License, Version 2.0 (the "License"); you may not use
+this file except in compliance with the License. You may obtain a copy of the
+License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software distributed
+under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR
+CONDITIONS OF ANY KIND, either express or implied. See the License for the
+specific language governing permissions and limitations under the License.

README.md

@@ -1,4 +1,4 @@
-# websocket-extensions [![Build status](https://secure.travis-ci.org/faye/websocket-extensions-node.svg)](http://travis-ci.org/faye/websocket-extensions-node)
+# websocket-extensions
 
 A minimal framework that supports the implementation of WebSocket extensions in
 a way that's decoupled from the main protocol. This library aims to allow a
@@ -234,8 +234,8 @@ then the `permessage-deflate` extension will receive the call:
 
 ```js
 ext.createServerSession([
-  {server_no_context_takeover: true, server_max_window_bits: 8},
-  {server_max_window_bits: 15}
+  { server_no_context_takeover: true, server_max_window_bits: 8 },
+  { server_max_window_bits: 15 }
 ]);
 ```
 
@@ -251,8 +251,8 @@ implement the following methods, as well as the *Session* API listed below.
 ```js
 clientSession.generateOffer()
 // e.g.  -> [
-//            {server_no_context_takeover: true, server_max_window_bits: 8},
-//            {server_max_window_bits: 15}
+//            { server_no_context_takeover: true, server_max_window_bits: 8 },
+//            { server_max_window_bits: 15 }
 //          ]
 ```
 
@@ -277,7 +277,7 @@ must implement the following methods, as well as the *Session* API listed below.
 
 ```js
 serverSession.generateResponse()
-// e.g.  -> {server_max_window_bits: 8}
+// e.g.  -> { server_max_window_bits: 8 }
 ```
 
 This returns the set of parameters the server session wants to send in its
@@ -327,28 +327,5 @@ the session to release any resources it's using.
 
 ## Examples
 
-* Consumer: [websocket-driver](https://github.com/faye/websocket-driver-node)
-* Provider: [permessage-deflate](https://github.com/faye/permessage-deflate-node)
-
-## License
-
-(The MIT License)
-
-Copyright (c) 2014-2015 James Coglan
-
-Permission is hereby granted, free of charge, to any person obtaining a copy of
-this software and associated documentation files (the 'Software'), to deal in
-the Software without restriction, including without limitation the rights to
-use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of
-the Software, and to permit persons to whom the Software is furnished to do so,
-subject to the following conditions:
-
-The above copyright notice and this permission notice shall be included in all
-copies or substantial portions of the Software.
-
-THE SOFTWARE IS PROVIDED 'AS IS', WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
-IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
-FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
-COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER
-IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
-CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+- Consumer: [websocket-driver](https://github.com/faye/websocket-driver-node)
+- Provider: [permessage-deflate](https://github.com/faye/permessage-deflate-node)

lib/parser.js

@@ -1,13 +1,15 @@
 'use strict';
 
-var TOKEN    = /([!#\$%&'\*\+\-\.\^_`\|~0-9a-z]+)/,
-    NOTOKEN  = /([^!#\$%&'\*\+\-\.\^_`\|~0-9a-z])/g,
-    QUOTED   = /"((?:\\[\x00-\x7f]|[^\x00-\x08\x0a-\x1f\x7f"])*)"/,
+var TOKEN    = /([!#\$%&'\*\+\-\.\^_`\|~0-9A-Za-z]+)/,
+    NOTOKEN  = /([^!#\$%&'\*\+\-\.\^_`\|~0-9A-Za-z])/g,
+    QUOTED   = /"((?:\\[\x00-\x7f]|[^\x00-\x08\x0a-\x1f\x7f"\\])*)"/,
     PARAM    = new RegExp(TOKEN.source + '(?:=(?:' + TOKEN.source + '|' + QUOTED.source + '))?'),
     EXT      = new RegExp(TOKEN.source + '(?: *; *' + PARAM.source + ')*', 'g'),
     EXT_LIST = new RegExp('^' + EXT.source + '(?: *, *' + EXT.source + ')*$'),
     NUMBER   = /^-?(0|[1-9][0-9]*)(\.[0-9]+)?$/;
 
+var hasOwnProperty = Object.prototype.hasOwnProperty;
+
 var Parser = {
   parseHeader: function(header) {
     var offers = new Offers();
@@ -35,7 +37,7 @@ var Parser = {
         }
         if (NUMBER.test(data)) data = parseFloat(data);
 
-        if (offer.hasOwnProperty(key)) {
+        if (hasOwnProperty.call(offer, key)) {
           offer[key] = [].concat(offer[key]);
           offer[key].push(data);
         } else {
@@ -77,9 +79,11 @@ var Offers = function() {
 };
 
 Offers.prototype.push = function(name, params) {
-  this._byName[name] = this._byName[name] || [];
+  if (!hasOwnProperty.call(this._byName, name))
+    this._byName[name] = [];
+
   this._byName[name].push(params);
-  this._inOrder.push({name: name, params: params});
+  this._inOrder.push({ name: name, params: params });
 };
 
 Offers.prototype.eachOffer = function(callback, context) {

lib/pipeline/README.md

@@ -124,11 +124,11 @@ var crypto = require('crypto'),
     large  = crypto.randomBytes(1 << 14),
     small  = new Buffer('hi');
 
-deflate.outgoing({data: large}, function() {
+deflate.outgoing({ data: large }, function() {
   console.log(1, 'large');
 });
 
-deflate.outgoing({data: small}, function() {
+deflate.outgoing({ data: small }, function() {
   console.log(2, 'small');
 });
 
@@ -205,7 +205,7 @@ order is preserved:
 
 ```js
 var stream  = require('stream'),
-    session = new stream.Transform({objectMode: true});
+    session = new stream.Transform({ objectMode: true });
 
 session._transform = function(message, _, callback) {
   var self = this;
@@ -228,7 +228,7 @@ is a mechanism to reorder the output so that message order is preserved for the
 next session in line.
 
 
-## Solution
+## Solution
 
 We now describe the model implemented here and how it meets the above design
 goals. The above diagram where a stack of extensions sit between the driver and
@@ -276,11 +276,11 @@ above:
 ```js
 var functor = new Functor(deflate, 'outgoing');
 
-functor.call({data: large}, function() {
+functor.call({ data: large }, function() {
   console.log(1, 'large');
 });
 
-functor.call({data: small}, function() {
+functor.call({ data: small }, function() {
   console.log(2, 'small');
 });
 
@@ -592,7 +592,7 @@ pipeline following *m2* will be processed by `A`, since it's upstream of the
 error. Those messages will be dropped by `B`.
 
 
-## Alternative ideas
+## Alternative ideas
 
 I am considering implementing `Functor` as an object-mode transform stream
 rather than what is essentially an async function. Being object-mode, a stream

lib/pipeline/cell.js

@@ -14,7 +14,8 @@ var Cell = function(tuple) {
 };
 
 Cell.prototype.pending = function(direction) {
-  this._functors[direction].pending += 1;
+  var functor = this._functors[direction];
+  if (!functor._stopped) functor.pending += 1;
 };
 
 Cell.prototype.incoming = function(error, message, callback, context) {

lib/pipeline/functor.js

@@ -15,7 +15,7 @@ Functor.QUEUE_SIZE = 8;
 Functor.prototype.call = function(error, message, callback, context) {
   if (this._stopped) return;
 
-  var record = {error: error, message: message, callback: callback, context: context, done: false},
+  var record = { error: error, message: message, callback: callback, context: context, done: false },
       called = false,
       self   = this;
 
@@ -27,7 +27,7 @@ Functor.prototype.call = function(error, message, callback, context) {
     return this._flushQueue();
   }
 
-  this._session[this._method](message, function(err, msg) {
+  var handler = function(err, msg) {
     if (!(called ^ (called = true))) return;
 
     if (err) {
@@ -40,7 +40,13 @@ Functor.prototype.call = function(error, message, callback, context) {
 
     record.done = true;
     self._flushQueue();
-  });
+  };
+
+  try {
+    this._session[this._method](message, handler);
+  } catch (err) {
+    handler(err);
+  }
 };
 
 Functor.prototype._stop = function() {
@@ -52,8 +58,13 @@ Functor.prototype._flushQueue = function() {
   var queue = this._queue, record;
 
   while (queue.length > 0 && queue.peek().done) {
-    this.pending -= 1;
     record = queue.shift();
+    if (record.error) {
+      this.pending = 0;
+      queue.clear();
+    } else {
+      this.pending -= 1;
+    }
     record.callback.call(record.context, record.error, record.message);
   }
 };

lib/pipeline/index.js

@@ -5,7 +5,7 @@ var Cell   = require('./cell'),
 
 var Pipeline = function(sessions) {
   this._cells   = sessions.map(function(session) { return new Cell(session) });
-  this._stopped = {incoming: false, outgoing: false};
+  this._stopped = { incoming: false, outgoing: false };
 };
 
 Pipeline.prototype.processIncomingMessage = function(message, callback, context) {
@@ -19,7 +19,7 @@ Pipeline.prototype.processOutgoingMessage = function(message, callback, context)
 };
 
 Pipeline.prototype.close = function(callback, context) {
-  this._stopped = {incoming: true, outgoing: true};
+  this._stopped = { incoming: true, outgoing: true };
 
   var closed = this._cells.map(function(a) { return a.close() });
   if (callback)

lib/pipeline/ring_buffer.js

@@ -1,10 +1,14 @@
 'use strict';
 
 var RingBuffer = function(bufferSize) {
-  this._buffer     = new Array(bufferSize);
   this._bufferSize = bufferSize;
+  this.clear();
+};
+
+RingBuffer.prototype.clear = function() {
+  this._buffer     = new Array(this._bufferSize);
   this._ringOffset = 0;
-  this._ringSize   = bufferSize;
+  this._ringSize   = this._bufferSize;
   this._head       = 0;
   this._tail       = 0;
   this.length      = 0;

lib/websocket_extensions.js

@@ -9,7 +9,7 @@ var Extensions = function() {
   this._byName   = {};
   this._inOrder  = [];
   this._sessions = [];
-  this._index    = {}
+  this._index    = {};
 };
 
 Extensions.MESSAGE_OPCODES = [1, 2];
@@ -89,9 +89,9 @@ var instance = {
   },
 
   generateResponse: function(header) {
-    var offers   = Parser.parseHeader(header),
-        sessions = [],
-        response = [];
+    var sessions = [],
+        response = [],
+        offers   = Parser.parseHeader(header);
 
     this._inOrder.forEach(function(ext) {
       var offer = offers.byName(ext.name);
@@ -112,7 +112,7 @@ var instance = {
   },
 
   validFrameRsv: function(frame) {
-    var allowed = {rsv1: false, rsv2: false, rsv3: false},
+    var allowed = { rsv1: false, rsv2: false, rsv3: false },
         ext;
 
     if (Extensions.MESSAGE_OPCODES.indexOf(frame.opcode) >= 0) {

package.json

@@ -1,20 +1,29 @@
-{ "name"            : "websocket-extensions"
-, "description"     : "Generic extension manager for WebSocket connections"
-, "homepage"        : "http://github.com/faye/websocket-extensions-node"
-, "author"          : "James Coglan <jcoglan@gmail.com> (http://jcoglan.com/)"
-, "keywords"        : ["websocket"]
-, "license"         : "MIT"
-
-, "version"         : "0.1.1"
-, "engines"         : {"node": ">=0.6.0"}
-, "main"            : "./lib/websocket_extensions"
-, "devDependencies" : {"jstest": ""}
-
-, "scripts"         : {"test": "jstest spec/runner.js"}
-
-, "repository"      : { "type"  : "git"
-                      , "url"   : "git://github.com/faye/websocket-extensions-node.git"
-                      }
-
-, "bugs"            : "http://github.com/faye/websocket-extensions-node/issues"
+{
+          "name": "websocket-extensions",
+          "description": "Generic extension manager for WebSocket connections",
+          "homepage": "http://github.com/faye/websocket-extensions-node",
+          "author": "James Coglan <jcoglan@gmail.com> (http://jcoglan.com/)",
+          "keywords": [
+                    "websocket"
+          ],
+          "license": "Apache-2.0",
+          "version": "0.1.4",
+          "engines": {
+                    "node": ">=0.8.0"
+          },
+          "files": [
+                    "lib"
+          ],
+          "main": "./lib/websocket_extensions",
+          "devDependencies": {
+                    "jstest": "*"
+          },
+          "scripts": {
+                    "test": "jstest spec/runner.js"
+          },
+          "repository": {
+                    "type": "git",
+                    "url": "git://github.com/faye/websocket-extensions-node.git"
+          },
+          "bugs": "http://github.com/faye/websocket-extensions-node/issues"
 }

spec/parser_spec.js

@@ -20,53 +20,69 @@ test.describe("Parser", function() { with(this) {
     }})
 
     it("parses one offer with no params", function() { with(this) {
-      assertEqual( [{name: "a", params: {}}],
+      assertEqual( [{ name: "a", params: {}}],
                    parse('a') )
     }})
 
     it("parses two offers with no params", function() { with(this) {
-      assertEqual( [{name: "a", params: {}}, {name: "b", params: {}}],
+      assertEqual( [{ name: "a", params: {}}, { name: "b", params: {}}],
                    parse('a, b') )
     }})
 
     it("parses a duplicate offer name", function() { with(this) {
-      assertEqual( [{name: "a", params: {}}, {name: "a", params: {}}],
+      assertEqual( [{ name: "a", params: {}}, { name: "a", params: {}}],
                    parse('a, a') )
     }})
 
     it("parses a flag", function() { with(this) {
-      assertEqual( [{name: "a", params: {b: true}}],
+      assertEqual( [{ name: "a", params: { b: true }}],
                    parse('a; b') )
     }})
 
     it("parses an unquoted param", function() { with(this) {
-      assertEqual( [{name: "a", params: {b: 1}}],
+      assertEqual( [{ name: "a", params: { b: 1 }}],
                    parse('a; b=1') )
     }})
 
     it("parses a quoted param", function() { with(this) {
-      assertEqual( [{name: "a", params: {b: 'hi, "there'}}],
+      assertEqual( [{ name: "a", params: { b: 'hi, "there' }}],
                    parse('a; b="hi, \\"there"') )
     }})
 
     it("parses multiple params", function() { with(this) {
-      assertEqual( [{name: "a", params: {b: true, c: 1, d: 'hi'}}],
+      assertEqual( [{ name: "a", params: { b: true, c: 1, d: 'hi' }}],
                    parse('a; b; c=1; d="hi"') )
     }})
 
     it("parses duplicate params", function() { with(this) {
-      assertEqual( [{name: "a", params: {b: [true, 'hi'], c: 1}}],
+      assertEqual( [{ name: "a", params: { b: [true, 'hi'], c: 1 }}],
                    parse('a; b; c=1; b="hi"') )
     }})
 
     it("parses multiple complex offers", function() { with(this) {
-      assertEqual( [{name: "a", params: {b: 1}},
-                    {name: "c", params: {}},
-                    {name: "b", params: {d: true}},
-                    {name: "c", params: {e: ['hi, there', true]}},
-                    {name: "a", params: {b: true}}],
+      assertEqual( [{ name: "a", params: { b: 1 }},
+                    { name: "c", params: {}},
+                    { name: "b", params: { d: true }},
+                    { name: "c", params: { e: ['hi, there', true] }},
+                    { name: "a", params: { b: true }}],
                    parse('a; b=1, c, b; d, c; e="hi, there"; e, a; b') )
     }})
+
+    it("parses an extension name that shadows an Object property", function() { with(this) {
+      assertEqual( [{ name: "hasOwnProperty", params: {}}],
+                   parse('hasOwnProperty') )
+    }})
+
+    it("parses an extension param that shadows an Object property", function() { with(this) {
+      var result = parse('foo; hasOwnProperty; x')[0]
+      assertEqual( result.params.hasOwnProperty, true )
+    }})
+
+    it("rejects a string missing its closing quote", function() { with(this) {
+      assertThrows(SyntaxError, function() {
+        parse('foo; bar="fooa\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a\\a')
+      })
+    }})
   }})
 
   describe("serializeParams", function() { with(this) {
@@ -75,23 +91,23 @@ test.describe("Parser", function() { with(this) {
     }})
 
     it("serializes a flag", function() { with(this) {
-      assertEqual( 'a; b', Parser.serializeParams('a', {b: true}) )
+      assertEqual( 'a; b', Parser.serializeParams('a', { b: true }) )
     }})
 
     it("serializes an unquoted param", function() { with(this) {
-      assertEqual( 'a; b=42', Parser.serializeParams('a', {b: '42'}) )
+      assertEqual( 'a; b=42', Parser.serializeParams('a', { b: '42' }) )
     }})
 
     it("serializes a quoted param", function() { with(this) {
-      assertEqual( 'a; b="hi, there"', Parser.serializeParams('a', {b: 'hi, there'}) )
+      assertEqual( 'a; b="hi, there"', Parser.serializeParams('a', { b: 'hi, there' }) )
     }})
 
     it("serializes multiple params", function() { with(this) {
-      assertEqual( 'a; b; c=1; d=hi', Parser.serializeParams('a', {b: true, c: 1, d: 'hi'}) )
+      assertEqual( 'a; b; c=1; d=hi', Parser.serializeParams('a', { b: true, c: 1, d: 'hi' }) )
     }})
 
     it("serializes duplicate params", function() { with(this) {
-      assertEqual( 'a; b; b=hi; c=1', Parser.serializeParams('a', {b: [true, 'hi'], c: 1}) )
+      assertEqual( 'a; b; b=hi; c=1', Parser.serializeParams('a', { b: [true, 'hi'], c: 1 }) )
     }})
   }})
 }})

spec/websocket_extensions_spec.js

@@ -6,7 +6,7 @@ test.describe("Extensions", function() { with(this) {
   before(function() { with(this) {
     this.extensions = new Extensions()
 
-    this.ext     = {name: "deflate", type: "permessage", rsv1: true, rsv2: false, rsv3: false}
+    this.ext     = { name: "deflate", type: "permessage", rsv1: true, rsv2: false, rsv3: false }
     this.session = {}
   }})
 
@@ -38,20 +38,20 @@ test.describe("Extensions", function() { with(this) {
 
   describe("client sessions", function() { with(this) {
     before(function() { with(this) {
-      this.offer = {mode: "compress"}
+      this.offer = { mode: "compress" }
       stub(ext, "createClientSession").returns(session)
       stub(session, "generateOffer").returns(offer)
       extensions.add(ext)
 
-      this.conflict = {name: "tar", type: "permessage", rsv1: true, rsv2: false, rsv3: false}
+      this.conflict = { name: "tar", type: "permessage", rsv1: true, rsv2: false, rsv3: false }
       this.conflictSession = {}
       stub(conflict, "createClientSession").returns(conflictSession)
-      stub(conflictSession, "generateOffer").returns({gzip: true})
+      stub(conflictSession, "generateOffer").returns({ gzip: true })
 
-      this.nonconflict = {name: "reverse", type: "permessage", rsv1: false, rsv2: true, rsv3: false}
+      this.nonconflict = { name: "reverse", type: "permessage", rsv1: false, rsv2: true, rsv3: false }
       this.nonconflictSession = {}
       stub(nonconflict, "createClientSession").returns(nonconflictSession)
-      stub(nonconflictSession, "generateOffer").returns({utf8: true})
+      stub(nonconflictSession, "generateOffer").returns({ utf8: true })
 
       stub(session, "activate").returns(true)
       stub(conflictSession, "activate").returns(true)
@@ -133,18 +133,18 @@ test.describe("Extensions", function() { with(this) {
       }})
 
       it("activates one session with a boolean param", function() { with(this) {
-        expect(session, "activate").given({gzip: true}).exactly(1).returning(true)
+        expect(session, "activate").given({ gzip: true }).exactly(1).returning(true)
         extensions.activate("deflate; gzip")
       }})
 
       it("activates one session with a string param", function() { with(this) {
-        expect(session, "activate").given({mode: "compress"}).exactly(1).returning(true)
+        expect(session, "activate").given({ mode: "compress" }).exactly(1).returning(true)
         extensions.activate("deflate; mode=compress")
       }})
 
       it("activates multiple sessions", function() { with(this) {
-        expect(session, "activate").given({a: true}).exactly(1).returning(true)
-        expect(nonconflictSession, "activate").given({b: true}).exactly(1).returning(true)
+        expect(session, "activate").given({ a: true }).exactly(1).returning(true)
+        expect(nonconflictSession, "activate").given({ b: true }).exactly(1).returning(true)
         extensions.activate("deflate; a, reverse; b")
       }})
 
@@ -180,17 +180,17 @@ test.describe("Extensions", function() { with(this) {
       it("processes messages in the reverse order given in the server's response", function() { with(this) {
         extensions.activate("deflate, reverse")
 
-        extensions.processIncomingMessage({frames: []}, function(error, message) {
-          assertNull(error)
+        extensions.processIncomingMessage({ frames: [] }, function(error, message) {
+          assertNull( error )
           assertEqual( ["reverse", "deflate"], message.frames )
         })
       }})
 
       it("yields an error if a session yields an error", function() { with(this) {
         extensions.activate("deflate")
-        stub(session, "processIncomingMessage").yields([{message: "ENOENT"}])
+        stub(session, "processIncomingMessage").yields([{ message: "ENOENT" }])
 
-        extensions.processIncomingMessage({frames: []}, function(error, message) {
+        extensions.processIncomingMessage({ frames: [] }, function(error, message) {
           assertEqual( "deflate: ENOENT", error.message )
           assertNull( message )
         })
@@ -198,11 +198,11 @@ test.describe("Extensions", function() { with(this) {
 
       it("does not call sessions after one has yielded an error", function() { with(this) {
         extensions.activate("deflate, reverse")
-        stub(nonconflictSession, "processIncomingMessage").yields([{message: "ENOENT"}])
+        stub(nonconflictSession, "processIncomingMessage").yields([{ message: "ENOENT" }])
 
         expect(session, "processIncomingMessage").exactly(0)
 
-        extensions.processIncomingMessage({frames: []}, function() {})
+        extensions.processIncomingMessage({ frames: [] }, function() {})
       }})
     }})
 
@@ -226,66 +226,89 @@ test.describe("Extensions", function() { with(this) {
       describe("error handling", function() { with(this) {
         include(FakeClock)
 
-        before(function() { with(this) {
-          clock.stub()
-          extensions.activate("deflate, reverse")
+        sharedExamplesFor("handles errors", function() { with(this) {
+          before(function() { with(this) {
+            clock.stub()
+            extensions.activate("deflate, reverse")
 
-          stub(session, "processOutgoingMessage", function(message, callback) {
-            setTimeout(function() { callback(null, message.concat("a")) }, 100)
-          })
+            stub(session, "processOutgoingMessage", function(message, callback) {
+              setTimeout(function() { callback(null, message.concat("a")) }, 100)
+            })
 
-          stub(nonconflictSession, "processOutgoingMessage", function(message, callback) {
-            setTimeout(function() { callback(null, message.concat("b")) }, 100)
-          })
+            stub(nonconflictSession, "processOutgoingMessage", function(message, callback) {
+              setTimeout(function() { callback(null, message.concat("b")) }, 100)
+            })
 
-          stub(nonconflictSession, "processIncomingMessage", function(message, callback) {
-            if (message[0] === 5)
-              setTimeout(function() { callback(new Error(""), null) }, 10)
-            else
+            stub(nonconflictSession, "processIncomingMessage", function(message, callback) {
+              if (message[0] === 5) return emitError(callback)
               setTimeout(function() { callback(null, message.concat("c")) }, 50)
+            })
+
+            stub(session, "processIncomingMessage", function(message, callback) {
+              setTimeout(function() { callback(null, message.concat("d")) }, 100)
+            })
+
+            stub(session, "close")
+            stub(nonconflictSession, "close")
+
+            this.messages = []
+
+            var push = function(error, message) {
+              if (error) extensions.close(function() { messages.push("close") })
+              messages.push(message)
+            }
+
+            ;[1, 2, 3].forEach(function(n) {
+              extensions.processOutgoingMessage([n], push)
+            })
+
+            ;[4, 5, 6].forEach(function(n, i) {
+              setTimeout(function() {
+                extensions.processIncomingMessage([n], push)
+              }, 20 * i)
+            })
+
+            clock.tick(200)
+          }})
+
+          it("allows the message before the error through to the end", function() { with(this) {
+            assertEqual( [4, "c", "d"], messages[0] )
+          }})
+
+          it("yields the error to the end of the pipeline", function() { with(this) {
+            assertNull( messages[1] )
+          }})
+
+          it("does not yield the message after the error", function() { with(this) {
+            assertNotEqual( arrayIncluding([6, "c", "d"]), messages )
+          }})
+
+          it("yields all the messages in the direction unaffected by the error", function() { with(this) {
+            assertEqual( [1, "a", "b"], messages[2] )
+            assertEqual( [2, "a", "b"], messages[3] )
+            assertEqual( [3, "a", "b"], messages[4] )
+          }})
+
+          it("closes after all messages are processed", function() { with(this) {
+            assertEqual( "close", messages[5] )
+            assertEqual( 6, messages.length )
+          }})
+        }})
+
+        describe("with a sync error", function() { with(this) {
+          define("emitError", function(callback) {
+            throw new Error("sync error")
           })
 
-          stub(session, "processIncomingMessage", function(message, callback) {
-            setTimeout(function() { callback(null, message.concat("d")) }, 100)
+          itShouldBehaveLike("handles errors")
+        }})
+
+        describe("with an async error", function() { with(this) {
+          define("emitError", function(callback) {
+            setTimeout(function() { callback(new Error("async error"), null) }, 10)
           })
 
-          stub(session, "close")
-          stub(nonconflictSession, "close")
-
-          this.messages = []
-
-          var push = function(error, message) {
-            if (error) extensions.close(function() { messages.push("close") })
-            messages.push(message)
-          }
-
-          ;[1, 2, 3].forEach(function(n) { extensions.processOutgoingMessage([n], push) })
-          ;[4, 5, 6].forEach(function(n) { extensions.processIncomingMessage([n], push) })
-
-          clock.tick(200)
-        }})
-
-        it("allows the message before the error through to the end", function() { with(this) {
-          assertEqual( [4, "c", "d"], messages[0] )
-        }})
-
-        it("yields the error to the end of the pipeline", function() { with(this) {
-          assertNull( messages[1] )
-        }})
-
-        it("does not yield the message after the error", function() { with(this) {
-          assertNotEqual( arrayIncluding([6, "c", "d"]), messages )
-        }})
-
-        it("yields all the messages in the direction unaffected by the error", function() { with(this) {
-          assertEqual( [1, "a", "b"], messages[2] )
-          assertEqual( [2, "a", "b"], messages[3] )
-          assertEqual( [3, "a", "b"], messages[4] )
-        }})
-
-        it("closes after all messages are processed", function() { with(this) {
-          assertEqual( "close", messages[5] )
-          assertEqual( 6, messages.length )
+          itShouldBehaveLike("handles errors")
         }})
       }})
 
@@ -313,11 +336,11 @@ test.describe("Extensions", function() { with(this) {
           extensions.activate("deflate, reverse")
 
           var out = []
-          extensions.processOutgoingMessage({frames: []}, function(error, message) { out.push(message) })
-          extensions.processOutgoingMessage({frames: [1]}, function(error, message) { out.push(message) })
+          extensions.processOutgoingMessage({ frames: [] }, function(error, message) { out.push(message) })
+          extensions.processOutgoingMessage({ frames: [1] }, function(error, message) { out.push(message) })
           clock.tick(200)
 
-          assertEqual( [{frames: ["a", "c"]}, {frames: [1, "b", "d"]}], out )
+          assertEqual( [{ frames: ["a", "c"] }, { frames: [1, "b", "d"] }], out )
         }})
 
         it("defers closing until the extension has finished processing", function() { with(this) {
@@ -326,7 +349,7 @@ test.describe("Extensions", function() { with(this) {
           var closed = false, notified = false
           stub(session, "close", function() { closed = true })
 
-          extensions.processOutgoingMessage({frames: []}, function() {})
+          extensions.processOutgoingMessage({ frames: [] }, function() {})
           extensions.close(function() { notified = true })
 
           clock.tick(50)
@@ -343,7 +366,7 @@ test.describe("Extensions", function() { with(this) {
           stub(session, "close", function() { closed[0] = true })
           stub(nonconflictSession, "close", function() { closed[1] = true })
 
-          extensions.processOutgoingMessage({frames: []}, function() {});
+          extensions.processOutgoingMessage({ frames: [] }, function() {});
           extensions.close(function() { notified = true })
 
           clock.tick(50)
@@ -361,7 +384,7 @@ test.describe("Extensions", function() { with(this) {
           extensions.activate("deflate")
           stub(session, "close", function() { closed = true })
 
-          extensions.processOutgoingMessage({frames: []}, function() {})
+          extensions.processOutgoingMessage({ frames: [] }, function() {})
           extensions.close()
           clock.tick(100)
 
@@ -374,8 +397,8 @@ test.describe("Extensions", function() { with(this) {
       it("processes messages in the order given in the server's response", function() { with(this) {
         extensions.activate("deflate, reverse")
 
-        extensions.processOutgoingMessage({frames: []}, function(error, message) {
-          assertNull(error)
+        extensions.processOutgoingMessage({ frames: [] }, function(error, message) {
+          assertNull( error )
           assertEqual( ["deflate", "reverse"], message.frames )
         })
       }})
@@ -383,17 +406,17 @@ test.describe("Extensions", function() { with(this) {
       it("processes messages in the server's order, not the client's order", function() { with(this) {
         extensions.activate("reverse, deflate")
 
-        extensions.processOutgoingMessage({frames: []}, function(error, message) {
-          assertNull(error)
+        extensions.processOutgoingMessage({ frames: [] }, function(error, message) {
+          assertNull( error )
           assertEqual( ["reverse", "deflate"], message.frames )
         })
       }})
 
       it("yields an error if a session yields an error", function() { with(this) {
         extensions.activate("deflate")
-        stub(session, "processOutgoingMessage").yields([{message: "ENOENT"}])
+        stub(session, "processOutgoingMessage").yields([{ message: "ENOENT" }])
 
-        extensions.processOutgoingMessage({frames: []}, function(error, message) {
+        extensions.processOutgoingMessage({ frames: [] }, function(error, message) {
           assertEqual( "deflate: ENOENT", error.message )
           assertNull( message )
         })
@@ -401,30 +424,30 @@ test.describe("Extensions", function() { with(this) {
 
       it("does not call sessions after one has yielded an error", function() { with(this) {
         extensions.activate("deflate, reverse")
-        stub(session, "processOutgoingMessage").yields([{message: "ENOENT"}])
+        stub(session, "processOutgoingMessage").yields([{ message: "ENOENT" }])
 
         expect(nonconflictSession, "processOutgoingMessage").exactly(0)
 
-        extensions.processOutgoingMessage({frames: []}, function() {})
+        extensions.processOutgoingMessage({ frames: [] }, function() {})
       }})
     }})
   }})
 
   describe("server sessions", function() { with(this) {
     before(function() { with(this) {
-      this.response = {mode: "compress"}
+      this.response = { mode: "compress" }
       stub(ext, "createServerSession").returns(session)
       stub(session, "generateResponse").returns(response)
 
-      this.conflict = {name: "tar", type: "permessage", rsv1: true, rsv2: false, rsv3: false}
+      this.conflict = { name: "tar", type: "permessage", rsv1: true, rsv2: false, rsv3: false }
       this.conflictSession = {}
       stub(conflict, "createServerSession").returns(conflictSession)
-      stub(conflictSession, "generateResponse").returns({gzip: true})
+      stub(conflictSession, "generateResponse").returns({ gzip: true })
 
-      this.nonconflict = {name: "reverse", type: "permessage", rsv1: false, rsv2: true, rsv3: false}
+      this.nonconflict = { name: "reverse", type: "permessage", rsv1: false, rsv2: true, rsv3: false }
       this.nonconflictSession = {}
       stub(nonconflict, "createServerSession").returns(nonconflictSession)
-      stub(nonconflictSession, "generateResponse").returns({utf8: true})
+      stub(nonconflictSession, "generateResponse").returns({ utf8: true })
 
       extensions.add(ext)
       extensions.add(conflict)
@@ -433,12 +456,12 @@ test.describe("Extensions", function() { with(this) {
 
     describe("generateResponse", function() { with(this) {
       it("asks the extension for a server session with the offer", function() { with(this) {
-        expect(ext, "createServerSession").given([{flag: true}]).exactly(1).returning(session)
+        expect(ext, "createServerSession").given([{ flag: true }]).exactly(1).returning(session)
         extensions.generateResponse("deflate; flag")
       }})
 
       it("asks the extension for a server session with multiple offers", function() { with(this) {
-        expect(ext, "createServerSession").given([{a: true}, {b: true}]).exactly(1).returning(session)
+        expect(ext, "createServerSession").given([{ a: true }, { b: true }]).exactly(1).returning(session)
         extensions.generateResponse("deflate; a, deflate; b")
       }})
 
@@ -489,7 +512,11 @@ test.describe("Extensions", function() { with(this) {
         assertEqual( "deflate; mode=compress", extensions.generateResponse("deflate, tar") )
       }})
 
-      it("returns a response for potentially conflicting extensions if their preceeding extensions don't build a session", function() { with(this) {
+      it("throws an error if the header is invalid", function() { with(this) {
+        assertThrows(SyntaxError, function() { extensions.generateResponse("x-webkit- -frame") })
+      }})
+
+      it("returns a response for potentially conflicting extensions if their preceding extensions don't build a session", function() { with(this) {
         stub(ext, "createServerSession").returns(null)
         assertEqual( "tar; gzip", extensions.generateResponse("deflate, tar") )
       }})