From 0025e48324b6f7db6566ecfb533b70ec8b42ed14 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Wed, 8 Apr 2026 11:55:50 +0200 Subject: [PATCH 1/5] src: coerce `spawnSync` args to string once --- src/spawn_sync.cc | 20 +++++++++---------- ...child-process-spawnsync-non-string-args.js | 7 +++++++ 2 files changed, 17 insertions(+), 10 deletions(-) create mode 100644 test/parallel/test-child-process-spawnsync-non-string-args.js diff --git a/src/spawn_sync.cc b/src/spawn_sync.cc index 47c4aaef348433..2d29d96769ce80 100644 --- a/src/spawn_sync.cc +++ b/src/spawn_sync.cc @@ -1155,10 +1155,12 @@ Maybe SyncProcessRunner::CopyJsStringArray(Local js_value, if (!js_value->IsArray()) return Just(UV_EINVAL); Local context = env()->context(); - js_array = js_value.As()->Clone().As(); + js_array = js_value.As(); length = js_array->Length(); data_size = 0; + Local values[length]; + // Index has a pointer to every string element, plus one more for a final // null pointer. list_size = (length + 1) * sizeof *list; @@ -1173,17 +1175,19 @@ Maybe SyncProcessRunner::CopyJsStringArray(Local js_value, return Nothing(); } - if (!value->IsString()) { + if (value->IsString()) { + values[i] = value.As(); + } else { Local string; if (!value->ToString(env()->isolate()->GetCurrentContext()) - .ToLocal(&string) || - js_array->Set(context, i, string).IsNothing()) { + .ToLocal(&string)) { return Nothing(); } + values[i] = string; } size_t maybe_size; - if (!StringBytes::StorageSize(isolate, value, UTF8).To(&maybe_size)) { + if (!StringBytes::StorageSize(isolate, values[i], UTF8).To(&maybe_size)) { return Nothing(); } data_size += maybe_size + 1; @@ -1197,14 +1201,10 @@ Maybe SyncProcessRunner::CopyJsStringArray(Local js_value, for (uint32_t i = 0; i < length; i++) { list[i] = buffer + data_offset; - Local value; - if (!js_array->Get(context, i).ToLocal(&value)) { - return Nothing(); - } data_offset += StringBytes::Write(isolate, buffer + data_offset, -1, - value, + values[i], UTF8); buffer[data_offset++] = '\0'; data_offset = nbytes::RoundUp(data_offset, sizeof(void*)); diff --git a/test/parallel/test-child-process-spawnsync-non-string-args.js b/test/parallel/test-child-process-spawnsync-non-string-args.js new file mode 100644 index 00000000000000..5ac14d821f7b0f --- /dev/null +++ b/test/parallel/test-child-process-spawnsync-non-string-args.js @@ -0,0 +1,7 @@ +'use strict'; +const common = require('../common'); +const { spawnSync } = require('child_process'); +const stateful = { + toString: common.mustCall(() => ';'), +}; +spawnSync(process.execPath, ['-e', stateful], { stdio: 'ignore' }); From 5de7a8778f1d03d57068fd927334b1e5b98a68da Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Wed, 8 Apr 2026 12:01:03 +0200 Subject: [PATCH 2/5] fixup! src: coerce `spawnSync` args to string once --- src/spawn_sync.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/spawn_sync.cc b/src/spawn_sync.cc index 2d29d96769ce80..0066d75a43d32a 100644 --- a/src/spawn_sync.cc +++ b/src/spawn_sync.cc @@ -1165,8 +1165,7 @@ Maybe SyncProcessRunner::CopyJsStringArray(Local js_value, // null pointer. list_size = (length + 1) * sizeof *list; - // Convert all array elements to string. Modify the js object itself if - // needed - it's okay since we cloned the original object. Also compute the + // Convert all array elements to string. Also compute the // length of all strings, including room for a null terminator after every // string. Align strings to cache lines. for (uint32_t i = 0; i < length; i++) { From 1c93a56750a970c0d672593b63332b9145425160 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Wed, 8 Apr 2026 12:06:49 +0200 Subject: [PATCH 3/5] fixup! src: coerce `spawnSync` args to string once --- src/spawn_sync.cc | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/spawn_sync.cc b/src/spawn_sync.cc index 0066d75a43d32a..2fbb3dd3c9a70b 100644 --- a/src/spawn_sync.cc +++ b/src/spawn_sync.cc @@ -1200,11 +1200,8 @@ Maybe SyncProcessRunner::CopyJsStringArray(Local js_value, for (uint32_t i = 0; i < length; i++) { list[i] = buffer + data_offset; - data_offset += StringBytes::Write(isolate, - buffer + data_offset, - -1, - values[i], - UTF8); + data_offset += + StringBytes::Write(isolate, buffer + data_offset, -1, values[i], UTF8); buffer[data_offset++] = '\0'; data_offset = nbytes::RoundUp(data_offset, sizeof(void*)); } From 3b101ad9daf06b461f425f000690da51e07af7b6 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Wed, 8 Apr 2026 12:28:15 +0200 Subject: [PATCH 4/5] fixup! src: coerce `spawnSync` args to string once --- src/spawn_sync.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/spawn_sync.cc b/src/spawn_sync.cc index 2fbb3dd3c9a70b..e6e6074fa9bba9 100644 --- a/src/spawn_sync.cc +++ b/src/spawn_sync.cc @@ -1159,7 +1159,7 @@ Maybe SyncProcessRunner::CopyJsStringArray(Local js_value, length = js_array->Length(); data_size = 0; - Local values[length]; + std::vector> values(length); // Index has a pointer to every string element, plus one more for a final // null pointer. From e3aa3a3e986abf09f5e87af59cf2f5e49f1d3242 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Wed, 8 Apr 2026 12:39:02 +0200 Subject: [PATCH 5/5] fixup! fixup! src: coerce `spawnSync` args to string once --- src/spawn_sync.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/spawn_sync.cc b/src/spawn_sync.cc index e6e6074fa9bba9..2da2e18950ad4d 100644 --- a/src/spawn_sync.cc +++ b/src/spawn_sync.cc @@ -44,6 +44,7 @@ using v8::Isolate; using v8::Just; using v8::JustVoid; using v8::Local; +using v8::LocalVector; using v8::Maybe; using v8::MaybeLocal; using v8::Nothing; @@ -1159,7 +1160,7 @@ Maybe SyncProcessRunner::CopyJsStringArray(Local js_value, length = js_array->Length(); data_size = 0; - std::vector> values(length); + LocalVector values(isolate, length); // Index has a pointer to every string element, plus one more for a final // null pointer.