src: parse dotenv with the rest of the options

Author: avivkeller

src/node.cc

@@ -851,28 +851,6 @@ static ExitCode InitializeNodeWithArgsInternal(
   HandleEnvOptions(per_process::cli_options->per_isolate->per_env);
 
   std::string node_options;
-  auto file_paths = node::Dotenv::GetPathFromArgs(*argv);
-
-  if (!file_paths.empty()) {
-    CHECK(!per_process::v8_initialized);
-
-    for (const auto& file_path : file_paths) {
-      switch (per_process::dotenv_file.ParsePath(file_path)) {
-        case Dotenv::ParseResult::Valid:
-          break;
-        case Dotenv::ParseResult::InvalidContent:
-          errors->push_back(file_path + ": invalid format");
-          break;
-        case Dotenv::ParseResult::FileError:
-          errors->push_back(file_path + ": not found");
-          break;
-        default:
-          UNREACHABLE();
-      }
-    }
-
-    per_process::dotenv_file.AssignNodeOptionsIfAvailable(&node_options);
-  }
 
 #if !defined(NODE_WITHOUT_NODE_OPTIONS)
   if (!(flags & ProcessInitializationFlags::kDisableNodeOptionsEnv)) {
@@ -909,6 +887,34 @@ static ExitCode InitializeNodeWithArgsInternal(
     if (exit_code != ExitCode::kNoFailure) return exit_code;
   }
 
+  if (!per_process::cli_options->per_isolate->per_env->env_file.empty()) {
+    CHECK(!per_process::v8_initialized);
+
+    for (const auto& file_path :
+         per_process::cli_options->per_isolate->per_env->env_file) {
+      switch (per_process::dotenv_file.ParsePath(file_path)) {
+        case Dotenv::ParseResult::Valid:
+          break;
+        case Dotenv::ParseResult::InvalidContent:
+          errors->push_back(file_path + ": invalid format");
+          break;
+        case Dotenv::ParseResult::FileError:
+          errors->push_back(file_path + ": not found");
+          break;
+        default:
+          UNREACHABLE();
+      }
+    }
+
+    std::vector<std::string> env_argv = ParseNodeOptionsEnvVar(
+        per_process::dotenv_file.GetNodeOptions(), errors);
+    env_argv.insert(env_argv.begin(), argv->at(0));
+
+    const ExitCode exit_code =
+        ProcessGlobalArgsInternal(&env_argv, nullptr, errors, kAllowedInEnvvar);
+    if (exit_code != ExitCode::kNoFailure) return exit_code;
+  }
+
   // Set the process.title immediately after processing argv if --title is set.
   if (!per_process::cli_options->title.empty())
     uv_set_process_title(per_process::cli_options->title.c_str());

src/node_dotenv.cc

@@ -11,38 +11,6 @@ using v8::NewStringType;
 using v8::Object;
 using v8::String;
 
-std::vector<std::string> Dotenv::GetPathFromArgs(
-    const std::vector<std::string>& args) {
-  const auto find_match = [](const std::string& arg) {
-    return arg == "--" || arg == "--env-file" || arg.starts_with("--env-file=");
-  };
-  std::vector<std::string> paths;
-  auto path = std::find_if(args.begin(), args.end(), find_match);
-
-  while (path != args.end()) {
-    if (*path == "--") {
-      return paths;
-    }
-    auto equal_char = path->find('=');
-
-    if (equal_char != std::string::npos) {
-      paths.push_back(path->substr(equal_char + 1));
-    } else {
-      auto next_path = std::next(path);
-
-      if (next_path == args.end()) {
-        return paths;
-      }
-
-      paths.push_back(*next_path);
-    }
-
-    path = std::find_if(++path, args.end(), find_match);
-  }
-
-  return paths;
-}
-
 void Dotenv::SetEnvironment(node::Environment* env) {
   auto isolate = env->isolate();
 
@@ -261,12 +229,9 @@ Dotenv::ParseResult Dotenv::ParsePath(const std::string_view path) {
   return ParseResult::Valid;
 }
 
-void Dotenv::AssignNodeOptionsIfAvailable(std::string* node_options) const {
-  auto match = store_.find("NODE_OPTIONS");
-
-  if (match != store_.end()) {
-    *node_options = match->second;
-  }
+std::string Dotenv::GetNodeOptions() const {
+  auto it = store_.find("NODE_OPTIONS");
+  return (it != store_.end()) ? it->second : "";
 }
 
 }  // namespace node

src/node_dotenv.h

@@ -23,7 +23,7 @@ class Dotenv {
 
   void ParseContent(const std::string_view content);
   ParseResult ParsePath(const std::string_view path);
-  void AssignNodeOptionsIfAvailable(std::string* node_options) const;
+  std::string GetNodeOptions() const;
   void SetEnvironment(Environment* env);
   v8::Local<v8::Object> ToObject(Environment* env) const;
 

src/node_options.h

@@ -176,7 +176,7 @@ class EnvironmentOptions : public Options {
 #endif  // HAVE_INSPECTOR
   std::string redirect_warnings;
   std::string diagnostic_dir;
-  std::string env_file;
+  std::vector<std::string> env_file;
   bool has_env_file_string = false;
   bool test_runner = false;
   uint64_t test_runner_concurrency = 0;

test/parallel/test-dotenv-edge-cases.js

@@ -113,3 +113,31 @@ describe('.env supports edge cases', () => {
     assert.strictEqual(child.code, 0);
   });
 });
+
+// Ref: https://github.com/nodejs/node/pull/54913
+it('CLI edge cases', async () => {
+  const edgeCases = [
+    {
+      flags: [fixtures.path('empty.js'), '--env-file=nonexistent.env'],
+    },
+    {
+      flags: ['--eval=""', '--', '--env-file=nonexistent.env'],
+    },
+    {
+      flags: ['--invalid-argument', '--env-file=nonexistent.env'],
+      error: 'bad option: --invalid-argument',
+    },
+    {
+      flags: ['--env-file-ABCD=nonexistent.env'],
+      error: 'bad option: --env-file-ABCD=nonexistent.env'
+    },
+  ];
+  for (const { flags, error } of edgeCases) {
+    const child = await common.spawnPromisified(process.execPath, flags);
+    if (error) {
+      assert.notStrictEqual(child.code, 0);
+      // Remove the leading '<path>: '
+      assert.strictEqual(child.stderr.substring(process.execPath.length + 2).trim(), error);
+    }
+  }
+});