Skip to content

Commit

Permalink
style(pass-style): Reduce nesting depth for input validation (merge #…
Browse files Browse the repository at this point in the history 
…2097)
  • Loading branch information
@gibson042
gibson042 authored Feb 22, 2024
2 parents 18fb8c0 + b8faeea commit 2753ac2
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 137 deletions.
18 changes: 7 additions & 11 deletions packages/pass-style/src/copyRecord.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}`))
);
});
},
Expand Down
51 changes: 19 additions & 32 deletions packages/pass-style/src/error.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -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': {
Expand All @@ -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 =>
Expand All @@ -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);
Expand All @@ -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;
}
Expand All @@ -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}`
);
}

Expand Down
43 changes: 18 additions & 25 deletions packages/pass-style/src/passStyle-helpers.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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)
);
};
Expand Down
70 changes: 27 additions & 43 deletions packages/pass-style/src/remotable.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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`))
);
};

Expand All @@ -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}`;

Expand All @@ -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)}`
);
}

Expand Down Expand Up @@ -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)
);
};
Expand All @@ -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)) {
Expand Down Expand Up @@ -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;
}
Expand All @@ -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') {
Expand All @@ -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),
Expand Down
Loading

0 comments on commit 2753ac2

Please sign in to comment.