From 48d020cd012bea77077db3fc392db14e56add37f Mon Sep 17 00:00:00 2001 From: Rongjian Zhang Date: Sun, 16 May 2021 19:22:48 +0800 Subject: [PATCH 1/2] lib: refactor to reuse validators --- lib/_http_agent.js | 12 ++++++------ lib/_tls_wrap.js | 28 ++++++++-------------------- lib/events.js | 11 +++-------- lib/internal/perf/timerify.js | 6 +++--- lib/internal/process/per_thread.js | 11 +++-------- lib/util.js | 13 ++++++------- lib/vm.js | 21 ++++++++------------- lib/wasi.js | 6 ++---- lib/zlib.js | 6 ++---- 9 files changed, 41 insertions(+), 73 deletions(-) diff --git a/lib/_http_agent.js b/lib/_http_agent.js index 8bc74c9cd43327..70ba3034876dae 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -51,12 +51,15 @@ const { AsyncResource } = require('async_hooks'); const { async_id_symbol } = require('internal/async_hooks').symbols; const { codes: { - ERR_INVALID_ARG_TYPE, ERR_OUT_OF_RANGE, }, } = require('internal/errors'); const { once } = require('internal/util'); -const { validateNumber, validateOneOf } = require('internal/validators'); +const { + validateNumber, + validateOneOf, + validateString +} = require('internal/validators'); const kOnKeylog = Symbol('onkeylog'); const kRequestOptions = Symbol('requestOptions'); @@ -344,10 +347,7 @@ function calculateServerName(options, req) { let servername = options.host; const hostHeader = req.getHeader('host'); if (hostHeader) { - if (typeof hostHeader !== 'string') { - throw new ERR_INVALID_ARG_TYPE('options.headers.host', - 'String', hostHeader); - } + validateString(hostHeader, 'options.headers.host'); // abc => abc // abc:123 => abc diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index 66ebc7b77869f7..271bcf0dfea332 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -84,9 +84,12 @@ const { getAllowUnauthorized, } = require('internal/options'); const { + validateBoolean, validateBuffer, validateCallback, + validateFunction, validateInt32, + validateNumber, validateObject, validateString, validateUint32, @@ -468,10 +471,8 @@ function TLSSocket(socket, opts) { process.emitWarning('Enabling --trace-tls can expose sensitive data in ' + 'the resulting log.'); } - } else if (typeof enableTrace !== 'boolean') { - throw new ERR_INVALID_ARG_TYPE( - 'options.enableTrace', 'boolean', enableTrace); - } + } else + validateBoolean(enableTrace, 'options.enableTrace'); if (tlsOptions.ALPNProtocols) tls.convertALPNProtocols(tlsOptions.ALPNProtocols, tlsOptions); @@ -783,11 +784,7 @@ TLSSocket.prototype._init = function(socket, wrap) { } if (options.pskCallback && ssl.enablePskCallback) { - if (typeof options.pskCallback !== 'function') { - throw new ERR_INVALID_ARG_TYPE('pskCallback', - 'function', - options.pskCallback); - } + validateFunction(options.pskCallback, 'pskCallback'); ssl[kOnPskExchange] = options.isServer ? onPskServerCallback : onPskClientCallback; @@ -796,13 +793,7 @@ TLSSocket.prototype._init = function(socket, wrap) { ssl.enablePskCallback(); if (options.pskIdentityHint) { - if (typeof options.pskIdentityHint !== 'string') { - throw new ERR_INVALID_ARG_TYPE( - 'options.pskIdentityHint', - 'string', - options.pskIdentityHint - ); - } + validateString(options.pskIdentityHint, 'options.pskIdentityHint'); ssl.setPskIdentityHint(options.pskIdentityHint); } } @@ -1215,10 +1206,7 @@ function Server(options, listener) { this[kPskCallback] = options.pskCallback; this[kPskIdentityHint] = options.pskIdentityHint; - if (typeof this[kHandshakeTimeout] !== 'number') { - throw new ERR_INVALID_ARG_TYPE( - 'options.handshakeTimeout', 'number', options.handshakeTimeout); - } + validateNumber(this[kHandshakeTimeout], 'options.handshakeTimeout'); if (this[kSNICallback] && typeof this[kSNICallback] !== 'function') { throw new ERR_INVALID_ARG_TYPE( diff --git a/lib/events.js b/lib/events.js index cc29b3358e0ce2..441b3ac4e4f984 100644 --- a/lib/events.js +++ b/lib/events.js @@ -63,6 +63,7 @@ const { const { validateAbortSignal, validateFunction, + validateBoolean } = require('internal/validators'); const kCapture = Symbol('kCapture'); @@ -89,10 +90,7 @@ ObjectDefineProperty(EventEmitter, 'captureRejections', { return EventEmitter.prototype[kCapture]; }, set(value) { - if (typeof value !== 'boolean') { - throw new ERR_INVALID_ARG_TYPE('EventEmitter.captureRejections', - 'boolean', value); - } + validateBoolean(value, 'EventEmitter.captureRejections'); EventEmitter.prototype[kCapture] = value; }, @@ -190,10 +188,7 @@ EventEmitter.init = function(opts) { if (opts?.captureRejections) { - if (typeof opts.captureRejections !== 'boolean') { - throw new ERR_INVALID_ARG_TYPE('options.captureRejections', - 'boolean', opts.captureRejections); - } + validateBoolean(opts.captureRejections, 'options.captureRejections'); this[kCapture] = Boolean(opts.captureRejections); } else { // Assigning the kCapture property directly saves an expensive diff --git a/lib/internal/perf/timerify.js b/lib/internal/perf/timerify.js index 1108f5efb91392..d730f62aae7eb1 100644 --- a/lib/internal/perf/timerify.js +++ b/lib/internal/perf/timerify.js @@ -15,7 +15,8 @@ const { } = require('internal/perf/perf'); const { - validateObject + validateFunction, + validateObject, } = require('internal/validators'); const { @@ -57,8 +58,7 @@ function processComplete(name, start, args, histogram) { } function timerify(fn, options = {}) { - if (typeof fn !== 'function') - throw new ERR_INVALID_ARG_TYPE('fn', 'function', fn); + validateFunction(fn, 'fn'); validateObject(options, 'options'); const { diff --git a/lib/internal/process/per_thread.js b/lib/internal/process/per_thread.js index e85029735a2c42..8f35051041c9a3 100644 --- a/lib/internal/process/per_thread.js +++ b/lib/internal/process/per_thread.js @@ -43,6 +43,7 @@ const { const format = require('internal/util/inspect').format; const { validateArray, + validateNumber, validateObject, } = require('internal/validators'); const constants = internalBinding('constants').os.signals; @@ -122,19 +123,13 @@ function wrapProcessMethods(binding) { if (!previousValueIsValid(prevValue.user)) { validateObject(prevValue, 'prevValue'); - if (typeof prevValue.user !== 'number') { - throw new ERR_INVALID_ARG_TYPE('prevValue.user', - 'number', prevValue.user); - } + validateNumber(prevValue.user, 'prevValue.user'); throw new ERR_INVALID_ARG_VALUE.RangeError('prevValue.user', prevValue.user); } if (!previousValueIsValid(prevValue.system)) { - if (typeof prevValue.system !== 'number') { - throw new ERR_INVALID_ARG_TYPE('prevValue.system', - 'number', prevValue.system); - } + validateNumber(prevValue.system, 'prevValue.system'); throw new ERR_INVALID_ARG_VALUE.RangeError('prevValue.system', prevValue.system); } diff --git a/lib/util.js b/lib/util.js index bcb1c81933c0c0..a8d9a356480a35 100644 --- a/lib/util.js +++ b/lib/util.js @@ -60,7 +60,10 @@ const { inspect } = require('internal/util/inspect'); const { debuglog } = require('internal/util/debuglog'); -const { validateNumber } = require('internal/validators'); +const { + validateFunction, + validateNumber, +} = require('internal/validators'); const { TextDecoder, TextEncoder } = require('internal/encoding'); const { isBuffer } = require('buffer').Buffer; const types = require('internal/util/types'); @@ -285,18 +288,14 @@ const callbackifyOnRejected = hideStackFrames((reason, cb) => { * } */ function callbackify(original) { - if (typeof original !== 'function') { - throw new ERR_INVALID_ARG_TYPE('original', 'Function', original); - } + validateFunction(original, 'original'); // We DO NOT return the promise as it gives the user a false sense that // the promise is actually somehow related to the callback's execution // and that the callback throwing will reject the promise. function callbackified(...args) { const maybeCb = ArrayPrototypePop(args); - if (typeof maybeCb !== 'function') { - throw new ERR_INVALID_ARG_TYPE('last argument', 'Function', maybeCb); - } + validateFunction(maybeCb, 'last argument'); const cb = FunctionPrototypeBind(maybeCb, this); // In true node style we process the callback on `nextTick` with all the // implications (stack, `uncaughtException`, `async_hooks`) diff --git a/lib/vm.js b/lib/vm.js index c873842c434c30..5c39429b3d1d5f 100644 --- a/lib/vm.js +++ b/lib/vm.js @@ -46,14 +46,15 @@ const { isArrayBufferView, } = require('internal/util/types'); const { - validateInt32, - validateUint32, - validateString, validateArray, validateBoolean, validateBuffer, + validateFunction, + validateInt32, validateObject, validateOneOf, + validateString, + validateUint32, } = require('internal/validators'); const { kVmBreakFirstLineSymbol, @@ -108,11 +109,8 @@ class Script extends ContextifyScript { } if (importModuleDynamically !== undefined) { - if (typeof importModuleDynamically !== 'function') { - throw new ERR_INVALID_ARG_TYPE('options.importModuleDynamically', - 'function', - importModuleDynamically); - } + validateFunction(importModuleDynamically, + 'options.importModuleDynamically'); const { importModuleDynamicallyWrap } = require('internal/vm/module'); const { callbackMap } = internalBinding('module_wrap'); @@ -373,11 +371,8 @@ function compileFunction(code, params, options = {}) { } if (importModuleDynamically !== undefined) { - if (typeof importModuleDynamically !== 'function') { - throw new ERR_INVALID_ARG_TYPE('options.importModuleDynamically', - 'function', - importModuleDynamically); - } + validateFunction(importModuleDynamically, + 'options.importModuleDynamically'); const { importModuleDynamicallyWrap } = require('internal/vm/module'); const { callbackMap } = internalBinding('module_wrap'); diff --git a/lib/wasi.js b/lib/wasi.js index b3c14fc2f14840..63209ce716cb9a 100644 --- a/lib/wasi.js +++ b/lib/wasi.js @@ -18,6 +18,7 @@ const { isArrayBuffer } = require('internal/util/types'); const { validateArray, validateBoolean, + validateFunction, validateInt32, validateObject, } = require('internal/validators'); @@ -118,10 +119,7 @@ class WASI { const { _start, _initialize } = this[kInstance].exports; - if (typeof _start !== 'function') { - throw new ERR_INVALID_ARG_TYPE( - 'instance.exports._start', 'function', _start); - } + validateFunction(_start, 'instance.exports._start'); if (_initialize !== undefined) { throw new ERR_INVALID_ARG_TYPE( 'instance.exports._initialize', 'undefined', _initialize); diff --git a/lib/zlib.js b/lib/zlib.js index c0c7f601758224..83116fd259c755 100644 --- a/lib/zlib.js +++ b/lib/zlib.js @@ -71,6 +71,7 @@ const { const { owner_symbol } = require('internal/async_hooks').symbols; const { validateFunction, + validateNumber, } = require('internal/validators'); const kFlushFlag = Symbol('kFlushFlag'); @@ -212,10 +213,7 @@ const checkFiniteNumber = hideStackFrames((number, name) => { return false; } - // Other non-numbers - if (typeof number !== 'number') { - throw new ERR_INVALID_ARG_TYPE(name, 'number', number); - } + validateNumber(number, name); // Infinite numbers throw new ERR_OUT_OF_RANGE(name, 'a finite number', number); From 9b29dccbdaba3fd5ad85c1f39ba0cad5a36c05c1 Mon Sep 17 00:00:00 2001 From: Rongjian Zhang Date: Sun, 16 May 2021 23:40:04 +0800 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Antoine du Hamel --- lib/_http_agent.js | 2 +- lib/_tls_wrap.js | 3 ++- lib/events.js | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/_http_agent.js b/lib/_http_agent.js index 70ba3034876dae..1e372f417ee5b2 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -58,7 +58,7 @@ const { once } = require('internal/util'); const { validateNumber, validateOneOf, - validateString + validateString, } = require('internal/validators'); const kOnKeylog = Symbol('onkeylog'); diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index 271bcf0dfea332..f596614af5fef9 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -471,8 +471,9 @@ function TLSSocket(socket, opts) { process.emitWarning('Enabling --trace-tls can expose sensitive data in ' + 'the resulting log.'); } - } else + } else { validateBoolean(enableTrace, 'options.enableTrace'); + } if (tlsOptions.ALPNProtocols) tls.convertALPNProtocols(tlsOptions.ALPNProtocols, tlsOptions); diff --git a/lib/events.js b/lib/events.js index 441b3ac4e4f984..aa1ae2165d095e 100644 --- a/lib/events.js +++ b/lib/events.js @@ -62,8 +62,8 @@ const { const { validateAbortSignal, + validateBoolean, validateFunction, - validateBoolean } = require('internal/validators'); const kCapture = Symbol('kCapture');