Skip to content

Commit

Permalink
Fix issues with Promise.all().
Browse files Browse the repository at this point in the history
The specification for Promise.all() contains some bugs.
Fix these as suggested in
domenic/promises-unwrapping#89

This might need to be revisited if the next revision of the spec
addresses these issues in a different way.
  • Loading branch information
cscott committed Feb 9, 2014
1 parent cd7afb7 commit 280230d
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 4 deletions.
11 changes: 7 additions & 4 deletions es6-shim.js
Original file line number Diff line number Diff line change
Expand Up @@ -1095,7 +1095,10 @@
});

var _promiseAllResolver = function(index, values, capability, remaining) {
var done = false;
return function(x) {
if (done) { return; } // protect against being called multiple times
done = true;
values[index] = x;
if ((--remaining.count) === 0) {
var resolve = capability.resolve;
Expand All @@ -1117,16 +1120,13 @@
typeof it.next !== 'function') {
throw new TypeError('bad iterable');
}
var values = [], remaining = { count: 0 };
var values = [], remaining = { count: 1 };
for (var index = 0; ; index++) {
var next = it.next();
if (next === null || typeof next !== 'object') {
throw new TypeError('bad iterator');
}
if (next.done) {
if (index === 0) {
resolve(values);
}
break;
}
var nextPromise = C.cast(next.value);
Expand All @@ -1136,6 +1136,9 @@
remaining.count++;
nextPromise.then(resolveElement, capability.reject);
}
if ((--remaining.count) === 0) {
resolve(values); // call w/ this===undefined
}
} catch (e) {
reject(e);
}
Expand Down
96 changes: 96 additions & 0 deletions test/promise/all.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,4 +89,100 @@ describe("Promise.all", function () {
}
);
});

// test cases from
// https://github.com/domenic/promises-unwrapping/issues/89#issuecomment-33110203
var tamper = function(p) {
return Object.assign(p, {
then: function(fulfill, reject) {
fulfill('tampered');
return Promise.prototype.then.call(this, fulfill, reject);
}
});
};
var failIfThrows = function(done) {
return function(e) { done(e); };
};

it("should be robust against tampering (1)", function(done) {
var g = [ tamper(Promise.resolve(0)) ];
// Prevent countdownHolder.[[Countdown]] from ever reaching zero
Promise.all(g).
then(function() { done(); }, failIfThrows(done));
});

it("should be robust against tampering (2)", function(done) {
var g = [
Promise.resolve(0),
tamper(Promise.resolve(1)),
Promise.resolve(2).then(function() {
assert(!fulfillCalled, 'should be resolved before all()');
}).then(function() {
assert(!fulfillCalled, 'should be resolved before all()');
})['catch'](failIfThrows(done))
];
// Promise from Promise.all resolved before arguments
var fulfillCalled = false;
Promise.all(g).
then(function() {
assert(!fulfillCalled, 'should be resolved last');
fulfillCalled = true;
}).
then(done, failIfThrows(done));
});

it("should be robust against tampering (3)", function(done) {
var g = [
Promise.resolve(0),
tamper(Promise.resolve(1)),
Promise.reject(2)
];
// Promise from Promise.all resolved despite rejected promise in arguments
Promise.all(g).
then(function(v) {
throw new Error('should not reach here!');
}, function(e) {
assert.strictEqual(e, 2);
}).then(done, failIfThrows(done));
});

it("should be robust against tampering (4)", function(done) {
var hijack = true;
var actualArguments = [];
var P = function(resolver) {
if (hijack) {
hijack = false;
Promise.call(this, function(resolve, reject) {
return resolver(function(values) {
// record arguments & # of times resolve function is called
actualArguments.push(values.slice());
return resolve(values);
}, reject);
});
} else {
Promise.call(this, resolver);
}
};
if (!P.__proto__) { return done(); } // skip test if on IE < 11
P.__proto__ = Promise;
P.prototype = Object.create(Promise.prototype);
P.prototype.constructor = P;
P.cast = function(p) { return p; };

var g = [
Promise.resolve(0),
tamper(Promise.resolve(1)),
Promise.resolve(2)
];

// Promise.all calls resolver twice
P.all(g)['catch'](failIfThrows(done));
Promise.
resolve().
then(function() {
assert.deepEqual(actualArguments, [ [ 0, 'tampered', 2 ] ]);
}).
then(done, failIfThrows(done));
});

});

0 comments on commit 280230d

Please sign in to comment.