Skip to content

Commit 622b405

Browse files
author
Ruben Bridgewater
committed
Reset the parser on protocol errors
1 parent 9d276a6 commit 622b405

File tree

4 files changed

+26
-4
lines changed

4 files changed

+26
-4
lines changed

README.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,11 @@ Node.js and therefor used as default in redis-parser if the hiredis parser is av
9090
Otherwise the pure js NodeRedis parser is choosen that is almost as fast as the
9191
hiredis parser besides some situations in which it'll be a bit slower.
9292

93+
## Protocol errors
94+
95+
To handle protocol errors (this is very unlikely to happen) gracefuly you should add the returnFatalError option, reject any still running command (they might have been processed properly but the reply is just wrong), destroy the socket and reconnect.
96+
Otherwise a chunk might still contain partial data of a following command that was already processed properly but answered in the same chunk as the command that resulted in the protocol error.
97+
9398
## Contribute
9499

95100
The js parser is already optimized but there are likely further optimizations possible.

changelog.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
2+
## v.1.1.0 - 26 Jan, 2016
3+
4+
Features
5+
6+
- The parser is from now on going to reset itself on protocol errors

lib/hiredis.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ var hiredis = require('hiredis');
44

55
function HiredisReplyParser(returnBuffers) {
66
this.name = 'hiredis';
7+
this.returnBuffers = returnBuffers;
78
this.reader = new hiredis.Reader({
89
return_buffers: returnBuffers
910
});
@@ -14,6 +15,10 @@ HiredisReplyParser.prototype.parseData = function () {
1415
return this.reader.get();
1516
} catch (err) {
1617
// Protocol errors land here
18+
// Reset the parser. Otherwise new commands can't be processed properly
19+
this.reader = new hiredis.Reader({
20+
return_buffers: this.returnBuffers
21+
});
1722
this.returnFatalError(err);
1823
return void 0;
1924
}

test/parsers.spec.js

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,11 +93,13 @@ describe('parsers', function () {
9393
assert.equal(replyCount, 1);
9494
});
9595

96-
it('parser error v2', function () {
96+
it('parser error resets the buffer', function () {
9797
var replyCount = 0;
9898
var errCount = 0;
9999
function checkReply (reply) {
100-
assert.strictEqual(reply[0], 'OK');
100+
assert.strictEqual(reply.length, 1);
101+
assert(Buffer.isBuffer(reply[0]));
102+
assert.strictEqual(reply[0].toString(), 'CCC');
101103
replyCount++;
102104
}
103105
function checkError (err) {
@@ -108,12 +110,16 @@ describe('parsers', function () {
108110
returnReply: checkReply,
109111
returnError: returnError,
110112
returnFatalError: checkError,
111-
name: parserName
113+
name: parserName,
114+
returnBuffers: true
112115
});
113116

114-
parser.execute(new Buffer('*1\r\n+OK\r\nb$1`zasd\r\na'));
117+
// The chunk contains valid data after the protocol error
118+
parser.execute(new Buffer('*1\r\n+CCC\r\nb$1\r\nz\r\n+abc\r\n'));
115119
assert.strictEqual(replyCount, 1);
116120
assert.strictEqual(errCount, 1);
121+
parser.execute(new Buffer('*1\r\n+CCC\r\n'));
122+
assert.strictEqual(replyCount, 2);
117123
});
118124

119125
it('parser error v3 without returnFatalError specified', function () {

0 commit comments

Comments
 (0)