Skip to content

Proposal: Introduce InternalPromise to keep Promise integrity #80

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

Closed
Constellation opened this issue Aug 28, 2015 · 8 comments
Closed

Comments

@Constellation
Copy link
Member

Now, the loader spec highly utilizes the Promise for its internal pipeline.
However, Promise itself is exposed to users freely. That means users can replace / modify then method to break the internal loader behavior.

For example, in RequestTranslate step 7-c, https://whatwg.github.io/loader/#request-translate

Return the result of transforming p1 with a fulfillment handler that, when called with argument source, runs the following steps:

It returns the promise inside the the other promise's fulfill handler. It finally invokes the "then" method in ECMA 262 6.0 section 25.4.1.3.2 step 8.
http://ecma-international.org/ecma-262/6.0/#sec-promise-resolve-functions
Thus, by replacing the Promise.prototype.then, users can break the loader pipeline.

So, we would like to propose InternalPromise. It is completely separated instances from the Promise exposed to users. This means InternalPromise !== Promise and InternalPromise.prototype !== Promise.prototype etc. But InternalPromise has the same functionality to the Promise.

@Constellation
Copy link
Member Author

NOTE: Now, we introduced InternalPromise into WebKit to implement the prototype.

@caridy
Copy link
Contributor

caridy commented Aug 28, 2015

this is related to #78. instead of introducing InternalPromise we will make sure that we never expose those promises that we hold in internal slots. we will solve this soon.

@Constellation
Copy link
Member Author

@caridy:

How about this case?

Promise.prototype.then = function () {
    return new Promise(function (resolve, reject) { reject("Always reject"); });
};

I think this will break the loader pipeline. Because Promise#then will be implicitly called in the resolve function. http://ecma-international.org/ecma-262/6.0/#sec-promise-resolve-functions

@caridy
Copy link
Contributor

caridy commented Sep 25, 2015

@Constellation I think we are covered at the algo level. E.g.: https://whatwg.github.io/loader/#request-fetch step 9, where we use the pass-thru of the promise returned by the hook. We can certainly improve https://whatwg.github.io/loader/#reacting-to-promises to protect us against this. Isn't that enough? Do you have any suggestion to protect against this? any precedent on this issue? /cc @domenic

@Constellation
Copy link
Member Author

@caridy, @domenic
Ah, ok :D
So we need to change some code like https://whatwg.github.io/loader/#request-translate 7-e's Return p1. because it finally invokes then, right?

@caridy
Copy link
Contributor

caridy commented Sep 25, 2015

correct, we want to guarantee that no matter how the promise gets resolved, the logic in 7.d.i and 7.d.2 gets executed. The same applies to RequestFetch(), RequestTranslate() and RequestInstantiate().

I plan to add some notes (now that ecmarkup supports notes inside algos) to provide more details.

@Constellation
Copy link
Member Author

@caridy:
Thanks for your clarification. Looks good to me too.

@caridy
Copy link
Contributor

caridy commented Oct 23, 2015

I think we can close this one, we have reduced the amount of tricks we are doing with promises, and we will continue doing so as part of #97

@caridy caridy closed this as completed Oct 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants