Skip to content

Commit

Permalink
lib: introduce internal/validators
Browse files Browse the repository at this point in the history 
Create a file to centralize argument validators that are used in
multiple internal modules.
Move validateInt32 and validateUint32 to this file.

PR-URL: #19973
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
@targos @BridgeAR
targos authored and BridgeAR committed Apr 26, 2018
1 parent d5e363b commit e836128
Show file tree
Hide file tree
Showing 8 changed files with 112 additions and 99 deletions.
16 changes: 9 additions & 7 deletions lib/fs.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ const internalUtil = require('internal/util');
const {
copyObject,
getOptions,
isUint32,
modeNum,
nullCheck,
preprocessSymlinkDestination,
Expand All @@ -61,16 +60,19 @@ const {
stringToSymlinkType,
toUnixTimestamp,
validateBuffer,
validateLen,
validateOffsetLengthRead,
validateOffsetLengthWrite,
validatePath,
validateUint32
validatePath
} = internalFS;
const {
CHAR_FORWARD_SLASH,
CHAR_BACKWARD_SLASH,
} = require('internal/constants');
const {
isUint32,
validateInt32,
validateUint32
} = require('internal/validators');

Object.defineProperty(exports, 'constants', {
configurable: false,
Expand Down Expand Up @@ -755,8 +757,8 @@ fs.ftruncate = function(fd, len = 0, callback) {
// TODO(BridgeAR): This does not seem right.
// There does not seem to be any validation before and if there is any, it
// should work similar to validateUint32 or not have a upper cap at all.
// This applies to all usage of `validateLen`.
validateLen(len);
// This applies to all usage of `validateInt32(len, 'len')`.
validateInt32(len, 'len');
len = Math.max(0, len);
const req = new FSReqWrap();
req.oncomplete = makeCallback(callback);
Expand All @@ -765,7 +767,7 @@ fs.ftruncate = function(fd, len = 0, callback) {

fs.ftruncateSync = function(fd, len = 0) {
validateUint32(fd, 'fd');
validateLen(len);
validateInt32(len, 'len');
len = Math.max(0, len);
const ctx = {};
binding.ftruncate(fd, len, undefined, ctx);
Expand Down
12 changes: 7 additions & 5 deletions lib/fs/promises.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,22 @@ const {
copyObject,
getOptions,
getStatsFromBinding,
isUint32,
modeNum,
nullCheck,
preprocessSymlinkDestination,
stringToFlags,
stringToSymlinkType,
toUnixTimestamp,
validateBuffer,
validateLen,
validateOffsetLengthRead,
validateOffsetLengthWrite,
validatePath,
validateUint32
validatePath
} = require('internal/fs');
const {
isUint32,
validateInt32,
validateUint32
} = require('internal/validators');
const pathModule = require('path');

const kHandle = Symbol('handle');
Expand Down Expand Up @@ -275,7 +277,7 @@ async function truncate(path, len = 0) {

async function ftruncate(handle, len = 0) {
validateFileHandle(handle);
validateLen(len);
validateInt32(len, 'len');
len = Math.max(0, len);
return binding.ftruncate(handle.fd, len, kUsePromises);
}
Expand Down
45 changes: 1 addition & 44 deletions lib/internal/fs.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,6 @@ function getOptions(options, defaultOptions) {
return options;
}

function isInt32(n) { return n === (n | 0); }
function isUint32(n) { return n === (n >>> 0); }

function modeNum(m, def) {
if (typeof m === 'number')
return m;
Expand Down Expand Up @@ -341,26 +338,6 @@ function validateBuffer(buffer) {
}
}

function validateLen(len) {
let err;

if (!isInt32(len)) {
if (typeof len !== 'number') {
err = new ERR_INVALID_ARG_TYPE('len', 'number', len);
} else if (!Number.isInteger(len)) {
err = new ERR_OUT_OF_RANGE('len', 'an integer', len);
} else {
// 2 ** 31 === 2147483648
err = new ERR_OUT_OF_RANGE('len', '> -2147483649 && < 2147483648', len);
}
}

if (err !== undefined) {
Error.captureStackTrace(err, validateLen);
throw err;
}
}

function validateOffsetLengthRead(offset, length, bufferLength) {
let err;

Expand Down Expand Up @@ -410,28 +387,10 @@ function validatePath(path, propName = 'path') {
}
}

function validateUint32(value, propName) {
if (!isUint32(value)) {
let err;
if (typeof value !== 'number') {
err = new ERR_INVALID_ARG_TYPE(propName, 'number', value);
} else if (!Number.isInteger(value)) {
err = new ERR_OUT_OF_RANGE(propName, 'an integer', value);
} else {
// 2 ** 32 === 4294967296
err = new ERR_OUT_OF_RANGE(propName, '>= 0 && < 4294967296', value);
}
Error.captureStackTrace(err, validateUint32);
throw err;
}
}

module.exports = {
assertEncoding,
copyObject,
getOptions,
isInt32,
isUint32,
modeNum,
nullCheck,
preprocessSymlinkDestination,
Expand All @@ -443,9 +402,7 @@ module.exports = {
SyncWriteStream,
toUnixTimestamp,
validateBuffer,
validateLen,
validateOffsetLengthRead,
validateOffsetLengthWrite,
validatePath,
validateUint32
validatePath
};
58 changes: 58 additions & 0 deletions lib/internal/validators.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
'use strict';

const {
ERR_INVALID_ARG_TYPE,
ERR_OUT_OF_RANGE
} = require('internal/errors').codes;

function isInt32(value) {
return value === (value | 0);
}

function isUint32(value) {
return value === (value >>> 0);
}

function validateInt32(value, name) {
if (!isInt32(value)) {
let err;
if (typeof value !== 'number') {
err = new ERR_INVALID_ARG_TYPE(name, 'number', value);
} else if (!Number.isInteger(value)) {
err = new ERR_OUT_OF_RANGE(name, 'an integer', value);
} else {
// 2 ** 31 === 2147483648
err = new ERR_OUT_OF_RANGE(name, '> -2147483649 && < 2147483648', value);
}
Error.captureStackTrace(err, validateInt32);
throw err;
}
}

function validateUint32(value, name, positive) {
if (!isUint32(value)) {
let err;
if (typeof value !== 'number') {
err = new ERR_INVALID_ARG_TYPE(name, 'number', value);
} else if (!Number.isInteger(value)) {
err = new ERR_OUT_OF_RANGE(name, 'an integer', value);
} else {
const min = positive ? 1 : 0;
// 2 ** 32 === 4294967296
err = new ERR_OUT_OF_RANGE(name, `>= ${min} && < 4294967296`, value);
}
Error.captureStackTrace(err, validateUint32);
throw err;
} else if (positive && value === 0) {
const err = new ERR_OUT_OF_RANGE(name, '>= 1 && < 4294967296', value);
Error.captureStackTrace(err, validateUint32);
throw err;
}
}

module.exports = {
isInt32,
isUint32,
validateInt32,
validateUint32
};
20 changes: 5 additions & 15 deletions lib/internal/vm/module.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ const { URL } = require('internal/url');
const { isContext } = process.binding('contextify');
const {
ERR_INVALID_ARG_TYPE,
ERR_OUT_OF_RANGE,
ERR_VM_MODULE_ALREADY_LINKED,
ERR_VM_MODULE_DIFFERENT_CONTEXT,
ERR_VM_MODULE_LINKING_ERRORED,
Expand All @@ -19,6 +18,7 @@ const {
customInspectSymbol,
} = require('internal/util');
const { SafePromise } = require('internal/safe_globals');
const { validateInt32, validateUint32 } = require('internal/validators');

const {
ModuleWrap,
Expand Down Expand Up @@ -92,8 +92,8 @@ class Module {
perContextModuleId.set(context, 1);
}

validateInteger(lineOffset, 'options.lineOffset');
validateInteger(columnOffset, 'options.columnOffset');
validateInt32(lineOffset, 'options.lineOffset');
validateInt32(columnOffset, 'options.columnOffset');

if (initializeImportMeta !== undefined) {
if (typeof initializeImportMeta === 'function') {
Expand Down Expand Up @@ -203,9 +203,8 @@ class Module {
let timeout = options.timeout;
if (timeout === undefined) {
timeout = -1;
} else if (!Number.isInteger(timeout) || timeout <= 0) {
throw new ERR_INVALID_ARG_TYPE('options.timeout', 'a positive integer',
timeout);
} else {
validateUint32(timeout, 'options.timeout', true);
}

const { breakOnSigint = false } = options;
Expand Down Expand Up @@ -243,15 +242,6 @@ class Module {
}
}

function validateInteger(prop, propName) {
if (!Number.isInteger(prop)) {
throw new ERR_INVALID_ARG_TYPE(propName, 'integer', prop);
}
if ((prop >> 0) !== prop) {
throw new ERR_OUT_OF_RANGE(propName, '32-bit integer', prop);
}
}

module.exports = {
Module,
initImportMetaMap,
Expand Down
24 changes: 6 additions & 18 deletions lib/vm.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,9 @@ const {
isContext: _isContext,
} = process.binding('contextify');

const {
ERR_INVALID_ARG_TYPE,
ERR_OUT_OF_RANGE
} = require('internal/errors').codes;
const { ERR_INVALID_ARG_TYPE } = require('internal/errors').codes;
const { isUint8Array } = require('internal/util/types');
const { validateInt32, validateUint32 } = require('internal/validators');

class Script extends ContextifyScript {
constructor(code, options = {}) {
Expand All @@ -56,8 +54,8 @@ class Script extends ContextifyScript {
if (typeof filename !== 'string') {
throw new ERR_INVALID_ARG_TYPE('options.filename', 'string', filename);
}
validateInteger(lineOffset, 'options.lineOffset');
validateInteger(columnOffset, 'options.columnOffset');
validateInt32(lineOffset, 'options.lineOffset');
validateInt32(columnOffset, 'options.columnOffset');
if (cachedData !== undefined && !isUint8Array(cachedData)) {
throw new ERR_INVALID_ARG_TYPE('options.cachedData',
['Buffer', 'Uint8Array'], cachedData);
Expand Down Expand Up @@ -119,15 +117,6 @@ function validateContext(sandbox) {
}
}

function validateInteger(prop, propName) {
if (!Number.isInteger(prop)) {
throw new ERR_INVALID_ARG_TYPE(propName, 'integer', prop);
}
if ((prop >> 0) !== prop) {
throw new ERR_OUT_OF_RANGE(propName, '32-bit integer', prop);
}
}

function validateString(prop, propName) {
if (prop !== undefined && typeof prop !== 'string')
throw new ERR_INVALID_ARG_TYPE(propName, 'string', prop);
Expand All @@ -151,9 +140,8 @@ function getRunInContextArgs(options = {}) {
let timeout = options.timeout;
if (timeout === undefined) {
timeout = -1;
} else if (!Number.isInteger(timeout) || timeout <= 0) {
throw new ERR_INVALID_ARG_TYPE('options.timeout', 'a positive integer',
timeout);
} else {
validateUint32(timeout, 'options.timeout', true);
}

const {
Expand Down
1 change: 1 addition & 0 deletions node.gyp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@
'lib/internal/v8.js',
'lib/internal/v8_prof_polyfill.js',
'lib/internal/v8_prof_processor.js',
'lib/internal/validators.js',
'lib/internal/stream_base_commons.js',
'lib/internal/vm/module.js',
'lib/internal/streams/lazy_transform.js',
Expand Down
Loading

0 comments on commit e836128

Please sign in to comment.