refactor: tweaks for landing in Node.js (#89)

Benjamin E. Coe · web-flow · commit 9549cf923477 · 2022-04-09T22:38:16.000Z
diff --git a/errors.js b/errors.js
@@ -7,8 +7,16 @@ class ERR_INVALID_ARG_TYPE extends TypeError {
   }
 }
 
+class ERR_INVALID_SHORT_OPTION extends TypeError {
+  constructor(longOption, shortOption) {
+    super(`options.${longOption}.short must be a single character, got '${shortOption}'`);
+    this.code = 'ERR_INVALID_SHORT_OPTION';
+  }
+}
+
 module.exports = {
   codes: {
     ERR_INVALID_ARG_TYPE,
+    ERR_INVALID_SHORT_OPTION
   }
 };
diff --git a/index.js b/index.js
@@ -6,7 +6,7 @@ const {
   ArrayPrototypeShift,
   ArrayPrototypeSlice,
   ArrayPrototypePush,
-  ObjectHasOwn,
+  ObjectPrototypeHasOwnProperty: ObjectHasOwn,
   ObjectEntries,
   StringPrototypeCharAt,
   StringPrototypeIncludes,
@@ -32,6 +32,12 @@ const {
   isShortOptionGroup
 } = require('./utils');
 
+const {
+  codes: {
+    ERR_INVALID_SHORT_OPTION,
+  },
+} = require('./errors');
+
 function getMainArgs() {
   // This function is a placeholder for proposed process.mainArgs.
   // Work out where to slice process.argv for user supplied arguments.
@@ -66,12 +72,17 @@ function getMainArgs() {
   return ArrayPrototypeSlice(process.argv, 2);
 }
 
+const protoKey = '__proto__';
 function storeOptionValue(options, longOption, value, result) {
   const optionConfig = options[longOption] || {};
 
   // Flags
   result.flags[longOption] = true;
 
+  if (longOption === protoKey) {
+    return;
+  }
+
   // Values
   if (optionConfig.multiple) {
     // Always store value in array, including for flags.
@@ -97,7 +108,7 @@ const parseArgs = ({
   validateObject(options, 'options');
   ArrayPrototypeForEach(
     ObjectEntries(options),
-    ([longOption, optionConfig]) => {
+    ({ 0: longOption, 1: optionConfig }) => {
       validateObject(optionConfig, `options.${longOption}`);
 
       if (ObjectHasOwn(optionConfig, 'type')) {
@@ -108,7 +119,7 @@ const parseArgs = ({
         const shortOption = optionConfig.short;
         validateString(shortOption, `options.${longOption}.short`);
         if (shortOption.length !== 1) {
-          throw new Error(`options.${longOption}.short must be a single character, got '${shortOption}'`);
+          throw new ERR_INVALID_SHORT_OPTION(longOption, shortOption);
         }
       }
 
diff --git a/test/index.js b/test/index.js
@@ -385,7 +385,9 @@ test('invalid short option length', function(t) {
   const passedArgs = [];
   const passedOptions = { foo: { short: 'fo' } };
 
-  t.throws(function() { parseArgs({ args: passedArgs, options: passedOptions }); });
+  t.throws(function() { parseArgs({ args: passedArgs, options: passedOptions }); }, {
+    code: 'ERR_INVALID_SHORT_OPTION'
+  });
 
   t.end();
 });
diff --git a/test/prototype-pollution.js b/test/prototype-pollution.js
@@ -0,0 +1,15 @@
+'use strict';
+/* eslint max-len: 0 */
+
+const test = require('tape');
+const { parseArgs } = require('../index.js');
+
+test('should not allow __proto__ key to be set on object', (t) => {
+  const passedArgs = ['--__proto__=hello'];
+  const expected = { flags: {}, values: {}, positionals: [] };
+
+  const result = parseArgs({ args: passedArgs });
+
+  t.deepEqual(result, expected);
+  t.end();
+});
diff --git a/utils.js b/utils.js
@@ -9,6 +9,10 @@ const {
   StringPrototypeStartsWith,
 } = require('./primordials');
 
+const {
+  validateObject
+} = require('./validators');
+
 // These are internal utilities to make the parsing logic easier to read, and
 // add lots of detail for the curious. They are in a separate file to allow
 // unit testing, although that is not essential (this could be rolled into
@@ -116,7 +120,8 @@ function isShortOptionGroup(arg, options) {
  * }) // returns true
  */
 function isShortOptionAndValue(arg, options) {
-  if (!options) throw new Error('Internal error, missing options argument');
+  validateObject(options, 'options');
+
   if (arg.length <= 2) return false;
   if (StringPrototypeCharAt(arg, 0) !== '-') return false;
   if (StringPrototypeCharAt(arg, 1) === '-') return false;
@@ -136,10 +141,10 @@ function isShortOptionAndValue(arg, options) {
  * }) // returns 'bar'
  */
 function findLongOptionForShort(shortOption, options) {
-  if (!options) throw new Error('Internal error, missing options argument');
-  const [longOption] = ArrayPrototypeFind(
+  validateObject(options, 'options');
+  const { 0: longOption } = ArrayPrototypeFind(
     ObjectEntries(options),
-    ([, optionConfig]) => optionConfig.short === shortOption
+    ({ 1: optionConfig }) => optionConfig.short === shortOption
   ) || [];
   return longOption || shortOption;
 }

Original file line number	Diff line number	Diff line change
`@@ -7,8 +7,16 @@ class ERR_INVALID_ARG_TYPE extends TypeError {`
`7`	`7`	`}`
`8`	`8`	`}`
`9`	`9`
	`10`	`+class ERR_INVALID_SHORT_OPTION extends TypeError {`
	`11`	`+ constructor(longOption, shortOption) {`
	`12`	+ super(`options.${longOption}.short must be a single character, got '${shortOption}'`);
	`13`	`+ this.code = 'ERR_INVALID_SHORT_OPTION';`
	`14`	`+ }`
	`15`	`+}`
	`16`	`+`
`10`	`17`	`module.exports = {`
`11`	`18`	`codes: {`
`12`	`19`	`ERR_INVALID_ARG_TYPE,`
	`20`	`+ ERR_INVALID_SHORT_OPTION`
`13`	`21`	`}`
`14`	`22`	`};`