Skip to content

Commit

Permalink
fix: throw upon invalid packet format
Browse files Browse the repository at this point in the history
Instead of returning an ERROR packet, the decoder will now throw an
error, since it means the other side is sending invalid payloads.

The server will then disconnect the client: [1]

[1] https://github.com/socketio/socket.io/blob/2.3.0/lib/client.js#L194-L198
  • Loading branch information
darrachequesne committed Sep 28, 2020
1 parent f516346 commit 388e5f5
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 14 deletions.
57 changes: 43 additions & 14 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,14 @@ exports.ERROR = 4;
exports.BINARY_EVENT = 5;
exports.BINARY_ACK = 6;

var errorPacket = {
type: exports.ERROR,
data: 'parser error'
var isInteger = Number.isInteger || function (value) {
return typeof value === 'number' &&
isFinite(value) &&
Math.floor(value) === value;
};

var isString = function (value) { return typeof value === 'string'; };

function Encoder () {}

Encoder.prototype.encode = function (packet, callback) {
Expand All @@ -45,20 +48,46 @@ Decoder.prototype.add = function (obj) {
};

Decoder.prototype.parseJSON = function (obj) {
try {
var decoded = JSON.parse(obj);
this.emit('decoded', decoded);
} catch (e) {
this.emit('decoded', errorPacket);
}
var decoded = JSON.parse(obj);
this.checkPacket(decoded);
this.emit('decoded', decoded);
};

Decoder.prototype.parseBinary = function (obj) {
try {
var decoded = msgpack.decode(obj);
this.emit('decoded', decoded);
} catch (e) {
this.emit('decoded', errorPacket);
var decoded = msgpack.decode(obj);
this.checkPacket(decoded);
this.emit('decoded', decoded);
};

function isDataValid (decoded) {
switch (decoded.type) {
case exports.CONNECT:
case exports.DISCONNECT:
return decoded.data === undefined;
case exports.ERROR:
return isString(decoded.data);
default:
return Array.isArray(decoded.data);
}
}

Decoder.prototype.checkPacket = function (decoded) {
var isTypeValid = isInteger(decoded.type) && decoded.type >= exports.CONNECT && decoded.type <= exports.BINARY_ACK;
if (!isTypeValid) {
throw new Error('invalid packet type');
}

if (!isString(decoded.nsp)) {
throw new Error('invalid namespace');
}

if (!isDataValid(decoded)) {
throw new Error('invalid payload');
}

var isAckValid = decoded.id === undefined || isInteger(decoded.id);
if (!isAckValid) {
throw new Error('invalid packet id');
}
};

Expand Down
21 changes: 21 additions & 0 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,4 +112,25 @@ describe('parser', () => {
done();
});
});

it('throws an error upon invalid format', () => {
const decoder = new customParser.Decoder();

expect(() => decoder.add('{')).to.throwError(/Unexpected end of JSON input/);
expect(() => decoder.add(Buffer.from([]))).to.throwError(/Could not parse/);

expect(() => decoder.add('{}')).to.throwError(/invalid packet type/);
expect(() => decoder.add('{"type":"a"}')).to.throwError(/invalid packet type/);
expect(() => decoder.add('{"type":7}')).to.throwError(/invalid packet type/);
expect(() => decoder.add(Buffer.from([1]))).to.throwError(/invalid packet type/);

expect(() => decoder.add('{"type":2}')).to.throwError(/invalid namespace/);
expect(() => decoder.add('{"type":2,"nsp":2}')).to.throwError(/invalid namespace/);

expect(() => decoder.add('{"type":2,"nsp":"/"}')).to.throwError(/invalid payload/);
expect(() => decoder.add('{"type":2,"nsp":"/","data":4}')).to.throwError(/invalid payload/);
expect(() => decoder.add('{"type":4,"nsp":"/","data":[]}')).to.throwError(/invalid payload/);

expect(() => decoder.add('{"type":2,"nsp":"/","data":[],"id":"a"}')).to.throwError(/invalid packet id/);
});
});

0 comments on commit 388e5f5

Please sign in to comment.