From b8faeeaf91914af299ebc7ed7c6e52edc67cbe03 Mon Sep 17 00:00:00 2001 From: Richard Gibson Date: Wed, 21 Feb 2024 16:54:20 -0500 Subject: [PATCH] style(pass-style): Reduce nesting depth for input validation --- packages/pass-style/src/copyRecord.js | 18 ++--- packages/pass-style/src/error.js | 51 ++++++-------- packages/pass-style/src/passStyle-helpers.js | 43 +++++------- packages/pass-style/src/remotable.js | 70 ++++++++------------ packages/pass-style/src/safe-promise.js | 44 +++++------- 5 files changed, 89 insertions(+), 137 deletions(-) diff --git a/packages/pass-style/src/copyRecord.js b/packages/pass-style/src/copyRecord.js index 38ee0d4ff9..64bb3a97c1 100644 --- a/packages/pass-style/src/copyRecord.js +++ b/packages/pass-style/src/copyRecord.js @@ -18,27 +18,23 @@ export const CopyRecordHelper = harden({ styleName: 'copyRecord', canBeValid: (candidate, check = undefined) => { - const reject = !!check && (details => check(false, details)); + const reject = !!check && ((T, ...subs) => check(false, X(T, ...subs))); if (getPrototypeOf(candidate) !== objectPrototype) { return ( reject && - reject(X`Records must inherit from Object.prototype: ${candidate}`) + reject`Records must inherit from Object.prototype: ${candidate}` ); } return ownKeys(candidate).every(key => { return ( (typeof key === 'string' || - (!!reject && - reject( - X`Records can only have string-named properties: ${candidate}`, - ))) && + (reject && + reject`Records can only have string-named properties: ${candidate}`)) && (!canBeMethod(candidate[key]) || - (!!reject && - reject( - // TODO: Update message now that there is no such thing as "implicit Remotable". - X`Records cannot contain non-far functions because they may be methods of an implicit Remotable: ${candidate}`, - ))) + (reject && + // TODO: Update message now that there is no such thing as "implicit Remotable". + reject`Records cannot contain non-far functions because they may be methods of an implicit Remotable: ${candidate}`)) ); }); }, diff --git a/packages/pass-style/src/error.js b/packages/pass-style/src/error.js index 5e70ec8baf..25996e349e 100644 --- a/packages/pass-style/src/error.js +++ b/packages/pass-style/src/error.js @@ -54,11 +54,11 @@ harden(getErrorConstructor); * @returns {boolean} */ const checkErrorLike = (candidate, check = undefined) => { - const reject = !!check && (details => check(false, details)); + const reject = !!check && ((T, ...subs) => check(false, X(T, ...subs))); // TODO: Need a better test than instanceof return ( candidate instanceof Error || - (reject && reject(X`Error expected: ${candidate}`)) + (reject && reject`Error expected: ${candidate}`) ); }; harden(checkErrorLike); @@ -98,25 +98,21 @@ export const checkRecursivelyPassableErrorPropertyDesc = ( passStyleOfRecur, check = undefined, ) => { - const reject = !!check && (details => check(false, details)); + const reject = !!check && ((T, ...subs) => check(false, X(T, ...subs))); if (desc.enumerable) { return ( reject && - reject( - X`Passable Error ${q( - propName, - )} own property must not be enumerable: ${desc}`, - ) + reject`Passable Error ${q( + propName, + )} own property must not be enumerable: ${desc}` ); } if (!hasOwn(desc, 'value')) { return ( reject && - reject( - X`Passable Error ${q( - propName, - )} own property must be a data property: ${desc}`, - ) + reject`Passable Error ${q( + propName, + )} own property must be a data property: ${desc}` ); } const { value } = desc; @@ -126,11 +122,9 @@ export const checkRecursivelyPassableErrorPropertyDesc = ( return ( typeof value === 'string' || (reject && - reject( - X`Passable Error ${q( - propName, - )} own property must be a string: ${value}`, - )) + reject`Passable Error ${q( + propName, + )} own property must be a string: ${value}`) ); } case 'cause': { @@ -141,11 +135,9 @@ export const checkRecursivelyPassableErrorPropertyDesc = ( if (!Array.isArray(value) || passStyleOfRecur(value) !== 'copyArray') { return ( reject && - reject( - X`Passable Error ${q( - propName, - )} own property must be a copyArray: ${value}`, - ) + reject`Passable Error ${q( + propName, + )} own property must be a copyArray: ${value}` ); } return value.every(err => @@ -158,8 +150,7 @@ export const checkRecursivelyPassableErrorPropertyDesc = ( } } return ( - reject && - reject(X`Passable Error has extra unpassed property ${q(propName)}`) + reject && reject`Passable Error has extra unpassed property ${q(propName)}` ); }; harden(checkRecursivelyPassableErrorPropertyDesc); @@ -175,7 +166,7 @@ export const checkRecursivelyPassableError = ( passStyleOfRecur, check = undefined, ) => { - const reject = !!check && (details => check(false, details)); + const reject = !!check && ((T, ...subs) => check(false, X(T, ...subs))); if (!checkErrorLike(candidate, check)) { return false; } @@ -185,18 +176,14 @@ export const checkRecursivelyPassableError = ( if (errConstructor === undefined || errConstructor.prototype !== proto) { return ( reject && - reject( - X`Passable Error must inherit from an error class .prototype: ${candidate}`, - ) + reject`Passable Error must inherit from an error class .prototype: ${candidate}` ); } const descs = getOwnPropertyDescriptors(candidate); if (!('message' in descs)) { return ( reject && - reject( - X`Passable Error must have an own "message" string property: ${candidate}`, - ) + reject`Passable Error must have an own "message" string property: ${candidate}` ); } diff --git a/packages/pass-style/src/passStyle-helpers.js b/packages/pass-style/src/passStyle-helpers.js index 65823d35cc..621cceb60e 100644 --- a/packages/pass-style/src/passStyle-helpers.js +++ b/packages/pass-style/src/passStyle-helpers.js @@ -93,32 +93,28 @@ export const checkNormalProperty = ( shouldBeEnumerable, check, ) => { - const reject = !!check && (details => check(false, details)); + const reject = !!check && ((T, ...subs) => check(false, X(T, ...subs))); const desc = getOwnPropertyDescriptor(candidate, propertyName); if (desc === undefined) { - return ( - reject && reject(X`${q(propertyName)} property expected: ${candidate}`) - ); + return reject && reject`${q(propertyName)} property expected: ${candidate}`; } return ( (hasOwnPropertyOf(desc, 'value') || (reject && - reject( - X`${q(propertyName)} must not be an accessor property: ${candidate}`, - ))) && + reject`${q( + propertyName, + )} must not be an accessor property: ${candidate}`)) && (shouldBeEnumerable ? desc.enumerable || (reject && - reject( - X`${q(propertyName)} must be an enumerable property: ${candidate}`, - )) + reject`${q( + propertyName, + )} must be an enumerable property: ${candidate}`) : !desc.enumerable || (reject && - reject( - X`${q( - propertyName, - )} must not be an enumerable property: ${candidate}`, - ))) + reject`${q( + propertyName, + )} must not be an enumerable property: ${candidate}`)) ); }; harden(checkNormalProperty); @@ -127,12 +123,12 @@ export const getTag = tagRecord => tagRecord[Symbol.toStringTag]; harden(getTag); export const checkPassStyle = (obj, expectedPassStyle, check) => { - const reject = !!check && (details => check(false, details)); + const reject = !!check && ((T, ...subs) => check(false, X(T, ...subs))); const actual = obj[PASS_STYLE]; return ( actual === expectedPassStyle || (reject && - reject(X`Expected ${q(expectedPassStyle)}, not ${q(actual)}: ${obj}`)) + reject`Expected ${q(expectedPassStyle)}, not ${q(actual)}: ${obj}`) ); }; harden(checkPassStyle); @@ -145,23 +141,20 @@ const makeCheckTagRecord = checkProto => { * @returns {boolean} */ const checkTagRecord = (tagRecord, passStyle, check) => { - const reject = !!check && (details => check(false, details)); + const reject = !!check && ((T, ...subs) => check(false, X(T, ...subs))); return ( (isObject(tagRecord) || - (reject && - reject(X`A non-object cannot be a tagRecord: ${tagRecord}`))) && + (reject && reject`A non-object cannot be a tagRecord: ${tagRecord}`)) && (isFrozen(tagRecord) || - (reject && reject(X`A tagRecord must be frozen: ${tagRecord}`))) && + (reject && reject`A tagRecord must be frozen: ${tagRecord}`)) && (!isArray(tagRecord) || - (reject && reject(X`An array cannot be a tagRecord: ${tagRecord}`))) && + (reject && reject`An array cannot be a tagRecord: ${tagRecord}`)) && checkNormalProperty(tagRecord, PASS_STYLE, false, check) && checkPassStyle(tagRecord, passStyle, check) && checkNormalProperty(tagRecord, Symbol.toStringTag, false, check) && (typeof getTag(tagRecord) === 'string' || (reject && - reject( - X`A [Symbol.toStringTag]-named property must be a string: ${tagRecord}`, - ))) && + reject`A [Symbol.toStringTag]-named property must be a string: ${tagRecord}`)) && checkProto(tagRecord, getPrototypeOf(tagRecord), check) ); }; diff --git a/packages/pass-style/src/remotable.js b/packages/pass-style/src/remotable.js index 0a2b32dd18..62388d92cc 100644 --- a/packages/pass-style/src/remotable.js +++ b/packages/pass-style/src/remotable.js @@ -32,23 +32,19 @@ const { * @param {Checker} [check] */ const checkIface = (iface, check) => { - const reject = !!check && (details => check(false, details)); + const reject = !!check && ((T, ...subs) => check(false, X(T, ...subs))); return ( // TODO other possible ifaces, once we have third party veracity (typeof iface === 'string' || (reject && - reject( - X`For now, interface ${iface} must be a string; unimplemented`, - ))) && + reject`For now, interface ${iface} must be a string; unimplemented`)) && (iface === 'Remotable' || iface.startsWith('Alleged: ') || iface.startsWith('DebugName: ') || (reject && - reject( - X`For now, iface ${q( - iface, - )} must be "Remotable" or begin with "Alleged: " or "DebugName: "; unimplemented`, - ))) + reject`For now, iface ${q( + iface, + )} must be "Remotable" or begin with "Alleged: " or "DebugName: "; unimplemented`)) ); }; @@ -71,7 +67,7 @@ harden(assertIface); * @returns {boolean} */ const checkRemotableProtoOf = (original, check) => { - const reject = !!check && (details => check(false, details)); + const reject = !!check && ((T, ...subs) => check(false, X(T, ...subs))); isObject(original) || Fail`Remotables must be objects or functions: ${original}`; @@ -92,8 +88,7 @@ const checkRemotableProtoOf = (original, check) => { const proto = getPrototypeOf(original); if (proto === objectPrototype || proto === null) { return ( - reject && - reject(X`Remotables must be explicitly declared: ${q(original)}`) + reject && reject`Remotables must be explicitly declared: ${q(original)}` ); } @@ -125,9 +120,9 @@ const checkRemotableProtoOf = (original, check) => { return ( (ownKeys(restDescs).length === 0 || (reject && - reject( - X`Unexpected properties on Remotable Proto ${ownKeys(restDescs)}`, - ))) && + reject`Unexpected properties on Remotable Proto ${ownKeys( + restDescs, + )}`)) && checkIface(iface, check) ); }; @@ -152,9 +147,9 @@ const checkRemotable = (val, check) => { if (confirmedRemotables.has(val)) { return true; } - const reject = !!check && (details => check(false, details)); + const reject = !!check && ((T, ...subs) => check(false, X(T, ...subs))); if (!isFrozen(val)) { - return reject && reject(X`cannot serialize non-frozen objects like ${val}`); + return reject && reject`cannot serialize non-frozen objects like ${val}`; } // eslint-disable-next-line no-use-before-define if (!RemotableHelper.canBeValid(val, check)) { @@ -188,14 +183,13 @@ export const RemotableHelper = harden({ styleName: 'remotable', canBeValid: (candidate, check = undefined) => { - const reject = !!check && (details => check(false, details)); + const reject = !!check && ((T, ...subs) => check(false, X(T, ...subs))); const validType = (isObject(candidate) || (reject && - reject(X`cannot serialize non-objects as Remotable ${candidate}`))) && + reject`cannot serialize non-objects as Remotable ${candidate}`)) && (!isArray(candidate) || - (reject && - reject(X`cannot serialize arrays as Remotable ${candidate}`))); + (reject && reject`cannot serialize arrays as Remotable ${candidate}`)); if (!validType) { return false; } @@ -209,22 +203,18 @@ export const RemotableHelper = harden({ // Typecast needed due to https://github.com/microsoft/TypeScript/issues/1863 (hasOwnPropertyOf(descs[/** @type {string} */ (key)], 'value') || (reject && - reject( - X`cannot serialize Remotables with accessors like ${q( - String(key), - )} in ${candidate}`, - ))) && + reject`cannot serialize Remotables with accessors like ${q( + String(key), + )} in ${candidate}`)) && ((key === Symbol.toStringTag && checkIface(candidate[key], check)) || ((canBeMethod(candidate[key]) || (reject && - reject( - X`cannot serialize Remotables with non-methods like ${q( - String(key), - )} in ${candidate}`, - ))) && + reject`cannot serialize Remotables with non-methods like ${q( + String(key), + )} in ${candidate}`)) && (key !== PASS_STYLE || (reject && - reject(X`A pass-by-remote cannot shadow ${q(PASS_STYLE)}`))))) + reject`A pass-by-remote cannot shadow ${q(PASS_STYLE)}`)))) ); }); } else if (typeof candidate === 'function') { @@ -241,27 +231,21 @@ export const RemotableHelper = harden({ return ( ((nameDesc && typeof nameDesc.value === 'string') || (reject && - reject(X`Far function name must be a string, in ${candidate}`))) && + reject`Far function name must be a string, in ${candidate}`)) && ((lengthDesc && typeof lengthDesc.value === 'number') || (reject && - reject( - X`Far function length must be a number, in ${candidate}`, - ))) && + reject`Far function length must be a number, in ${candidate}`)) && (toStringTagDesc === undefined || ((typeof toStringTagDesc.value === 'string' || (reject && - reject( - X`Far function @@toStringTag must be a string, in ${candidate}`, - ))) && + reject`Far function @@toStringTag must be a string, in ${candidate}`)) && checkIface(toStringTagDesc.value, check))) && (restKeys.length === 0 || (reject && - reject( - X`Far functions unexpected properties besides .name and .length ${restKeys}`, - ))) + reject`Far functions unexpected properties besides .name and .length ${restKeys}`)) ); } - return reject && reject(X`unrecognized typeof ${candidate}`); + return reject && reject`unrecognized typeof ${candidate}`; }, assertValid: candidate => checkRemotable(candidate, assertChecker), diff --git a/packages/pass-style/src/safe-promise.js b/packages/pass-style/src/safe-promise.js index 80c425d843..071abbf019 100644 --- a/packages/pass-style/src/safe-promise.js +++ b/packages/pass-style/src/safe-promise.js @@ -16,7 +16,7 @@ const { toStringTag } = Symbol; * @returns {pr is Promise} Whether it is a safe promise */ const checkPromiseOwnKeys = (pr, check) => { - const reject = details => check(false, details); + const reject = (T, ...subs) => check(false, X(T, ...subs)); const keys = ownKeys(pr); if (keys.length === 0) { @@ -37,9 +37,7 @@ const checkPromiseOwnKeys = (pr, check) => { ); if (unknownKeys.length !== 0) { - return reject( - X`${pr} - Must not have any own properties: ${q(unknownKeys)}`, - ); + return reject`${pr} - Must not have any own properties: ${q(unknownKeys)}`; } /** @@ -71,17 +69,15 @@ const checkPromiseOwnKeys = (pr, check) => { assert(tagDesc !== undefined); return ( (hasOwnPropertyOf(tagDesc, 'value') || - reject( - X`Own @@toStringTag must be a data property, not an accessor: ${q( - tagDesc, - )}`, - )) && + reject`Own @@toStringTag must be a data property, not an accessor: ${q( + tagDesc, + )}`) && (typeof tagDesc.value === 'string' || - reject( - X`Own @@toStringTag value must be a string: ${q(tagDesc.value)}`, - )) && + reject`Own @@toStringTag value must be a string: ${q( + tagDesc.value, + )}`) && (!tagDesc.enumerable || - reject(X`Own @@toStringTag must not be enumerable: ${q(tagDesc)}`)) + reject`Own @@toStringTag must not be enumerable: ${q(tagDesc)}`) ); } const val = pr[key]; @@ -107,11 +103,9 @@ const checkPromiseOwnKeys = (pr, check) => { return true; } } - return reject( - X`Unexpected Node async_hooks additions to promise: ${pr}.${q( - String(key), - )} is ${val}`, - ); + return reject`Unexpected Node async_hooks additions to promise: ${pr}.${q( + String(key), + )} is ${val}`; }; return keys.every(checkSafeOwnKey); @@ -134,16 +128,14 @@ const checkPromiseOwnKeys = (pr, check) => { * @returns {pr is Promise} Whether it is a safe promise */ const checkSafePromise = (pr, check) => { - const reject = details => check(false, details); + const reject = (T, ...subs) => check(false, X(T, ...subs)); return ( - (isFrozen(pr) || reject(X`${pr} - Must be frozen`)) && - (isPromise(pr) || reject(X`${pr} - Must be a promise`)) && + (isFrozen(pr) || reject`${pr} - Must be frozen`) && + (isPromise(pr) || reject`${pr} - Must be a promise`) && (getPrototypeOf(pr) === Promise.prototype || - reject( - X`${pr} - Must inherit from Promise.prototype: ${q( - getPrototypeOf(pr), - )}`, - )) && + reject`${pr} - Must inherit from Promise.prototype: ${q( + getPrototypeOf(pr), + )}`) && checkPromiseOwnKeys(/** @type {Promise} */ (pr), check) ); };