Skip to content

Commit

Permalink
fix handling of bareword args matching node flags; closes #3761
Browse files Browse the repository at this point in the history
  • Loading branch information
boneskull committed Feb 25, 2019
1 parent 6535965 commit e91ef1c
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 10 deletions.
32 changes: 24 additions & 8 deletions lib/cli/node-flags.js
Expand Up @@ -29,16 +29,32 @@ const debugFlags = new Set(['debug', 'debug-brk', 'inspect', 'inspect-brk']);
* - `--v8-*` (but *not* `--v8-options`)
* @summary Whether or not to pass a flag along to the `node` executable.
* @param {string} flag - Flag to test
* @returns {boolean}
* @param {boolean} [bareword=true] - If `false`, we expect `flag` to have one or two leading dashes.
* @returns {boolean} If the flag is considered a "Node" flag.
* @private
*/
exports.isNodeFlag = flag =>
!/^(?:require|r)$/.test(flag) &&
(nodeFlags.has(flag) ||
debugFlags.has(flag) ||
/(?:preserve-symlinks(?:-main)?|harmony(?:[_-]|$)|(?:trace[_-].+$)|gc(?:[_-]global)?$|es[_-]staging$|use[_-]strict$|v8[_-](?!options).+?$)/.test(
flag
));
exports.isNodeFlag = (flag, bareword = true) => {
if (!bareword) {
// check if the flag begins with dashes; if not, not a node flag.
if (!/^--?/.test(flag)) {
return false;
}
// strip the leading dashes to match against subsequent checks
flag = flag.replace(/^--?/, '');
}
return (
// treat --require/-r as Mocha flag even though it's also a node flag
!(flag === 'require' || flag === 'r') &&
// check actual node flags from `process.allowedNodeEnvironmentFlags`,
// then historical support for various V8 and non-`NODE_OPTIONS` flags
// and also any V8 flags with `--v8-` prefix
(nodeFlags.has(flag) ||
debugFlags.has(flag) ||
/(?:preserve-symlinks(?:-main)?|harmony(?:[_-]|$)|(?:trace[_-].+$)|gc(?:[_-]global)?$|es[_-]staging$|use[_-]strict$|v8[_-](?!options).+?$)/.test(
flag
))
);
};

/**
* Returns `true` if the flag is a "debug-like" flag. These require timeouts
Expand Down
5 changes: 3 additions & 2 deletions lib/cli/options.js
Expand Up @@ -85,8 +85,9 @@ const parse = (args = [], ...configObjects) => {
const nodeArgs = (Array.isArray(args) ? args : args.split(' ')).reduce(
(acc, arg) => {
const pair = arg.split('=');
const flag = pair[0].replace(/^--?/, '');
if (isNodeFlag(flag)) {
let flag = pair[0];
if (isNodeFlag(flag, false)) {
flag = flag.replace(/^--?/, '');
return arg.includes('=')
? acc.concat([[flag, pair[1]]])
: acc.concat([[flag, true]]);
Expand Down
19 changes: 19 additions & 0 deletions test/integration/options/node-flags.spec.js
@@ -0,0 +1,19 @@
'use strict';

var invokeMocha = require('../helpers').invokeMocha;

describe('node flags', function() {
it('should not consider argument values to be node flags', function(done) {
invokeMocha(
['--require', 'trace-dependency'],
function(err, res) {
if (err) {
return done(err);
}
expect(res, 'not to have failed with output', /bad option/i);
done();
},
'pipe'
);
});
});
12 changes: 12 additions & 0 deletions test/node-unit/cli/node-flags.spec.js
Expand Up @@ -15,6 +15,18 @@ describe('node-flags', function() {
});
});

describe('when expecting leading dashes', function() {
it('should require leading dashes', function() {
expect(isNodeFlag('throw-deprecation', false), 'to be false');
expect(isNodeFlag('--throw-deprecation', false), 'to be true');
});

it('should return false for --require/-r', function() {
expect(isNodeFlag('--require', false), 'to be false');
expect(isNodeFlag('-r', false), 'to be false');
});
});

describe('special cases', function() {
it('should return true for flags starting with "preserve-symlinks"', function() {
expect(isNodeFlag('preserve-symlinks'), 'to be true');
Expand Down

0 comments on commit e91ef1c

Please sign in to comment.