Test Promise thenable's onResolve realm

[jridgewell]

Re: #2910, I'm not able to find a similar test that checks the realm of the onResolve/onReject when using a cross realm thenable:

var other = $262.createRealm().global;

var then = other.eval('(onThen, onCatch) => { globalThis.onThen = onThen; globalThis.onCatch = onCatch; }');

Promise.resolve({
  then: then
});

Promise.resolve().then(() => {
  assert.sameValue(other.onThen.constructor, then.constructor);
  assert.sameValue(other.onCatch.constructor, then.constructor);
}).then($DONE, $DONE);

I believe this was part of tc39/ecma262#1597?

[mhofman]

This is not equivalent to the test proposed in #2910, right?

This test seem to use the realm of the then function of the thenable to create the onResolve / onReject callbacks, not the realm of the finally argument.

The equivalent would be:

var other = $262.createRealm().global;

var otherThen = other.eval('() => {}');

var onThen, onCatch;

Promise.prototype.then.call({
  then(onThen_, onCatch_) {
    onThen= onThen_;
    onCatch= onCatch_;
  }
}, otherThen);

assert.sameValue(thenFinally.constructor, onFinally.constructor);
assert.sameValue(catchFinally.constructor, onFinally.constructor);

Which is highlighting what I don't understand. I believe it wouldn't be a concern to use the realm of the then of thenable to construct the callback, as that wouldn't reveal anything to the thenable it didn't already have. From what I understand, the concern is that just having access to a callable from another realm should not give you the ability to create functions in that realm.

[jridgewell]

The equivalent would be:

Yes, but not quite. then can't accept a thenable as the this, it must be a Promise instance. And assimilation doesn't respect the current instance's .then value. The thenFinally in #2910 is checking a synthetic function's realm, which is easily visible in finally because it attempts to call the instance's current .then with the synthetic functions.

The only way to access the synthetic functions in then (and Promise.resolve, because they use the same abstractions) is to return a thenable from a onResolve:

Promise.resolve().then(() => {
  return {
    then(onThen) {
      // onThen is a synthetic function. What realm does it belong to?
    }
  };
});

The only way to test, then, is to check if the realm of onThen is the same as the thenable's .then method, because the thenable's realm doesn't matter.

[mhofman]

So one thing I still don't understand is why isn't the realm of Promise.prototype.then or of Promise.resolve used to create the callback? In which cases is it unavailable such that we have to fallback onto something else?

[jridgewell]

There's a lot of history in:

• https://bugzilla.mozilla.org/show_bug.cgi?id=1663090
• https://bugzilla.mozilla.org/show_bug.cgi?id=1674505
• Sort out interaction of promises with document unloading and browsing context teardown whatwg/html#2621
• Do not execute scripts in navigated-away-from documents whatwg/html#1989

Essentially, if we used Promise.prototype.then, HTML will have a bad time if the thenable's .then's realm is dying. AMP actually gets bitten by this a lot in our test suites, because we throw away iframes all the time, and if the promise reactions ran after the iframe is cleaned, it causes all kinds of failures.

[mhofman]

Thanks for the links, still going through them.

From a quick read, this sound like an HTML specific decision to not process enqueued promise jobs under some circumstances, which ends up affecting the specified scoping of JavaScript promises. Feels weird to me to have dynamic scoping wart in JS for this. Afaik there are other issues with detached iframes, like debugger not working anymore.

Naïve question, why can't promises in the queue of a detached document be rejected? Would that actually not be web compatible since it seems currently the behavior of some browsers is to hang?

[jridgewell]

this sound like an HTML specific decision to not process enqueued promise jobs under some circumstances, which ends up affecting the specified scoping of JavaScript promises.

It's a security invariant, too. Imagine you were able to grab the global postMessage or fetch because finally invoked you with the wrong context. In this case, using the realm of the thenable's .then ensures that .then can't get access to another realm though the synthetic callbacks passed to it.

Feels weird to me to have dynamic scoping wart in JS for this.

Easing the integration for our most important consumer is important.

why can't promises in the queue of a detached document be rejected? Would that actually not be web compatible since it seems currently the behavior of some browsers is to hang?

See whatwg/html#2621 (comment)

[mhofman]

Right, dynamic scoping is a security issue, and I don't see how the current approach satisfies that invariant.

I wrote the following test: https://gist.github.com/mhofman/df7047ee89942038cbc4629c41b8c7e0

First, implementations seem inconsistent, so I don't think we should claim the current behavior is beyond repair.

The way I see it, the truly sane value for any of those is yellow, as that's what a userland library would be able to produce. Anything else amounts to dynamic scoping.

The least bad of those is probably blue (v8 and sm), as it doesn't reveal a realm to the blue function that wasn't specifically introduced.

red as it seems implemented by JSC and engine262 appears a little strange at first, but explainable if the realm of the explicit then call is ignored, which make it more similar to my expected yellow result. It's still unexpected from the program's perspective.

Thankfully purple is currently nowhere in the results, as that would be truly dynamic, and spooky action at a distance. The concern is that from what I understand it's the behavior expected from the proposed change to finally.

[jridgewell]

Thankfully purple is currently nowhere in the results, as that would be truly dynamic, and spooky action at a distance. The concern is that from what I understand it's the behavior expected from the proposed change to finally.

I don't see how the blue results (which you consider least bad) and a purple result are materially different. They're both using a realm that isn't the currently-invoked function's. If we didn't have finally, I'd be calling the thenable's .then directly with my purple onFinally, so it didn't really expose a new realm to the thenable.

Consider instead https://output.jsbin.com/hidozel/quiet. In this case, we have an unexpected difference between then and finally due to the finally using finally's realm instead of the callback's.

[mhofman]

They're both using a realm that isn't the currently-invoked function's

blue is the realm of the invoked function, hence why it's acceptable.

I'd be calling the thenable's .then directly with my purple onFinally

Sure, but neither example calls blue's then directly with the purple function. In both cases the thenable gets adopted into a native promise first, and the function given to the thenable's then is a spec created function to perform the adoption. Which is why I argue that this spec created function should have the realm of the Promise function invoked by the program that causes the adoption of the thenable.

Consider instead https://output.jsbin.com/hidozel/quiet. In this case, we have an unexpected difference between then and finally due to the finally using finally's realm instead of the callback's.

What is the difference? I'm seeing the following in the console in Chrome 96:

Yay from then!
Yay from finally!

Regardless, I'm not arguing for a difference between the two. I'm arguing that they should both behave the same, but that if a thenable is involved, that the user code's thenable is not able to get access to a function constructor they wouldn't have had access to otherwise. In the example I gave, I believe it should all print yellow.

[codehag]

I think we do have a web compatibility issue though I would have to look more deeply about which browser has it right -- there may be pressure to align with sm/chrome if there is a major library that relies on this behavior (hopefully unlikely) so we may have to follow web reality in that case -- otherwise this might be a correction issue. Without looking deeply i think SM/chrome might be correct so that may not be an issue? It looks like we are missing a test for this on promise.then as justin pointed out. I think we are in agreement that we likely need to align on this.

[codehag]

Also thank you mhofman for the test, that was interesting to read.

[mhofman]

From a conceptual point of view, I think JSC is closer to being correct, as it's the creator of the adopting promise which creates the callback function in its own realm and gives them to the thenable. However what the implication is on dying realms and existing code, I'm not sure. It's likely some Chrome/Firefox specific programs would be impacted if the realm of the adopting promise was used instead of the realm of the thenable's then.

[codehag]

At present, I disagree, both regarding that the answer here should be red or yellow. I believe chrome and firefox have the intended behaviour, but maybe i spent too much time looking at it. The binding is explicit to the blue realm, that it should change to the red or yellow realm seems odd and exposes multiple realms when there should only be one. However, I am on break starting today. I will pick this thread up again once i am back from winter break.

[mhofman]

My opinion derives from what a userland library implementing Promise would likely do if the only operation available was "EnqueueJob". The library in the adopting realm (yellow) would likely just create a function using syntax, and pass it to then. In effect having a blue result here is the equivalent of the library doing new (then.constructor) to create that function instead.

While I find that surprising as form of dynamic scoping, it does indeed not reveal functions from another realm to blue's then.

Enjoy the break!