Skip to content
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

special-case Symbol.asyncIterator as method name #2619

Closed
warner opened this issue Mar 11, 2021 · 1 comment · Fixed by #2623
Closed

special-case Symbol.asyncIterator as method name #2619

warner opened this issue Mar 11, 2021 · 1 comment · Fixed by #2623
Assignees
Labels
enhancement New feature or request SwingSet package: SwingSet

Comments

@warner
Copy link
Member

warner commented Mar 11, 2021

What is the Problem Being Solved?

We can address #2092 (using Symbol.asyncIterator as a method name) thoroughly by the serialization change described in #2481 (which would also fix #2612), but it's going to take a little while because it touches many different codepaths through the kernel and comms layer. We can apply a quick fix by special-casing that one method name.

Description of the Design

Liveslots (in queueMessage) will add:

  if (prop === Symbol.asyncIterator) {
    prop = 'Symbol.asyncIterator';
  }

On the receiving side, in deliver, we'll add:

  if (method === 'Symbol.asyncIterator') {
    method = Symbol.asyncIterator;
  }

This will, of course, prohibit access to a method named with the string Symbol.asyncIterator, because anyone who invokes E(remote)['Symbol.asyncIterator']() will wind up invoking target[Symbol.asyncIterator]() instead of target['Symbol.asyncIterator'](). This falls into the same lack-of-JS-fidelity bug bucket that prevents cross-machine invocation of e.g. E(remote)['method:with:colon'](), tracked in #2612.

Security Considerations

There shouldn't be any security consequences of this, since we consider Symbol.asyncIterator to be an ambient ability. However we need to pay attention to the compatibility consequences. We'll have three phases:

Assuming we don't consider upgrading some vats and not others (which we don't even have the ability to do now, since all vats get the same liveslots), then the question is compatibility between separate machines (chains/ag-solo) that might be at different stages.

If a stage-2 machine attempts to invoke the symbol method on a stage-1 machine, it will be delivered as a string method invocation, which will probably fail safely. If a stage-1 machine happened to try to invoke E(target)['Symbol.asyncIterator'](), which would be pretty unlikely, it would be delivered as a symbol method invocation, which would be surprising. All other methods will interoperate just fine.

A stage-1/2 machine won't be able to interoperate with a stage-3 machine at all, unless we take on the task of trying to do format-translation between the two. This could be done entirely within the comms vat, but it'd be kind of gross so I'm hoping to avoid it.

Test Plan

Normal unit tests.

@warner warner added enhancement New feature or request SwingSet package: SwingSet labels Mar 11, 2021
@warner warner self-assigned this Mar 11, 2021
@erights
Copy link
Member

erights commented Mar 11, 2021

As a short term stop gap until we fix #2092 , I love this plan. Thanks!

warner added a commit that referenced this issue Mar 12, 2021
Liveslots special-cases this one symbol by converting it into a (string)
method name of `"Symbol.asyncIterator"` for `syscall.send`, and back again
during `dispatch.deliver`. This has several limitations:

* no other Symbols are accepted yet
* `E(target)[Symbol.asyncIterator](args)` and
`E[target]['Symbol.asyncIterator'](args)` will invoke the same target method,
which is clearly wrong
* a method named `'Symbol.asyncIterator'` (as a string) cannot be invoked
remotely

This should all be fixed someday by #2481.

closes #2619
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request SwingSet package: SwingSet
Projects
None yet
2 participants