Fix buffer corruption

Fixes #6

Add changelog entry

Author: Ruben Bridgewater

changelog.md

@@ -1,4 +1,10 @@
-## v.2.0.1 - 0x Jun, 2016
+## v.2.0.2 - 08 Jun, 2016
+
+Bugfixes
+
+-  Fixed parser with returnBuffers option returning corrupted data
+
+## v.2.0.1 - 04 Jun, 2016
 
 Bugfixes
 

lib/parser.js

@@ -1,7 +1,8 @@
 'use strict'
 
 var ReplyError = require('./replyError')
-var bufferPool = new Buffer(64 * 1024)
+var bufferPool = new Buffer(32 * 1024)
+var bufferOffset = 0
 var interval = null
 
 /**
@@ -135,6 +136,9 @@ function parseBulkString (parser) {
   var offsetEnd = parser.offset + length
   if (offsetEnd + 2 > parser.buffer.length) {
     parser.bigStrSize = offsetEnd + 2
+    parser.bigOffset = parser.offset
+    parser.totalChunkSize = parser.buffer.length
+    parser.bufferCache.push(parser.buffer)
     return
   }
 
@@ -320,18 +324,26 @@ function decreaseBufferPool () {
  */
 function concatBuffer (parser, length) {
   var list = parser.bufferCache
-  var pos = 0
-  if (bufferPool.length < length) {
-    bufferPool = new Buffer(length)
+  var pos = bufferOffset
+  length -= parser.offset
+  if (bufferPool.length < length + bufferOffset) {
+    // Increase the bufferPool size by three times the current needed length
+    bufferPool = new Buffer(length * 3 + bufferOffset)
+    bufferOffset = 0
+    pos = 0
     if (interval === null) {
-      interval = setInterval(decreaseBufferPool, 50)
+      interval = setInterval(decreaseBufferPool, 30)
     }
   }
-  for (var i = 0; i < list.length; i++) {
+  list[0].copy(bufferPool, pos, parser.offset, list[0].length)
+  pos += list[0].length - parser.offset
+  for (var i = 1; i < list.length; i++) {
     list[i].copy(bufferPool, pos)
     pos += list[i].length
   }
-  return bufferPool.slice(0, length)
+  var buffer = bufferPool.slice(bufferOffset, length + bufferOffset)
+  bufferOffset += length
+  return buffer
 }
 
 /**
@@ -345,22 +357,21 @@ JavascriptRedisParser.prototype.execute = function (buffer) {
     this.offset = 0
   } else if (this.bigStrSize === 0) {
     var oldLength = this.buffer.length
-    var newLength = oldLength + buffer.length
-    // ~ 5% speed increase over using new Buffer(length) all the time
-    if (bufferPool.length < newLength) { // We can't rely on the chunk size
-      bufferPool = new Buffer(newLength)
-    }
-    this.buffer.copy(bufferPool, 0, 0, oldLength)
-    buffer.copy(bufferPool, oldLength, 0, buffer.length)
-    this.buffer = bufferPool.slice(0, newLength)
+    var remainingLength = oldLength - this.offset
+    var bufferPool = new Buffer(remainingLength + buffer.length)
+    this.buffer.copy(bufferPool, 0, this.offset, oldLength)
+    buffer.copy(bufferPool, remainingLength, 0, buffer.length)
+    this.buffer = bufferPool
+    this.offset = 0
   } else if (this.totalChunkSize + buffer.length >= this.bigStrSize) {
     this.bufferCache.push(buffer)
     // The returned type might be Array * (42) and in that case we can't improve the parsing currently
-    if (this.optionReturnBuffers === false && this.buffer[0] === 36) {
+    if (this.optionReturnBuffers === false && this.buffer[this.offset] === 36) {
       this.returnReply(concatBulkString(this))
       this.buffer = buffer
     } else { // This applies for arrays too
       this.buffer = concatBuffer(this, this.totalChunkSize + buffer.length)
+      this.offset = 0
     }
     this.bigStrSize = 0
     this.totalChunkSize = 0
@@ -376,19 +387,7 @@ JavascriptRedisParser.prototype.execute = function (buffer) {
     var type = this.buffer[this.offset++]
     var response = parseType(this, type)
     if (response === undefined) {
-      if (this.buffer === null) {
-        return
-      }
-      var tempBuffer = new Buffer(this.buffer.length - offset)
-      this.buffer.copy(tempBuffer, 0, offset, this.buffer.length)
-      this.buffer = tempBuffer
-      if (this.bigStrSize !== 0) {
-        this.bigStrSize -= offset
-        this.bigOffset = this.offset - offset
-        this.totalChunkSize = this.buffer.length
-        this.bufferCache.push(this.buffer)
-      }
-      this.offset = 0
+      this.offset = offset
       return
     }
 

test/parsers.spec.js

@@ -126,6 +126,32 @@ describe('parsers', function () {
         parserOne.execute(new Buffer('tttttttttttttttttttttt\r\n'))
       })
 
+      it('returned buffers do not get mutated', function () {
+        var replyCount = 0
+        var results = [new Buffer('aaaaaaaaaa'), new Buffer('zzzzzzzzzz')]
+        function checkReply (reply) {
+          assert.deepEqual(results[replyCount], reply)
+          results[replyCount] = reply
+          replyCount++
+        }
+        var parser = new Parser({
+          returnReply: checkReply,
+          returnError: returnError,
+          returnFatalError: returnFatalError,
+          returnBuffers: true
+        })
+        parser.execute(new Buffer('$10\r\naaaaa'))
+        parser.execute(new Buffer('aaaaa\r\n'))
+        assert.strictEqual(replyCount, 1)
+        parser.execute(new Buffer('$10\r\nzzzzz'))
+        parser.execute(new Buffer('zzzzz\r\n'))
+        assert.strictEqual(replyCount, 2)
+        var str = results[0].toString()
+        for (var i = 0; i < str.length; i++) {
+          assert.strictEqual(str.charAt(i), 'a')
+        }
+      })
+
       it('chunks getting to big for the bufferPool', function () {
         // This is a edge case. Chunks should not exceed Math.pow(2, 16) bytes
         var lorem = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, ' +