From 9b3b0901198c509b8392150d5a5813d654b24c9f Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 20 Oct 2015 10:30:41 +0200 Subject: [PATCH 1/2] Simplify parser and remove dead code --- lib/parsers/javascript.js | 35 +++++++++-------------------------- 1 file changed, 9 insertions(+), 26 deletions(-) diff --git a/lib/parsers/javascript.js b/lib/parsers/javascript.js index 4eb863c380..7da5c3ed14 100644 --- a/lib/parsers/javascript.js +++ b/lib/parsers/javascript.js @@ -22,13 +22,9 @@ JavascriptReplyParser.prototype._parseResult = function (type) { if (type === 43 || type === 58 || type === 45) { // + or : or - // up to the delimiter - end = this._packetEndOffset() - 1; + end = this._packetEndOffset(); start = this._offset; - if (end > this._buffer.length) { - throw new IncompleteReadBuffer('Wait for more data.'); - } - // include the delimiter this._offset = end + 2; @@ -91,13 +87,15 @@ JavascriptReplyParser.prototype._parseResult = function (type) { } return reply; + } else { + return null; } }; JavascriptReplyParser.prototype.execute = function (buffer) { this.append(buffer); - var type, ret, offset; + var type, offset; while (true) { offset = this._offset; @@ -109,30 +107,16 @@ JavascriptReplyParser.prototype.execute = function (buffer) { try { type = this._buffer[this._offset++]; - if (type === 43) { // Strings + - ret = this._parseResult(type); - - this.send_reply(ret); + if (type === 43 || type === 58 || type === 36) { // Strings + // Integers : // Bulk strings $ + this.send_reply(this._parseResult(type)); } else if (type === 45) { // Errors - - ret = this._parseResult(type); - - this.send_error(ret); - } else if (type === 58) { // Integers : - ret = this._parseResult(type); - - this.send_reply(ret); - } else if (type === 36) { // Bulk strings $ - ret = this._parseResult(type); - - this.send_reply(ret); + this.send_error(this._parseResult(type)); } else if (type === 42) { // Arrays * // set a rewind point. if a failure occurs, // wait for the next execute()/append() and try again offset = this._offset - 1; - ret = this._parseResult(type); - - this.send_reply(ret); + this.send_reply(this._parseResult(type)); } else if (type === 10 || type === 13) { break; } else { @@ -162,7 +146,7 @@ JavascriptReplyParser.prototype.append = function (newBuffer) { }; JavascriptReplyParser.prototype.parseHeader = function () { - var end = this._packetEndOffset(), + var end = this._packetEndOffset() + 1, value = this._buffer.toString('ascii', this._offset, end - 1) | 0; this._offset = end + 1; @@ -182,7 +166,6 @@ JavascriptReplyParser.prototype._packetEndOffset = function () { } } - offset++; return offset; }; From 5d08132f7ce6dd77507afc20d8998c2d0983c59a Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 27 Oct 2015 11:13:43 +0100 Subject: [PATCH 2/2] Fix: do not stop parsing a chunk if the first character is a line break Add changelog entry --- changelog.md | 4 ++++ lib/parsers/javascript.js | 4 +--- test/parser.spec.js | 7 ++----- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/changelog.md b/changelog.md index d0cc69f8b6..ef585e082d 100644 --- a/changelog.md +++ b/changelog.md @@ -1,6 +1,10 @@ Changelog ========= +Bugfixes + +- Fixed a js parser error that could result in a timeout ([@BridgeAR](https://github.com/BridgeAR)) + ## v.2.2.5 - 18 Oct, 2015 Bugfixes diff --git a/lib/parsers/javascript.js b/lib/parsers/javascript.js index 7da5c3ed14..5d0208b46b 100644 --- a/lib/parsers/javascript.js +++ b/lib/parsers/javascript.js @@ -117,9 +117,7 @@ JavascriptReplyParser.prototype.execute = function (buffer) { offset = this._offset - 1; this.send_reply(this._parseResult(type)); - } else if (type === 10 || type === 13) { - break; - } else { + } else if (type !== 10 && type !== 13) { var err = new Error('Protocol error, got "' + String.fromCharCode(type) + '" as reply type byte'); this.send_error(err); } diff --git a/test/parser.spec.js b/test/parser.spec.js index f54181ba78..01dd38e539 100644 --- a/test/parser.spec.js +++ b/test/parser.spec.js @@ -55,7 +55,7 @@ describe('parsers', function () { return done(); }); - it('line breaks in the beginning', function (done) { + it('line breaks in the beginning of the last chunk', function (done) { var parser = new Parser(); var reply_count = 0; function check_reply(reply) { @@ -68,10 +68,7 @@ describe('parsers', function () { parser.execute(new Buffer('*1\r\n*1\r\n$1\r\na')); parser.execute(new Buffer('\r\n*1\r\n*1\r')); - parser.execute(new Buffer('\n$1\r\na\r\n')); - - parser.execute(new Buffer('*1\r\n*1\r\n')); - parser.execute(new Buffer('$1\r\na\r\n')); + parser.execute(new Buffer('\n$1\r\na\r\n*1\r\n*1\r\n$1\r\na\r\n')); assert.equal(reply_count, 3, "check reply should have been called three times"); return done();