Skip to content

Commit facb55b

Browse files
author
Ruben Bridgewater
committed
Add another regression test and refactor some lines away
1 parent b999dd5 commit facb55b

File tree

2 files changed

+38
-9
lines changed

2 files changed

+38
-9
lines changed

lib/parser.js

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,6 @@ function parseBulkString (parser) {
135135
var offsetEnd = parser.offset + length
136136
if (offsetEnd + 2 > parser.buffer.length) {
137137
parser.bigStrSize = offsetEnd + 2
138-
parser.bigOffset = parser.offset
139-
parser.totalChunkSize = parser.buffer.length
140138
return
141139
}
142140

@@ -333,7 +331,7 @@ function concatBuffer (parser, length) {
333331
list[i].copy(bufferPool, pos)
334332
pos += list[i].length
335333
}
336-
return bufferPool.slice(parser.offset, length)
334+
return bufferPool.slice(0, length)
337335
}
338336

339337
/**
@@ -355,17 +353,14 @@ JavascriptRedisParser.prototype.execute = function (buffer) {
355353
this.buffer.copy(bufferPool, 0, 0, oldLength)
356354
buffer.copy(bufferPool, oldLength, 0, buffer.length)
357355
this.buffer = bufferPool.slice(0, newLength)
358-
this.offset = 0
359356
} else if (this.totalChunkSize + buffer.length >= this.bigStrSize) {
360-
this.bufferCache.unshift(this.buffer)
361357
this.bufferCache.push(buffer)
362358
// The returned type might be Array * (42) and in that case we can't improve the parsing currently
363359
if (this.optionReturnBuffers === false && this.buffer[0] === 36) {
364360
this.returnReply(concatBulkString(this))
365361
this.buffer = buffer
366362
} else { // This applies for arrays too
367363
this.buffer = concatBuffer(this, this.totalChunkSize + buffer.length)
368-
this.offset = 0
369364
}
370365
this.bigStrSize = 0
371366
this.totalChunkSize = 0
@@ -389,8 +384,9 @@ JavascriptRedisParser.prototype.execute = function (buffer) {
389384
this.buffer = tempBuffer
390385
if (this.bigStrSize !== 0) {
391386
this.bigStrSize -= offset
392-
this.bigOffset -= offset
393-
this.totalChunkSize -= offset
387+
this.bigOffset = this.offset - offset
388+
this.totalChunkSize = this.buffer.length
389+
this.bufferCache.push(this.buffer)
394390
}
395391
this.offset = 0
396392
return

test/parsers.spec.js

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ var parsers = [JavascriptParser, HiredisParser]
1111
// Mock the not needed return functions
1212
function returnReply () { throw new Error('failed') }
1313
function returnError () { throw new Error('failed') }
14-
function returnFatalError () { throw new Error('failed') }
14+
function returnFatalError (err) { throw err }
1515

1616
describe('parsers', function () {
1717
describe('general parser functionality', function () {
@@ -93,6 +93,39 @@ describe('parsers', function () {
9393
assert.strictEqual(replyCount, 3)
9494
})
9595

96+
it('multiple parsers do not interfere with bulk strings in arrays', function () {
97+
var replyCount = 0
98+
var results = [['foo', 'foo bar baz'], [1234567890, 'hello world', 'the end'], 'ttttttttttttttttttttttttttttttttttttttttttttttt']
99+
function checkReply (reply) {
100+
console.log(reply)
101+
assert.deepEqual(results[replyCount], reply)
102+
replyCount++
103+
}
104+
var parserOne = new Parser({
105+
returnReply: checkReply,
106+
returnError: returnError,
107+
returnFatalError: returnFatalError
108+
})
109+
var parserTwo = new Parser({
110+
returnReply: checkReply,
111+
returnError: returnError,
112+
returnFatalError: returnFatalError
113+
})
114+
parserOne.execute(new Buffer('*2\r\n+foo\r\n$11\r\nfoo '))
115+
parserOne.execute(new Buffer('bar '))
116+
assert.strictEqual(replyCount, 0)
117+
parserTwo.execute(new Buffer('*3\r\n:1234567890\r\n$11\r\nhello '))
118+
assert.strictEqual(replyCount, 0)
119+
parserOne.execute(new Buffer('baz\r\n+ttttttttttttttttttttttttt'))
120+
assert.strictEqual(replyCount, 1)
121+
parserTwo.execute(new Buffer('wor'))
122+
parserTwo.execute(new Buffer('ld\r\n'))
123+
assert.strictEqual(replyCount, 1)
124+
parserTwo.execute(new Buffer('+the end\r\n'))
125+
assert.strictEqual(replyCount, 2)
126+
parserOne.execute(new Buffer('tttttttttttttttttttttt\r\n'))
127+
})
128+
96129
it('chunks getting to big for the bufferPool', function () {
97130
// This is a edge case. Chunks should not exceed Math.pow(2, 16) bytes
98131
var lorem = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, ' +

0 commit comments

Comments
 (0)