Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why we still fail to endow liveSlots #2407

Closed
erights opened this issue Aug 12, 2024 · 6 comments · Fixed by #2408
Closed

Why we still fail to endow liveSlots #2407

erights opened this issue Aug 12, 2024 · 6 comments · Fixed by #2408

Comments

@erights
Copy link
Contributor

erights commented Aug 12, 2024

Uses Object.keys which only includes string-named properties.

My first thought is to change it to Reflect.ownKeys, but this includes non-enumerable properties as well. What did we decide was correct in general for these global endowments? Should we change to Reflect.ownKeys and then filter out non-enumerables? Or should we just use Reflect.ownKeys?

@kriskowal
Copy link
Member

We agreed that the goal state would be for the globals to use the semantics of Object.assign except that all properties would be non-enumerable by default, and to leave a door open for globalProperties which would be copied by the semantics of Object.getOwnPropertyDescriptors and Object.defineProperties. We were content to omit a globalProperties option from Compartment since the behavior is allowed with direct manipulation of compartment.globalThis, but the compartment mapper would need one for those semantics. We also agreed to leave open the possibility of a parallel design for inescapableGlobals and inescapableGlobalProperties on the Compartment constructor.

@erights
Copy link
Contributor Author

erights commented Aug 12, 2024

So concretely, this specific line should do Reflect.ownKeys and then filter out the non-enumerables, in order to be consistent with Object.assign behavior?

@erights
Copy link
Contributor Author

erights commented Aug 12, 2024

Btw, even with this line corrected, we're still failing to endow passStyleOf. Still tracking that down. Attn @warner

@kriskowal
Copy link
Member

I am open to relitigating. I would be fine with a proposal that all owned, enumerable string and symbol names of globals be copied by assignment and made non-enumerable in the goal state. We would need to either use a feature flag to enable the new behavior or do a trial to verify that the change is inconsequential to existing applications.

@kriskowal
Copy link
Member

attn @naugtur since this is in endomoat policy enforcement.

@naugtur
Copy link
Member

naugtur commented Aug 13, 2024

Last time we discussed this I was initially in favor of copying all own properties, enumerable or not.
The only thing that raises an eyebrow for me here is all owned, enumerable string and symbol names of globals be copied by assignment and made non-enumerable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants