Merge pull request #903 from NodeRedis/parser

Fix and simplify parser

Fixes #875 
Fixes #902

Author: BridgeAR

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

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,33 +107,17 @@ 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);
-            } else if (type === 10 || type === 13) {
-                break;
-            } else {
+                this.send_reply(this._parseResult(type));
+            } else if (type !== 10 && type !== 13) {
                 var err = new Error('Protocol error, got "' + String.fromCharCode(type) + '" as reply type byte');
                 this.send_error(err);
             }
@@ -162,7 +144,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 +164,6 @@ JavascriptReplyParser.prototype._packetEndOffset = function () {
         }
     }
 
-    offset++;
     return offset;
 };
 

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();