From ae8dd88995dbd7f89c97e5cc15e5b489fa0efece Mon Sep 17 00:00:00 2001 From: Damien Arrachequesne Date: Thu, 19 Jan 2023 09:04:38 +0100 Subject: [PATCH] fix: do not modify the input packet upon encoding Note: this issue has existed since Socket.IO v1.0 (see [1]), because the `deconstructPacket()` method also mutates its input argument. This also explains why some adapters (like [2]) need to use `process.nextTick()` when extending the `broadcast()` method, because `Adapter.broadcast()` calls `Encoder.encode()` ([3]). Related: - https://github.com/socketio/socket.io/issues/4374 - https://github.com/socketio/socket.io-mongo-adapter/issues/10 [1]: https://github.com/socketio/socket.io-parser/commit/299849b00294c3bc95817572441f3aca8ffb1f65 [2]: https://github.com/socketio/socket.io-postgres-adapter/blob/0.3.0/lib/index.ts#L587-L590 [3]: https://github.com/socketio/socket.io-adapter/blob/2.4.0/lib/index.ts#L148 --- lib/binary.ts | 2 +- lib/index.ts | 21 ++++++++++++--------- test/arraybuffer.js | 16 ++++++++++++++++ test/helpers.js | 3 --- 4 files changed, 29 insertions(+), 13 deletions(-) diff --git a/lib/binary.ts b/lib/binary.ts index 3bc41b4..078bf4d 100644 --- a/lib/binary.ts +++ b/lib/binary.ts @@ -53,7 +53,7 @@ function _deconstructPacket(data, buffers) { export function reconstructPacket(packet, buffers) { packet.data = _reconstructPacket(packet.data, buffers); - packet.attachments = undefined; // no longer useful + delete packet.attachments; // no longer useful return packet; } diff --git a/lib/index.ts b/lib/index.ts index 1170d17..b3035d3 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -53,11 +53,15 @@ export class Encoder { if (obj.type === PacketType.EVENT || obj.type === PacketType.ACK) { if (hasBinary(obj)) { - obj.type = - obj.type === PacketType.EVENT - ? PacketType.BINARY_EVENT - : PacketType.BINARY_ACK; - return this.encodeAsBinary(obj); + return this.encodeAsBinary({ + type: + obj.type === PacketType.EVENT + ? PacketType.BINARY_EVENT + : PacketType.BINARY_ACK, + nsp: obj.nsp, + data: obj.data, + id: obj.id, + }); } } return [this.encodeAsString(obj)]; @@ -149,10 +153,9 @@ export class Decoder extends Emitter<{}, {}, DecoderReservedEvents> { throw new Error("got plaintext data when reconstructing a packet"); } packet = this.decodeString(obj); - if ( - packet.type === PacketType.BINARY_EVENT || - packet.type === PacketType.BINARY_ACK - ) { + const isBinaryEvent = packet.type === PacketType.BINARY_EVENT; + if (isBinaryEvent || packet.type === PacketType.BINARY_ACK) { + packet.type = isBinaryEvent ? PacketType.EVENT : PacketType.ACK; // binary packet's json this.reconstructor = new BinaryReconstructor(packet); diff --git a/test/arraybuffer.js b/test/arraybuffer.js index 3e8884e..7103c80 100644 --- a/test/arraybuffer.js +++ b/test/arraybuffer.js @@ -89,4 +89,20 @@ describe("ArrayBuffer", () => { decoder.destroy(); // destroy before all data added expect(decoder.reconstructor.buffers.length).to.be(0); // expect that buffer is clean }); + + it("should not modify the input packet", () => { + const packet = { + type: PacketType.EVENT, + nsp: "/", + data: ["a", Uint8Array.of(1, 2, 3), Uint8Array.of(4, 5, 6)], + }; + + encoder.encode(packet); + + expect(packet).to.eql({ + type: PacketType.EVENT, + nsp: "/", + data: ["a", Uint8Array.of(1, 2, 3), Uint8Array.of(4, 5, 6)], + }); + }); }); diff --git a/test/helpers.js b/test/helpers.js index 8a50adb..d3fe1f2 100644 --- a/test/helpers.js +++ b/test/helpers.js @@ -20,13 +20,10 @@ module.exports.test = (obj) => { // tests encoding of binary packets module.exports.test_bin = (obj) => { return new Promise((resolve) => { - const originalData = obj.data; const encodedPackets = encoder.encode(obj); const decoder = new parser.Decoder(); decoder.on("decoded", (packet) => { - obj.data = originalData; - obj.attachments = undefined; expect(obj).to.eql(packet); resolve(); });