-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Function
and indirect eval
should not capture context from where they are called.
#3160
Comments
If Chrome ends up implementing this fix for a faulty assertion in the timer initialization steps, would it follow that setTimeout there would also end being affected by this? |
Function
and indirect eval
shuold not capture context from where they are called.Function
and indirect eval
should not capture context from where they are called.
Fixing this would make these two code snippets equivalent: // mod1.js
import { call } from "./mod2.js";
function run(f, x) {
// ... do something
call(f, x);
// ... do something else
}
// mod2.js
export function call(fn, arg) {
fn.call(arg);
} // mod1.js
function run(f, x) {
// ... do something
f.call(x);
// ... do something else
} |
What does this mean for |
What do you mean by looking at the caller? |
I should have been more precise, I meant the https://html.spec.whatwg.org/multipage/nav-history-apis.html#dom-location-href setter. Which uses the "incumbent document" for referrer and base URL purposes. |
DISCLAIMER: I only learned about the difference between active realm and incumbent realm today. This answer is based on my reading of the spec but I still have to figure out how to write a test that shows what the incumbent document used by This PR does not directly affect those operations (when not used together with However, this change is based on the assumption that HTML's usage of the incumbent document / settings object is "bad"/"legacy" (and looking at where it's used in HTML, it seems to only be old APIs?). Specifically, if this change gets consensus then it would be considered bad that these examples code behave differently: <!DOCTYPE html>
<iframe></iframe>
<script>
function set(obj, key, value) {
obj[key] = value;
}
</script>
<script>
globalThis.set(location, "href", "https://example.com");
</script> <!DOCTYPE html>
<iframe></iframe>
<script>
globalThis.set = Reflect.set;
</script>
<script>
globalThis.set(location, "href", "https://example.com");
</script> <!DOCTYPE html>
<iframe>
<!-- pretend this is an <iframe src=...> loading these contents -->
<script>
function set(obj, key, value) {
obj[key] = value;
}
</script>
</iframe>
<script>
frames[0].set(location, "href", "https://example.com");
</script> <!DOCTYPE html>
<iframe>
<!-- pretend this is an <iframe src=...> loading these contents -->
<script>
globalThis.set = Reflect.set;
</script>
</iframe>
<script>
frames[0].set(location, "href", "https://example.com");
</script> From my understanding of what the incumbent settings object is, HTML defines the 3rd example to behave differently from the other ones. "Good behavior" would be that they all behave the same, because they are all setting the same property on the same object with the same value. When setting A concrete difference for <!DOCTYPE html>
<iframe></iframe>
<script>
frames[0].eval(`location.href = "https://exmaple.com"`);
</script> currently the incumbent document is the outer one, while with this change it would be null and thus fallback to the backup incumbent settings object stack. |
I wonder if "incumbent" is still bad in light of shadow realms. I think when we initially stopped using it we did not anticipate adding more cross-realm behavior. If a shadow realm is handed a |
I personally think these two examples should behave the same: const descriptorSet = Object.getOwnPropertyDescriptor(location, "href").set;
const setLocation = url => descriptorSet.call(location, url);
shadowRealm.evaluate(`set => set("https://example.com")`)(setLocation); const descriptorSet = Object.getOwnPropertyDescriptor(location, "href").set;
const setLocation = descriptorSet.bind(location);
shadowRealm.evaluate(`set => set("https://example.com")`)(setLocation); since they are two different "styles" of writing the same code. This would mean not using the incumbent though. |
Here is why the web currently depends on incumbent: // code running on https://example.com/foo.html
const w = window.open();
w.location.href = "bar.html"; This is a common pattern. It opens a new window whose URL/base URL is If you resolve If we were designing the web from scratch, we would not add this affordance. New APIs, like This is actually not the only important use case for incumbent. It's just the simplest to explain, and the one that has the clearest parallel to the active script case. Check out this document I wrote previously for more. What does this mean for the question at hand? Well, basically there are two schools of thought.
Previously, we had been working toward (1). Firefox is very good about (1), in particular. And the spec follows them. Over the last few years, the pendulum has swung a little bit more toward (2). Chrome and Safari have not made movements toward improving incumbent support. And I think when they went to implement finalization registry callbacks, Firefox decided to pass on incumbent support there too? Not sure. Side note: (2) might be a little dangerous for the sourceDocument usage of incumbent (discussed in the doc), since it changes which document's security policies apply to a navigation. But, probably it's not a serious problem, since I think you can only use it to "impersonate" same-origin frames, and possibly only cooperating same-origin frames. How does this relate to active script? Well, as you note, they're distinct mechanisms, but basically the same question. Right now active script only influences <!-- https://example.com/index.html -->
<script src="sub/path/foo.js"></script> // sub/path/foo.js
import("./example1.mjs");
setTimeout(eval, 0, 'import(`./example2.mjs`)'); This will definitely import In whatwg/webidl#902 I tried to move us more toward (1) for active script, but have mostly given up. Some discussions there show no browsers propagating the active script in the way that PR suggests they should. |
As long as you can set @domenic I think that means that |
Thank you so much to both of you for the context. I will present this to the next TC39 plenary, showing the options we have and what the web platform needs. I still need to finish going through all the linked resources. I do not consider in scope any change to the machinery that lets However, given the current implementation divergence, I will propose that when it comes to ECMA-262 API, we update |
I want to make sure I understand what you're proposing. Given the following: function call(f, x) { f(x); }
import("./foo.mjs");
eval(`import("./foo.mjs");`);
(0, eval)(`import("./foo.mjs");`);
call(eval, `import("./foo.mjs");`); Currently all of these behave the same: they use the script's base URL. What case are you proposing to change? Your original post involves 4 files and I haven't been able to understand the implications. But your latest reply makes it sound like you're changing the simple cases above. |
I'm proposing to change the last two lines. Direct eval ( The last two lines on the other hand use indirect eval, that is meant to not capture any context about where it is being called (except for what is needed for web-platform-specific features, such as the |
OK. Well, I don't object, but I think everyone should carefully consider the implications of such changes, and of moving away from what is currently interoperable behavior, at least between Firefox and Chrome. Having all four lines behave the same is more in line with what I would expect. But it's probably web compatible to change such edge cases, if implementers are up for it. |
Description: Consider these two programs:
For any given string
str
, their behavior should be identical. Only directeval
should capture context from where it's being called, whileFunction
and indirecteval
(andsetTimeout
, even if it's in HTML) should not.However,
Function("import(...)")
is defined to use theFunction
/(0, eval)
's caller asimport()
's referrer, introducing a dynamic scope case other than direct eval.eshost Output:
Browsers behavior varies:
Function
,eval
, andsetTimeout
Function
andeval
, but notsetTimeout
You can find a test with various cases at https://github.com/nicolo-ribaudo/function-dynamic-scoping.
Proposed behavior:
In all these cases,
import()
's referrer should not capture any script or module, and instead fallback to using the current realm as the referrer.I believe that this is what is already happening in the following case, as described in #871:
The text was updated successfully, but these errors were encountered: