[acl][hook] Local variables in a with block are mistreated as global variables in ACL

[t2ym]

[acl][hook] Local variables in a with block are mistreated as global variables in ACL

Root Cause

• Local variables in a with block are hooked as with-scoped variables but they cannot be found in the with block scope chain and then mistreated as global variables in applying ACL, while the variables are just local

Reproducible Code

{
  let with_scoped_chain = 1;
  with ({with_scoped2: 2}) {
    let local_scoped = 3;
    // local_scoped is unexpectedly hooked as a with-scoped variable
    with_scoped_chain = local_scoped;
    global_scoped; // global if the scope object does not have global_scoped property
  }
}

Fix

• Avoid unexpected hooking of local variables within a with-scoped block

[t2ym]

It seems there are some regressions in the changes for 0.4.0-alpha.11

Status: Fix Verification

Root Cause

• In the native JavaScript, var variables are overridden by the corresponding with scope object properties if such properties exist, while they are in the function scope if there are no corresponding properties in the with scope object
• In the hooked code, var variable declarations are treated as declarations in their function scope, which can be the global scope if they are not in a function
• In the hooked code, var variable references are treated as references via the with scope

Workaround Candidate (not verified yet)

• Reproducible Original Code

with (scope) {
  var variable = init;
}

• Code with Workaround

with (scope) {
  var variable; variable = init; // only for var; not for const and let
}

Reproducible Code (This may not cover all the issues)

{
  with ({ g2: 'g2_in_with_scope_object' }) {
    var g = 'g_in_the_global_scope';

    // Expected: referencing the `with` scope
    // Actual: hooked as a global variable declaration and initialization
    //   Note: How can the preprocessor know whether it is in a global scope or not?
    //  Fix Candidate: Hook "var v = i" as "var v; v = i"
    var g2 = 'g2_in_with_scope_block';

    g2; // referencing the `with` scope object
    if (g2 === 'g2_in_with_scope_block') {
      console.log('g2 value is OK', g2);
    }
    else {
      console.error('g2 value is not OK, expected =', 'g2_in_with_scope_block', 'actual =', g2);
    }
  }
  if (typeof g2 === 'undefined') {
    console.log('g2 value is OK', g2);
  }
  else {
    console.error('g2 value is not OK, expected =', undefined, 'actual =', g2);
  }
  debugger;
}

Result

g2 value is not OK, expected = g2_in_with_scope_block actual = g2_in_with_scope_object
g2 value is not OK, expected = undefined actual = g2_in_with_scope_block

Fix

• Hoist global var declarations in a with scope to the beginning of the script
• Hook such variables as with-scoped variables

Example Code

• Original Code

with ({g: 'from_with_scope_object'}) {
  var g = 'from_with_scope_block';
  var g2 = 'from_with_scope_block;
}
g === undefined;
g2 === 'from_with_scope_block';

• Hooked Like This with Fix

var g, g2; // converted to $hook$.global('g'), $hook$.global('g2')
with ({g: 'from_with_scope_object'}) {
  g = 'from_with_scope_block'; // treated as a with-scoped variable
  g2 = 'from_with_scope_block; // treated as a with-scoped variable
}
g === undefined;
g2 === 'from_with_scope_block';

• Hooked (Fixed)

const _c_ = $hook$.$(__hook__, [
  'demo/with-test3.js',
  '_p_g;demo/with-test3.js',
  '_p_g2;demo/with-test3.js',
  '_p_undefined;demo/with-test3.js'
]);
$hook$.global(__hook__, _c_[0], 'g', 'var')[_c_[1]], $hook$.global(__hook__, _c_[0], 'g2', 'var')[_c_[2]];
with ($hook$.with({ g: 'from_with_scope_object' }, {})) {
  __hook__('w.=', __with__, [
    'g',
    {
      set ['='](v) {
        g = v;
      },
      get ['=']() {
        return g;
      }
    }
  ], _c_[0], false)['='] = 'from_with_scope_block';
  __hook__('w.=', __with__, [
    'g2',
    {
      set ['='](v) {
        g2 = v;
      },
      get ['=']() {
        return g2;
      }
    }
  ], _c_[0], false)['='] = 'from_with_scope_block';
}
$hook$.global(__hook__, _c_[0], 'g', 'get')[_c_[1]] === $hook$.global(__hook__, _c_[0], 'undefined', 'get')[_c_[3]];
$hook$.global(__hook__, _c_[0], 'g2', 'get')[_c_[2]] === 'from_with_scope_block';

[t2ym]

Fix Candidate rev. 1 to the regression issue

diff --git a/lib/preprocess.js b/lib/preprocess.js
index cc8998e0..69a458b7 100644
--- a/lib/preprocess.js
+++ b/lib/preprocess.js
@@ -491,6 +491,12 @@ module.exports = function(espree, escodegen, htmlparser, createHash, convert, he
             ast.name = nonOverridableIdentifiers.get(ast.name);
           }
           (functionScopeTypes[scope] ? functionScope : blockScope)[ast.name] = scope;
+          if (functionScopeTypes[scope] && functionScope === moduleScope && hookWith && blockScope[hookWith]) {
+            if (!moduleScope.hasOwnProperty(Symbol.for('__with__'))) {
+              moduleScope[Symbol.for('__with__')] = {};
+            }
+            moduleScope[Symbol.for('__with__')][ast.name] = scope;
+          }
         }
       }
       break;
@@ -502,6 +508,12 @@ module.exports = function(espree, escodegen, htmlparser, createHash, convert, he
             ast.left.name = nonOverridableIdentifiers.get(ast.left.name);
           }
           (functionScopeTypes[scope] ? functionScope : blockScope)[ast.left.name] = scope;
+          if (functionScopeTypes[scope] && functionScope === moduleScope && hookWith && blockScope[hookWith]) {
+            if (!moduleScope.hasOwnProperty(Symbol.for('__with__'))) {
+              moduleScope[Symbol.for('__with__')] = {};
+            }
+            moduleScope[Symbol.for('__with__')][ast.left.name] = scope;
+          }
         }
       }
       break;
@@ -554,6 +566,12 @@ module.exports = function(espree, escodegen, htmlparser, createHash, convert, he
                 ast.argument.name = nonOverridableIdentifiers.get(ast.argument.name);
               }
               (functionScopeTypes[scope] ? functionScope : blockScope)[ast.argument.name] = scope;
+              if (functionScopeTypes[scope] && functionScope === moduleScope && hookWith && blockScope[hookWith]) {
+                if (!moduleScope.hasOwnProperty(Symbol.for('__with__'))) {
+                  moduleScope[Symbol.for('__with__')] = {};
+                }
+                moduleScope[Symbol.for('__with__')][ast.argument.name] = scope;
+              }
             }
             break;
           case 'ArrayPattern':
@@ -580,6 +598,12 @@ module.exports = function(espree, escodegen, htmlparser, createHash, convert, he
                 ast.argument.name = nonOverridableIdentifiers.get(ast.argument.name);
               }
               (functionScopeTypes[scope] ? functionScope : blockScope)[ast.argument.name] = scope;
+              if (functionScopeTypes[scope] && functionScope === moduleScope && hookWith && blockScope[hookWith]) {
+                if (!moduleScope.hasOwnProperty(Symbol.for('__with__'))) {
+                  moduleScope[Symbol.for('__with__')] = {};
+                }
+                moduleScope[Symbol.for('__with__')][ast.argument.name] = scope;
+              }
             }
             break;
           default:
@@ -2246,6 +2270,11 @@ module.exports = function(espree, escodegen, htmlparser, createHash, convert, he
         ast.__hooked__ = undefined;
       }
       else if (_hookProperty) {
+        if (_hookGlobal && ast.__global__ && ast.__LHS__ && ast.name && !blockScope[ast.name] && // is a global LHS variable
+            hookWith && blockScope[hookWith] && // in a with scope
+            globalScope.hasOwnProperty(Symbol.for('__with__')) && globalScope[Symbol.for('__with__')][ast.name]) { // declared in a with scope
+          ast.__with__ = 'set'; // treat as a LHS with variable
+        }
         if (ast.__with__) {
           switch (ast.__with__) {
           case 'get':
@@ -3030,6 +3059,33 @@ module.exports = function(espree, escodegen, htmlparser, createHash, convert, he
       if (ast.body.length > 0) {
         ast.body.splice(spliceIndex, 0, f);
       }
+      if (globalScope[Symbol.for('__with__')]) {
+        //console.log('BlockStatement globalScope[Symbol.for(__with__)]', globalScope[Symbol.for('__with__')]);
+        let template = {
+          "type": "VariableDeclaration",
+          "declarations": [],
+          "kind": "var",
+        };
+        for (let v in globalScope[Symbol.for('__with__')]) {
+          if (globalScope[Symbol.for('__with__')][v] === 'VariableDeclaration_var') {
+            template.declarations.push({
+              "type": "VariableDeclarator",
+              "id": {
+                "type": "Identifier",
+                "name": v,
+              },
+              "init": null,
+            });
+          }
+          else {
+            console.error('unknown declaration type in a with scope for ', v, globalScope[Symbol.for('__with__')][v]);
+          }
+        }
+        if (ast.body.length > 0) {
+          spliceIndex++;
+          ast.body.splice(spliceIndex, 0, template);
+        }
+      }
       break;
     default:
       break;

[t2ym]

Closing as the test goes well