From 9d27faa2c4147e3aa8c8b83ce91330f8ae96798b Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Wed, 19 Oct 2011 11:01:08 -0700 Subject: [PATCH] Revert "Fix #1801 vm: Use 'sandbox' as global_prototype" Accidentally committed. Revert until review. This reverts commit 200df8641b43902adc73cce6b89d3e84a91dd3e6. --- src/node_script.cc | 25 ++++--------- test/simple/test-script-context.js | 9 +---- test/simple/test-script-new.js | 31 ++-------------- test/simple/test-script-static-context.js | 40 ++++++++++++++++++++ test/simple/test-script-static-new.js | 62 +++++++++++++++++++++++++++++++ test/simple/test-script-static-this.js | 56 ++++++++++++++++++++++++++++ test/simple/test-script-this.js | 2 +- 7 files changed, 171 insertions(+), 54 deletions(-) create mode 100644 test/simple/test-script-static-context.js create mode 100644 test/simple/test-script-static-new.js create mode 100644 test/simple/test-script-static-this.js diff --git a/src/node_script.cc b/src/node_script.cc index 512c477..6e4caff 100644 --- a/src/node_script.cc +++ b/src/node_script.cc @@ -364,25 +364,12 @@ Handle 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 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 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 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 index d273f59..5819744 100644 --- 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 index 8e2a9a2..7894bb9 100644 --- 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 new file mode 100644 index 0000000..d2cb701 --- /dev/null +++ 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 new file mode 100644 index 0000000..eaac2d9 --- /dev/null +++ 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 new file mode 100644 index 0000000..63c1133 --- /dev/null +++ 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 index 46d3159..a55a490 100644 --- 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); -- 2.7.4