Overriden thens?

[domenic]

Hoping for feedback by @erights especially on this issue raised by @wycats.

As specced, we do one-level unwrapping of true promises in Resolve. That is, we transfer over [[Following]], [[Value]], and [[Reason]]. In contrast, thenables get stored in [[Value]], and only unwrapped later, inside UpdateDerived (via Then).

However, this one-level unwrapping is somewhat "magic". In particular, it ignores overriden then methods, e.g.:

var p = new Promise(() => {});
p.then = f => f(5);

var q = new Promise(resolve => resolve(p));

// q is forever-pending, instead of fulfilled with 5.

This is a bit odd. Even worse, overwritten thens can "fool" (in some sense) Promise.cast:

Promise.cast(p).then(x => console.log(x)); // logs 5, synchronously

Lest you think the fix is just to make then non-configurable, also consider the case

var p2 = Object.create(new Promise(() => {}));
p2.then = f => f(5);

This p2 is just as bad as p, slipping through all our IsPromise checks, while behaving badly with its synchronous-then and unexpected behavior when used as an argument to resolve.

---

Site note: recall that the one-level unwrapping exists mainly for efficiency, as @erights pointed out on es-discuss. Otherwise we would just store the promise in [[Value]] like we do the thenables. (I think this would lead to killing [[Following]] as well.)

But even if we moved the unwrapping totally over to the then side, that would still leave the question of how to unwrap the overwritten-then promise: using its overwritten then, or reaching into its internal state as we do now.

---

Background note: @wycats's desire, which I do not really share, is that thenables should be able to fully participate in the adopt-internal-properties assimilation path. If I recall correctly, he envisions unique symbols for [[Following]], [[Value]], and [[Reason]]. In particular, he doesn't like it when one promise reaches into another promise's internal state, saying that it should work through the public API to do so instead, so that promises and non-promise thenables are on the same level playing field. He then brought up the overwritten-then case, which highlights the specific way in which promise objects (i.e. those created via Promise[@@create]()) are not on the same playing field as thenables. I pointed out that they were not supposed to be, since real promises were supposed to be high integrity, but then realized that because of "attacks" like the above, we don't really have a good way of guaranteeing high integrity promises.

[erights]

See the Q-like high integrity promises in makeQ at https://code.google.com/p/es-lab/source/browse/trunk/src/ses/makeQ.js

These haven't yet been adjusted to be AP2-like, in that they still do their assimilation on the output side of .then. But I think that's orthogonal to the issues being raised here. Please see if they are vulnerable to any of the attacks you have in mind. Thanks.

[domenic]

@erights I think I see. Promise.cast (or Q, in makeQ) is only meant to ensure you have an instance of Promise, and since the makeQ promises deeply-freeze Promise (including Promise.prototype), Q produces safe promises. Presumably in ES6, someone who wanted safe promises would deeply-freeze Promise as well.

I think this is still susceptible to the following "attack" (not sure if it's actually dangerous):

import { def } from "ses";

def(Promise);

const p = Object.create(new Promise(() => {}), { then: { value(f) { f(5); } } });

Promise.cast(p).then(x => console.log(x)); // logs 5, synchronously

---

There is also the conceptual issue of whether promises with overriden thens should be treated as thenables, or as promises.

[erights]

On Sun, Sep 29, 2013 at 8:18 PM, Domenic Denicola
[email protected]:

@erights https:/erights I think I see. Promise.cast (or Q,
in makeQ) is only meant to ensure you have an instance of Promise, and
since the makeQ promises deeply-freeze Promise (including
Promise.prototype), Q produces safe promises. Presumably in ES6, someone
who wanted safe promises would deeply-freeze Promise as well.

I think this is still susceptible to the following "attack" (not sure if
it's actually dangerous):

If that attack would succeed, it would definitely be dangerous. Please look
back at the plan interference attack scenarios. However, that attack won't
succeed because p is not a promise, it is an object that inherits from a
promise. Promise.cast therefore must not return p itself.

import { def } from "ses";
def(Promise);
const p = Object.create(new Promise(() => {}), { then: { value(f) { f(5); } } });

Promise.cast(p).then(x => console.log(x)); // logs 5, synchronously

---

There is also the conceptual issue of whether promises with overriden thens
should be treated as thenables, or as promises.

—
Reply to this email directly or view it on GitHubhttps://issues/42#issuecomment-25335707
.

Text by me above is hereby placed in the public domain

Cheers,
--MarkM

[domenic]

OK, in that case there is a bug in the definition of Promise.cast. It currently works by storing the constructor that was used to create the promise object in the internal [[PromiseConstructor]] class, and returning the input if input.[[PromiseConstructor]] equals Promise (or, in general, this). This is not correct, it seems.

I am not sure what the best test would be. Perhaps add in a input.[[GetPrototypeOf]]() === this.prototype.

[erights]

In general, when the spec talks about getting the [[Foo]] internal property of bar, it means only to obtain an internal own property. Internal properties are not usually thought of as being subject to inheritance.

[domenic]

That can't be right; otherwise you would not be able to subclass anything. E.g. the [[MapData]] lookup for maps could not be own.

[erights]

I don't understand. [[MapData]] is on the instance. It is an own lookup. It is not inherited.

[domenic]

I believe it is inherited. (It might be time for @allenwb to step in here.) In other words, this should work:

class MyMap extends Map {}

const m = new MyMap();

m.set(1, 2);
m.get(1);

Consider e.g. http://people.mozilla.org/~jorendorff/es6-draft.html#sec-23.1.3.6:

1. Let M be the this value.
2. If Type(M) is not Object, then throw a TypeError exception.
3. If M does not have a [[MapData]] internal data property throw a TypeError exception.

It seems like if [[MapData]] was not inherited, this, i.e. m in the above example, would have no [[MapData]], so m.set would fail with a TypeError.

[erights]

The extends goes along with having the subclass constructor delegate some own instance initialization to the superclass constructor. The superclass constructor initializes these properties directly on the same instance that the subclass initializes.

Inheritance only comes into play in this pattern for the properties that are not per-instance. Those are inherited from prototypes.

[domenic]

Oh, right, that makes sense! Thanks for sticking with me on that.

[domenic]

So then, this should fail?

const m = Object.create(new Map());

m.set(1, 2); // Fails with a TypeError, because `m` has no `[[MapData]]`.

[erights]

Yes. (@allenwb, please confirm)

[allenwb]

Map's @@create method allocates an object that has a [[MapData]] internal data property.
See http://people.mozilla.org/~jorendorff/es6-draft.html#sec-23.1.2.2

That (static) method is inherited by MyMap and called by the new operator. Its state is initialized by the Map constructor http://people.mozilla.org/~jorendorff/es6-draft.html#sec-23.1.1.1

Because MyMap does not define a constructor body, one is automatically created for it that just does a super.constructor call (which will call the Map constructor). If you provide a constructor body, then you have to explicitly code the super.constructor call within it.

Object.create does not work to subclass built-ins. All it does is create an ordinary object instance and set it's [[Prototype]]. However, you could manually do:

MyMap.call(MyMap[Symbol.create]( ))

which is essentially equivalent to
new MyMap;

[allenwb]

Also,

Internal data properties, as used in the ES spec, are never inherited. They aren't even actually properties. They are just private state of objects that is provide in some implementation dependent manner. Inheriting the @@create method provides the behavior of allocating an object with the desired internal data properties.

[wycats]

Can someone tell me, succinctly, the benefit(s) of having both Promises and Thenables, with mostly equivalent, but occasionally slightly divergent semantics?

[erights]

If we didn't have legacy, we'd still have promises. Promises guarantee turn
isolation.

Assimilating thenables is a kludge we tolerate because of legacy. A
thenable can have any behavior, and so it not assumed to have any behavior
reliably.

On Tue, Oct 8, 2013 at 3:18 PM, Yehuda Katz [email protected]:

Can someone tell me, succinctly, the benefit(s) of having both Promises
and Thenables, with mostly equivalent, but occasionally slightly divergent
semantics?

—
Reply to this email directly or view it on GitHubhttps://issues/42#issuecomment-25932572
.

Text by me above is hereby placed in the public domain

Cheers,
--MarkM

[wycats]

What specific behavior of Promises are you trying to guarantee, and in what situations is it actually guaranteed?

[erights]

I thought you wanted succinct ;).

var p = Promise.cast(arb0);
assert(p === Promise.cast(p));
var p2 = p.then(arb1, arb2);
assert(p2 === Promise.cast(p2));

Where arb0, arb1, and arb2 are three variables holding arbitrary values contributed by our untrusted client.

No code contributed by arb0, arb1, or arb2 executes during the above code sequence. Assuming nothing else in the enclosing turn uses arb0, arb1, and arb2, no code contributed by these executes during this turn. All this is needed to protect against plan interference attacks.

[wycats]

Why is it important to treat "secure" promises differently here? Why couldn't we just do exactly the same thing. Implementations could of course use an optimized implementation for the actual cast if they can see that a DOM Promise (with an unmodified then) was provided.

[erights]

I do not understand the question. Differently from what? Exactly the same thing as what? Fewer pronouns and more explicit context please.

[wycats]

I am asking why we can't have Promise.cast do the same round-tripping through .then and the event loop that we do for thenables.

[erights]

Are you suggesting that

var p2 = Q(p0).then(v => p1);

where p1 is a promise, that p1.then would be called, in order for p2 to effectively follow p1? If so, you lose the ability to support lazy evaluation and promise pipelining. To support these, p2 must follow p1 by internal promise-to-promise interaction not mediated by p1.then. This applies to .flatMap too, or it wouldn't be flat.

[wycats]

Let me pop up a level. What I am suggesting is:

If there is a capability that "real" promises have that userland promises do not have, we should either:

• explain why the distinction pays for itself
• expose a protocol for userland promises to have the same capability

Your last comment says that there is, indeed, a capability that only true promises have ("internal promise-to-promise interaction not mediated by p1.then"). If we think this capability is necessary, again, we should either explain why we can't give it to userland promises or make it work.

[domenic]

I am closing this and opening a new issue to summarize. I think @wycats is probably right.