src: fix config file handling

Signed-off-by: Marco Ippolito <marcoippolito54@gmail.com>
PR-URL: #66431
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com>

Author: marco-ippolito

src/node_config_file.cc

@@ -3,6 +3,7 @@
 #include "node_version.h"
 #include "simdjson.h"
 
+#include <algorithm>
 #include <cinttypes>
 
 namespace node {
@@ -21,20 +22,67 @@ inline bool HasEqualsPrefix(std::string_view arg, std::string_view flag) {
          arg[flag.size()] == '=';
 }
 
+inline bool IsBareConfigFileFlag(std::string_view arg) {
+  return arg == kConfigFileFlag || arg == kExperimentalConfigFileFlag ||
+         arg == kDefaultConfigFileFlag;
+}
+
+inline bool IsConfigFileFlag(std::string_view arg) {
+  return IsBareConfigFileFlag(arg) || HasEqualsPrefix(arg, kConfigFileFlag) ||
+         HasEqualsPrefix(arg, kExperimentalConfigFileFlag) ||
+         HasEqualsPrefix(arg, kDefaultConfigFileFlag);
+}
+
+inline std::string DefaultConfigFileArg() {
+  return std::string(kConfigFileFlag) + "=" +
+         std::string(kDefaultConfigFileName);
+}
+
+// Returns the index of the first entry in `args` that is neither the program
+// name nor an option for Node.js itself. Everything from there on is the
+// script and its arguments, or follows `--`, so it must not be read as a
+// config file flag.
+size_t GetNodeOptionsEnd(const std::vector<std::string>& args) {
+  // The bare flags take no value. Spell them out the way GetDataFromArgs()
+  // does before the real parse, otherwise the parser would consume the next
+  // argument as the path.
+  std::vector<std::string> remaining;
+  remaining.reserve(args.size());
+  for (const std::string& arg : args) {
+    remaining.push_back(IsBareConfigFileFlag(arg) ? DefaultConfigFileArg()
+                                                  : arg);
+  }
+
+  // Parsing into throwaway options stops exactly where the real parse will.
+  // Any errors are reported by the real parse later on.
+  PerProcessOptions options;
+  std::vector<std::string> v8_args;
+  std::vector<std::string> errors;
+  options_parser::Parse(
+      &remaining, nullptr, &v8_args, &options, kDisallowedInEnvvar, &errors);
+
+  // `remaining` is left with the program name and the unparsed arguments.
+  return args.size() - remaining.size() + 1;
+}
+
 std::optional<std::string_view> ConfigReader::GetDataFromArgs(
     std::vector<std::string>* args) {
   std::optional<std::string_view> result;
   invalid_default_config_file_argument_ = false;
 
-  for (size_t i = 0; i < args->size(); ++i) {
+  // Only pay for the extra parse when a config file flag might be present.
+  if (args->empty() || std::ranges::none_of(*args, IsConfigFileFlag)) {
+    return result;
+  }
+
+  const size_t node_options_end = GetNodeOptionsEnd(*args);
+  for (size_t i = 1; i < node_options_end; ++i) {
     std::string& arg = (*args)[i];
 
-    if (arg == kConfigFileFlag || arg == kExperimentalConfigFileFlag ||
-        arg == kDefaultConfigFileFlag) {
+    if (IsBareConfigFileFlag(arg)) {
       // --config-file, --experimental-config-file or
       // --experimental-default-config-file
-      arg = std::string(kConfigFileFlag) + "=" +
-            std::string(kDefaultConfigFileName);
+      arg = DefaultConfigFileArg();
       result = kDefaultConfigFileName;
     } else if (HasEqualsPrefix(arg, kConfigFileFlag) ||
                HasEqualsPrefix(arg, kExperimentalConfigFileFlag)) {

test/parallel/test-config-file.js

@@ -617,6 +617,58 @@ test('should treat a space-separated config file path as the script',
        assert.strictEqual(result.code, 1);
      });
 
+describe('config file flags that are not Node.js options', () => {
+  const argsVariants = [
+    ['--config-file'],
+    ['--config-file', 'tool.config.mjs'],
+    ['--config-file=i-do-not-exist.json'],
+    ['--experimental-config-file'],
+    ['--experimental-config-file=i-do-not-exist.json'],
+    ['--experimental-default-config-file'],
+    ['--experimental-default-config-file=i-do-not-exist.json'],
+  ];
+
+  for (const args of argsVariants) {
+    it(`should not read ${args.join(' ')} after the script`, async () => {
+      const result = await spawnPromisified(process.execPath, [
+        fixtures.path('printA.js'),
+        ...args,
+      ], {
+        cwd: fixtures.path('rc'),
+      });
+      assert.strictEqual(result.stderr, '');
+      assert.strictEqual(result.stdout, 'A\n');
+      assert.strictEqual(result.code, 0);
+    });
+
+    it(`should not read ${args.join(' ')} after --`, async () => {
+      const result = await spawnPromisified(process.execPath, [
+        '-p', 'process.argv.slice(1).join(" ")',
+        '--',
+        ...args,
+      ], {
+        cwd: fixtures.path('rc'),
+      });
+      assert.strictEqual(result.stderr, '');
+      assert.strictEqual(result.stdout, `${args.join(' ')}\n`);
+      assert.strictEqual(result.code, 0);
+    });
+  }
+});
+
+test('should read the config file after an option that takes a separate value',
+     onlyIfNodeOptionsSupport, async () => {
+       const result = await spawnPromisified(process.execPath, [
+         '--no-warnings',
+         '--title', 'config-file-test',
+         `--config-file=${fixtures.path('rc/default/node.config.json')}`,
+         '-p', 'http.maxHeaderSize',
+       ]);
+       assert.strictEqual(result.stderr, '');
+       assert.strictEqual(result.stdout, '10\n');
+       assert.strictEqual(result.code, 0);
+     });
+
 test('should error when --config-file= has empty argument',
      onlyIfNodeOptionsSupport, async () => {
        const result = await spawnPromisified(process.execPath, [