src: whitelist v8 options with '_' or '-'

V8 options allow either '_' or '-' to be used in options as a seperator, such as "--abort-on_uncaught-exception". Allow these case variations when used with NODE_OPTIONS. PR-URL: #14093 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Jul 18, 2017 · 50913b1 · 50913b1
1 parent b12b8c2
commit 50913b1
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 10 deletions.
diff --git a/doc/api/cli.md b/doc/api/cli.md
@@ -466,7 +466,7 @@ Node options that are allowed are:
 
 V8 options that are allowed are:
 - `--abort-on-uncaught-exception`
-- `--max_old_space_size`
+- `--max-old-space-size`
 
 ### `NODE_PENDING_DEPRECATION=1`
 <!-- YAML

diff --git a/src/node.cc b/src/node.cc
@@ -3729,15 +3729,34 @@ static void PrintHelp() {
 }
 
 
+static bool ArgIsAllowed(const char* arg, const char* allowed) {
+  for (; *arg && *allowed; arg++, allowed++) {
+    // Like normal strcmp(), except that a '_' in `allowed` matches either a '-'
+    // or '_' in `arg`.
+    if (*allowed == '_') {
+      if (!(*arg == '_' || *arg == '-'))
+        return false;
+    } else {
+      if (*arg != *allowed)
+        return false;
+    }
+  }
+
+  // "--some-arg=val" is allowed for "--some-arg"
+  if (*arg == '=')
+    return true;
+
+  // Both must be null, or one string is just a prefix of the other, not a
+  // match.
+  return !*arg && !*allowed;
+}
+
+
 static void CheckIfAllowedInEnv(const char* exe, bool is_env,
                                 const char* arg) {
   if (!is_env)
     return;
 
-  // Find the arg prefix when its --some_arg=val
-  const char* eq = strchr(arg, '=');
-  size_t arglen = eq ? eq - arg : strlen(arg);
-
   static const char* whitelist[] = {
     // Node options, sorted in `node --help` order for ease of comparison.
     "--require", "-r",
@@ -3765,14 +3784,14 @@ static void CheckIfAllowedInEnv(const char* exe, bool is_env,
     "--openssl-config",
     "--icu-data-dir",
 
-    // V8 options
-    "--abort-on-uncaught-exception",
+    // V8 options (define with '_', which allows '-' or '_')
+    "--abort_on_uncaught_exception",
     "--max_old_space_size",
   };
 
   for (unsigned i = 0; i < arraysize(whitelist); i++) {
     const char* allowed = whitelist[i];
-    if (strlen(allowed) == arglen && strncmp(allowed, arg, arglen) == 0)
+    if (ArgIsAllowed(arg, allowed))
       return;
   }
 

diff --git a/test/parallel/test-cli-node-options.js b/test/parallel/test-cli-node-options.js
@@ -26,6 +26,7 @@ disallow('--interactive');
 disallow('-i');
 disallow('--v8-options');
 disallow('--');
+disallow('--no_warnings'); // Node options don't allow '_' instead of '-'.
 
 function disallow(opt) {
   const options = {env: {NODE_OPTIONS: opt}};
@@ -40,7 +41,6 @@ function disallow(opt) {
 
 const printA = require.resolve('../fixtures/printA.js');
 
-expect('--abort-on-uncaught-exception', 'B\n');
 expect(`-r ${printA}`, 'A\nB\n');
 expect('--no-deprecation', 'B\n');
 expect('--no-warnings', 'B\n');
@@ -59,8 +59,13 @@ if (common.hasCrypto) {
   expect('--openssl-config=_ossl_cfg', 'B\n');
 }
 
-    // V8 options
+// V8 options
+expect('--abort-on-uncaught-exception', 'B\n');
+expect('--abort_on_uncaught_exception', 'B\n');
+expect('--abort_on-uncaught_exception', 'B\n');
 expect('--max_old_space_size=0', 'B\n');
+expect('--max-old_space-size=0', 'B\n');
+expect('--max-old-space-size=0', 'B\n');
 
 function expect(opt, want) {
   const printB = require.resolve('../fixtures/printB.js');