From a9bb53a2ef9a5351c87fd489a5252464ec17ddf4 Mon Sep 17 00:00:00 2001 From: "me.gorkov" Date: Fri, 29 Nov 2024 13:09:13 +0300 Subject: [PATCH 1/6] child_process: fix parsing messages with splitted length field Fixes: https://github.com/nodejs/node/issues/55834 --- lib/internal/child_process/serialization.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/internal/child_process/serialization.js b/lib/internal/child_process/serialization.js index 7be39f0d48c3c2..1a8f34c1849641 100644 --- a/lib/internal/child_process/serialization.js +++ b/lib/internal/child_process/serialization.js @@ -61,7 +61,12 @@ const advanced = { *parseChannelMessages(channel, readData) { if (readData.length === 0) return; - ArrayPrototypePush(channel[kMessageBuffer], readData); + if (channel[kMessageBuffer].length && channel[kMessageBuffer][0].length < 4) { + // message length splitted into two buffers, so let's concat it + channel[kMessageBuffer][0] = Buffer.concat([channel[kMessageBuffer][0], readData]); + } else { + ArrayPrototypePush(channel[kMessageBuffer], readData); + } channel[kMessageBufferSize] += readData.length; // Index 0 should always be present because we just pushed data into it. From 065942d2c87d0712cd38b81775b27d95633f63f0 Mon Sep 17 00:00:00 2001 From: "me.gorkov" Date: Mon, 9 Dec 2024 16:44:01 +0300 Subject: [PATCH 2/6] child_process: add test for issue 55834 --- ...ced-serialization-splitted-length-field.js | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 test/parallel/test-child-process-advanced-serialization-splitted-length-field.js diff --git a/test/parallel/test-child-process-advanced-serialization-splitted-length-field.js b/test/parallel/test-child-process-advanced-serialization-splitted-length-field.js new file mode 100644 index 00000000000000..a58832a5cb4a23 --- /dev/null +++ b/test/parallel/test-child-process-advanced-serialization-splitted-length-field.js @@ -0,0 +1,26 @@ +'use strict'; +const common = require('../common'); +const child_process = require('child_process'); + +// Regression test for https://github.com/nodejs/node/issues/55834 +const msgLen = 65521; +let cnt = 10; + +if (process.argv[2] === 'child') { + const msg = Buffer.allocUnsafe(msgLen); + (function send() { + if (cnt--) { + process.send(msg, send); + } else { + process.nextTick(() => { + process.exit(0); + }); + } + })() +} else { + const child = child_process.spawn(process.execPath, [__filename, 'child'], { + stdio: ['inherit', 'inherit', 'inherit', 'ipc'], + serialization: 'advanced' + }); + child.on('message', common.mustCall(cnt)); +} From d9379af932137b3a63d235795da5a01e2a4792af Mon Sep 17 00:00:00 2001 From: Maksim Gorkov <33923276+MGorkov@users.noreply.github.com> Date: Fri, 13 Dec 2024 17:37:56 +0300 Subject: [PATCH 3/6] Update test/parallel/test-child-process-advanced-serialization-splitted-length-field.js Co-authored-by: Luigi Pinca --- ...-process-advanced-serialization-splitted-length-field.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/parallel/test-child-process-advanced-serialization-splitted-length-field.js b/test/parallel/test-child-process-advanced-serialization-splitted-length-field.js index a58832a5cb4a23..80711c2a9897eb 100644 --- a/test/parallel/test-child-process-advanced-serialization-splitted-length-field.js +++ b/test/parallel/test-child-process-advanced-serialization-splitted-length-field.js @@ -12,11 +12,9 @@ if (process.argv[2] === 'child') { if (cnt--) { process.send(msg, send); } else { - process.nextTick(() => { - process.exit(0); - }); + process.disconnect(); } - })() + })() } else { const child = child_process.spawn(process.execPath, [__filename, 'child'], { stdio: ['inherit', 'inherit', 'inherit', 'ipc'], From 0a89f5eb15daaebc736d53655a56347deeac3537 Mon Sep 17 00:00:00 2001 From: Maksim Gorkov <33923276+MGorkov@users.noreply.github.com> Date: Fri, 13 Dec 2024 17:38:08 +0300 Subject: [PATCH 4/6] Update lib/internal/child_process/serialization.js Co-authored-by: Luigi Pinca --- lib/internal/child_process/serialization.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/child_process/serialization.js b/lib/internal/child_process/serialization.js index 1a8f34c1849641..664371e29d4f00 100644 --- a/lib/internal/child_process/serialization.js +++ b/lib/internal/child_process/serialization.js @@ -62,7 +62,7 @@ const advanced = { if (readData.length === 0) return; if (channel[kMessageBuffer].length && channel[kMessageBuffer][0].length < 4) { - // message length splitted into two buffers, so let's concat it + // Message length split into two buffers, so let's concatenate it. channel[kMessageBuffer][0] = Buffer.concat([channel[kMessageBuffer][0], readData]); } else { ArrayPrototypePush(channel[kMessageBuffer], readData); From a7ef03ad5e3a8931ca231c17635cb681f2e6c6c6 Mon Sep 17 00:00:00 2001 From: Maksim Gorkov <33923276+MGorkov@users.noreply.github.com> Date: Fri, 13 Dec 2024 17:43:04 +0300 Subject: [PATCH 5/6] Update lib/internal/child_process/serialization.js Co-authored-by: Luigi Pinca --- lib/internal/child_process/serialization.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/child_process/serialization.js b/lib/internal/child_process/serialization.js index 664371e29d4f00..46bb1faaf9fc21 100644 --- a/lib/internal/child_process/serialization.js +++ b/lib/internal/child_process/serialization.js @@ -61,7 +61,7 @@ const advanced = { *parseChannelMessages(channel, readData) { if (readData.length === 0) return; - if (channel[kMessageBuffer].length && channel[kMessageBuffer][0].length < 4) { + if (channel[kMessageBufferSize] && channel[kMessageBuffer][0].length < 4) { // Message length split into two buffers, so let's concatenate it. channel[kMessageBuffer][0] = Buffer.concat([channel[kMessageBuffer][0], readData]); } else { From 73a8e0d62219618c2148f17d19e3120e4a663e91 Mon Sep 17 00:00:00 2001 From: "me.gorkov" Date: Mon, 16 Dec 2024 15:12:14 +0300 Subject: [PATCH 6/6] child_process: fix linter error for issue 55834 --- ...hild-process-advanced-serialization-splitted-length-field.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-child-process-advanced-serialization-splitted-length-field.js b/test/parallel/test-child-process-advanced-serialization-splitted-length-field.js index 80711c2a9897eb..5407a56f495c8f 100644 --- a/test/parallel/test-child-process-advanced-serialization-splitted-length-field.js +++ b/test/parallel/test-child-process-advanced-serialization-splitted-length-field.js @@ -14,7 +14,7 @@ if (process.argv[2] === 'child') { } else { process.disconnect(); } - })() + })(); } else { const child = child_process.spawn(process.execPath, [__filename, 'child'], { stdio: ['inherit', 'inherit', 'inherit', 'ipc'],