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 Fishrock123 committed Jul 19, 2017
1 parent 4d41502 commit 3cdaae2
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 10 deletions.
2 changes: 1 addition & 1 deletion doc/api/cli.md
Expand Up @@ -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
Expand Down
33 changes: 26 additions & 7 deletions src/node.cc
Expand Up @@ -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",
Expand Down Expand Up @@ -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;
}

Expand Down
9 changes: 7 additions & 2 deletions test/parallel/test-cli-node-options.js
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 @@ -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');
Expand All @@ -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');
Expand Down

0 comments on commit 3cdaae2

Please sign in to comment.