src: parse dotenv with the rest of the options

src/node.cc

@@ -800,6 +800,34 @@ int ProcessGlobalArgs(std::vector<std::string>* args,
 
 static std::atomic_bool init_called{false};
 
+static ExitCode ProcessEnvFiles(std::vector<std::string>* argv,
+                                std::vector<std::string>* errors) {
+  const auto& cli_options = per_process::cli_options->per_isolate->per_env;
+  // Early exit if both env_files and optional_env_files are empty
+  if (cli_options->env_files.empty() &&
+      cli_options->optional_env_files.empty()) {
+    return ExitCode::kNoFailure;
+  }
+
+  CHECK(!per_process::v8_initialized);
+  per_process::dotenv_file.ProcessEnvFilesFromCLI(
+      cli_options, per_process::cli_options->cmdline, errors);
+
+#if !defined(NODE_WITHOUT_NODE_OPTIONS)
+  // Parse and process Node.js options from the environment
+  std::vector<std::string> env_argv =
+      ParseNodeOptionsEnvVar(per_process::dotenv_file.GetNodeOptions(), errors);
+  env_argv.insert(env_argv.begin(), argv->at(0));
+
+  // Process global arguments
+  const ExitCode exit_code =
+      ProcessGlobalArgsInternal(&env_argv, nullptr, errors, kAllowedInEnvvar);
+  if (exit_code != ExitCode::kNoFailure) return exit_code;
+#endif
+
+  return ExitCode::kNoFailure;
+}
+
 // TODO(addaleax): Turn this into a wrapper around InitializeOncePerProcess()
 // (with the corresponding additional flags set), then eventually remove this.
 static ExitCode InitializeNodeWithArgsInternal(
@@ -851,34 +879,6 @@ static ExitCode InitializeNodeWithArgsInternal(
   HandleEnvOptions(per_process::cli_options->per_isolate->per_env);
 
   std::string node_options;
-  auto env_files = node::Dotenv::GetDataFromArgs(*argv);
-
-  if (!env_files.empty()) {
-    CHECK(!per_process::v8_initialized);
-
-    for (const auto& file_data : env_files) {
-      switch (per_process::dotenv_file.ParsePath(file_data.path)) {
-        case Dotenv::ParseResult::Valid:
-          break;
-        case Dotenv::ParseResult::InvalidContent:
-          errors->push_back(file_data.path + ": invalid format");
-          break;
-        case Dotenv::ParseResult::FileError:
-          if (file_data.is_optional) {
-            fprintf(stderr,
-                    "%s not found. Continuing without it.\n",
-                    file_data.path.c_str());
-            continue;
-          }
-          errors->push_back(file_data.path + ": not found");
-          break;
-        default:
-          UNREACHABLE();
-      }
-    }
-
-    per_process::dotenv_file.AssignNodeOptionsIfAvailable(&node_options);
-  }
 
 #if !defined(NODE_WITHOUT_NODE_OPTIONS)
   if (!(flags & ProcessInitializationFlags::kDisableNodeOptionsEnv)) {
@@ -915,6 +915,9 @@ static ExitCode InitializeNodeWithArgsInternal(
     if (exit_code != ExitCode::kNoFailure) return exit_code;
   }
 
+  const ExitCode exit_code = ProcessEnvFiles(argv, errors);
+  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

@@ -1,5 +1,7 @@
 #include "node_dotenv.h"
 #include <unordered_set>
+
+#include "node_options-inl.h"
 #include "env-inl.h"
 #include "node_file.h"
 #include "uv.h"
@@ -11,52 +13,46 @@ using v8::NewStringType;
 using v8::Object;
 using v8::String;
 
-std::vector<Dotenv::env_file_data> Dotenv::GetDataFromArgs(
-    const std::vector<std::string>& args) {
-  const std::string_view optional_env_file_flag = "--env-file-if-exists";
-
-  const auto find_match = [](const std::string& arg) {
-    return arg == "--" || arg == "--env-file" ||
-           arg.starts_with("--env-file=") || arg == "--env-file-if-exists" ||
-           arg.starts_with("--env-file-if-exists=");
-  };
-
-  std::vector<Dotenv::env_file_data> env_files;
-  // This will be an iterator, pointing to args.end() if no matches are found
-  auto matched_arg = std::find_if(args.begin(), args.end(), find_match);
-
-  while (matched_arg != args.end()) {
-    if (*matched_arg == "--") {
-      return env_files;
+void Dotenv::ProcessEnvFilesFromCLI(
+    const std::shared_ptr<EnvironmentOptions> cli_options,
+    std::vector<std::string> cmdline,
+    std::vector<std::string>* errors) {
+  // Helper function to process environment files
+  auto process_file = [&](const std::string& file_path, bool is_optional) {
+    switch (this->ParsePath(file_path)) {
+      case Dotenv::ParseResult::Valid:
+        break;
+      case Dotenv::ParseResult::InvalidContent:
+        errors->emplace_back(file_path + ": invalid format");
+        break;
+      case Dotenv::ParseResult::FileError:
+        if (is_optional) {
+          fprintf(stderr,
+                  "%s not found. Continuing without it.\n",
+                  file_path.c_str());
+        } else {
+          errors->emplace_back(file_path + ": not found");
+        }
+        break;
+      default:
+        UNREACHABLE();
     }
+  };
 
-    auto equal_char_index = matched_arg->find('=');
-
-    if (equal_char_index != std::string::npos) {
-      // `--env-file=path`
-      auto flag = matched_arg->substr(0, equal_char_index);
-      auto file_path = matched_arg->substr(equal_char_index + 1);
-
-      struct env_file_data env_file_data = {
-          file_path, flag.starts_with(optional_env_file_flag)};
-      env_files.push_back(env_file_data);
-    } else {
-      // `--env-file path`
-      auto file_path = std::next(matched_arg);
-
-      if (file_path == args.end()) {
-        return env_files;
-      }
-
-      struct env_file_data env_file_data = {
-          *file_path, matched_arg->starts_with(optional_env_file_flag)};
-      env_files.push_back(env_file_data);
+  // Process env files and optional env files based on the command-line
+  // arguments
+  // TODO(RedYetiDev): Find a way to get the index of each argument, in order to
+  // create a more robust method of determining the order of env files.
+  int env_file_idx = 0;
+  int optional_env_file_idx = 0;
+  for (const auto& arg : cmdline) {
+    if (arg.starts_with("--env-file-if-exists")) {
+      process_file(cli_options->optional_env_files[optional_env_file_idx++],
+                   true);
+    } else if (arg.starts_with("--env-file")) {
+      process_file(cli_options->env_files[env_file_idx++], false);
     }
-
-    matched_arg = std::find_if(++matched_arg, args.end(), find_match);
   }
-
-  return env_files;
 }
 
 void Dotenv::SetEnvironment(node::Environment* env) {
@@ -277,12 +273,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

@@ -3,6 +3,7 @@
 
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 
+#include "node_options-inl.h"
 #include "util-inl.h"
 #include "v8.h"
 
@@ -25,9 +26,13 @@ class Dotenv {
   Dotenv& operator=(const Dotenv& d) = delete;
   ~Dotenv() = default;
 
+  void ProcessEnvFilesFromCLI(
+      const std::shared_ptr<EnvironmentOptions> cli_options,
+      std::vector<std::string> cmdline,
+      std::vector<std::string>* errors);
   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.cc

@@ -638,11 +638,11 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
       "[has_env_file_string]", "", &EnvironmentOptions::has_env_file_string);
   AddOption("--env-file",
             "set environment variables from supplied file",
-            &EnvironmentOptions::env_file);
+            &EnvironmentOptions::env_files);
   Implies("--env-file", "[has_env_file_string]");
   AddOption("--env-file-if-exists",
             "set environment variables from supplied file",
-            &EnvironmentOptions::optional_env_file);
+            &EnvironmentOptions::optional_env_files);
   Implies("--env-file-if-exists", "[has_env_file_string]");
   AddOption("--test",
             "launch test runner on startup",

src/node_options.h

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

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

@@ -135,17 +135,39 @@ describe('.env supports edge cases', () => {
     assert.strictEqual(child.code, 0);
   });
 
-  it('should handle when --env-file is passed along with --', async () => {
-    const child = await common.spawnPromisified(
-      process.execPath,
-      [
-        '--eval', `require('assert').strictEqual(process.env.BASIC, undefined);`,
-        '--', '--env-file', validEnvFilePath,
-      ],
-      { cwd: __dirname },
-    );
-    assert.strictEqual(child.stdout, '');
-    assert.strictEqual(child.stderr, '');
-    assert.strictEqual(child.code, 0);
+  // Ref: https://github.com/nodejs/node/pull/54913
+  it('should handle CLI edge cases', async () => {
+    const edgeCases = [
+      {
+        // If the flag is passed AFTER the script, ignore it
+        flags: [fixtures.path('empty.js'), '--env-file=nonexistent.env'],
+      },
+      {
+        // If the flag is passed AFTER '--', ignore it
+        flags: ['--eval=""', '--', '--env-file=nonexistent.env'],
+      },
+      {
+        // If the flag is passed AFTER an invalid argument, check the argument first
+        flags: ['--invalid-argument', '--env-file=nonexistent.env'],
+        error: 'bad option: --invalid-argument',
+      },
+      {
+        // If the flag is passed as an invalid argument, check the argument first
+        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);
+      } else {
+        assert.strictEqual(child.code, 0);
+        assert.strictEqual(child.stderr, '');
+        assert.strictEqual(child.stdout, '');
+      }
+    }
   });
 });

test/parallel/test-dotenv-without-node-options.js

@@ -0,0 +1,26 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('node:assert');
+const { test } = require('node:test');
+
+if (!process.config.variables.node_without_node_options) {
+  common.skip('Requires the lack of NODE_OPTIONS support');
+}
+
+const relativePath = '../fixtures/dotenv/node-options.env';
+
+test('.env does not support NODE_OPTIONS', async () => {
+  const code = 'assert.strictEqual(process.permission, undefined)';
+  const child = await common.spawnPromisified(
+    process.execPath,
+    [ `--env-file=${relativePath}`, '--eval', code ],
+    { cwd: __dirname },
+  );
+  // NODE_NO_WARNINGS is set, so `stderr` should not contain
+  // "ExperimentalWarning: Permission is an experimental feature" message
+  // and thus be empty
+  assert.strictEqual(child.stdout, '');
+  assert.strictEqual(child.stderr, '');
+  assert.strictEqual(child.code, 0);
+});