Merge pull request #38 from Agoric/transform-rewrite-endowments

evaluators.js: allow transform.rewrite() to change endowments
Agoric · Aug 29, 2019 · 01b8558 · 01b8558
2 parents 9b8af86 + bdfd688
commit 01b8558
Show file tree

Hide file tree

Showing 2 changed files with 82 additions and 64 deletions.
diff --git a/src/evaluators.js b/src/evaluators.js
@@ -94,30 +94,6 @@ export function createSafeEvaluatorFactory(
       ...mandatoryTransforms
     ];
 
-    // Add endowments needed by the transforms, threading through state as necessary.
-    const endowmentState = allTransforms.reduce(
-      (es, transform) => (transform.endow ? transform.endow(es) : es),
-      { endowments }
-    );
-    endowments = endowmentState.endowments;
-
-    // todo (shim limitation): scan endowments, throw error if endowment
-    // overlaps with the const optimization (which would otherwise
-    // incorrectly shadow endowments), or if endowments includes 'eval'. Also
-    // prohibit accessor properties (to be able to consistently explain
-    // things in terms of shimming the global lexical scope).
-    // writeable-vs-nonwritable == let-vs-const, but there's no
-    // global-lexical-scope equivalent of an accessor, outside what we can
-    // explain/spec
-    const scopeTarget = create(
-      safeGlobal,
-      getOwnPropertyDescriptors(endowments)
-    );
-    const scopeProxy = new Proxy(scopeTarget, scopeHandler);
-    const scopedEvaluator = apply(scopedEvaluatorFactory, safeGlobal, [
-      scopeProxy
-    ]);
-
     // We use the the concise method syntax to create an eval without a
     // [[Construct]] behavior (such that the invocation "new eval()" throws
     // TypeError: eval is not a constructor"), but which still accepts a
@@ -128,10 +104,19 @@ export function createSafeEvaluatorFactory(
         // Rewrite the source, threading through rewriter state as necessary.
         const rewriterState = allTransforms.reduce(
           (rs, transform) => (transform.rewrite ? transform.rewrite(rs) : rs),
-          { src }
+          { src, endowments }
         );
         src = rewriterState.src;
 
+        const scopeTarget = create(
+          safeGlobal,
+          getOwnPropertyDescriptors(rewriterState.endowments)
+        );
+        const scopeProxy = new Proxy(scopeTarget, scopeHandler);
+        const scopedEvaluator = apply(scopedEvaluatorFactory, safeGlobal, [
+          scopeProxy
+        ]);
+
         scopeHandler.allowUnsafeEvaluatorOnce();
         let err;
         try {

diff --git a/test/module/evaluators.js b/test/module/evaluators.js
@@ -127,53 +127,86 @@ test('createSafeEvaluatorWhichTakesEndowments - options.sloppyGlobals', t => {
 });
 
 test('createSafeEvaluatorWhichTakesEndowments - options.transforms', t => {
-  t.plan(4);
-
-  // Mimic repairFunctions.
-  // eslint-disable-next-line no-proto
-  sinon.stub(Function.__proto__, 'constructor').callsFake(() => {
-    throw new TypeError();
-  });
-
-  const safeGlobal = Object.create(null, {
-    foo: { value: 1 },
-    bar: { value: 2, writable: true }
-  });
+  try {
+    // Mimic repairFunctions.
+    // eslint-disable-next-line no-proto
+    sinon.stub(Function.__proto__, 'constructor').callsFake(() => {
+      throw new TypeError();
+    });
 
-  const realmTransforms = [
-    {
-      endow(es) {
-        return { ...es, endowments: { ...es.endowments, abc: 123 } };
-      },
-      rewrite(ss) {
-        return { ...ss, src: ss.src === 'ABC' ? 'abc' : ss.src };
-      }
-    }
-  ];
+    const safeGlobal = Object.create(null, {
+      foo: { value: 1 },
+      bar: { value: 2, writable: true }
+    });
 
-  const safeEval = createSafeEvaluatorWhichTakesEndowments(
-    createSafeEvaluatorFactory(unsafeRecord, safeGlobal, realmTransforms)
-  );
-  const options = {
-    transforms: [
+    const realmTransforms = [
       {
         rewrite(ss) {
-          return { ...ss, src: ss.src === 'ABC' ? 'def' : ss.src };
+          if (!ss.endowments.abc) {
+            ss.endowments.abc = 123;
+          }
+          return { ...ss, src: ss.src === 'ABC' ? 'abc' : ss.src };
         }
       }
-    ]
-  };
+    ];
 
-  // The realmTransforms rewrite ABC.
-  t.equal(safeEval('abc', {}), 123);
-  t.equal(safeEval('ABC', { ABC: 234 }), 123);
-  // The endowed abc is overridden by the realmTransforms.
-  t.equal(safeEval('ABC', { ABC: 234, abc: 'notused' }), 123);
-  // The specified options.transforms rewrite ABC first.
-  t.equal(safeEval('ABC', { def: 789 }, options), 789);
+    const safeEval = createSafeEvaluatorWhichTakesEndowments(
+      createSafeEvaluatorFactory(unsafeRecord, safeGlobal, realmTransforms)
+    );
+    const options = {
+      transforms: [
+        {
+          rewrite(ss) {
+            return { ...ss, src: ss.src === 'ABC' ? 'def' : ss.src };
+          }
+        }
+      ]
+    };
 
-  // eslint-disable-next-line no-proto
-  Function.__proto__.constructor.restore();
+    t.equal(safeEval('abc', {}), 123, 'no rewrite');
+    t.equal(safeEval('ABC', { ABC: 234 }), 123, 'realmTransforms rewrite ABC');
+    t.equal(
+      safeEval('ABC', { ABC: 234, abc: false }),
+      123,
+      'falsey abc is overridden'
+    );
+    t.equal(
+      safeEval('ABC', { def: 789 }, options),
+      789,
+      `options.transform rewrite ABC first`
+    );
+
+    const optionsEndow = abcVal => ({
+      transforms: [
+        {
+          rewrite(ss) {
+            ss.endowments.abc = abcVal;
+            return ss;
+          }
+        }
+      ]
+    });
+
+    // The endowed abc is overridden by realmTransforms, then optionsEndow.
+    t.equal(
+      safeEval('abc', { ABC: 234, abc: 'notused' }, optionsEndow(321)),
+      321,
+      `optionsEndow replace endowment`
+    );
+
+    t.equal(
+      safeEval('ABC', { ABC: 234, abc: 'notused' }, optionsEndow(231)),
+      231,
+      `optionsEndow replace rewritten endowment`
+    );
+
+    // eslint-disable-next-line no-proto
+    Function.__proto__.constructor.restore();
+  } catch (e) {
+    t.isNot(e, e, 'unexpected exception');
+  } finally {
+    t.end();
+  }
 });
 
 test('createSafeEvaluatorWhichTakesEndowments', t => {