From 7947be7803a3a3848079b271314c587508a3e5db Mon Sep 17 00:00:00 2001
From: Brian Warner <warner@lothar.com>
Date: Fri, 12 Mar 2021 00:46:15 -0800
Subject: [PATCH] fix(swingset): allow Symbol.asyncIterator as a method name

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
---
 packages/SwingSet/docs/liveslots.md       |  2 +
 packages/SwingSet/src/kernel/liveSlots.js | 13 ++++
 packages/SwingSet/test/test-liveslots.js  | 76 +++++++++++++++++++++++
 3 files changed, 91 insertions(+)

diff --git a/packages/SwingSet/docs/liveslots.md b/packages/SwingSet/docs/liveslots.md
index b22a0eff776..f57e5f413d2 100644
--- a/packages/SwingSet/docs/liveslots.md
+++ b/packages/SwingSet/docs/liveslots.md
@@ -51,6 +51,8 @@ const p = E(target).foo('arg1');
 p.then(obj2 => E(obj2).bar('arg2'))
 ```
 
+The method name being invoked can be any string, or the special `Symbol.asyncIterator`. All other Symbol-named methods are currently rejected, but see #2612 for plans to accept anything that JavaScript will accept.
+
 All vats are subject to the "tildot transformation", which means these calls may also be written like:
 
 ```js
diff --git a/packages/SwingSet/src/kernel/liveSlots.js b/packages/SwingSet/src/kernel/liveSlots.js
index 6c1059a056a..6d0b352f28b 100644
--- a/packages/SwingSet/src/kernel/liveSlots.js
+++ b/packages/SwingSet/src/kernel/liveSlots.js
@@ -360,6 +360,15 @@ function build(syscall, forVatID, cacheSize, vatPowers, vatParameters) {
   }
 
   function queueMessage(targetSlot, prop, args, returnedP) {
+    if (typeof prop === 'symbol') {
+      if (prop === Symbol.asyncIterator) {
+        // special-case this Symbol for now, will be replaced in #2481
+        prop = 'Symbol.asyncIterator';
+      } else {
+        throw Error(`arbitrary Symbols cannot be used as method names`);
+      }
+    }
+
     const serArgs = m.serialize(harden(args));
     const resultVPID = allocatePromiseID();
     lsdebug(`Promise allocation ${forVatID}:${resultVPID} in queueMessage`);
@@ -459,6 +468,10 @@ function build(syscall, forVatID, cacheSize, vatPowers, vatParameters) {
     // the same vpid as a result= twice, or getting a result= for an exported
     // promise (for which we were already the decider).
 
+    if (method === 'Symbol.asyncIterator') {
+      method = Symbol.asyncIterator;
+    }
+
     const args = m.unserialize(argsdata);
 
     // If the method is missing, or is not a Function, or the method throws a
diff --git a/packages/SwingSet/test/test-liveslots.js b/packages/SwingSet/test/test-liveslots.js
index 65775d3f89b..e34a49d26ef 100644
--- a/packages/SwingSet/test/test-liveslots.js
+++ b/packages/SwingSet/test/test-liveslots.js
@@ -477,3 +477,79 @@ test('liveslots retires result promise IDs after resolve to data', async t => {
 test('liveslots retires result promise IDs after reject', async t => {
   await doResultPromise(t, 'reject');
 });
+
+test('liveslots vs symbols', async t => {
+  const { log, syscall } = buildSyscall();
+
+  function build(_vatPowers) {
+    return Far('root', {
+      [Symbol.asyncIterator](arg) {
+        return ['ok', 'asyncIterator', arg];
+      },
+      good(target) {
+        E(target)[Symbol.asyncIterator]('arg');
+      },
+      bad(target) {
+        return E(target)
+          [Symbol.for('nope')]('arg')
+          .then(
+            _ok => 'oops no error',
+            err => ['caught', err],
+          );
+      },
+    });
+  }
+  const dispatch = makeDispatch(syscall, build);
+  t.deepEqual(log, []);
+  const rootA = 'o+0';
+  const target = 'o-1';
+
+  // E(root)[Symbol.asyncIterator]('one')
+  const rp1 = 'p-1';
+  dispatch.deliver(rootA, 'Symbol.asyncIterator', capargs(['one']), 'p-1');
+  await waitUntilQuiescent();
+  t.deepEqual(log.shift(), {
+    type: 'resolve',
+    resolutions: [[rp1, false, capargs(['ok', 'asyncIterator', 'one'])]],
+  });
+  t.deepEqual(log, []);
+
+  // root~.good(target) -> send(methodname=Symbol.asyncIterator)
+  dispatch.deliver(
+    rootA,
+    'good',
+    capargs([{ '@qclass': 'slot', index: 0 }], [target]),
+    undefined,
+  );
+  await waitUntilQuiescent();
+  t.deepEqual(log.shift(), {
+    type: 'send',
+    targetSlot: target,
+    method: 'Symbol.asyncIterator',
+    args: capargs(['arg']),
+    resultSlot: 'p+5',
+  });
+  t.deepEqual(log.shift(), { type: 'subscribe', target: 'p+5' });
+  t.deepEqual(log, []);
+
+  // root~.bad(target) -> error because other Symbols are rejected
+  const rp2 = 'p-2';
+  const expErr = {
+    '@qclass': 'error',
+    errorId: 'error:liveSlots:vatA#1',
+    message: 'arbitrary Symbols cannot be used as method names',
+    name: 'Error',
+  };
+  dispatch.deliver(
+    rootA,
+    'bad',
+    capargs([{ '@qclass': 'slot', index: 0 }], [target]),
+    rp2,
+  );
+  await waitUntilQuiescent();
+  t.deepEqual(log.shift(), {
+    type: 'resolve',
+    resolutions: [[rp2, false, capargs(['caught', expErr])]],
+  });
+  t.deepEqual(log, []);
+});