lib: allow to validate enums with validateOneOf

PR-URL: #34070 Reviewed-By: Zeyu Yang <himself65@outlook.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: James M Snell <jasnell@gmail.com>
nodejs · Aug 17, 2020 · 6726246 · 6726246
1 parent c62cf1d
commit 6726246
Show file tree

Hide file tree

Showing 10 changed files with 108 additions and 40 deletions.
diff --git a/lib/_http_agent.js b/lib/_http_agent.js
@@ -39,12 +39,11 @@ const { async_id_symbol } = require('internal/async_hooks').symbols;
 const {
   codes: {
     ERR_INVALID_ARG_TYPE,
-    ERR_INVALID_OPT_VALUE,
     ERR_OUT_OF_RANGE,
   },
 } = require('internal/errors');
 const { once } = require('internal/util');
-const { validateNumber } = require('internal/validators');
+const { validateNumber, validateOneOf } = require('internal/validators');
 
 const kOnKeylog = Symbol('onkeylog');
 const kRequestOptions = Symbol('requestOptions');
@@ -99,9 +98,7 @@ function Agent(options) {
   this.maxTotalSockets = this.options.maxTotalSockets;
   this.totalSocketCount = 0;
 
-  if (this.scheduling !== 'fifo' && this.scheduling !== 'lifo') {
-    throw new ERR_INVALID_OPT_VALUE('scheduling', this.scheduling);
-  }
+  validateOneOf(this.scheduling, 'scheduling', ['fifo', 'lifo'], true);
 
   if (this.maxTotalSockets !== undefined) {
     validateNumber(this.maxTotalSockets, 'maxTotalSockets');

diff --git a/lib/dns.js b/lib/dns.js
@@ -49,6 +49,7 @@ const {
 const {
   validatePort,
   validateString,
+  validateOneOf,
 } = require('internal/validators');
 
 const {
@@ -114,8 +115,7 @@ function lookup(hostname, options, callback) {
     family = options >>> 0;
   }
 
-  if (family !== 0 && family !== 4 && family !== 6)
-    throw new ERR_INVALID_OPT_VALUE('family', family);
+  validateOneOf(family, 'family', [0, 4, 6], true);
 
   if (!hostname) {
     emitInvalidHostnameWarning(hostname);

diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js
@@ -21,7 +21,7 @@ const {
     ERR_MISSING_ARGS
   }
 } = require('internal/errors');
-const { validateString } = require('internal/validators');
+const { validateString, validateOneOf } = require('internal/validators');
 const EventEmitter = require('events');
 const net = require('net');
 const dgram = require('dgram');
@@ -345,13 +345,9 @@ ChildProcess.prototype.spawn = function(options) {
   const ipcFd = stdio.ipcFd;
   stdio = options.stdio = stdio.stdio;
 
-  if (options.serialization !== undefined &&
-      options.serialization !== 'json' &&
-      options.serialization !== 'advanced') {
-    throw new ERR_INVALID_OPT_VALUE('options.serialization',
-                                    options.serialization);
-  }
 
+  validateOneOf(options.serialization, 'options.serialization',
+                [undefined, 'json', 'advanced'], true);
   const serialization = options.serialization || 'json';
 
   if (ipc !== undefined) {

diff --git a/lib/internal/dns/promises.js b/lib/internal/dns/promises.js
@@ -31,7 +31,8 @@ const {
 } = codes;
 const {
   validatePort,
-  validateString
+  validateString,
+  validateOneOf,
 } = require('internal/validators');
 
 function onlookup(err, addresses) {
@@ -116,8 +117,7 @@ function lookup(hostname, options) {
     family = options >>> 0;
   }
 
-  if (family !== 0 && family !== 4 && family !== 6)
-    throw new ERR_INVALID_OPT_VALUE('family', family);
+  validateOneOf(family, 'family', [0, 4, 6], true);
 
   return createLookupPromise(family, hostname, all, hints, verbatim);
 }

diff --git a/lib/internal/validators.js b/lib/internal/validators.js
@@ -13,6 +13,7 @@ const {
     ERR_SOCKET_BAD_PORT,
     ERR_INVALID_ARG_TYPE,
     ERR_INVALID_ARG_VALUE,
+    ERR_INVALID_OPT_VALUE,
     ERR_OUT_OF_RANGE,
     ERR_UNKNOWN_SIGNAL,
     ERR_INVALID_CALLBACK,
@@ -126,6 +127,21 @@ function validateNumber(value, name) {
     throw new ERR_INVALID_ARG_TYPE(name, 'number', value);
 }
 
+const validateOneOf = hideStackFrames((value, name, oneOf, option = false) => {
+  if (!oneOf.includes(value)) {
+    const allowed = oneOf
+      .map((v) => (typeof v === 'string' ? `'${v}'` : String(v)))
+      .join(', ');
+    if (!option) {
+      const reason = 'must be one of: ' + allowed;
+      throw new ERR_INVALID_ARG_VALUE(name, value, reason);
+    } else {
+      const reason = 'Must be one of: ' + allowed;
+      throw new ERR_INVALID_OPT_VALUE(name, value, reason);
+    }
+  }
+});
+
 function validateBoolean(value, name) {
   if (typeof value !== 'boolean')
     throw new ERR_INVALID_ARG_TYPE(name, 'boolean', value);
@@ -212,6 +228,7 @@ module.exports = {
   validateInteger,
   validateNumber,
   validateObject,
+  validateOneOf,
   validatePort,
   validateSignalName,
   validateString,

diff --git a/lib/vm.js b/lib/vm.js
@@ -39,7 +39,6 @@ const {
 const {
   ERR_CONTEXT_NOT_INITIALIZED,
   ERR_INVALID_ARG_TYPE,
-  ERR_INVALID_ARG_VALUE,
 } = require('internal/errors').codes;
 const {
   isArrayBufferView,
@@ -52,6 +51,7 @@ const {
   validateBoolean,
   validateBuffer,
   validateObject,
+  validateOneOf,
 } = require('internal/validators');
 const {
   kVmBreakFirstLineSymbol,
@@ -246,17 +246,11 @@ function createContext(contextObject = {}, options = {}) {
 
   let microtaskQueue = null;
   if (microtaskMode !== undefined) {
-    validateString(microtaskMode, 'options.microtaskMode');
+    validateOneOf(microtaskMode, 'options.microtaskMode',
+                  ['afterEvaluate', undefined]);
 
-    if (microtaskMode === 'afterEvaluate') {
+    if (microtaskMode === 'afterEvaluate')
       microtaskQueue = new MicrotaskQueue();
-    } else {
-      throw new ERR_INVALID_ARG_VALUE(
-        'options.microtaskQueue',
-        microtaskQueue,
-        'must be \'afterEvaluate\' or undefined'
-      );
-    }
   }
 
   makeContext(contextObject, name, origin, strings, wasm, microtaskQueue);
@@ -395,16 +389,8 @@ function measureMemory(options = {}) {
   emitExperimentalWarning('vm.measureMemory');
   validateObject(options, 'options');
   const { mode = 'summary', execution = 'default' } = options;
-  if (mode !== 'summary' && mode !== 'detailed') {
-    throw new ERR_INVALID_ARG_VALUE(
-      'options.mode', options.mode,
-      'must be either \'summary\' or \'detailed\'');
-  }
-  if (execution !== 'default' && execution !== 'eager') {
-    throw new ERR_INVALID_ARG_VALUE(
-      'options.execution', options.execution,
-      'must be either \'default\' or \'eager\'');
-  }
+  validateOneOf(mode, 'options.mode', ['summary', 'detailed']);
+  validateOneOf(execution, 'options.execution', ['default', 'eager']);
   const result = _measureMemory(measureMemoryModes[mode],
                                 measureMemoryExecutions[execution]);
   if (result === undefined) {

diff --git a/test/parallel/test-child-process-advanced-serialization.js b/test/parallel/test-child-process-advanced-serialization.js
@@ -11,7 +11,8 @@ if (process.argv[2] !== 'child') {
     }, {
       code: 'ERR_INVALID_OPT_VALUE',
       message: `The value "${value}" is invalid ` +
-        'for option "options.serialization"'
+      'for option "options.serialization". ' +
+      "Must be one of: undefined, 'json', 'advanced'"
     });
   }
 

diff --git a/test/parallel/test-dns-lookup.js b/test/parallel/test-dns-lookup.js
@@ -72,7 +72,8 @@ assert.throws(() => {
   const err = {
     code: 'ERR_INVALID_OPT_VALUE',
     name: 'TypeError',
-    message: 'The value "20" is invalid for option "family"'
+    message: 'The value "20" is invalid for option "family". ' +
+    'Must be one of: 0, 4, 6'
   };
   const options = {
     hints: 0,

diff --git a/test/parallel/test-http-agent-scheduling.js b/test/parallel/test-http-agent-scheduling.js
@@ -137,7 +137,8 @@ function badSchedulingOptionTest() {
     assert.strictEqual(err.code, 'ERR_INVALID_OPT_VALUE');
     assert.strictEqual(
       err.message,
-      'The value "filo" is invalid for option "scheduling"'
+      'The value "filo" is invalid for option "scheduling". ' +
+      "Must be one of: 'fifo', 'lifo'"
     );
   }
 }

diff --git a/test/parallel/test-internal-validators-validateoneof.js b/test/parallel/test-internal-validators-validateoneof.js
@@ -0,0 +1,69 @@
+// Flags: --expose-internals
+'use strict';
+
+require('../common');
+const assert = require('assert');
+const { validateOneOf } = require('internal/validators');
+
+{
+  // validateOneOf number incorrect.
+  const allowed = [2, 3];
+  assert.throws(() => validateOneOf(1, 'name', allowed), {
+    code: 'ERR_INVALID_ARG_VALUE',
+    // eslint-disable-next-line quotes
+    message: `The argument 'name' must be one of: 2, 3. Received 1`
+  });
+  assert.throws(() => validateOneOf(1, 'name', allowed, true), {
+    code: 'ERR_INVALID_OPT_VALUE',
+    message: 'The value "1" is invalid for option "name". ' +
+      'Must be one of: 2, 3'
+  });
+}
+
+{
+  // validateOneOf number correct.
+  validateOneOf(2, 'name', [1, 2]);
+}
+
+{
+  // validateOneOf string incorrect.
+  const allowed = ['b', 'c'];
+  assert.throws(() => validateOneOf('a', 'name', allowed), {
+    code: 'ERR_INVALID_ARG_VALUE',
+    // eslint-disable-next-line quotes
+    message: `The argument 'name' must be one of: 'b', 'c'. Received 'a'`
+  });
+  assert.throws(() => validateOneOf('a', 'name', allowed, true), {
+    code: 'ERR_INVALID_OPT_VALUE',
+    // eslint-disable-next-line quotes
+    message: `The value "a" is invalid for option "name". ` +
+    "Must be one of: 'b', 'c'",
+  });
+}
+
+{
+  // validateOneOf string correct.
+  validateOneOf('two', 'name', ['one', 'two']);
+}
+
+{
+  // validateOneOf Symbol incorrect.
+  const allowed = [Symbol.for('b'), Symbol.for('c')];
+  assert.throws(() => validateOneOf(Symbol.for('a'), 'name', allowed), {
+    code: 'ERR_INVALID_ARG_VALUE',
+    // eslint-disable-next-line quotes
+    message: `The argument 'name' must be one of: Symbol(b), Symbol(c). ` +
+      'Received Symbol(a)'
+  });
+  assert.throws(() => validateOneOf(Symbol.for('a'), 'name', allowed, true), {
+    code: 'ERR_INVALID_OPT_VALUE',
+    message: 'The value "Symbol(a)" is invalid for option "name". ' +
+      'Must be one of: Symbol(b), Symbol(c)',
+  });
+}
+
+{
+  // validateOneOf Symbol correct.
+  const allowed = [Symbol.for('b'), Symbol.for('c')];
+  validateOneOf(Symbol.for('b'), 'name', allowed);
+}