9.5.12 [[OwnPropertyKeys]] ( )
The array returned from ownKeys() can be mutated after all invariant checks were performed in 9.5.12. That way it is still possible to violate the object invariants from 6.1.7.3.
Test case:
---
let o = Object.defineProperty({}, "prop", {value: 123, configurable: false});
let p = new Proxy(o, {
ownKeys() {
return Object.defineProperty([], "0", {
get() {
// Alternatively return a different property name here.
this.length = 0;
return "prop";
}, configurable: true
});
}
});
print("own-names:", Object.getOwnPropertyNames(p));
---
Tom, Mark
What do you think?
The cleanest way I can think of for fixing this would be to respecify [[OwnPropertyKey]] as returning a List rather than an array. Of course a trap handler would still return an Array but Proxy [[OwnPropertyKey]] would convert it to a List before performing its other invariant checks.
The array to list conversion could also be used to enforce the invariant the elements are all valid property keys (strings or symbols). Se Bug 3026
(In reply to Allen Wirfs-Brock from comment #1)
> Tom, Mark
> What do you think?
>
> The cleanest way I can think of for fixing this would be to respecify
> [[OwnPropertyKey]] as returning a List rather than an array. Of course a
> trap handler would still return an Array but Proxy [[OwnPropertyKey]] would
> convert it to a List before performing its other invariant checks.
>
> The array to list conversion could also be used to enforce the invariant
> the elements are all valid property keys (strings or symbols). Se Bug 3026
That seems reasonable to me. What are the possible downsides or alternatives that would still fix the problem?
(In reply to Mark Miller from comment #2)
>
> That seems reasonable to me. What are the possible downsides or alternatives
> that would still fix the problem?
We'd have to validate that now of the array indexed properties (or the 'length') property of the trap result are accessor properties. (we already forbid it being a Proxy, but that restriction could be eliminated if we to List conversion)
Using a List means that identify of the result array is not preserved from the trap call through a Object.getOwnPropertyKeys call. Requires copying, while the validation approach could be done in place. Passing the same object from handler to getOwnPropertyKeys caller also means that the result array as a communications channel for additional information from handler to caller (and possibly back).
(In reply to Allen Wirfs-Brock from comment #3)
> (In reply to Mark Miller from comment #2)
> >
> > That seems reasonable to me. What are the possible downsides or alternatives
> > that would still fix the problem?
>
> We'd have to validate that now of the array indexed properties (or the
> 'length') property of the trap result are accessor properties. (we already
> forbid it being a Proxy, but that restriction could be eliminated if we to
> List conversion)
>
> Using a List means that identify of the result array is not preserved from
> the trap call through a Object.getOwnPropertyKeys call. Requires copying,
> while the validation approach could be done in place. Passing the same
> object from handler to getOwnPropertyKeys caller also means that the result
> array as a communications channel for additional information from handler to
> caller (and possibly back).
All this makes me even more convinced of the List approach. This is all very parallel to the issues around the descriptor returned by getOwnPropertyDescriptor, in which we avoided the similar accidental communications channel issue.
fixed in rev26 editor's draft
Made the result of [[OwnPropertyKeys]] a List
(In reply to Allen Wirfs-Brock from comment #5)
> fixed in rev26 editor's draft
>
uh, rev27
Yes, +1 for either returning a List or having [[OwnPropertyKeys]] reconstruct an Array from scratch.
Interestingly, the latter is what I already did for my harmony-reflect shim. And as was mentioned, this also allows us to make sure the result contains only Strings and Symbols.
fixed in rev27 draft