Skip to content

Commit

Permalink
lib: consolidate arrayBufferView validation
Browse files Browse the repository at this point in the history
There are lots of places that validate for arrayBufferView and we
have multiple functions that do the same thing. Instead, move the
validation into `internal/validators` so all files can use that
instead.

There are more functions throughout the code that do the same but
it takes some more work to fully consolidate all of those.

PR-URL: nodejs#26809
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
BridgeAR committed Mar 27, 2019
1 parent 751c92d commit 7bddfcc
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 19 deletions.
2 changes: 1 addition & 1 deletion lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ const {
stringToFlags,
stringToSymlinkType,
toUnixTimestamp,
validateBuffer,
validateOffsetLengthRead,
validateOffsetLengthWrite,
validatePath
Expand All @@ -81,6 +80,7 @@ const {
const {
isUint32,
parseMode,
validateBuffer,
validateInteger,
validateInt32,
validateUint32
Expand Down
11 changes: 4 additions & 7 deletions lib/internal/crypto/certificate.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ const {
certExportPublicKey,
certVerifySpkac
} = internalBinding('crypto');
const {
validateBuffer
} = require('internal/validators');

const { ERR_INVALID_ARG_TYPE } = require('internal/errors').codes;
const { isArrayBufferView } = require('internal/util/types');
Expand All @@ -14,13 +17,7 @@ const {
} = require('internal/crypto/util');

function verifySpkac(spkac) {
if (!isArrayBufferView(spkac)) {
throw new ERR_INVALID_ARG_TYPE(
'spkac',
['Buffer', 'TypedArray', 'DataView'],
spkac
);
}
validateBuffer(spkac, 'spkac');
return certVerifySpkac(spkac);
}

Expand Down
2 changes: 1 addition & 1 deletion lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ const {
stringToFlags,
stringToSymlinkType,
toUnixTimestamp,
validateBuffer,
validateOffsetLengthRead,
validateOffsetLengthWrite,
validatePath
} = require('internal/fs/utils');
const {
parseMode,
validateBuffer,
validateInteger,
validateUint32
} = require('internal/validators');
Expand Down
10 changes: 0 additions & 10 deletions lib/internal/fs/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ const {
} = require('internal/errors');
const {
isUint8Array,
isArrayBufferView,
isDate
} = require('internal/util/types');
const { once } = require('internal/util');
Expand Down Expand Up @@ -393,14 +392,6 @@ function toUnixTimestamp(time, name = 'time') {
throw new ERR_INVALID_ARG_TYPE(name, ['Date', 'Time in seconds'], time);
}

const validateBuffer = hideStackFrames((buffer) => {
if (!isArrayBufferView(buffer)) {
throw new ERR_INVALID_ARG_TYPE('buffer',
['Buffer', 'TypedArray', 'DataView'],
buffer);
}
});

const validateOffsetLengthRead = hideStackFrames(
(offset, length, bufferLength) => {
if (offset < 0 || offset >= bufferLength) {
Expand Down Expand Up @@ -453,7 +444,6 @@ module.exports = {
stringToSymlinkType,
Stats,
toUnixTimestamp,
validateBuffer,
validateOffsetLengthRead,
validateOffsetLengthWrite,
validatePath
Expand Down
15 changes: 15 additions & 0 deletions lib/internal/validators.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ const {
ERR_OUT_OF_RANGE
}
} = require('internal/errors');
const {
isArrayBufferView
} = require('internal/util/types');

function isInt32(value) {
return value === (value | 0);
Expand Down Expand Up @@ -107,10 +110,22 @@ function validateNumber(value, name) {
throw new ERR_INVALID_ARG_TYPE(name, 'number', value);
}

// TODO(BridgeAR): We have multiple validation functions that call
// `require('internal/utils').toBuf()` before validating for array buffer views.
// Those should likely also be consolidated in here.
const validateBuffer = hideStackFrames((buffer, name = 'buffer') => {
if (!isArrayBufferView(buffer)) {
throw new ERR_INVALID_ARG_TYPE(name,
['Buffer', 'TypedArray', 'DataView'],
buffer);
}
});

module.exports = {
isInt32,
isUint32,
parseMode,
validateBuffer,
validateInteger,
validateInt32,
validateUint32,
Expand Down

0 comments on commit 7bddfcc

Please sign in to comment.