Revert "Fix #1801 vm: Use 'sandbox' as global_prototype"

Accidentally committed. Revert until review. This reverts commit 200df86.
nodejs · Oct 19, 2011 · 9d27faa · 9d27faa
1 parent b0f78af
commit 9d27faa
Show file tree

Hide file tree

Showing 7 changed files with 171 additions and 54 deletions.
diff --git a/src/node_script.cc b/src/node_script.cc
@@ -364,25 +364,12 @@ Handle<Value> WrappedScript::EvalMachine(const Arguments& args) {
 
   // New and user context share code. DRY it up.
   if (context_flag == userContext || context_flag == newContext) {
-    // 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.
-    Handle<Object> global_proxy = context->Global();
-
-    context->DetachGlobal();
-    context->Global()->SetPrototype(sandbox);
-    context->ReattachGlobal(global_proxy);
-
+    // Enter the context
     context->Enter();
+
+    // 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());
   }
 
   // Catch errors
@@ -422,6 +409,7 @@ Handle<Value> WrappedScript::EvalMachine(const Arguments& args) {
     result = script->Run();
     if (result.IsEmpty()) {
       if (context_flag == newContext) {
+        context->DetachGlobal();
         context->Exit();
         context.Dispose();
       }
@@ -444,6 +432,7 @@ 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) {

diff --git a/test/simple/test-script-context.js b/test/simple/test-script-context.js
@@ -32,8 +32,7 @@ var result = script.runInContext(context);
 assert.equal('passed', result);
 
 common.debug('create a new pre-populated context');
-var sandbox = {'foo': 'bar', 'thing': 'lala'};
-context = script.createContext(sandbox);
+context = script.createContext({'foo': 'bar', 'thing': 'lala'});
 assert.equal('bar', context.foo);
 assert.equal('lala', context.thing);
 
@@ -43,12 +42,6 @@ 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');

diff --git a/test/simple/test-script-new.js b/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;
-sandbox = { foo: 0, baz: 3 };
+obj = { foo: 0, baz: 3 };
 script = new Script(code);
-var baz = script.runInNewContext(sandbox);
-assert.equal(1, sandbox.foo);
-assert.equal(2, sandbox.bar);
+var baz = script.runInNewContext(obj);
+assert.equal(1, obj.foo);
+assert.equal(2, obj.bar);
 assert.equal(2, foo);
 
 common.debug('call a function by reference');
@@ -85,29 +85,6 @@ 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\';');

diff --git a/test/simple/test-script-static-context.js b/test/simple/test-script-static-context.js
@@ -0,0 +1,40 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+var common = require('../common');
+var assert = require('assert');
+
+var Script = require('vm').Script;
+
+common.debug('run in a new empty context');
+var context = Script.createContext();
+var result = Script.runInContext('"passed";', context);
+assert.equal('passed', result);
+
+common.debug('create a new pre-populated context');
+context = Script.createContext({'foo': 'bar', 'thing': 'lala'});
+assert.equal('bar', context.foo);
+assert.equal('lala', context.thing);
+
+common.debug('test updating context');
+result = Script.runInContext('var foo = 3;', context);
+assert.equal(3, context.foo);
+assert.equal('lala', context.thing);
diff --git a/test/simple/test-script-static-new.js b/test/simple/test-script-static-new.js
@@ -0,0 +1,62 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+var common = require('../common');
+var assert = require('assert');
+var Script = require('vm').Script;
+
+common.globalCheck = false;
+
+common.debug('run a string');
+var result = Script.runInNewContext('\'passed\';');
+assert.equal('passed', result);
+
+common.debug('thrown error');
+assert.throws(function() {
+  Script.runInNewContext('throw new Error(\'test\');');
+});
+
+hello = 5;
+Script.runInNewContext('hello = 2');
+assert.equal(5, hello);
+
+
+common.debug('pass values in and out');
+code = 'foo = 1;' +
+       'bar = 2;' +
+       'if (baz !== 3) throw new Error(\'test fail\');';
+foo = 2;
+obj = { foo: 0, baz: 3 };
+var baz = Script.runInNewContext(code, obj);
+assert.equal(1, obj.foo);
+assert.equal(2, obj.bar);
+assert.equal(2, foo);
+
+common.debug('call a function by reference');
+function changeFoo() { foo = 100 }
+Script.runInNewContext('f()', { f: changeFoo });
+assert.equal(foo, 100);
+
+common.debug('modify an object by reference');
+var f = { a: 1 };
+Script.runInNewContext('f.a = 2', { f: f });
+assert.equal(f.a, 2);
+
diff --git a/test/simple/test-script-static-this.js b/test/simple/test-script-static-this.js
@@ -0,0 +1,56 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+var common = require('../common');
+var assert = require('assert');
+var Script = require('vm').Script;
+
+common.globalCheck = false;
+
+common.debug('run a string');
+var result = Script.runInThisContext('\'passed\';');
+assert.equal('passed', result);
+
+common.debug('thrown error');
+assert.throws(function() {
+  Script.runInThisContext('throw new Error(\'test\');');
+});
+
+hello = 5;
+Script.runInThisContext('hello = 2');
+assert.equal(2, hello);
+
+
+common.debug('pass values');
+code = 'foo = 1;' +
+       'bar = 2;' +
+       'if (typeof baz !== \'undefined\') throw new Error(\'test fail\');';
+foo = 2;
+obj = { foo: 0, baz: 3 };
+var baz = Script.runInThisContext(code);
+assert.equal(0, obj.foo);
+assert.equal(2, bar);
+assert.equal(1, foo);
+
+common.debug('call a function');
+f = function() { foo = 100 };
+Script.runInThisContext('f()');
+assert.equal(100, foo);
diff --git a/test/simple/test-script-this.js b/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.runInThisContext(script);
 assert.equal(0, obj.foo);
 assert.equal(2, bar);
 assert.equal(1, foo);