src: coerce spawnSync args to string once

PR-URL: https://github.com/nodejs/node/pull/62633
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>

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;
@@ -1155,16 +1156,17 @@ Maybe<int> SyncProcessRunner::CopyJsStringArray(Local<Value> js_value,
   if (!js_value->IsArray()) return Just<int>(UV_EINVAL);
 
   Local<Context> context = env()->context();
-  js_array = js_value.As<Array>()->Clone().As<Array>();
+  js_array = js_value.As<Array>();
   length = js_array->Length();
   data_size = 0;
 
+  LocalVector<String> values(isolate, length);
+
   // Index has a pointer to every string element, plus one more for a final
   // 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++) {
@@ -1173,17 +1175,19 @@ Maybe<int> SyncProcessRunner::CopyJsStringArray(Local<Value> js_value,
       return Nothing<int>();
     }
 
-    if (!value->IsString()) {
+    if (value->IsString()) {
+      values[i] = value.As<String>();
+    } else {
       Local<String> string;
       if (!value->ToString(env()->isolate()->GetCurrentContext())
-               .ToLocal(&string) ||
-          js_array->Set(context, i, string).IsNothing()) {
+               .ToLocal(&string)) {
         return Nothing<int>();
       }
+      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<int>();
     }
     data_size += maybe_size + 1;
@@ -1197,15 +1201,8 @@ Maybe<int> SyncProcessRunner::CopyJsStringArray(Local<Value> js_value,
 
   for (uint32_t i = 0; i < length; i++) {
     list[i] = buffer + data_offset;
-    Local<Value> value;
-    if (!js_array->Get(context, i).ToLocal(&value)) {
-      return Nothing<int>();
-    }
-    data_offset += StringBytes::Write(isolate,
-                                      buffer + data_offset,
-                                      -1,
-                                      value,
-                                      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*));
   }

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' });