fix(marshal)!: Reject null as a substitute for Object.prototype #1328
Conversation
| (!canBeMethod(candidate[key]) || | ||
| (!!reject && | ||
| reject( | ||
| X`Records cannot contain non-far functions because they may be methods of an implicit Remotable: ${candidate}`, |
There was a problem hiding this comment.
Error message needs revision because there is no longer any such thing as an "implicit Remotable". For this PR, a TODO is adequate.
| reject( | ||
| X`Records can only have string-named properties: ${candidate}`, | ||
| ))) && | ||
| (!canBeMethod(candidate[key]) || |
There was a problem hiding this comment.
Before this PR, canBeMethod tests desc.value. This PR tests candidate[key]. Where does the new code ensure that candidate is frozen and has no own accessor properties? Were we previously being reflective unnecessarily? Isn't reflective examination the safer style anyway? Is a reflective check significantly slower on XS?
There was a problem hiding this comment.
This is in canBeValid, where my understanding is that we should return true because e.g. { get foo(){ return nonFunction; } } cannot be any kind of passable other than copyRecord. assertValid still uses checkNormalProperty to reject it, and I've added some tests for coverage.
| (!!check && | ||
| check( | ||
| false, |
| check(false, X`A tagRecord must inherit from Object.prototype: ${val}`)), | ||
| ); | ||
|
|
||
| export const checkFunctionTagRecord = makeCheckTagRecord( |
There was a problem hiding this comment.
What is our stance on builtin Function subclasses, such as async functions, generator functions, or async generators? Can these be far functions? These do not directly inherit from Function.prototype, but they do inherit from known primordial prototypes.
There was a problem hiding this comment.
This preserves the existing behavior, which allows far async functions if their prototype chain has been suitably manipulated (e.g., setPrototypeOf(async () => {}, makeTagishRecord('Remotable', getPrototypeOf(async () => {})))) while rejecting generators and async generators—and classes, for that matter—by dint of a non-removable "prototype" property ("Far functions unexpected properties besides .name and .length"). But an (async)generator is basically a callable factory for (async)iterator objects which at their core are a bag of inherited methods, so I could definitely see extending support in that direction, probably with a helper like ExoGenerator(async function* () { … }) to alter the prototype chain and harden responses.
gibson042
force-pushed
the
gibson-1324-reject-null-proto
branch
from
06060c5 to
843fa3e
Compare
gibson042
force-pushed
the
gibson-1324-reject-null-proto
branch
from
843fa3e to
02744b9
Compare
Fixes #1324