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

Make Async-from-Sync iterator object inaccessible to ECMAScript code #1172

Open
anba opened this Issue Apr 12, 2018 · 6 comments

Comments

Projects
None yet
3 participants
@anba
Contributor

anba commented Apr 12, 2018

25.1.4.1 CreateAsyncFromSyncIterator should not expose the Async-from-Sync iterator object to ECMAScript code.

Let's take this example:

async function* g() {
    yield* [];
}
g().next();

14.4.13 Runtime Semantics: Evaluation for YieldExpression : yield * AssignmentExpression.

  1. Let iteratorRecord be ? GetIterator(value, generatorKind).

with generatorKind = async.

7.4.1 GetIterator ( obj [ , hint [ , method ] ] )

3.a.ii.3. Return ? CreateAsyncFromSyncIterator(syncIteratorRecord).

25.1.4.1 CreateAsyncFromSyncIterator

  1. Return ? GetIterator(asyncIterator, async).

where asyncIterator is an Async-from-Sync iterator object. Its prototype is %AsyncFromSyncIteratorPrototype% and the prototype of %AsyncFromSyncIteratorPrototype% is %AsyncIteratorPrototype%. %AsyncIteratorPrototype% is accessible from ECMAScript code, which means it's possible to redefine %AsyncIteratorPrototype%[@@asyncIterator]:

var AsyncIteratorPrototype = Object.getPrototypeOf(async function*(){}.constructor.prototype.prototype);
var AsyncIteratorPrototype_asyncIterator = Object.getOwnPropertyDescriptor(AsyncIteratorPrototype, Symbol.asyncIterator).value;
Object.defineProperty(AsyncIteratorPrototype, Symbol.asyncIterator, {
    get() {
        print("HI");
        return AsyncIteratorPrototype_asyncIterator;
    }
});

That in turn means the GetIterator call in CreateAsyncFromSyncIterator can be used to get access to the Async-from-Sync iterator object. And I don't think we don't want to that to happen.

Note: All engines implementing async generators assume Async-from-Sync iterator object are never directly accessible to ECMAScript code, so the above code won't print "HI" to the console.

@littledan

This comment has been minimized.

Show comment
Hide comment
@littledan

littledan Apr 12, 2018

Member

@anba Any idea for a fix? Is there some change we should make to yield* in an async generator, for example?

Cc @domenic @caitp @gschakov @arai-a

Member

littledan commented Apr 12, 2018

@anba Any idea for a fix? Is there some change we should make to yield* in an async generator, for example?

Cc @domenic @caitp @gschakov @arai-a

@anba

This comment has been minimized.

Show comment
Hide comment
@anba

anba Apr 12, 2018

Contributor

Any idea for a fix?

CreateAsyncFromSyncIterator, step 3 could be changed to:

  1. Let nextMethod be %AsyncFromSyncIteratorPrototype_next%.
  2. Return the Record {[[Iterator]]: asyncIterator, [[NextMethod]]: nextMethod, [[Done]]: false}.

where %AsyncFromSyncIteratorPrototype_next% is a new intrinsic for 25.1.4.2.1 %AsyncFromSyncIteratorPrototype%.next.

Or alternatively add %AsyncFromSyncIteratorPrototype%[@@asyncIterator] with the same steps as 25.1.3.1 %AsyncIteratorPrototype% [ @@asyncIterator ] ( ).

Contributor

anba commented Apr 12, 2018

Any idea for a fix?

CreateAsyncFromSyncIterator, step 3 could be changed to:

  1. Let nextMethod be %AsyncFromSyncIteratorPrototype_next%.
  2. Return the Record {[[Iterator]]: asyncIterator, [[NextMethod]]: nextMethod, [[Done]]: false}.

where %AsyncFromSyncIteratorPrototype_next% is a new intrinsic for 25.1.4.2.1 %AsyncFromSyncIteratorPrototype%.next.

Or alternatively add %AsyncFromSyncIteratorPrototype%[@@asyncIterator] with the same steps as 25.1.3.1 %AsyncIteratorPrototype% [ @@asyncIterator ] ( ).

@littledan

This comment has been minimized.

Show comment
Hide comment
@littledan

littledan Apr 12, 2018

Member

It sounds like the first option would be what browsers currently implement. Is that your understanding?

Member

littledan commented Apr 12, 2018

It sounds like the first option would be what browsers currently implement. Is that your understanding?

@anba

This comment has been minimized.

Show comment
Hide comment
@anba

anba Apr 12, 2018

Contributor

Both options result in the same behaviour, except the first one is easier to reason about.

Contributor

anba commented Apr 12, 2018

Both options result in the same behaviour, except the first one is easier to reason about.

@caitp

This comment has been minimized.

Show comment
Hide comment
@caitp

caitp Apr 12, 2018

Contributor

In particular, v8's implementation doesn't include tc39/proposal-async-iteration@201c814, so there's no subsequent GetIterator(asyncIterator, async) after the Async-from-Sync Iterator is created.

The actual behaviour:

  1. method = obj[Symbol.asyncIterator] -> undefined
  2. If method is undefined/null
    1. syncIterator = obj[Symbol.iterator]() -> [object Array Iterator]
    2. asyncIterator = %_CreateAsyncFromSyncIterator(syncIterator) -> [object Async-from-Sync Iterator]
  3. next = asyncIterator.next -> %AsyncFromSyncIteratorPrototype.next%

But the way the spec is written, we should be loading @@asyncIterator after %_CreateAsyncFromSyncIterator(syncIterator), which would potentially call an accessor and expose the Async-from-Sync Iterator receiver to it.

AFAIU we don't want to expose the Async-from-Sync Iterator directly, so it only makes sense to avoid recursing into the GetIterator algorithm. Also, the GetIterator in step 3 is potentially recursive/non-terminating behaviour, so we also probably don't want that.

+1 to @anba's suggestion which doesn't include a GetIterator at the end of CreateAsyncFromSyncIterator, because it's easier to reason about if we don't jump back and forth between those 2 algorithms, imho

Contributor

caitp commented Apr 12, 2018

In particular, v8's implementation doesn't include tc39/proposal-async-iteration@201c814, so there's no subsequent GetIterator(asyncIterator, async) after the Async-from-Sync Iterator is created.

The actual behaviour:

  1. method = obj[Symbol.asyncIterator] -> undefined
  2. If method is undefined/null
    1. syncIterator = obj[Symbol.iterator]() -> [object Array Iterator]
    2. asyncIterator = %_CreateAsyncFromSyncIterator(syncIterator) -> [object Async-from-Sync Iterator]
  3. next = asyncIterator.next -> %AsyncFromSyncIteratorPrototype.next%

But the way the spec is written, we should be loading @@asyncIterator after %_CreateAsyncFromSyncIterator(syncIterator), which would potentially call an accessor and expose the Async-from-Sync Iterator receiver to it.

AFAIU we don't want to expose the Async-from-Sync Iterator directly, so it only makes sense to avoid recursing into the GetIterator algorithm. Also, the GetIterator in step 3 is potentially recursive/non-terminating behaviour, so we also probably don't want that.

+1 to @anba's suggestion which doesn't include a GetIterator at the end of CreateAsyncFromSyncIterator, because it's easier to reason about if we don't jump back and forth between those 2 algorithms, imho

@anba

This comment has been minimized.

Show comment
Hide comment
@anba

anba Apr 12, 2018

Contributor

Thanks for digging out the commit which introduced this behaviour!

Contributor

anba commented Apr 12, 2018

Thanks for digging out the commit which introduced this behaviour!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment