chore(ses): Update comments of makeEvaluate to reflect reality
MODIFY
packages/ses/src/make-evaluate.js
packages/ses/src/make-evaluate.js
@@ -5,17 +5,19 @@ import { getScopeConstants } from './scope-constants.js';
/**
* buildOptimizer()
- * Given an array of indentifier, the optimizer return a `const` declaration
- * destructring `this`.
+ * Given an array of identifiers, the optimizer returns a `const` declaration
+ * destructuring `this.${name}`.
*
* @param {Array<string>} constants
+ * @param {string} name
*/
-function buildOptimizer(constants) {
+function buildOptimizer(constants, name) {
// No need to build an optimizer when there are no constants.
if (constants.length === 0) return '';
- // Use 'this' to avoid going through the scope proxy, which is unecessary
+ // Use 'this' to avoid going through the scope proxy, which is unnecessary
// since the optimizer only needs references to the safe global.
- return `const {${arrayJoin(constants, ',')}} = this.optimizerObject;`;
+ // Destructure the constants from the target scope object.
+ return `const {${arrayJoin(constants, ',')}} = this.${name};`;
}
/**
@@ -47,35 +49,46 @@ export const makeEvaluate = context => {
// eval, and thus in strict mode in the same scope. We must be very careful
// to not create new names in this scope
- // 1: we use 'with' (around a Proxy) to catch all free variable names. The
- // `this` value holds the Proxy which safely wraps the safeGlobal
- // 2: 'optimizer' catches constant variable names for speed.
- // 3: The inner strict function is effectively passed two parameters:
+ // 1: we use multiple nested 'with' to catch all free variable names. The
+ // `this` value of the outer sloppy function holds the different scope
+ // layers, from inner to outer:
+ // a) `evalScope` which must release the `FERAL_EVAL` as 'eval' once for
+ // every invocation of the inner `evaluate` function in order to
+ // trigger direct eval. The direct eval semantics is what allows the
+ // evaluated code to lookup free variable names on the other scope
+ // objects and not in global scope.
+ // b) `globalLexicals` which provide a way to introduce free variables
+ // that are not available on the globalObject.
+ // c) `globalObject` is the global scope object of the evaluator, aka the
+ // Compartment's `globalThis`.
+ // d) `scopeTerminator` is a proxy object which prevents free variable
+ // lookups to escape to the start compartment's global object.
+ // 2: `optimizer`s catch constant variable names for speed.
+ // 3: The inner strict `evaluate` function should be passed two parameters:
// a) its arguments[0] is the source to be directly evaluated.
// b) its 'this' is the this binding seen by the code being
// directly evaluated (the globalObject).
- // 4: The outer sloppy function is passed one parameter, the scope proxy.
- // as the `this` parameter.
// Notes:
- // - everything in the 'optimizer' string is looked up in the proxy
- // (including an 'arguments[0]', which points at the Proxy).
- // - keywords like 'function' which are reserved keywords, and cannot be
- // used as a variable, so they are not part of the optimizer.
- // - when 'eval' is looked up in the proxy, and it's the first time it is
- // looked up after allowNextEvalToBeUnsafe is turned on, the proxy returns
- // the powerful, unsafe eval intrinsic, and flips allowNextEvalToBeUnsafe
- // back to false. Any reference to 'eval' in that string will get the tamed
- // evaluator.
+ // - The `optimizer` strings only lookup values on the `globalObject` and
+ // `globalLexicals` objects by construct. Keywords like 'function' are
+ // reserved and cannot be used as a variable, so they are excluded from the
+ // optimizer. Furthermore to prevent shadowing 'eval', while a valid
+ // identifier, that name is also explicitly excluded.
+ // - when 'eval' is looked up in the `evalScope`, the powerful unsafe eval
+ // intrinsic is returned after automatically removing it from the
+ // `evalScope`. Any further reference to 'eval' in the evaluate string will
+ // get the tamed evaluator from the `globalObject`, if any.
// TODO https://github.com/endojs/endo/issues/816
// The optimizer currently runs under sloppy mode, and although we doubt that
// there is any vulnerability introduced just by running the optimizer
// sloppy, we are much more confident in the semantics of strict mode.
- // The motivation for having the optimizer in sloppy mode is that it can be
- // reused for multiple evaluations, but in practice we have no such calls.
- // We could probably both move the optimizer into the inner function
- // and we could also simplify makeEvaluateFactory to simply evaluate.
+ // The `evaluate` function can be and is reused across multiple evaluations.
+ // Since the optimizer should not be re-evaluated every time, it cannot be
+ // inside the `evaluate` closure. While we could potentially nest an
+ // intermediate layer of `() => {'use strict'; ${optimizers}; ...`, it
+ // doesn't seem worth the overhead and complexity.
const evaluateFactory = FERAL_FUNCTION(`
with (this.scopeTerminator) {
with (this.globalObject) {

chore(ses): Update comments of makeEvaluate to reflect reality