worker: allow execArgv and eval in combination

addaleax · addaleax · commit 3ad8f6123640 · 2019-03-10T22:25:15.000Z
It was likely an oversight that `execArgv` was only read when no filename/URL was provided. PR-URL: #26533 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
diff --git a/src/node_worker.cc b/src/node_worker.cc
@@ -439,63 +439,64 @@ void Worker::New(const FunctionCallbackInfo<Value>& args) {
   std::vector<std::string> exec_argv_out;
   bool has_explicit_exec_argv = false;
 
+  CHECK_EQ(args.Length(), 2);
   // Argument might be a string or URL
-  if (args.Length() > 0 && !args[0]->IsNullOrUndefined()) {
+  if (!args[0]->IsNullOrUndefined()) {
     Utf8Value value(
         args.GetIsolate(),
         args[0]->ToString(env->context()).FromMaybe(v8::Local<v8::String>()));
     url.append(value.out(), value.length());
+  }
 
-    if (args.Length() > 1 && args[1]->IsArray()) {
-      v8::Local<v8::Array> array = args[1].As<v8::Array>();
-      // The first argument is reserved for program name, but we don't need it
-      // in workers.
-      has_explicit_exec_argv = true;
-      std::vector<std::string> exec_argv = {""};
-      uint32_t length = array->Length();
-      for (uint32_t i = 0; i < length; i++) {
-        v8::Local<v8::Value> arg;
-        if (!array->Get(env->context(), i).ToLocal(&arg)) {
-          return;
-        }
-        v8::MaybeLocal<v8::String> arg_v8_string =
-            arg->ToString(env->context());
-        if (arg_v8_string.IsEmpty()) {
-          return;
-        }
-        Utf8Value arg_utf8_value(
-            args.GetIsolate(),
-            arg_v8_string.FromMaybe(v8::Local<v8::String>()));
-        std::string arg_string(arg_utf8_value.out(), arg_utf8_value.length());
-        exec_argv.push_back(arg_string);
+  if (args[1]->IsArray()) {
+    v8::Local<v8::Array> array = args[1].As<v8::Array>();
+    // The first argument is reserved for program name, but we don't need it
+    // in workers.
+    has_explicit_exec_argv = true;
+    std::vector<std::string> exec_argv = {""};
+    uint32_t length = array->Length();
+    for (uint32_t i = 0; i < length; i++) {
+      v8::Local<v8::Value> arg;
+      if (!array->Get(env->context(), i).ToLocal(&arg)) {
+        return;
       }
-
-      std::vector<std::string> invalid_args{};
-      std::vector<std::string> errors{};
-      per_isolate_opts.reset(new PerIsolateOptions());
-
-      // Using invalid_args as the v8_args argument as it stores unknown
-      // options for the per isolate parser.
-      options_parser::Parse(
-          &exec_argv,
-          &exec_argv_out,
-          &invalid_args,
-          per_isolate_opts.get(),
-          kDisallowedInEnvironment,
-          &errors);
-
-      // The first argument is program name.
-      invalid_args.erase(invalid_args.begin());
-      if (errors.size() > 0 || invalid_args.size() > 0) {
-        v8::Local<v8::Value> error =
-            ToV8Value(env->context(),
-                      errors.size() > 0 ? errors : invalid_args)
-                .ToLocalChecked();
-        Local<String> key =
-            FIXED_ONE_BYTE_STRING(env->isolate(), "invalidExecArgv");
-        USE(args.This()->Set(env->context(), key, error).FromJust());
+      v8::MaybeLocal<v8::String> arg_v8_string =
+          arg->ToString(env->context());
+      if (arg_v8_string.IsEmpty()) {
         return;
       }
+      Utf8Value arg_utf8_value(
+          args.GetIsolate(),
+          arg_v8_string.FromMaybe(v8::Local<v8::String>()));
+      std::string arg_string(arg_utf8_value.out(), arg_utf8_value.length());
+      exec_argv.push_back(arg_string);
+    }
+
+    std::vector<std::string> invalid_args{};
+    std::vector<std::string> errors{};
+    per_isolate_opts.reset(new PerIsolateOptions());
+
+    // Using invalid_args as the v8_args argument as it stores unknown
+    // options for the per isolate parser.
+    options_parser::Parse(
+        &exec_argv,
+        &exec_argv_out,
+        &invalid_args,
+        per_isolate_opts.get(),
+        kDisallowedInEnvironment,
+        &errors);
+
+    // The first argument is program name.
+    invalid_args.erase(invalid_args.begin());
+    if (errors.size() > 0 || invalid_args.size() > 0) {
+      v8::Local<v8::Value> error =
+          ToV8Value(env->context(),
+                    errors.size() > 0 ? errors : invalid_args)
+              .ToLocalChecked();
+      Local<String> key =
+          FIXED_ONE_BYTE_STRING(env->isolate(), "invalidExecArgv");
+      USE(args.This()->Set(env->context(), key, error).FromJust());
+      return;
     }
   }
   if (!has_explicit_exec_argv)
diff --git a/test/parallel/test-worker-execargv.js b/test/parallel/test-worker-execargv.js
@@ -19,6 +19,13 @@ if (!process.env.HAS_STARTED_WORKER) {
       /Warning: some warning[\s\S]*at Object\.<anonymous>/.test(error)
     );
   }));
+
+  new Worker(
+    "require('worker_threads').parentPort.postMessage(process.execArgv)",
+    { eval: true, execArgv: ['--trace-warnings'] })
+    .on('message', common.mustCall((data) => {
+      assert.deepStrictEqual(data, ['--trace-warnings']);
+    }));
 } else {
   process.emitWarning('some warning');
   assert.deepStrictEqual(process.execArgv, ['--trace-warnings']);
