Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

lib: refactor to use validateObject #37028

Merged
merged 1 commit into from Jan 28, 2021
Merged

lib: refactor to use validateObject #37028

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 5 additions & 7 deletions lib/_tls_wrap.js
View file
Open in desktop
Expand Up @@ -84,9 +84,10 @@ const {
getAllowUnauthorized,
} = require('internal/options');
const {
validateBuffer,
validateCallback,
validateObject,
validateString,
validateBuffer,
validateUint32
} = require('internal/validators');
const traceTls = getOptionValue('--trace-tls');
Expand Down Expand Up @@ -364,8 +365,7 @@ function onPskClientCallback(hint, maxPskLen, maxIdentityLen) {
if (ret == null)
return undefined;

if (typeof ret !== 'object')
throw new ERR_INVALID_ARG_TYPE('ret', 'Object', ret);
validateObject(ret, 'ret');

validateBuffer(ret.psk, 'psk');
if (ret.psk.length > maxPskLen) {
Expand Down Expand Up @@ -823,8 +823,7 @@ TLSSocket.prototype._init = function(socket, wrap) {
};

TLSSocket.prototype.renegotiate = function(options, callback) {
if (options === null || typeof options !== 'object')
throw new ERR_INVALID_ARG_TYPE('options', 'Object', options);
validateObject(options, 'options');
if (callback !== undefined) {
validateCallback(callback);
}
Expand Down Expand Up @@ -1237,8 +1236,7 @@ exports.createServer = function createServer(options, listener) {


Server.prototype.setSecureContext = function(options) {
if (options === null || typeof options !== 'object')
throw new ERR_INVALID_ARG_TYPE('options', 'Object', options);
validateObject(options, 'options');

if (options.pfx)
this.pfx = options.pfx;
Expand Down
7 changes: 4 additions & 3 deletions lib/child_process.js
View file
Open in desktop
Expand Up @@ -72,9 +72,10 @@ const {
} = errorCodes;
const { clearTimeout, setTimeout } = require('timers');
const {
validateString,
isInt32,
validateAbortSignal,
validateObject,
validateString,
} = require('internal/validators');
const child_process = require('internal/child_process');
const {
Expand Down Expand Up @@ -452,8 +453,8 @@ function normalizeSpawnArguments(file, args, options) {

if (options === undefined)
options = {};
else if (options === null || typeof options !== 'object')
throw new ERR_INVALID_ARG_TYPE('options', 'object', options);
else
validateObject(options, 'options');

// Validate the cwd, if present.
if (options.cwd != null) {
Expand Down
6 changes: 3 additions & 3 deletions lib/inspector.js
View file
Open in desktop
Expand Up @@ -16,7 +16,6 @@ const {
ERR_INSPECTOR_NOT_CONNECTED,
ERR_INSPECTOR_NOT_ACTIVE,
ERR_INSPECTOR_NOT_WORKER,
ERR_INVALID_ARG_TYPE,
} = require('internal/errors').codes;

const { hasInspector } = internalBinding('config');
Expand All @@ -27,6 +26,7 @@ const EventEmitter = require('events');
const { queueMicrotask } = require('internal/process/task_queues');
const {
validateCallback,
validateObject,
validateString,
} = require('internal/validators');
const { isMainThread } = require('worker_threads');
Expand Down Expand Up @@ -99,8 +99,8 @@ class Session extends EventEmitter {
callback = params;
params = null;
}
if (params && typeof params !== 'object') {
throw new ERR_INVALID_ARG_TYPE('params', 'Object', params);
if (params) {
validateObject(params, 'params');
}
if (callback) {
validateCallback(callback);
Expand Down
10 changes: 4 additions & 6 deletions lib/internal/assert/assertion_error.js
View file
Open in desktop
Expand Up @@ -18,12 +18,12 @@ const {
} = primordials;

const { inspect } = require('internal/util/inspect');
const { codes: {
ERR_INVALID_ARG_TYPE
} } = require('internal/errors');
const {
removeColors,
} = require('internal/util');
const {
validateObject,
} = require('internal/validators');

let blue = '';
let green = '';
Expand Down Expand Up @@ -315,9 +315,7 @@ function createErrDiff(actual, expected, operator) {

class AssertionError extends Error {
constructor(options) {
if (typeof options !== 'object' || options === null) {
throw new ERR_INVALID_ARG_TYPE('options', 'Object', options);
}
validateObject(options, 'options');
const {
message,
operator,
Expand Down
10 changes: 4 additions & 6 deletions lib/internal/child_process.js
View file
Open in desktop
Expand Up @@ -31,6 +31,7 @@ const {
} = require('internal/errors');
const {
validateArray,
validateObject,
validateOneOf,
validateString,
} = require('internal/validators');
Expand Down Expand Up @@ -345,9 +346,7 @@ function closePendingHandle(target) {
ChildProcess.prototype.spawn = function(options) {
let i = 0;

if (options === null || typeof options !== 'object') {
throw new ERR_INVALID_ARG_TYPE('options', 'Object', options);
}
validateObject(options, 'options');

// If no `stdio` option was given - use default
let stdio = options.stdio || 'pipe';
Expand Down Expand Up @@ -713,9 +712,8 @@ function setupChannel(target, channel, serializationMode) {
} else if (typeof options === 'function') {
callback = options;
options = undefined;
} else if (options !== undefined &&
(options === null || typeof options !== 'object')) {
throw new ERR_INVALID_ARG_TYPE('options', 'Object', options);
} else if (options !== undefined) {
validateObject(options, 'options');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, this change broke cheerio through the workerpool module: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/2604/nodes=fedora-last-latest-x64/testReport/junit/(root)/citgm/cheerio_v1_0_0_rc_5/

See https://github.com/josdejong/workerpool/blob/b0e72695fc26df60e224c2d2eafc402f3097a6f4/src/WorkerHandler.js#L262

It would probably be nice to open a PR to fix that module. It seems to have been working by accident until now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in josdejong/workerpool#230.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably also do #37047.

}

options = { swallowErrors: false, ...options };
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/fs/utils.js
View file
Open in desktop
Expand Up @@ -48,6 +48,7 @@ const {
validateBoolean,
validateInt32,
validateInteger,
validateObject,
validateUint32,
} = require('internal/validators');
const pathModule = require('path');
Expand Down Expand Up @@ -762,8 +763,7 @@ const validateRmdirOptions = hideStackFrames(
(options, defaults = defaultRmdirOptions) => {
if (options === undefined)
return defaults;
if (options === null || typeof options !== 'object')
throw new ERR_INVALID_ARG_TYPE('options', 'object', options);
validateObject(options, 'options');

options = { ...defaults, ...options };

Expand Down
8 changes: 5 additions & 3 deletions lib/internal/process/per_thread.js
View file
Open in desktop
Expand Up @@ -35,7 +35,10 @@ const {
}
} = require('internal/errors');
const format = require('internal/util/inspect').format;
const { validateArray } = require('internal/validators');
const {
validateArray,
validateObject,
} = require('internal/validators');
const constants = internalBinding('constants').os.signals;

function assert(x, msg) {
Expand Down Expand Up @@ -109,8 +112,7 @@ function wrapProcessMethods(binding) {
// If a previous value was passed in, ensure it has the correct shape.
if (prevValue) {
if (!previousValueIsValid(prevValue.user)) {
if (typeof prevValue !== 'object')
throw new ERR_INVALID_ARG_TYPE('prevValue', 'object', prevValue);
validateObject(prevValue, 'prevValue');

if (typeof prevValue.user !== 'number') {
throw new ERR_INVALID_ARG_TYPE('prevValue.user',
Expand Down
11 changes: 6 additions & 5 deletions lib/internal/process/report.js
View file
Open in desktop
Expand Up @@ -4,9 +4,10 @@ const {
ERR_SYNTHETIC
} = require('internal/errors').codes;
const {
validateBoolean,
validateObject,
validateSignalName,
validateString,
validateBoolean,
} = require('internal/validators');
const nr = internalBinding('report');
const {
Expand All @@ -21,17 +22,17 @@ const report = {
throw new ERR_INVALID_ARG_TYPE('file', 'String', file);
} else if (err === undefined) {
err = new ERR_SYNTHETIC();
} else if (err === null || typeof err !== 'object') {
throw new ERR_INVALID_ARG_TYPE('err', 'Object', err);
} else {
validateObject(err, 'err');
}

return nr.writeReport('JavaScript API', 'API', file, err);
},
getReport(err) {
if (err === undefined)
err = new ERR_SYNTHETIC();
else if (err === null || typeof err !== 'object')
throw new ERR_INVALID_ARG_TYPE('err', 'Object', err);
else
validateObject(err, 'err');

return JSONParse(nr.getReport(err));
},
Expand Down
6 changes: 3 additions & 3 deletions lib/internal/streams/end-of-stream.js
View file
Open in desktop
Expand Up @@ -4,12 +4,12 @@
'use strict';

const {
ERR_INVALID_ARG_TYPE,
ERR_STREAM_PREMATURE_CLOSE
} = require('internal/errors').codes;
const { once } = require('internal/util');
const {
validateFunction,
validateObject,
} = require('internal/validators');

function isSocket(stream) {
Expand Down Expand Up @@ -68,8 +68,8 @@ function eos(stream, options, callback) {
options = {};
} else if (options == null) {
options = {};
} else if (typeof options !== 'object') {
throw new ERR_INVALID_ARG_TYPE('options', 'object', options);
} else {
validateObject(options, 'options');
}
validateFunction(callback, 'callback');

Expand Down
10 changes: 7 additions & 3 deletions lib/internal/url.js
View file
Open in desktop
Expand Up @@ -64,7 +64,10 @@ const {
} = require('internal/constants');
const path = require('path');

const { validateCallback } = require('internal/validators');
const {
validateCallback,
validateObject,
} = require('internal/validators');

// Lazy loaded for startup performance.
let querystring;
Expand Down Expand Up @@ -413,8 +416,9 @@ ObjectDefineProperties(URL.prototype, {
configurable: false,
// eslint-disable-next-line func-name-matching
value: function format(options) {
if (options && typeof options !== 'object')
throw new ERR_INVALID_ARG_TYPE('options', 'Object', options);
if (options)
validateObject(options, 'options');

options = {
fragment: true,
unicode: false,
Expand Down
7 changes: 4 additions & 3 deletions lib/internal/util/inspect.js
View file
Open in desktop
Expand Up @@ -135,6 +135,9 @@ const {
const assert = require('internal/assert');

const { NativeModule } = require('internal/bootstrap/loaders');
const {
validateObject,
} = require('internal/validators');

let hexSlice;

Expand Down Expand Up @@ -338,9 +341,7 @@ ObjectDefineProperty(inspect, 'defaultOptions', {
return inspectDefaultOptions;
},
set(options) {
if (options === null || typeof options !== 'object') {
throw new ERR_INVALID_ARG_TYPE('options', 'Object', options);
}
validateObject(options, 'options');
return ObjectAssign(inspectDefaultOptions, options);
}
});
Expand Down
18 changes: 5 additions & 13 deletions lib/internal/vm/module.js
View file
Open in desktop
Expand Up @@ -42,6 +42,7 @@ const {
validateBoolean,
validateFunction,
validateInt32,
validateObject,
validateUint32,
validateString,
} = require('internal/validators');
Expand Down Expand Up @@ -92,9 +93,7 @@ class Module {
} = options;

if (context !== undefined) {
if (typeof context !== 'object' || context === null) {
throw new ERR_INVALID_ARG_TYPE('options.context', 'Object', context);
}
validateObject(context, 'context');
if (!isContext(context)) {
throw new ERR_INVALID_ARG_TYPE('options.context', 'vm.Context',
context);
Expand Down Expand Up @@ -204,9 +203,7 @@ class Module {
throw new ERR_VM_MODULE_NOT_MODULE();
}

if (typeof options !== 'object' || options === null) {
throw new ERR_INVALID_ARG_TYPE('options', 'Object', options);
}
validateObject(options, 'options');

let timeout = options.timeout;
if (timeout === undefined) {
Expand Down Expand Up @@ -261,10 +258,7 @@ class SourceTextModule extends Module {

constructor(sourceText, options = {}) {
validateString(sourceText, 'sourceText');

if (typeof options !== 'object' || options === null) {
throw new ERR_INVALID_ARG_TYPE('options', 'Object', options);
}
validateObject(options, 'options');

const {
lineOffset = 0,
Expand Down Expand Up @@ -408,9 +402,7 @@ class SyntheticModule extends Module {
}
validateFunction(evaluateCallback, 'evaluateCallback');

if (typeof options !== 'object' || options === null) {
throw new ERR_INVALID_ARG_TYPE('options', 'Object', options);
}
validateObject(options, 'options');

const { context, identifier } = options;

Expand Down
10 changes: 5 additions & 5 deletions lib/path.js
View file
Open in desktop
Expand Up @@ -28,7 +28,6 @@ const {
StringPrototypeSlice,
StringPrototypeToLowerCase,
} = primordials;
const { ERR_INVALID_ARG_TYPE } = require('internal/errors').codes;
const {
CHAR_UPPERCASE_A,
CHAR_LOWERCASE_A,
Expand All @@ -40,7 +39,10 @@ const {
CHAR_COLON,
CHAR_QUESTION_MARK,
} = require('internal/constants');
const { validateString } = require('internal/validators');
const {
validateObject,
validateString,
} = require('internal/validators');

function isPathSeparator(code) {
return code === CHAR_FORWARD_SLASH || code === CHAR_BACKWARD_SLASH;
Expand Down Expand Up @@ -121,9 +123,7 @@ function normalizeString(path, allowAboveRoot, separator, isPathSeparator) {
}

function _format(sep, pathObject) {
if (pathObject === null || typeof pathObject !== 'object') {
throw new ERR_INVALID_ARG_TYPE('pathObject', 'Object', pathObject);
}
validateObject(pathObject, 'pathObject');
const dir = pathObject.dir || pathObject.root;
const base = pathObject.base ||
`${pathObject.name || ''}${pathObject.ext || ''}`;
Expand Down