Make evalcx work like it's supposed to.

1. Move the context->Enter() call so that the global obj is available for writing.
2. On success, copy the modified global out to the sandbox object.
3. Don't copy functions in either direction.  They have scope and closures, and make for craziness when trying to keep contexts separate.
4. Only do the ->ToObject->Clone() on objects, so that simple values stay simple.
5. Update the test so that it tests all this stuff.

diff --git a/src/node.cc b/src/node.cc
index cedcc64d745612859aabf4caf1971d490aee9676..78b0def92e1b66098dcaa80b5cc743999c5422b9 100644 (file)
--- a/src/node.cc
+++ b/src/node.cc
@@ -862,6 +862,9 @@ Handle<Value> EvalCX(const Arguments& args) {
   // Create the new context
   Persistent<Context> context = Context::New();
 
+  // Enter and compile script
+  context->Enter();
+
   // Copy objects from global context, to our brand new context
   Handle<Array> keys = sandbox->GetPropertyNames();
 
@@ -869,12 +872,13 @@ Handle<Value> EvalCX(const Arguments& args) {
   for (i = 0; i < keys->Length(); i++) {
     Handle<String> key = keys->Get(Integer::New(i))->ToString();
     Handle<Value> value = sandbox->Get(key);
-    context->Global()->Set(key, value->ToObject()->Clone());
+    if (value->IsFunction()) continue;
+    if (value->IsObject()) {
+      value = value->ToObject()->Clone();
+    }
+    context->Global()->Set(key, value);
   }
 
-  // Enter and compile script
-  context->Enter();
-
   // Catch errors
   TryCatch try_catch;
 
@@ -887,6 +891,18 @@ Handle<Value> EvalCX(const Arguments& args) {
     result = script->Run();
     if (result.IsEmpty()) {
       result = ThrowException(try_catch.Exception());
+    } else {
+      // success! copy changes back onto the sandbox object.
+      keys = context->Global()->GetPropertyNames();
+      for (i = 0; i < keys->Length(); i++) {
+        Handle<String> key = keys->Get(Integer::New(i))->ToString();
+        Handle<Value> value = context->Global()->Get(key);
+        if (value->IsFunction()) continue;
+        if (value->IsObject()) {
+          value = value->ToObject()->Clone();
+        }
+        sandbox->Set(key, value);
+      }
     }
   }
 

diff --git a/test/simple/test-eval-cx.js b/test/simple/test-eval-cx.js
index 32d5c944367f9175093f863e4400b3f32fc59b38..49b8ca10a03a1342136bd1ed76b629548e68fa50 100644 (file)
--- a/test/simple/test-eval-cx.js
+++ b/test/simple/test-eval-cx.js
@@ -14,14 +14,19 @@ process.evalcx('hello = 2');
 assert.equal(5, hello);
 
 
-code = "foo = 1; bar = 2;";
-foo = 2;
-obj = { foo : 0 };
-process.evalcx(code, obj);
+code = "foo = 1;"
+     + "bar = 2;"
+     + "if (baz !== 3) throw new Error('test fail');"
+     + "quux.pwned = true;";
 
-/* TODO?
+foo = 2;
+var quux = { pwned : false };
+obj = { foo : 0, baz : 3, quux : quux };
+var baz = process.evalcx(code, obj);
 assert.equal(1, obj.foo);
 assert.equal(2, obj.bar);
-*/
+assert.equal(obj.quux.pwned, true);
+assert.equal(quux.pwned, false);
+assert.notEqual(quux, obj.quux);
 
 assert.equal(2, foo);