Promise.all() will have unexpected behavior if Array.prototype.then exists and is callable (?)

[smikes]

Consider the case where Promise.all([p1, p2]) is called, and p1 and p2 are both unsettled (non-foreign) promises. Then Promise.all installs an onFulfilled handler on p1, p2 which is a Promise.all Resolve Element Function

Assuming both p1, p2 eventually are fulfilled, whichever one fulfills second triggers the final steps of that algorithm, specifically:

 10 If remainingElementsCount.[[value]] is 0,
  a Let valuesArray be CreateArrayFromList(values).
  b Return the result of calling the [[Call]] internal method of promiseCapability.[[Resolve]] with undefined as thisArgument and (valuesArray) as argumentsList.
 11 Return undefined.

CreateArrayFromList creates an Array object. If Array.prototype.then exists and is callable, then instead of fulfilling the Promise with the array, the "foreign thenable" code path in promiseCapability.[[Resolve]] will be exercised, and the Promise returned from Promise.all will be "locked in" to whatever thenable Array.prototype.then looks like.

It's hard for me to think of a good design reason that I would want to add a method then to Array.prototype or Object.prototype, so this is not something most people will encounter in normal development.

Now this is all obviously insane, and possibly just a minor nuisance, but there may be a security risk here in that an attacker who can modify Array.prototype (which is already quite bad, I understand!) would be able to break any Promise.all resolutions in the same ... Realm? Vat? execution context, maybe.

/cc @domenic
Edited above to clarify meaning.

[domenic]

Nope, this is working as intended. You should never be able to fulfill with a thenable; that's why resolving with a thenable triggers its then function.

[domenic]

An analogous situation: if you remove [Symbol.iterator] from Array.prototype, then passing an array in to Promise.all will not work as you expect.

[smikes]

Although I see your point, I believe this behavior will be surprising to many -- similar to Promise.race([]) returning a never-fulfilled promise; it makes sense, but it isn't expected.

In particular, the difference between deleting Symbol.iterator and adding Array.prototype.then is that deleting Symbol.iterator is obviously bad, and will visibly break lots of syntax; while adding a 'then' to a object prototype will only affect Promise.all.

Psychology plays into this as well. It's easier to understand that removing something causes damage than adding something seemingly innocuous which happens to have a magic name.

Consider:

var p1 = Promise.resolve(1);
var p2 = Promise.resolve(2);
var logger = console.log.bind(console);

Promise.all([p1, p2]).then(logger); // outputs [1, 2]

Array.prototype.then = function (a, b) { a('foo'); }

Promise.all([p1, p2]).then(logger); // outputs foo

Or how about this:

Object.prototype.then = function (a, b) { a('baz'); }

Promise.resolve({}).then(logger);  // outputs baz

I think that's pretty surprising. Especially if the definition of then comes from loading a library or some other code.

This behavior is visible in Chrome 38.0.2125.104 m

[smikes]

i thought of two more:

O.p.then = function (a, b) { b(new SyntaxError("illegal character")); }

O.p.then = function () {} // all promises fulfilled with an object will pend forever

And another:

O.p.then = function (a) { console.log(JSON.stringify(this)); a(this); } // log all promise fulfillment args

[domenic]

Shrug. This is just how promises work. then has essentially become a reserved property name.

[smikes]

OK. I will add tests to cover this behavior.