Skip to content

Commit

Permalink
fix(marshal): #2435 make getInterfaceOf truthful
Browse files Browse the repository at this point in the history
  • Loading branch information
@erights @michaelfig
erights authored and michaelfig committed Apr 27, 2021
1 parent cddd949 commit ff19f93
Show file tree
Hide file tree
Showing 3 changed files with 282 additions and 101 deletions.
37 changes: 14 additions & 23 deletions packages/marshal/src/marshal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
getInterfaceOf,
getErrorConstructor,
assertCanBeRemotable,
assertIface,
} from './passStyleOf';

import './types';
Expand Down Expand Up @@ -119,18 +120,23 @@ function pureCopy(val, already = new WeakMap()) {
harden(pureCopy);
export { pureCopy };

const makeRemotableProto = (oldProto, allegedName) => {
/**
* @param {Object|null} oldProto
* @param {InterfaceSpec} iface
* @returns {Object}
*/
const makeRemotableProto = (oldProto, iface) => {
assert(
oldProto === objectPrototype || oldProto === null,
X`For now, remotables cannot inherit from anything unusual`,
);
// Assign the arrow function to a variable to set its .name.
const toString = () => `[${allegedName}]`;
const toString = () => `[${iface}]`;
return harden(
create(oldProto, {
[PASS_STYLE]: { value: 'remotable' },
toString: { value: toString },
[Symbol.toStringTag]: { value: allegedName },
[Symbol.toStringTag]: { value: iface },
}),
);
};
Expand Down Expand Up @@ -629,24 +635,12 @@ export function makeMarshal(
* @param {object} [remotable={}] The object used as the remotable
* @returns {object} remotable, modified for debuggability
*/
const Remotable = (iface = 'Remotable', props = undefined, remotable = {}) => {
// TODO unimplemented
assert.typeof(
iface,
'string',
X`Interface ${iface} must be a string; unimplemented`,
);
// TODO unimplemented
assert(
iface === 'Remotable' || iface.startsWith('Alleged: '),
X`For now, iface ${q(
iface,
)} must be "Remotable" or begin with "Alleged: "; unimplemented`,
);
function Remotable(iface = 'Remotable', props = undefined, remotable = {}) {
assertIface(iface);
iface = pureCopy(harden(iface));
assert(iface);
// TODO: When iface is richer than just string, we need to get the allegedName
// in a different way.
const allegedName = iface;
assert(props === undefined, X`Remotable props not yet implemented ${props}`);

// Fail fast: check that the unmodified object is able to become a Remotable.
Expand All @@ -661,10 +655,7 @@ const Remotable = (iface = 'Remotable', props = undefined, remotable = {}) => {
);
// Ensure that the remotable isn't already frozen.
assert(!isFrozen(remotable), X`Remotable ${remotable} is already frozen`);
const remotableProto = makeRemotableProto(
getPrototypeOf(remotable),
allegedName,
);
const remotableProto = makeRemotableProto(getPrototypeOf(remotable), iface);

// Take a static copy of the enumerable own properties as data properties.
// const propDescs = getOwnPropertyDescriptors({ ...props });
Expand All @@ -685,7 +676,7 @@ const Remotable = (iface = 'Remotable', props = undefined, remotable = {}) => {
// We're committed, so keep the interface for future reference.
assert(iface !== undefined); // To make TypeScript happy
return remotable;
};
}

harden(Remotable);
export { Remotable };
Expand Down
265 changes: 187 additions & 78 deletions packages/marshal/src/passStyleOf.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,15 @@ import { isPromise } from '@agoric/promise-kit';

import './types';

// Setting this flag to true is what allows objects with `null` or
// `Object.prototype` prototypes to be treated as remotable. Setting to `false`
// means that only objects declared with `Remotable(...)`, including `Far(...)`
// can be used as remotables.
//
// TODO: once the policy changes to force remotables to be explicit, remove this
// flag entirely and fix code that uses it (as if it were always `false`).
const ALLOW_IMPLICIT_REMOTABLES = true;

const {
getPrototypeOf,
getOwnPropertyDescriptors,
Expand All @@ -19,24 +28,6 @@ const { ownKeys } = Reflect;

export const PASS_STYLE = Symbol.for('passStyle');

/** @type {MarshalGetInterfaceOf} */
export function getInterfaceOf(val) {
if (typeof val !== 'object' || val === null) {
return undefined;
}
if (val[PASS_STYLE] !== 'remotable') {
return undefined;
}
assert(isFrozen(val), X`Remotable ${val} must be frozen`, TypeError);
const iface = val[Symbol.toStringTag];
assert.typeof(
iface,
'string',
X`Remotable interface currently can only be a string`,
);
return iface;
}

const errorConstructors = new Map([
['Error', Error],
['EvalError', EvalError],
Expand Down Expand Up @@ -190,95 +181,213 @@ function isPassByCopyRecord(val) {
return true;
}

// Below we have a series of predicate functions and their (curried) assertion
// functions. The semantics of the assertion function is just to assert that
// the corresponding predicate function would have returned true. But it
// reproduces the internal tests so failures can give a better error message.

/**
* Throw if val is not the correct shape for the prototype of a Remotable.
*
* TODO: It would be nice to typedef this shape and then declare that this
* function asserts it, but we can't declare a type with PASS_STYLE from JSDoc.
* @callback Checker
* @param {boolean} cond
* @param {Parameters<typeof assert>[1]} details
* @returns {boolean}
*/

/** @type {Checker} */
const assertChecker = (cond, details) => {
assert(cond, details);
return true;
};

/**
* @param {InterfaceSpec} iface
* @param {Checker} check
*/
const checkIface = (iface, check = x => x) => {
return (
// TODO other possible ifaces, once we have third party veracity
check(
typeof iface === 'string',
X`Interface ${iface} must be a string; unimplemented`,
) &&
check(
iface === 'Remotable' || iface.startsWith('Alleged: '),
X`For now, iface ${q(
iface,
)} must be "Remotable" or begin with "Alleged: "; unimplemented`,
)
);
};

/**
* @param {InterfaceSpec} iface
*/
const assertIface = iface => checkIface(iface, assertChecker);
harden(assertIface);
export { assertIface };

/**
* TODO: It would be nice to typedef this shape, but we can't declare a type
* with PASS_STYLE from JSDoc.
*
* @param {{ [PASS_STYLE]: string, [Symbol.toStringTag]: string, toString: () =>
* void}} val the value to verify
* @param {{
* [PASS_STYLE]: string,
* [Symbol.toStringTag]: string,
* toString: () => void }} val the value to verify
* @param {Checker} [check]
* @returns {boolean}
*/
const assertRemotableProto = val => {
assert.typeof(val, 'object', X`cannot serialize non-objects like ${val}`);
assert(!Array.isArray(val), X`Arrays cannot be pass-by-remote`);
assert(val !== null, X`null cannot be pass-by-remote`);
const checkRemotableProto = (val, check = x => x) => {
if (
!(
check(
typeof val === 'object',
X`cannot serialize non-objects like ${val}`,
) &&
check(!Array.isArray(val), X`Arrays cannot be pass-by-remote`) &&
check(val !== null, X`null cannot be pass-by-remote`)
)
) {
return false;
}

const protoProto = getPrototypeOf(val);
assert(
protoProto === objectPrototype || protoProto === null,
X`The Remotable Proto marker cannot inherit from anything unusual`,
);
assert(isFrozen(val), X`The Remotable proto must be frozen`);
if (
!(
check(
protoProto === objectPrototype || protoProto === null,
X`The Remotable Proto marker cannot inherit from anything unusual`,
) && check(isFrozen(val), X`The Remotable proto must be frozen`)
)
) {
return false;
}

const {
[PASS_STYLE]: { value: passStyleValue },
toString: { value: toStringValue },
[PASS_STYLE]: passStyleDesc,
toString: toStringDesc,
// @ts-ignore https://github.com/microsoft/TypeScript/issues/1863
[Symbol.toStringTag]: { value: toStringTagValue },
[Symbol.toStringTag]: ifaceDesc,
...rest
} = getOwnPropertyDescriptors(val);
assert(
ownKeys(rest).length === 0,
X`Unexpect properties on Remotable Proto ${ownKeys(rest)}`,
);
assert(
passStyleValue === 'remotable',
X`Expected 'remotable', not ${q(passStyleValue)}`,

return (
check(
ownKeys(rest).length === 0,
X`Unexpected properties on Remotable Proto ${ownKeys(rest)}`,
) &&
check(!!passStyleDesc, X`Remotable must have a [PASS_STYLE]`) &&
check(
passStyleDesc.value === 'remotable',
X`Expected 'remotable', not ${q(passStyleDesc.value)}`,
) &&
check(
typeof toStringDesc.value === 'function',
X`toString must be a function`,
) &&
checkIface(ifaceDesc && ifaceDesc.value, check)
);
assert.typeof(toStringValue, 'function', X`toString must be a function`);
assert.typeof(toStringTagValue, 'string', X`@@toStringTag must be a string`);
};

/**
* Ensure that val could become a legitimate remotable. This is used
* internally both in the construction of a new remotable and
* mustPassByRemote.
* Ensure that val could become a legitimate remotable. This is used internally
* both in the construction of a new remotable and checkRemotable.
*
* @param {*} val The remotable candidate to check
* @param {Checker} [check]
* @returns {boolean}
*/
export function assertCanBeRemotable(val) {
// throws exception if cannot
assert.typeof(val, 'object', X`cannot serialize non-objects like ${val}`);
assert(!Array.isArray(val), X`Arrays cannot be pass-by-remote`);
assert(val !== null, X`null cannot be pass-by-remote`);
function checkCanBeRemotable(val, check = x => x) {
if (
!(
check(
typeof val === 'object',
X`cannot serialize non-objects like ${val}`,
) &&
check(!Array.isArray(val), X`Arrays cannot be pass-by-remote`) &&
check(val !== null, X`null cannot be pass-by-remote`)
)
) {
return false;
}

const descs = getOwnPropertyDescriptors(val);
const keys = ownKeys(descs); // enumerable-and-not, string-or-Symbol
keys.forEach(key => {
assert(
return keys.every(
key =>
// Typecast needed due to https://github.com/microsoft/TypeScript/issues/1863
!('get' in descs[/** @type {string} */ (key)]),
X`cannot serialize objects with getters like ${q(String(key))} in ${val}`,
);
assert.typeof(
// @ts-ignore https://github.com/microsoft/TypeScript/issues/1863
val[key],
'function',
X`cannot serialize objects with non-methods like ${q(
String(key),
)} in ${val}`,
);
assert(
key !== PASS_STYLE,
X`A pass-by-remote cannot shadow ${q(PASS_STYLE)}`,
);
});
check(
!('get' in descs[/** @type {string} */ (key)]),
X`cannot serialize objects with getters like ${q(
String(key),
)} in ${val}`,
) &&
check(
typeof val[key] === 'function',
X`cannot serialize objects with non-methods like ${q(
String(key),
)} in ${val}`,
) &&
check(
key !== PASS_STYLE,
X`A pass-by-remote cannot shadow ${q(PASS_STYLE)}`,
),
);
}

const canBeRemotable = val => checkCanBeRemotable(val);
harden(canBeRemotable);
export { canBeRemotable };

const assertCanBeRemotable = val => {
checkCanBeRemotable(val, assertChecker);
};
harden(assertCanBeRemotable);
export { assertCanBeRemotable };

/**
* @param {Remotable} val
* @param {Checker} [check]
* @returns {boolean}
*/
function assertRemotable(val) {
assert(isFrozen(val), X`cannot serialize non-frozen objects like ${val}`);

assertCanBeRemotable(val);

function checkRemotable(val, check = x => x) {
const not = (cond, details) => !check(cond, details);
if (not(isFrozen(val), X`cannot serialize non-frozen objects like ${val}`)) {
return false;
}
if (!checkCanBeRemotable(val, check)) {
return false;
}
const p = getPrototypeOf(val);
if (p !== null && p !== objectPrototype) {
assertRemotableProto(p);

if (ALLOW_IMPLICIT_REMOTABLES && (p === null || p === objectPrototype)) {
return true;
}
return checkRemotableProto(p, check);
}

/**
* @param {Remotable} val
*/
const assertRemotable = val => {
checkRemotable(val, assertChecker);
};

/** @type {MarshalGetInterfaceOf} */
const getInterfaceOf = val => {
if (
typeof val !== 'object' ||
val === null ||
val[PASS_STYLE] !== 'remotable' ||
!checkRemotable(val)
) {
return undefined;
}
return val[Symbol.toStringTag];
};
harden(getInterfaceOf);
export { getInterfaceOf };

/**
* objects can only be passed in one of two/three forms:
* 1: pass-by-remote: all properties (own and inherited) are methods,
Expand Down
Loading

0 comments on commit ff19f93

Please sign in to comment.