From d53e53699c3a06c8543676e21717e698e5085e06 Mon Sep 17 00:00:00 2001
From: theanarkh <theratliter@gmail.com>
Date: Thu, 23 May 2024 23:27:50 +0800
Subject: [PATCH] src: fix execArgv in worker

PR-URL: https://github.com/nodejs/node/pull/53029
Fixes: https://github.com/nodejs/node/issues/53011
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
---
 src/node_worker.cc                       |  7 ++++---
 test/parallel/test-worker-cli-options.js | 21 ++++++++++++++++++---
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/src/node_worker.cc b/src/node_worker.cc
index d06df25ee1ff66..571baf88b94691 100644
--- a/src/node_worker.cc
+++ b/src/node_worker.cc
@@ -625,9 +625,8 @@ void Worker::New(const FunctionCallbackInfo<Value>& args) {
         exec_argv.push_back(arg_string);
       }
     } else {
-      exec_argv_out = env->exec_argv();
       exec_argv.insert(
-          exec_argv.end(), exec_argv_out.begin(), exec_argv_out.end());
+          exec_argv.end(), env->exec_argv().begin(), env->exec_argv().end());
     }
 
     std::vector<std::string> invalid_args{};
@@ -643,7 +642,9 @@ void Worker::New(const FunctionCallbackInfo<Value>& args) {
 
     // The first argument is program name.
     invalid_args.erase(invalid_args.begin());
-    if (errors.size() > 0 || invalid_args.size() > 0) {
+    // Only fail for explicitly provided execArgv, this protects from failures
+    // when execArgv from parent's execArgv is used (which is the default).
+    if (errors.size() > 0 || (invalid_args.size() > 0 && args[2]->IsArray())) {
       Local<Value> error;
       if (!ToV8Value(env->context(), errors.size() > 0 ? errors : invalid_args)
                .ToLocal(&error)) {
diff --git a/test/parallel/test-worker-cli-options.js b/test/parallel/test-worker-cli-options.js
index 98682243f2d16c..ae59dcb6e62d48 100644
--- a/test/parallel/test-worker-cli-options.js
+++ b/test/parallel/test-worker-cli-options.js
@@ -1,16 +1,31 @@
-// Flags: --expose-internals
+// Flags: --expose-internals --expose-gc
 'use strict';
 require('../common');
 const { Worker } = require('worker_threads');
+const assert = require('assert');
 
 const CODE = `
 // If the --expose-internals flag does not pass to worker
 // require function will throw an error
 require('internal/options');
+global.gc();
 `;
-// Test if the flags is passed to worker threads
+
+// Test if the flags is passed to worker threads correctly
+// and do not throw an error with the invalid execArgv
+// when execArgv is inherited from parent
 // See https://github.com/nodejs/node/issues/52825
+// See https://github.com/nodejs/node/issues/53011
+
+// Inherited env, execArgv from the parent will be ok
 new Worker(CODE, { eval: true });
-new Worker(CODE, { eval: true, env: process.env, execArgv: ['--expose-internals'] });
+// Pass process.env explicitly and inherited execArgv from parent will be ok
 new Worker(CODE, { eval: true, env: process.env });
+// Inherited env from the parent and pass execArgv (Node.js options) explicitly will be ok
 new Worker(CODE, { eval: true, execArgv: ['--expose-internals'] });
+// Pass process.env and execArgv (Node.js options) explicitly will be ok
+new Worker(CODE, { eval: true, env: process.env, execArgv: ['--expose-internals'] });
+//  Pass execArgv (V8 options) explicitly will throw an error
+assert.throws(() => {
+  new Worker(CODE, { eval: true, execArgv: ['--expose-gc'] });
+}, /ERR_WORKER_INVALID_EXEC_ARGV/);