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

Waiting for promises might not work correctly if the page messes with its standard objects #45

Closed
bzbarsky opened this issue Oct 29, 2015 · 4 comments

Comments

@bzbarsky
Copy link

The spec says:

The result of waiting for all of a collection of promises is a promise created by calling Promise.all(promiseArray), where promiseArray is that collection in array form and we use the initial value of Promise.all.

If the intent is that this actually work reliably, then this is not enough. The output of the initial value of Promise.all in this case will depend on the current values of at least the following: Promise[Symbol.species], %ArrayPrototype%[Symbol.iterator], %ArrayIteratorPrototype%.next (if no one has changed out the array iterator on you), Promise.resolve, Promise.prototype.then (if no one has changed out Promise.resolve on you).

Really, you probably just want a different algorithm here.

@domenic
Copy link
Member

domenic commented Nov 4, 2015

Yeah, this whole section is far too wishy-washy...

Do you want to try defining it over in Web IDL (#27)? I'd happily pull the section and redirect incoming links over there...

@bzbarsky
Copy link
Author

bzbarsky commented Nov 4, 2015

I'm fine with defining it whereever; I'm just not sure yet how to define it.

@ricea
Copy link

ricea commented Nov 30, 2017

There is also a security concern if an algorithm uses "waiting for all" without protection against re-entrancy.

@bzbarsky
Copy link
Author

It's worth checking what UAs actually do. SpiderMonkey has an API, which Gecko uses, which has the following characteristics:

  1. Takes a list of promises in the form of a spec List, effectively, not an Array. This means it does not depend on any of the array iterator stuff.
  2. Assumes that all the things in the list are in fact Promise instances; it's the caller's responsibility to ensure this. This means it does not need to Promise.resolve.
  3. Always uses the canonical Promise constructor to create the return value Promise, thus avoiding a dependency on Promise[Symbol.species].
  4. Directly adds reactions to the Promise instances in the list, so doesn't depend on Promise.prototype.then. Also doesn't involve constructing a return-value promise, unlike calling then, so eliminates that dependency on Promise[Symbol.species].

Looks to me like we use this API in the following spec-related places:

Of those, the former links to https://www.w3.org/2001/tag/doc/promises-guide/#waiting-for-all and the latter does not.

domenic added a commit that referenced this issue Sep 7, 2018
domenic added a commit that referenced this issue Oct 26, 2018
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

No branches or pull requests

3 participants