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

Normative: Do not capture the script/module in new Function and indirect eval #3374

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nicolo-ribaudo
Copy link
Member

@nicolo-ribaudo nicolo-ribaudo commented Jul 15, 2024

We have three ways of dynamically executing code:

  • direct eval
  • indirect eval
  • new Function&friends

The main difference is that direct eval has some magic behavior, capturing the context where it's being called, while indirect eval and new Function are supposed to be "normal functions" that do not perform any dynamic scoping.

There is however a case where indirect eval and new Function are affected by where they are being called: they propagate the active script or module.

This pull request changes them to instead run their code with a null ScriptOrModule, similarly to code run in HTML event handlers. This ensures that the behavior of indirect eval and new Function does not depend on where they are called. The observable change is that in (eval, 0)("import('./foo')"), ./foo is resolved relative to the base URL of the Realm and not of the script or module calling eval (and same for new Function).

I believe that this PR can be web compatible because there is currently different behaviors across different browsers (see #3160 for more details).

Fixes #3160.

@nicolo-ribaudo nicolo-ribaudo added normative change Affects behavior required to correctly evaluate some ECMAScript source text needs consensus This needs committee consensus before it can be eligible to be merged. needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 labels Jul 15, 2024
@@ -13137,10 +13146,10 @@ <h1>ECMAScript Function Objects</h1>
[[ScriptOrModule]]
</td>
<td>
a Script Record or a Module Record
a Script Record, a Module Record, *null* or ~hidden~.
Copy link
Member

@devsnek devsnek Jul 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't think this can be null (or appear to be null in the case of ~hidden~?). See for example #1670.

and so this leads me to my question, why not just create a new script record for indirect evaluators?

Copy link
Member Author

@nicolo-ribaudo nicolo-ribaudo Jul 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be null when you define a function inside an inline event handler in HTML, since in that case GetActiveScriptOrModule() is null.

and so this leads me to my question, why not just create a new script record for indirect evaluators?

That would work too, but we probably need a new host hook to let hosts define the [[HostDefined]] slot of those new script records created within ECMA-262.

Copy link
Member

@devsnek devsnek Jul 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess another perspective is that the ecmascript code being evaluated is not technically from a Script or a Module. Though it would seem to clean up the existence of HideParentScriptOrModule if we set something there explicitly.

Comment on lines +13293 to +13297
1. If _F_.[[ScriptOrModule]] is ~hidden~, then
1. Set the ScriptOrModule of _calleeContext_ to *null*.
1. Set the HideParentScriptOrModule of _calleeContext_ to *true*.
1. Else,
1. Set the ScriptOrModule of _calleeContext_ to _F_.[[ScriptOrModule]].
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be unchanged and propagate ~hidden~ up to GetActiveScriptOrModule, which could turn it into null?

HideParentScriptOrModule
</td>
<td>
A Boolean indicating wether, when looking for an execution context with a non-*null* ScriptOrModule component, this execution context should allow looking at its ancestors or not. If not explicitly set, defaults to *false*.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
A Boolean indicating wether, when looking for an execution context with a non-*null* ScriptOrModule component, this execution context should allow looking at its ancestors or not. If not explicitly set, defaults to *false*.
A Boolean indicating whether, when looking for an execution context with a non-*null* ScriptOrModule component, this execution context should allow looking at its ancestors or not. If not explicitly set, defaults to *false*.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs consensus This needs committee consensus before it can be eligible to be merged. needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 normative change Affects behavior required to correctly evaluate some ECMAScript source text
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Function and indirect eval should not capture context from where they are called.
3 participants