Skip to content

Improve js parser #846

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 16, 2015
Merged

Improve js parser #846

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 15 additions & 46 deletions lib/parser/javascript.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ ReplyParser.prototype._parseResult = function (type) {
throw new IncompleteReadBuffer("Wait for more data.");
}

if (type === 45) {
var result = this._buffer.toString(this._encoding, start, end);
return new Error(result);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #751

}

if (this.options.return_buffers) {
return this._buffer.slice(start, end);
} else {
Expand All @@ -76,12 +81,8 @@ ReplyParser.prototype._parseResult = function (type) {
throw new IncompleteReadBuffer("Wait for more data.");
}

if (this.options.return_buffers) {
return this._buffer.slice(start, end);
}

// return the coerced numeric value
return +small_toString(this._buffer, start, end);
return +this._buffer.toString('ascii', start, end);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're looking for numbers we're safe to choose the fast ascii parser

} else if (type === 36) { // $
// set a rewind point, as the packet could be larger than the
// buffer in memory
Expand Down Expand Up @@ -123,7 +124,7 @@ ReplyParser.prototype._parseResult = function (type) {
throw new IncompleteReadBuffer("Wait for more data.");
}

var reply = [ ];
var reply = [];
var ntype, i, res;

offset = this._offset - 1;
Expand Down Expand Up @@ -152,36 +153,25 @@ ReplyParser.prototype.execute = function (buffer) {

while (true) {
offset = this._offset;
try {
// at least 4 bytes: :1\r\n
if (this._bytesRemaining() < 4) {
break;
}
// at least 4 bytes: :1\r\n
if (this._bytesRemaining() < 4) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can't throw, so we are able to exclude it from the try block.

break;
}

try {
type = this._buffer[this._offset++];

if (type === 43) { // +
ret = this._parseResult(type);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These four checks are all dead code.

if (ret === null) {
break;
}

this.send_reply(ret);
} else if (type === 45) { // -
ret = this._parseResult(type);

if (ret === null) {
break;
}
this.send_error(new Error(ret));
this.send_error(ret);
} else if (type === 58) { // :
ret = this._parseResult(type);

if (ret === null) {
break;
}

this.send_reply(ret);
} else if (type === 36) { // $
ret = this._parseResult(type);
Expand Down Expand Up @@ -219,9 +209,6 @@ ReplyParser.prototype.execute = function (buffer) {
};

ReplyParser.prototype.append = function (newBuffer) {
if (!newBuffer) {
return;
}

// first run
if (this._buffer === null) {
Expand All @@ -238,27 +225,13 @@ ReplyParser.prototype.append = function (newBuffer) {
return;
}

// very large packet
// check for concat, if we have it, use it
if (Buffer.concat !== undefined) {
this._buffer = Buffer.concat([this._buffer.slice(this._offset), newBuffer]);
} else {
var remaining = this._bytesRemaining(),
newLength = remaining + newBuffer.length,
tmpBuffer = new Buffer(newLength);

this._buffer.copy(tmpBuffer, 0, this._offset);
newBuffer.copy(tmpBuffer, remaining, 0);

this._buffer = tmpBuffer;
}

this._buffer = Buffer.concat([this._buffer.slice(this._offset), newBuffer]);
this._offset = 0;
};

ReplyParser.prototype.parseHeader = function () {
var end = this._packetEndOffset(),
value = small_toString(this._buffer, this._offset, end - 1);
value = this._buffer.toString('ascii', this._offset, end - 1);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The headers should all be sent in ascii


this._offset = end + 1;

Expand All @@ -270,10 +243,6 @@ ReplyParser.prototype._packetEndOffset = function () {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not support node < 0.10

while (this._buffer[offset] !== 0x0d && this._buffer[offset + 1] !== 0x0a) {
offset++;

if (offset >= this._buffer.length) {
throw new IncompleteReadBuffer("didn't see LF after NL reading multi bulk count (" + offset + " => " + this._buffer.length + ", " + this._offset + ")");
}
}

offset++;
Expand Down