From 22d719fe953aad77a031bd9675752b4afcdf5741 Mon Sep 17 00:00:00 2001 From: Tim Perry Date: Mon, 14 Jun 2021 19:35:28 +0200 Subject: [PATCH] Attach error codes to all receiver errors This change also moves message data and close codes into constants, rather than repeating it at each point where the error is thrown. --- lib/constants.js | 15 ++++++- lib/permessage-deflate.js | 8 ++-- lib/receiver.js | 64 +++++++++++++++------------- test/create-websocket-stream.test.js | 1 + test/receiver.test.js | 22 ++++++++++ test/websocket.test.js | 1 + 6 files changed, 77 insertions(+), 34 deletions(-) diff --git a/lib/constants.js b/lib/constants.js index 4082981f8..cc61c8ce8 100644 --- a/lib/constants.js +++ b/lib/constants.js @@ -6,5 +6,18 @@ module.exports = { kStatusCode: Symbol('status-code'), kWebSocket: Symbol('websocket'), EMPTY_BUFFER: Buffer.alloc(0), - NOOP: () => {} + NOOP: () => {}, + ERRORS: { + 'WS_INVALID_OPCODE': { closeCode: 1002, message: "invalid opcode" }, + 'WS_INVALID_CLOSE_CODE': { closeCode: 1002, message: "invalid status code" }, + 'WS_UNEXPECTED_RSV_1': { closeCode: 1002, message: "RSV1 must be clear" }, + 'WS_UNEXPECTED_RSV_2_3': { closeCode: 1002, message: "RSV2 and RSV3 must be clear" }, + 'WS_EXPECTED_FIN': { closeCode: 1002, message: "FIN must be set" }, + 'WS_EXPECTED_MASK': { closeCode: 1002, message: "MASK must be set" }, + 'WS_UNEXPECTED_MASK': { closeCode: 1002, message: "MASK must be clear" }, + 'WS_INVALID_CONTROL_PAYLOAD_LENGTH': { closeCode: 1002, message: "invalid payload length" }, + + 'WS_INVALID_UTF8': { closeCode: 1007, message: "invalid UTF-8 sequence" }, + 'WS_INVALID_DATA_PAYLOAD_LENGTH': { closeCode: 1009, message: "Max payload size exceeded" } + } }; diff --git a/lib/permessage-deflate.js b/lib/permessage-deflate.js index a8974b988..7e2ada609 100644 --- a/lib/permessage-deflate.js +++ b/lib/permessage-deflate.js @@ -4,7 +4,7 @@ const zlib = require('zlib'); const bufferUtil = require('./buffer-util'); const Limiter = require('./limiter'); -const { kStatusCode, NOOP } = require('./constants'); +const { kStatusCode, NOOP, ERRORS } = require('./constants'); const TRAILER = Buffer.from([0x00, 0x00, 0xff, 0xff]); const kPerMessageDeflate = Symbol('permessage-deflate'); @@ -494,8 +494,10 @@ function inflateOnData(chunk) { return; } - this[kError] = new RangeError('Max payload size exceeded'); - this[kError][kStatusCode] = 1009; + const code = 'WS_INVALID_DATA_PAYLOAD_LENGTH'; + this[kError] = new RangeError(ERRORS[code].message); + this[kError].code = code; + this[kError][kStatusCode] = ERRORS[code].closeCode; this.removeListener('data', inflateOnData); this.reset(); } diff --git a/lib/receiver.js b/lib/receiver.js index 65a5ab45f..c26ef1e18 100644 --- a/lib/receiver.js +++ b/lib/receiver.js @@ -7,7 +7,8 @@ const { BINARY_TYPES, EMPTY_BUFFER, kStatusCode, - kWebSocket + kWebSocket, + ERRORS } = require('./constants'); const { concat, toArrayBuffer, unmask } = require('./buffer-util'); const { isValidStatusCode, isValidUTF8 } = require('./validation'); @@ -168,14 +169,14 @@ class Receiver extends Writable { if ((buf[0] & 0x30) !== 0x00) { this._loop = false; - return error(RangeError, 'RSV2 and RSV3 must be clear', true, 1002); + return error(RangeError, 'WS_UNEXPECTED_RSV_2_3'); } const compressed = (buf[0] & 0x40) === 0x40; if (compressed && !this._extensions[PerMessageDeflate.extensionName]) { this._loop = false; - return error(RangeError, 'RSV1 must be clear', true, 1002); + return error(RangeError, 'WS_UNEXPECTED_RSV_1'); } this._fin = (buf[0] & 0x80) === 0x80; @@ -185,45 +186,44 @@ class Receiver extends Writable { if (this._opcode === 0x00) { if (compressed) { this._loop = false; - return error(RangeError, 'RSV1 must be clear', true, 1002); + return error(RangeError, 'WS_UNEXPECTED_RSV_1'); } if (!this._fragmented) { this._loop = false; - return error(RangeError, 'invalid opcode 0', true, 1002); + return error(RangeError, 'WS_INVALID_OPCODE', 'invalid opcode 0'); } this._opcode = this._fragmented; } else if (this._opcode === 0x01 || this._opcode === 0x02) { if (this._fragmented) { this._loop = false; - return error(RangeError, `invalid opcode ${this._opcode}`, true, 1002); + return error(RangeError, 'WS_INVALID_OPCODE', `invalid opcode ${this._opcode}`); } this._compressed = compressed; } else if (this._opcode > 0x07 && this._opcode < 0x0b) { if (!this._fin) { this._loop = false; - return error(RangeError, 'FIN must be set', true, 1002); + return error(RangeError, 'WS_EXPECTED_FIN'); } if (compressed) { this._loop = false; - return error(RangeError, 'RSV1 must be clear', true, 1002); + return error(RangeError, 'WS_UNEXPECTED_RSV_1'); } if (this._payloadLength > 0x7d) { this._loop = false; return error( RangeError, - `invalid payload length ${this._payloadLength}`, - true, - 1002 + 'WS_INVALID_CONTROL_PAYLOAD_LENGTH', + `invalid payload length ${this._payloadLength}` ); } } else { this._loop = false; - return error(RangeError, `invalid opcode ${this._opcode}`, true, 1002); + return error(RangeError, 'WS_INVALID_OPCODE', `invalid opcode ${this._opcode}`); } if (!this._fin && !this._fragmented) this._fragmented = this._opcode; @@ -232,11 +232,11 @@ class Receiver extends Writable { if (this._isServer) { if (!this._masked) { this._loop = false; - return error(RangeError, 'MASK must be set', true, 1002); + return error(RangeError, 'WS_EXPECTED_MASK'); } } else if (this._masked) { this._loop = false; - return error(RangeError, 'MASK must be clear', true, 1002); + return error(RangeError, 'WS_UNEXPECTED_MASK'); } if (this._payloadLength === 126) this._state = GET_PAYLOAD_LENGTH_16; @@ -283,9 +283,8 @@ class Receiver extends Writable { this._loop = false; return error( RangeError, - 'Unsupported WebSocket frame: payload length > 2^53 - 1', - false, - 1009 + 'WS_INVALID_DATA_PAYLOAD_LENGTH', + 'Unsupported WebSocket frame: payload length > 2^53 - 1' ); } @@ -304,7 +303,7 @@ class Receiver extends Writable { this._totalPayloadLength += this._payloadLength; if (this._totalPayloadLength > this._maxPayload && this._maxPayload > 0) { this._loop = false; - return error(RangeError, 'Max payload size exceeded', false, 1009); + return error(RangeError, 'WS_INVALID_DATA_PAYLOAD_LENGTH'); } } @@ -384,7 +383,7 @@ class Receiver extends Writable { this._messageLength += buf.length; if (this._messageLength > this._maxPayload && this._maxPayload > 0) { return cb( - error(RangeError, 'Max payload size exceeded', false, 1009) + error(RangeError, 'WS_INVALID_DATA_PAYLOAD_LENGTH') ); } @@ -431,7 +430,7 @@ class Receiver extends Writable { if (!isValidUTF8(buf)) { this._loop = false; - return error(Error, 'invalid UTF-8 sequence', true, 1007); + return error(Error, 'WS_INVALID_UTF8'); } this.emit('message', buf.toString()); @@ -456,18 +455,18 @@ class Receiver extends Writable { this.emit('conclude', 1005, ''); this.end(); } else if (data.length === 1) { - return error(RangeError, 'invalid payload length 1', true, 1002); + return error(RangeError, 'WS_INVALID_CONTROL_PAYLOAD_LENGTH', 'invalid payload length 1'); } else { const code = data.readUInt16BE(0); if (!isValidStatusCode(code)) { - return error(RangeError, `invalid status code ${code}`, true, 1002); + return error(RangeError, 'WS_INVALID_CLOSE_CODE', `invalid status code ${code}`); } const buf = data.slice(2); if (!isValidUTF8(buf)) { - return error(Error, 'invalid UTF-8 sequence', true, 1007); + return error(Error, 'WS_INVALID_UTF8'); } this.emit('conclude', code, buf.toString()); @@ -489,19 +488,24 @@ module.exports = Receiver; * Builds an error object. * * @param {(Error|RangeError)} ErrorCtor The error constructor - * @param {String} message The error message - * @param {Boolean} prefix Specifies whether or not to add a default prefix to - * `message` - * @param {Number} statusCode The status code + * @param {String} errorCode The exposed error code (must be a key of ERRORS constant) + * @param {String} message An optional error message, overriding the default for the code * @return {(Error|RangeError)} The error * @private */ -function error(ErrorCtor, message, prefix, statusCode) { +function error(ErrorCtor, errorCode, customMessage) { + const errorDefinition = ERRORS[errorCode]; + + const shouldPrefix = errorDefinition.closeCode !== 1009; // Long payloads aren't technically 'invalid frames' + + const message = customMessage || errorDefinition.message; + const err = new ErrorCtor( - prefix ? `Invalid WebSocket frame: ${message}` : message + shouldPrefix ? `Invalid WebSocket frame: ${message}` : message ); Error.captureStackTrace(err, error); - err[kStatusCode] = statusCode; + err.code = errorCode; + err[kStatusCode] = errorDefinition.closeCode; return err; } diff --git a/test/create-websocket-stream.test.js b/test/create-websocket-stream.test.js index ddccb56b2..a16a5dfc5 100644 --- a/test/create-websocket-stream.test.js +++ b/test/create-websocket-stream.test.js @@ -210,6 +210,7 @@ describe('createWebSocketStream', () => { duplex.on('error', (err) => { assert.ok(err instanceof RangeError); + assert.strictEqual(err.code, 'WS_INVALID_OPCODE'); assert.strictEqual( err.message, 'Invalid WebSocket frame: invalid opcode 5' diff --git a/test/receiver.test.js b/test/receiver.test.js index a70cc8dbe..ed4a8b471 100644 --- a/test/receiver.test.js +++ b/test/receiver.test.js @@ -513,6 +513,7 @@ describe('Receiver', () => { receiver.on('error', (err) => { assert.ok(err instanceof RangeError); + assert.strictEqual(err.code, 'WS_UNEXPECTED_RSV_1'); assert.strictEqual( err.message, 'Invalid WebSocket frame: RSV1 must be clear' @@ -534,6 +535,7 @@ describe('Receiver', () => { receiver.on('error', (err) => { assert.ok(err instanceof RangeError); + assert.strictEqual(err.code, 'WS_UNEXPECTED_RSV_1'); assert.strictEqual( err.message, 'Invalid WebSocket frame: RSV1 must be clear' @@ -550,6 +552,7 @@ describe('Receiver', () => { receiver.on('error', (err) => { assert.ok(err instanceof RangeError); + assert.strictEqual(err.code, 'WS_UNEXPECTED_RSV_2_3'); assert.strictEqual( err.message, 'Invalid WebSocket frame: RSV2 and RSV3 must be clear' @@ -566,6 +569,7 @@ describe('Receiver', () => { receiver.on('error', (err) => { assert.ok(err instanceof RangeError); + assert.strictEqual(err.code, 'WS_UNEXPECTED_RSV_2_3'); assert.strictEqual( err.message, 'Invalid WebSocket frame: RSV2 and RSV3 must be clear' @@ -582,6 +586,7 @@ describe('Receiver', () => { receiver.on('error', (err) => { assert.ok(err instanceof RangeError); + assert.strictEqual(err.code, 'WS_INVALID_OPCODE'); assert.strictEqual( err.message, 'Invalid WebSocket frame: invalid opcode 0' @@ -598,6 +603,7 @@ describe('Receiver', () => { receiver.on('error', (err) => { assert.ok(err instanceof RangeError); + assert.strictEqual(err.code, 'WS_INVALID_OPCODE'); assert.strictEqual( err.message, 'Invalid WebSocket frame: invalid opcode 1' @@ -615,6 +621,7 @@ describe('Receiver', () => { receiver.on('error', (err) => { assert.ok(err instanceof RangeError); + assert.strictEqual(err.code, 'WS_INVALID_OPCODE'); assert.strictEqual( err.message, 'Invalid WebSocket frame: invalid opcode 2' @@ -632,6 +639,7 @@ describe('Receiver', () => { receiver.on('error', (err) => { assert.ok(err instanceof RangeError); + assert.strictEqual(err.code, 'WS_EXPECTED_FIN'); assert.strictEqual( err.message, 'Invalid WebSocket frame: FIN must be set' @@ -653,6 +661,7 @@ describe('Receiver', () => { receiver.on('error', (err) => { assert.ok(err instanceof RangeError); + assert.strictEqual(err.code, 'WS_UNEXPECTED_RSV_1'); assert.strictEqual( err.message, 'Invalid WebSocket frame: RSV1 must be clear' @@ -669,6 +678,7 @@ describe('Receiver', () => { receiver.on('error', (err) => { assert.ok(err instanceof RangeError); + assert.strictEqual(err.code, 'WS_EXPECTED_FIN'); assert.strictEqual( err.message, 'Invalid WebSocket frame: FIN must be set' @@ -685,6 +695,7 @@ describe('Receiver', () => { receiver.on('error', (err) => { assert.ok(err instanceof RangeError); + assert.strictEqual(err.code, 'WS_EXPECTED_MASK'); assert.strictEqual( err.message, 'Invalid WebSocket frame: MASK must be set' @@ -701,6 +712,7 @@ describe('Receiver', () => { receiver.on('error', (err) => { assert.ok(err instanceof RangeError); + assert.strictEqual(err.code, 'WS_UNEXPECTED_MASK'); assert.strictEqual( err.message, 'Invalid WebSocket frame: MASK must be clear' @@ -719,6 +731,7 @@ describe('Receiver', () => { receiver.on('error', (err) => { assert.ok(err instanceof RangeError); + assert.strictEqual(err.code, 'WS_INVALID_CONTROL_PAYLOAD_LENGTH'); assert.strictEqual( err.message, 'Invalid WebSocket frame: invalid payload length 126' @@ -735,6 +748,7 @@ describe('Receiver', () => { receiver.on('error', (err) => { assert.ok(err instanceof RangeError); + assert.strictEqual(err.code, 'WS_INVALID_DATA_PAYLOAD_LENGTH'); assert.strictEqual( err.message, 'Unsupported WebSocket frame: payload length > 2^53 - 1' @@ -756,6 +770,7 @@ describe('Receiver', () => { receiver.on('error', (err) => { assert.ok(err instanceof Error); + assert.strictEqual(err.code, 'WS_INVALID_UTF8'); assert.strictEqual( err.message, 'Invalid WebSocket frame: invalid UTF-8 sequence' @@ -778,6 +793,7 @@ describe('Receiver', () => { receiver.on('error', (err) => { assert.ok(err instanceof Error); + assert.strictEqual(err.code, 'WS_INVALID_UTF8'); assert.strictEqual( err.message, 'Invalid WebSocket frame: invalid UTF-8 sequence' @@ -799,6 +815,7 @@ describe('Receiver', () => { receiver.on('error', (err) => { assert.ok(err instanceof RangeError); + assert.strictEqual(err.code, 'WS_INVALID_CONTROL_PAYLOAD_LENGTH'); assert.strictEqual( err.message, 'Invalid WebSocket frame: invalid payload length 1' @@ -815,6 +832,7 @@ describe('Receiver', () => { receiver.on('error', (err) => { assert.ok(err instanceof RangeError); + assert.strictEqual(err.code, 'WS_INVALID_CLOSE_CODE'); assert.strictEqual( err.message, 'Invalid WebSocket frame: invalid status code 0' @@ -831,6 +849,7 @@ describe('Receiver', () => { receiver.on('error', (err) => { assert.ok(err instanceof Error); + assert.strictEqual(err.code, 'WS_INVALID_UTF8'); assert.strictEqual( err.message, 'Invalid WebSocket frame: invalid UTF-8 sequence' @@ -860,6 +879,7 @@ describe('Receiver', () => { receiver.on('error', (err) => { assert.ok(err instanceof RangeError); + assert.strictEqual(err.code, 'WS_INVALID_DATA_PAYLOAD_LENGTH'); assert.strictEqual(err.message, 'Max payload size exceeded'); assert.strictEqual(err[kStatusCode], 1009); done(); @@ -884,6 +904,7 @@ describe('Receiver', () => { receiver.on('error', (err) => { assert.ok(err instanceof RangeError); + assert.strictEqual(err.code, 'WS_INVALID_DATA_PAYLOAD_LENGTH'); assert.strictEqual(err.message, 'Max payload size exceeded'); assert.strictEqual(err[kStatusCode], 1009); done(); @@ -913,6 +934,7 @@ describe('Receiver', () => { receiver.on('error', (err) => { assert.ok(err instanceof RangeError); + assert.strictEqual(err.code, 'WS_INVALID_DATA_PAYLOAD_LENGTH'); assert.strictEqual(err.message, 'Max payload size exceeded'); assert.strictEqual(err[kStatusCode], 1009); done(); diff --git a/test/websocket.test.js b/test/websocket.test.js index a0557980e..b42b207ca 100644 --- a/test/websocket.test.js +++ b/test/websocket.test.js @@ -435,6 +435,7 @@ describe('WebSocket', () => { ws.on('error', (err) => { assert.ok(err instanceof RangeError); + assert.strictEqual(err.code, 'WS_INVALID_OPCODE'); assert.strictEqual( err.message, 'Invalid WebSocket frame: invalid opcode 5'