From 080cd664621bac5be54511009fe9f4dc0fed458d Mon Sep 17 00:00:00 2001 From: mpoc Date: Wed, 16 Aug 2023 17:29:00 +0100 Subject: [PATCH 1/3] Fix parsing of IPC packets with multiple or partial messages --- src/transports/ipc.js | 60 ++++++++++++++++++++++++++++++------------- 1 file changed, 42 insertions(+), 18 deletions(-) diff --git a/src/transports/ipc.js b/src/transports/ipc.js index f050a01..3750893 100644 --- a/src/transports/ipc.js +++ b/src/transports/ipc.js @@ -66,9 +66,10 @@ function encode(op, data) { return packet; } -const working = { - full: '', +const accumulatedData = { + payload: Buffer.alloc(0), op: undefined, + expectedLength: 0, }; function decode(socket, callback) { @@ -77,23 +78,46 @@ function decode(socket, callback) { return; } - let { op } = working; - let raw; - if (working.full === '') { - op = working.op = packet.readInt32LE(0); - const len = packet.readInt32LE(4); - raw = packet.slice(8, len + 8); - } else { - raw = packet.toString(); - } + accumulatedData.payload = Buffer.concat([accumulatedData.payload, packet]); - try { - const data = JSON.parse(working.full + raw); - callback({ op, data }); // eslint-disable-line callback-return - working.full = ''; - working.op = undefined; - } catch (err) { - working.full += raw; + while (accumulatedData.payload.length > 0) { + if (accumulatedData.expectedLength === 0) { + // We are at the start of a new payload + accumulatedData.op = accumulatedData.payload.readInt32LE(0); + accumulatedData.expectedLength = accumulatedData.payload.readInt32LE(4); + accumulatedData.payload = accumulatedData.payload.subarray(8); // Remove opcode and length + } + + if (accumulatedData.payload.length >= accumulatedData.expectedLength) { + // Accumulated data has the full payload and possibly the beginning of the next payload + const currentPayload = accumulatedData.payload.subarray(0, accumulatedData.expectedLength); + const nextPayload = accumulatedData.payload.subarray(accumulatedData.expectedLength); + + accumulatedData.payload = nextPayload; // Keep remainder for next payload + + try { + callback({ + op: accumulatedData.op, + data: JSON.parse(currentPayload.toString('utf8')), + }); + + // Reset for next payload + accumulatedData.op = undefined; + accumulatedData.expectedLength = 0; + } catch (err) { + // Full payload has been received, but is not valid JSON + console.error('Error parsing payload:', err); + + // Reset for next payload + accumulatedData.op = undefined; + accumulatedData.expectedLength = 0; + + break; + } + } else { + // Full payload hasn't been received yet, wait for more data + break; + } } decode(socket, callback); From f4fa49fec79e8b7a77e4f63c4f7709ea529013fc Mon Sep 17 00:00:00 2001 From: Lucio Cuddeford Date: Wed, 16 Aug 2023 18:38:06 +0100 Subject: [PATCH 2/3] Add early return for clarity --- src/transports/ipc.js | 54 +++++++++++++++++++++---------------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/src/transports/ipc.js b/src/transports/ipc.js index 3750893..57d7fb5 100644 --- a/src/transports/ipc.js +++ b/src/transports/ipc.js @@ -88,36 +88,36 @@ function decode(socket, callback) { accumulatedData.payload = accumulatedData.payload.subarray(8); // Remove opcode and length } - if (accumulatedData.payload.length >= accumulatedData.expectedLength) { - // Accumulated data has the full payload and possibly the beginning of the next payload - const currentPayload = accumulatedData.payload.subarray(0, accumulatedData.expectedLength); - const nextPayload = accumulatedData.payload.subarray(accumulatedData.expectedLength); - - accumulatedData.payload = nextPayload; // Keep remainder for next payload - - try { - callback({ - op: accumulatedData.op, - data: JSON.parse(currentPayload.toString('utf8')), - }); - - // Reset for next payload - accumulatedData.op = undefined; - accumulatedData.expectedLength = 0; - } catch (err) { - // Full payload has been received, but is not valid JSON - console.error('Error parsing payload:', err); - - // Reset for next payload - accumulatedData.op = undefined; - accumulatedData.expectedLength = 0; - - break; - } - } else { + if (accumulatedData.payload.length < accumulatedData.expectedLength) { // Full payload hasn't been received yet, wait for more data break; } + + // Accumulated data has the full payload and possibly the beginning of the next payload + const currentPayload = accumulatedData.payload.subarray(0, accumulatedData.expectedLength); + const nextPayload = accumulatedData.payload.subarray(accumulatedData.expectedLength); + + accumulatedData.payload = nextPayload; // Keep remainder for next payload + + try { + callback({ + op: accumulatedData.op, + data: JSON.parse(currentPayload.toString('utf8')), + }); + + // Reset for next payload + accumulatedData.op = undefined; + accumulatedData.expectedLength = 0; + } catch (err) { + // Full payload has been received, but is not valid JSON + console.error('Error parsing payload:', err); + + // Reset for next payload + accumulatedData.op = undefined; + accumulatedData.expectedLength = 0; + + break; + } } decode(socket, callback); From 4b025cd9c6e3db6688cbe9acdd6d3dc246bf82b7 Mon Sep 17 00:00:00 2001 From: mpoc Date: Mon, 21 Aug 2023 11:32:56 +0100 Subject: [PATCH 3/3] Emit error event when received payload is not valid JSON --- src/transports/ipc.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/transports/ipc.js b/src/transports/ipc.js index 57d7fb5..073afcd 100644 --- a/src/transports/ipc.js +++ b/src/transports/ipc.js @@ -110,7 +110,7 @@ function decode(socket, callback) { accumulatedData.expectedLength = 0; } catch (err) { // Full payload has been received, but is not valid JSON - console.error('Error parsing payload:', err); + callback({ error: new Error('Received payload with malformed JSON', { cause: err }) }); // Reset for next payload accumulatedData.op = undefined; @@ -141,7 +141,11 @@ class IPCTransport extends EventEmitter { })); socket.pause(); socket.on('readable', () => { - decode(socket, ({ op, data }) => { + decode(socket, ({ error, op, data }) => { + if (error) { + this.client.emit('error', error); + return; + } switch (op) { case OPCodes.PING: this.send(data, OPCodes.PONG);