Skip to content

Commit

Permalink
src: fix execArgv in worker
Browse files Browse the repository at this point in the history
PR-URL: #53029
Fixes: #53011
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
  • Loading branch information
theanarkh authored May 23, 2024
1 parent 124aed2 commit c137d6e
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 6 deletions.
7 changes: 4 additions & 3 deletions src/node_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -590,9 +590,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{};
Expand All @@ -608,7 +607,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)) {
Expand Down
21 changes: 18 additions & 3 deletions test/parallel/test-worker-cli-options.js
Original file line number Diff line number Diff line change
@@ -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/);

0 comments on commit c137d6e

Please sign in to comment.