diff --git a/index.js b/index.js index 9e25e05..ac5b0ad 100644 --- a/index.js +++ b/index.js @@ -6,8 +6,9 @@ const { ArrayPrototypeShift, ArrayPrototypeSlice, ArrayPrototypePush, - ObjectPrototypeHasOwnProperty: ObjectHasOwn, + ObjectDefineProperty, ObjectEntries, + ObjectPrototypeHasOwnProperty: ObjectHasOwn, StringPrototypeCharAt, StringPrototypeIncludes, StringPrototypeIndexOf, @@ -29,7 +30,9 @@ const { isLongOptionAndValue, isOptionValue, isShortOptionAndValue, - isShortOptionGroup + isShortOptionGroup, + objectGetOwn, + optionsGetOwn } = require('./utils'); const { @@ -74,61 +77,83 @@ function getMainArgs() { return ArrayPrototypeSlice(process.argv, 2); } -const protoKey = '__proto__'; - -function storeOption({ - strict, - options, - result, - longOption, - shortOption, - optionValue, -}) { - const hasOptionConfig = ObjectHasOwn(options, longOption); - const optionConfig = hasOptionConfig ? options[longOption] : {}; - - if (strict) { - if (!hasOptionConfig) { - throw new ERR_PARSE_ARGS_UNKNOWN_OPTION(shortOption == null ? `--${longOption}` : `-${shortOption}`); - } - - const shortOptionErr = ObjectHasOwn(optionConfig, 'short') ? `-${optionConfig.short}, ` : ''; +/** + * In strict mode, throw for usage errors. + * + * @param {string} longOption - long option name e.g. 'foo' + * @param {string|undefined} optionValue - value from user args + * @param {Object} options - option configs, from parseArgs({ options }) + * @param {string} shortOrLong - option used, with dashes e.g. `-l` or `--long` + * @param {boolean} strict - show errors, from parseArgs({ strict }) + */ +function checkOptionUsage(longOption, optionValue, options, + shortOrLong, strict) { + // Strict and options are used from local context. + if (!strict) return; - if (options[longOption].type === 'string' && optionValue == null) { - throw new ERR_PARSE_ARGS_INVALID_OPTION_VALUE(`Option '${shortOptionErr}--${longOption} ' argument missing`); - } + if (!ObjectHasOwn(options, longOption)) { + throw new ERR_PARSE_ARGS_UNKNOWN_OPTION(shortOrLong); + } - if (options[longOption].type === 'boolean' && optionValue != null) { - throw new ERR_PARSE_ARGS_INVALID_OPTION_VALUE(`Option '${shortOptionErr}--${longOption}' does not take an argument`); - } + const short = optionsGetOwn(options, longOption, 'short'); + const shortAndLong = short ? `-${short}, --${longOption}` : `--${longOption}`; + const type = optionsGetOwn(options, longOption, 'type'); + if (type === 'string' && typeof optionValue !== 'string') { + throw new ERR_PARSE_ARGS_INVALID_OPTION_VALUE(`Option '${shortAndLong} ' argument missing`); + } + // (Idiomatic test for undefined||null, expecting undefined.) + if (type === 'boolean' && optionValue != null) { + throw new ERR_PARSE_ARGS_INVALID_OPTION_VALUE(`Option '${shortAndLong}' does not take an argument`); } +} - if (longOption === protoKey) { +/** + * Store the option value in `values`. + * + * @param {string} longOption - long option name e.g. 'foo' + * @param {string|undefined} optionValue - value from user args + * @param {Object} options - option configs, from parseArgs({ options }) + * @param {Object} values - option values returned in `values` by parseArgs + */ +function storeOption(longOption, optionValue, options, values) { + if (longOption === '__proto__') { return; } - // Values - const usedAsFlag = optionValue === undefined; - const newValue = usedAsFlag ? true : optionValue; - if (optionConfig.multiple) { - // Always store value in array, including for flags. - // result.values[longOption] starts out not present, + // Can be removed when value has a null prototype + const safeAssignProperty = (obj, prop, value) => { + ObjectDefineProperty(obj, prop, { + value, + writable: true, + enumerable: true, + configurable: true + }); + }; + + // We store based on the option value rather than option type, + // preserving the users intent for author to deal with. + const newValue = optionValue ?? true; + if (optionsGetOwn(options, longOption, 'multiple')) { + // Always store value in array, including for boolean. + // values[longOption] starts out not present, // first value is added as new array [newValue], // subsequent values are pushed to existing array. - if (result.values[longOption] !== undefined) - ArrayPrototypePush(result.values[longOption], newValue); - else - result.values[longOption] = [newValue]; + if (ObjectHasOwn(values, longOption)) { + ArrayPrototypePush(values[longOption], newValue); + } else { + safeAssignProperty(values, longOption, [newValue]); + } } else { - result.values[longOption] = newValue; + safeAssignProperty(values, longOption, newValue); } } -const parseArgs = ({ - args = getMainArgs(), - strict = true, - options = {} -} = {}) => { +const parseArgs = (config = { __proto__: null }) => { + const args = objectGetOwn(config, 'args') ?? getMainArgs(); + const strict = objectGetOwn(config, 'strict') ?? true; + const options = objectGetOwn(config, 'options') ?? { __proto__: null }; + + // Validate input configuration. validateArray(args, 'args'); validateBoolean(strict, 'strict'); validateObject(options, 'options'); @@ -137,7 +162,8 @@ const parseArgs = ({ ({ 0: longOption, 1: optionConfig }) => { validateObject(optionConfig, `options.${longOption}`); - validateUnion(optionConfig.type, `options.${longOption}.type`, ['string', 'boolean']); + // type is required + validateUnion(objectGetOwn(optionConfig, 'type'), `options.${longOption}.type`, ['string', 'boolean']); if (ObjectHasOwn(optionConfig, 'short')) { const shortOption = optionConfig.short; @@ -183,18 +209,13 @@ const parseArgs = ({ const shortOption = StringPrototypeCharAt(arg, 1); const longOption = findLongOptionForShort(shortOption, options); let optionValue; - if (options[longOption]?.type === 'string' && isOptionValue(nextArg)) { + if (optionsGetOwn(options, longOption, 'type') === 'string' && + isOptionValue(nextArg)) { // e.g. '-f', 'bar' optionValue = ArrayPrototypeShift(remainingArgs); } - storeOption({ - strict, - options, - result, - longOption, - shortOption, - optionValue, - }); + checkOptionUsage(longOption, optionValue, options, arg, strict); + storeOption(longOption, optionValue, options, result.values); continue; } @@ -204,7 +225,7 @@ const parseArgs = ({ for (let index = 1; index < arg.length; index++) { const shortOption = StringPrototypeCharAt(arg, index); const longOption = findLongOptionForShort(shortOption, options); - if (options[longOption]?.type !== 'string' || + if (optionsGetOwn(options, longOption, 'type') !== 'string' || index === arg.length - 1) { // Boolean option, or last short in group. Well formed. ArrayPrototypePush(expanded, `-${shortOption}`); @@ -224,14 +245,8 @@ const parseArgs = ({ const shortOption = StringPrototypeCharAt(arg, 1); const longOption = findLongOptionForShort(shortOption, options); const optionValue = StringPrototypeSlice(arg, 2); - storeOption({ - strict, - options, - result, - longOption, - shortOption, - optionValue, - }); + checkOptionUsage(longOption, optionValue, options, `-${shortOption}`, strict); + storeOption(longOption, optionValue, options, result.values); continue; } @@ -239,11 +254,13 @@ const parseArgs = ({ // e.g. '--foo' const longOption = StringPrototypeSlice(arg, 2); let optionValue; - if (options[longOption]?.type === 'string' && isOptionValue(nextArg)) { + if (optionsGetOwn(options, longOption, 'type') === 'string' && + isOptionValue(nextArg)) { // e.g. '--foo', 'bar' optionValue = ArrayPrototypeShift(remainingArgs); } - storeOption({ strict, options, result, longOption, optionValue }); + checkOptionUsage(longOption, optionValue, options, arg, strict); + storeOption(longOption, optionValue, options, result.values); continue; } @@ -252,7 +269,8 @@ const parseArgs = ({ const index = StringPrototypeIndexOf(arg, '='); const longOption = StringPrototypeSlice(arg, 2, index); const optionValue = StringPrototypeSlice(arg, index + 1); - storeOption({ strict, options, result, longOption, optionValue }); + checkOptionUsage(longOption, optionValue, options, `--${longOption}`, strict); + storeOption(longOption, optionValue, options, result.values); continue; } diff --git a/test/prototype-pollution.js b/test/prototype-pollution.js index 3f28f59..f1c82c4 100644 --- a/test/prototype-pollution.js +++ b/test/prototype-pollution.js @@ -4,6 +4,23 @@ const test = require('tape'); const { parseArgs } = require('../index.js'); +// These tests are not synced upstream with node, in case of possible side-effects. +// See index.js for tests shared with upstream. + +function setObjectPrototype(prop, value) { + const oldDescriptor = Object.getOwnPropertyDescriptor(Object.prototype, prop); + Object.prototype[prop] = value; + return oldDescriptor; +} + +function restoreObjectPrototype(prop, oldDescriptor) { + if (oldDescriptor == null) { + delete Object.prototype[prop]; + } else { + Object.defineProperty(Object.prototype, prop, oldDescriptor); + } +} + test('should not allow __proto__ key to be set on object', (t) => { const passedArgs = ['--__proto__=hello']; const expected = { values: {}, positionals: [] }; @@ -13,3 +30,58 @@ test('should not allow __proto__ key to be set on object', (t) => { t.deepEqual(result, expected); t.end(); }); + +test('when prototype has multiple then ignored', (t) => { + const args = ['--foo', '1', '--foo', '2']; + const options = { foo: { type: 'string' } }; + const expectedResult = { values: { foo: '2' }, positionals: [] }; + + const holdDescriptor = setObjectPrototype('multiple', true); + const result = parseArgs({ args, options }); + restoreObjectPrototype('multiple', holdDescriptor); + t.deepEqual(result, expectedResult); + t.end(); +}); + +test('when prototype has type then ignored', (t) => { + const args = ['--foo', '1']; + const options = { foo: { } }; + + const holdDescriptor = setObjectPrototype('type', 'string'); + t.throws(() => { + parseArgs({ args, options }); + }); + restoreObjectPrototype('type', holdDescriptor); + t.end(); +}); + +test('when prototype has short then ignored', (t) => { + const args = ['-f', '1']; + const options = { foo: { type: 'string' } }; + + const holdDescriptor = setObjectPrototype('short', 'f'); + t.throws(() => { + parseArgs({ args, options }); + }); + restoreObjectPrototype('short', holdDescriptor); + t.end(); +}); + +test('when prototype has strict then ignored', (t) => { + const args = ['-f']; + + const holdDescriptor = setObjectPrototype('strict', false); + t.throws(() => { + parseArgs({ args }); + }); + restoreObjectPrototype('strict', holdDescriptor); + t.end(); +}); + +test('when prototype has args then ignored', (t) => { + const holdDescriptor = setObjectPrototype('args', ['--foo']); + const result = parseArgs({ strict: false }); + restoreObjectPrototype('args', holdDescriptor); + t.false(result.values.foo); + t.end(); +}); diff --git a/utils.js b/utils.js index b718c01..739f49d 100644 --- a/utils.js +++ b/utils.js @@ -3,6 +3,7 @@ const { ArrayPrototypeFind, ObjectEntries, + ObjectPrototypeHasOwnProperty: ObjectHasOwn, StringPrototypeCharAt, StringPrototypeIncludes, StringPrototypeSlice, @@ -20,6 +21,22 @@ const { // // These routines are for internal use, not for export to client. +/** + * Return the named property, but only if it is an own property. + */ +function objectGetOwn(obj, prop) { + if (ObjectHasOwn(obj, prop)) + return obj[prop]; +} + +/** + * Return the named options property, but only if it is an own property. + */ +function optionsGetOwn(options, longOption, prop) { + if (ObjectHasOwn(options, longOption)) + return objectGetOwn(options[longOption], prop); +} + /** * Determines if the argument may be used as an option value. * NB: We are choosing not to accept option-ish arguments. @@ -107,7 +124,7 @@ function isShortOptionGroup(arg, options) { const firstShort = StringPrototypeCharAt(arg, 1); const longOption = findLongOptionForShort(firstShort, options); - return options[longOption]?.type !== 'string'; + return optionsGetOwn(options, longOption, 'type') !== 'string'; } /** @@ -128,7 +145,7 @@ function isShortOptionAndValue(arg, options) { const shortOption = StringPrototypeCharAt(arg, 1); const longOption = findLongOptionForShort(shortOption, options); - return options[longOption]?.type === 'string'; + return optionsGetOwn(options, longOption, 'type') === 'string'; } /** @@ -144,7 +161,7 @@ function findLongOptionForShort(shortOption, options) { validateObject(options, 'options'); const { 0: longOption } = ArrayPrototypeFind( ObjectEntries(options), - ({ 1: optionConfig }) => optionConfig.short === shortOption + ({ 1: optionConfig }) => objectGetOwn(optionConfig, 'short') === shortOption ) || []; return longOption || shortOption; } @@ -156,5 +173,7 @@ module.exports = { isLongOptionAndValue, isOptionValue, isShortOptionAndValue, - isShortOptionGroup + isShortOptionGroup, + objectGetOwn, + optionsGetOwn };