Skip to content
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

Jump to bottom

Make sure that no string is emitted as an error #789

Merged
merged 1 commit into from Jul 20, 2016
Merged

Make sure that no string is emitted as an error #789

Show file tree
Hide file tree
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 3 additions & 13 deletions lib/Receiver.hixie.js
View file
Open in desktop
Expand Up @@ -40,7 +40,6 @@ class Receiver {
*/

add(data) {
if (this.dead) return;
var self = this;
function doAdd() {
if (self.state === EMPTY) {
Expand All @@ -56,7 +55,7 @@ class Receiver {
} else {

if (data[0] !== 0x00) {
self.error('payload must start with 0x00 byte', true);
self.error(new Error('payload must start with 0x00 byte'), true);
return;
}
data = data.slice(1);
Expand Down Expand Up @@ -146,18 +145,9 @@ class Receiver {
* @api private
*/

error(reason, terminate) {
if (this.dead) return;
error(err, terminate) {
this.reset();
if (typeof reason == 'string'){
this.onerror(new Error(reason), terminate);
}
else if (reason.constructor == Error){
this.onerror(reason, terminate);
}
else {
this.onerror(new Error('An error occured'), terminate);
}
this.onerror(err, terminate)
return this;
}

Expand Down
47 changes: 20 additions & 27 deletions lib/Receiver.js
View file
Open in desktop
Expand Up @@ -191,12 +191,12 @@ class Receiver {
processPacket (data) {
if (this.extensions[PerMessageDeflate.extensionName]) {
if ((data[0] & 0x30) != 0) {
this.error('reserved fields (2, 3) must be empty', 1002);
this.error(new Error('reserved fields (2, 3) must be empty'), 1002);
return;
}
} else {
if ((data[0] & 0x70) != 0) {
this.error('reserved fields must be empty', 1002);
this.error(new Error('reserved fields must be empty'), 1002);
return;
}
}
Expand All @@ -206,24 +206,24 @@ class Receiver {
var opcode = data[0] & 0xf;
if (opcode === 0) {
if (compressed) {
this.error('continuation frame cannot have the Per-message Compressed bits', 1002);
this.error(new Error('continuation frame cannot have the Per-message Compressed bits'), 1002);
return;
}
// continuation frame
this.state.fragmentedOperation = true;
this.state.opcode = this.state.activeFragmentedOperation;
if (!(this.state.opcode == 1 || this.state.opcode == 2)) {
this.error('continuation frame cannot follow current opcode', 1002);
this.error(new Error('continuation frame cannot follow current opcode'), 1002);
return;
}
}
else {
if (opcode < 3 && this.state.activeFragmentedOperation != null) {
this.error('data frames after the initial data frame must have opcode 0', 1002);
this.error(new Error('data frames after the initial data frame must have opcode 0'), 1002);
return;
}
if (opcode >= 8 && compressed) {
this.error('control frames cannot have the Per-message Compressed bits', 1002);
this.error(new Error('control frames cannot have the Per-message Compressed bits'), 1002);
return;
}
this.state.compressed = compressed;
Expand All @@ -235,7 +235,9 @@ class Receiver {
else this.state.fragmentedOperation = false;
}
var handler = opcodes[this.state.opcode];
if (typeof handler == 'undefined') this.error('no handler for opcode ' + this.state.opcode, 1002);
if (typeof handler == 'undefined') {
this.error(new Error(`no handler for opcode ${this.state.opcode}`), 1002);
}
else {
handler.start.call(this, data);
}
Expand Down Expand Up @@ -298,7 +300,7 @@ class Receiver {
* @api private
*/

unmask (mask, buf, binary) {
unmask(mask, buf, binary) {
if (mask != null && buf != null) bufferUtil.unmask(buf, mask);
if (binary) return buf;
return buf != null ? buf.toString('utf8') : '';
Expand All @@ -310,18 +312,9 @@ class Receiver {
* @api private
*/

Copy link
Member Author

Choose a reason for hiding this comment

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

This was added here 913afa3#diff-ee39beab4c8223a58e34e180ba59f345 but the issue it attempted to fix (#680) was actually caused by reentrance after cleanup.
This is no longer the case in master so it has been removed.

error (reason, protocolErrorCode) {
if (this.dead) return;
error(err, protocolErrorCode) {
this.reset();
if (typeof reason == 'string'){
this.onerror(new Error(reason), protocolErrorCode);
}
else if (reason.constructor == Error){
this.onerror(reason, protocolErrorCode);
}
else {
this.onerror(new Error('An error occured'), protocolErrorCode);
}
this.onerror(err, protocolErrorCode);
return this;
}

Expand Down Expand Up @@ -382,7 +375,7 @@ class Receiver {
this.currentPayloadLength = fullLength;
return false;
}
this.error('payload cannot exceed ' + this.maxPayload + ' bytes', 1009);
this.error(new Error(`payload cannot exceed ${this.maxPayload} bytes`), 1009);
this.cleanup();

return true;
Expand Down Expand Up @@ -461,7 +454,7 @@ var opcodes = {
else if (firstLength == 127) {
this.expectHeader(8, (data) => {
if (readUInt32BE.call(data, 0) != 0) {
this.error('packets with length spanning more than 32 bit is currently not supported', 1008);
this.error(new Error('packets with length spanning more than 32 bit is currently not supported'), 1008);
return;
}
var length = readUInt32BE.call(data, 4);
Expand Down Expand Up @@ -491,7 +484,7 @@ var opcodes = {
this.messageHandlers.push((callback) => {
this.applyExtensions(packet, state.lastFragment, state.compressed, (err, buffer) => {
if (err) {
this.error(err.message, err.closeCode === 1009 ? 1009 : 1007);
this.error(err, err.closeCode === 1009 ? 1009 : 1007);
return;
}

Expand All @@ -503,7 +496,7 @@ var opcodes = {
else {
this.currentMessage = [];
this.currentMessageLength = 0;
this.error('payload cannot exceed ' + this.maxPayload + ' bytes', 1009);
this.error(new Error(`payload cannot exceed ${this.maxPayload} bytes`), 1009);
return;
}
this.currentMessageLength += buffer.length;
Expand All @@ -513,7 +506,7 @@ var opcodes = {
this.currentMessage = [];
this.currentMessageLength = 0;
if (!Validation.isValidUTF8(messageBuffer)) {
this.error('invalid utf8 sequence', 1007);
this.error(new Error('invalid utf8 sequence'), 1007);
return;
}
this.ontext(messageBuffer.toString('utf8'), {masked: state.masked, buffer: messageBuffer});
Expand Down Expand Up @@ -544,7 +537,7 @@ var opcodes = {
else if (firstLength == 127) {
this.expectHeader(8, (data) => {
if (readUInt32BE.call(data, 0) != 0) {
this.error('packets with length spanning more than 32 bit is currently not supported', 1008);
this.error(new Error('packets with length spanning more than 32 bit is currently not supported'), 1008);
return;
}
var length = readUInt32BE.call(data, 4, true);
Expand Down Expand Up @@ -574,7 +567,7 @@ var opcodes = {
this.messageHandlers.push((callback) => {
this.applyExtensions(packet, state.lastFragment, state.compressed, (err, buffer) => {
if (err) {
this.error(err.message, err.closeCode === 1009 ? 1009 : 1007);
this.error(err, err.closeCode === 1009 ? 1009 : 1007);
return;
}

Expand All @@ -586,7 +579,7 @@ var opcodes = {
else {
this.currentMessage = [];
this.currentMessageLength = 0;
this.error('payload cannot exceed ' + this.maxPayload + ' bytes', 1009);
this.error(new Error(`payload cannot exceed ${this.maxPayload} bytes`), 1009);
return;
}
this.currentMessageLength += buffer.length;
Expand Down
14 changes: 2 additions & 12 deletions lib/Sender.hixie.js
View file
Open in desktop
Expand Up @@ -68,7 +68,7 @@ class Sender extends EventEmitter {
try {
this.socket.write(buffer, 'binary', cb);
} catch (e) {
this.error(e.toString());
this.emit('error', e)
}
}

Expand All @@ -85,7 +85,7 @@ class Sender extends EventEmitter {
if (this.continuationFrame) this.socket.write(new Buffer([0xff], 'binary'));
this.socket.write(new Buffer([0xff, 0x00]), 'binary', cb);
} catch (e) {
this.error(e.toString());
this.emit('error', e);
}
}

Expand All @@ -103,16 +103,6 @@ class Sender extends EventEmitter {
* @api public
*/
pong(data, options) {}

/**
* Handles an error
*
* @api private
*/
error(reason) {
this.emit('error', reason);
return this;
}
}

module.exports = Sender;
14 changes: 7 additions & 7 deletions lib/WebSocket.js
View file
Open in desktop
Expand Up @@ -716,7 +716,7 @@ function initAsClient(address, protocols, options) {
var error;

if (!self.emit('unexpected-response', req, res)) {
error = new Error('unexpected server response (' + res.statusCode + ')');
error = new Error(`unexpected server response (${res.statusCode})`);
req.abort();
self.emit('error', error);
}
Expand All @@ -735,7 +735,7 @@ function initAsClient(address, protocols, options) {

var serverKey = res.headers['sec-websocket-accept'];
if (typeof serverKey === 'undefined' || serverKey !== expectedServerKey) {
self.emit('error', 'invalid server key');
self.emit('error', new Error('invalid server key'));
self.removeAllListeners();
socket.end();
return;
Expand All @@ -754,7 +754,7 @@ function initAsClient(address, protocols, options) {
}

if (protError) {
self.emit('error', protError);
self.emit('error', new Error(protError));
self.removeAllListeners();
socket.end();
return;
Expand All @@ -767,7 +767,7 @@ function initAsClient(address, protocols, options) {
try {
perMessageDeflate.accept(serverExtensions[PerMessageDeflate.extensionName]);
} catch (err) {
self.emit('error', 'invalid extension parameter');
self.emit('error', new Error('invalid extension parameter'));
self.removeAllListeners();
socket.end();
return;
Expand Down Expand Up @@ -870,10 +870,10 @@ function establishConnection(ReceiverClass, SenderClass, socket, upgradeHead) {
self.close(code, data);
};

self._receiver.onerror = function onerror(reason, errorCode) {
self._receiver.onerror = function onerror(error, errorCode) {
// close the connection when the receiver reports a HyBi error code
self.close(typeof errorCode !== 'undefined' ? errorCode : 1002, '');
self.emit('error', (reason instanceof Error) ? reason : (new Error(reason)));
self.close(errorCode, '');
Copy link
Member Author

Choose a reason for hiding this comment

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

errorCode is always specified.

self.emit('error', error);
Copy link
Member Author

Choose a reason for hiding this comment

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

error is always an Error instance.

};

// finalize the client
Expand Down
34 changes: 0 additions & 34 deletions test/WebSocket.test.js
View file
Open in desktop
Expand Up @@ -46,40 +46,6 @@ describe('WebSocket', function() {
ws.should.be.an.instanceOf(WebSocket);
done();
});

it('should emit an error object when the receiver throws an error string', function(done) {

var wss = new WebSocketServer({port: ++port}, function() {

var ws = new WebSocket('ws://localhost:' + port);

ws.on('open', function () {
ws._receiver.error('This is an error string', 1002);
});

ws.on('error', function (error) {
error.should.be.an.instanceof(Error);
done();
});
});
});

it('should emit an error object when the receiver throws an error object', function(done) {

var wss = new WebSocketServer({port: ++port}, function() {

var ws = new WebSocket('ws://localhost:' + port);

ws.on('open', function () {
ws._receiver.error(new Error('This is an error object'), 1002);
});

ws.on('error', function (error) {
error.should.be.an.instanceof(Error);
done();
});
});
});
});

Copy link
Member Author

Choose a reason for hiding this comment

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

No longer needed tests.

describe('options', function() {
Expand Down