Skip to content

Commit

Permalink
src: whitelist v8 options with '_' or '-'
Browse files Browse the repository at this point in the history
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 <riclau@uk.ibm.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
sam-github authored and MylesBorins committed Nov 14, 2017
1 parent 8b70ad1 commit 4bdbcc3
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 8 deletions.
2 changes: 1 addition & 1 deletion doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,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_REPL_HISTORY=file`
Expand Down
33 changes: 26 additions & 7 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3786,15 +3786,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",
Expand All @@ -3820,14 +3839,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;
}

Expand Down
4 changes: 4 additions & 0 deletions test/parallel/test-cli-node-options.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}};
Expand All @@ -43,6 +44,7 @@ const printA = require.resolve('../fixtures/printA.js');

expect(`-r ${printA}`, 'A\nB\n');
expect('--abort-on-uncaught-exception', 'B\n');
expect('--abort_on_uncaught_exception', 'B\n');
expect('--no-deprecation', 'B\n');
expect('--no-warnings', 'B\n');
expect('--trace-warnings', 'B\n');
Expand All @@ -61,6 +63,8 @@ expect('--icu-data-dir=_d', 'B\n');

// V8 options
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');
Expand Down

0 comments on commit 4bdbcc3

Please sign in to comment.