vm module behaves oddly W.R.T. context-objects and functions in exterior scopes

src/node_script.cc

@@ -364,12 +364,27 @@ Handle<Value> WrappedScript::EvalMachine(const Arguments& args) {
 
   // New and user context share code. DRY it up.
   if (context_flag == userContext || context_flag == newContext) {
-    // Enter the context
-    context->Enter();
+    // First, we grab the “global proxy” (see v8’s documentation for an
+    // explanation of this) by calling `Context::Global()` *before* we call
+    // `Context::DetachGlobal()`, which will then give us access to the *actual*
+    // global object.
+    // 
+    // We have to set the prototype of the *actual* global object, because the
+    // prototype of v8’s ‘global proxy’ is the global object itself, and v8
+    // blows up in approximately twenty different ways if you mess with that
+    // relationship.
+    // 
+    // Once we have the `global_proxy` reference, we utilize that to
+    // `ReattachGlobal()` before entering the context.
+    // — elliottcable <oss@ell.io>
+    Handle<Object> global_proxy = // »
+        context->Global();
 
-    // Copy everything from the passed in sandbox (either the persistent
-    // context for runInContext(), or the sandbox arg to runInNewContext()).
-    CloneObject(args.This(), sandbox, context->Global()->GetPrototype());
+        context->DetachGlobal();
+        context->Global()->SetPrototype(sandbox);
+        context->ReattachGlobal(global_proxy);
+
+        context->Enter();
   }
 
   // Catch errors
@@ -409,7 +424,6 @@ Handle<Value> WrappedScript::EvalMachine(const Arguments& args) {
     result = script->Run();
     if (result.IsEmpty()) {
       if (context_flag == newContext) {
-        context->DetachGlobal();
         context->Exit();
         context.Dispose();
       }
@@ -432,7 +446,6 @@ Handle<Value> WrappedScript::EvalMachine(const Arguments& args) {
 
   if (context_flag == newContext) {
     // Clean up, clean up, everybody everywhere!
-    context->DetachGlobal();
     context->Exit();
     context.Dispose();
   } else if (context_flag == userContext) {

test/simple/test-script-context.js

@@ -32,7 +32,8 @@ var result = script.runInContext(context);
 assert.equal('passed', result);
 
 common.debug('create a new pre-populated context');
-context = script.createContext({'foo': 'bar', 'thing': 'lala'});
+var sandbox = {'foo': 'bar', 'thing': 'lala'};
+context = script.createContext(sandbox);
 assert.equal('bar', context.foo);
 assert.equal('lala', context.thing);
 
@@ -42,6 +43,12 @@ result = script.runInContext(context);
 assert.equal(3, context.foo);
 assert.equal('lala', context.thing);
 
+// Issue GH-1801
+common.debug('test dynamic modification');
+context.fun = function(){ context.widget = 42 };
+script = new Script('fun(); widget');
+result = script.runInContext(context);
+assert.equal(42, result);
 
 // Issue GH-227:
 Script.runInNewContext('', null, 'some.js');

test/simple/test-script-new.js

@@ -66,11 +66,11 @@ code = 'foo = 1;' +
        'bar = 2;' +
        'if (baz !== 3) throw new Error(\'test fail\');';
 foo = 2;
-obj = { foo: 0, baz: 3 };
+sandbox = { foo: 0, baz: 3 };
 script = new Script(code);
-var baz = script.runInNewContext(obj);
-assert.equal(1, obj.foo);
-assert.equal(2, obj.bar);
+var baz = script.runInNewContext(sandbox);
+assert.equal(1, sandbox.foo);
+assert.equal(2, sandbox.bar);
 assert.equal(2, foo);
 
 common.debug('call a function by reference');
@@ -85,6 +85,29 @@ var f = { a: 1 };
 script.runInNewContext({ f: f });
 assert.equal(f.a, 2);
 
+// Issue GH-1801
+common.debug('test dynamic modification');
+sandbox.fun = function(){ sandbox.widget = 42 };
+script = new Script('fun(); widget');
+result = script.runInNewContext(sandbox);
+assert.equal(42, result);
+
+// Issue GH-1801
+common.debug('indirectly modify an object');
+var sandbox = { proxy: {}, common: common, process: process };
+sandbox.finish = function(){
+   process.nextTick(function(){
+      common.debug('(FINISH)');
+      assert.equal(42, sandbox.proxy.widget)
+   })
+};
+script = new Script("  process.nextTick(function(){ " +
+                    "    common.debug('(TICK)');    " +
+                    "    proxy.widget = 42;         " +
+                    "  });                          " +
+                    "  finish()                     ");
+script.runInNewContext(sandbox);
+
 common.debug('invalid this');
 assert.throws(function() {
   script.runInNewContext.call('\'hello\';');

test/simple/test-script-this.js

@@ -49,7 +49,7 @@ code = 'foo = 1;' +
 foo = 2;
 obj = { foo: 0, baz: 3 };
 script = new Script(code);
-script.runInThisContext(script);
+script.runInThisContext();
 assert.equal(0, obj.foo);
 assert.equal(2, bar);
 assert.equal(1, foo);