src: add validation for duplicate options in node and namespace options

Author: pmarchini

src/node_config_file.cc

@@ -52,6 +52,14 @@ ParseResult ConfigReader::ParseNodeOptions(
     std::string prefix = "--";
     auto it = env_options_map.find(prefix.append(key));
     if (it != env_options_map.end()) {
+      // If the option has already been set in the namespace options
+      // we return an invalid content error
+      if (unique_namespace_options_.contains(it->first)) {
+        FPrintF(stderr,
+                "Option %s is already set in namespace options\n",
+                it->first.c_str());
+        return ParseResult::InvalidContent;
+      }
       ParseResult result = ProcessOptionValue(key,
                                               it->first,
                                               ondemand_value,
@@ -283,6 +291,14 @@ ParseResult ConfigReader::ParseNamespaceOptions(
     std::string prefix = "--";
     auto it = options_map.find(prefix.append(key));
     if (it != options_map.end()) {
+      // If the option has already been set in the nodeOptions
+      // we return an invalid content error
+      if (unique_node_options_.contains(it->first)) {
+        FPrintF(stderr,
+                "Option %s is already set in nodeOptions\n",
+                it->first.c_str());
+        return ParseResult::InvalidContent;
+      }
       ParseResult result = ProcessOptionValue(key,
                                               it->first,
                                               ondemand_value,

test/fixtures/rc/override-namespace-inverse.json renamed to test/fixtures/rc/override-node-option-with-namespace.json

@@ -401,29 +401,29 @@ describe('namespaced options', () => {
     strictEqual(result.code, 0);
   });
 
-  it('should override a non-namespaced option with a namespaced option', async () => {
+  it('should throw an error if a namespaced option has already been set in node options', async () => {
     const result = await spawnPromisified(process.execPath, [
       '--no-warnings',
       '--expose-internals',
       '--experimental-config-file',
-      fixtures.path('rc/override-namespace.json'),
+      fixtures.path('rc/override-node-option-with-namespace.json'),
       '-p', 'require("internal/options").getOptionValue("--test-isolation")',
     ]);
-    strictEqual(result.stderr, '');
-    strictEqual(result.stdout, 'process\n');
-    strictEqual(result.code, 0);
+    match(result.stderr, /Option --test-isolation is already set in nodeOptions/);
+    strictEqual(result.stdout, '');
+    strictEqual(result.code, 9);
   });
 
-  it('should override a non-namespaced option with a namespaced option inverse order', async () => {
+  it('should throw an error if a node option has already been set in a namespaced option', async () => {
     const result = await spawnPromisified(process.execPath, [
       '--no-warnings',
       '--expose-internals',
       '--experimental-config-file',
-      fixtures.path('rc/override-namespace-inverse.json'),
+      fixtures.path('rc/override-namespace.json'),
       '-p', 'require("internal/options").getOptionValue("--test-isolation")',
     ]);
-    strictEqual(result.stderr, '');
-    strictEqual(result.stdout, 'process\n');
-    strictEqual(result.code, 0);
+    match(result.stderr, /Option --test-isolation is already set in namespace options/);
+    strictEqual(result.stdout, '');
+    strictEqual(result.code, 9);
   });
 });