From 3b3a70e26d60a8496cf13aa5b686d84626b08480 Mon Sep 17 00:00:00 2001 From: Denys Otrishko Date: Fri, 26 Jun 2020 13:14:48 +0300 Subject: [PATCH] lib: allow to validate enums with validateOneOf MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/34070 Reviewed-By: Zeyu Yang Reviewed-By: Tobias Nießen Reviewed-By: James M Snell --- lib/_http_agent.js | 7 +- lib/dns.js | 4 +- lib/internal/child_process.js | 10 +-- lib/internal/dns/promises.js | 6 +- lib/internal/validators.js | 17 +++++ lib/vm.js | 26 ++----- ...st-child-process-advanced-serialization.js | 3 +- test/parallel/test-dns-lookup.js | 3 +- test/parallel/test-http-agent-scheduling.js | 3 +- .../test-internal-validators-validateoneof.js | 69 +++++++++++++++++++ 10 files changed, 108 insertions(+), 40 deletions(-) create mode 100644 test/parallel/test-internal-validators-validateoneof.js diff --git a/lib/_http_agent.js b/lib/_http_agent.js index c4430e86ec3aca..fc20260baf642c 100644 --- 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 index 92b70c73effbf2..0393069f811711 100644 --- 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 index 1295950f5d51ae..b068a740817c60 100644 --- 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 index 16cd97e06ac5ca..19ad6e5bfb3a6b 100644 --- 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 index deab53d4b8221e..71726f700518e3 100644 --- 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 index ca7bdb2259dd8e..45a2edf0bb20b3 100644 --- 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 index 0303fc719d331c..d75f0b59989bc6 100644 --- 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 index 80e13d308d3217..0cc6d4240f5253 100644 --- 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 index bcf07863b0fb61..7214ef5f877d7e 100644 --- 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 new file mode 100644 index 00000000000000..38a79a05151241 --- /dev/null +++ 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); +}