From: isaacs <i@izs.me>
Date: Tue, 16 Mar 2010 06:27:21 +0000 (-0700)
Subject: Make evalcx work like it's supposed to.
X-Git-Tag: v0.1.33~13
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=943b2c61a80d24d67e353c90abd7ec6f1549f6d4;p=platform%2Fupstream%2Fnodejs.git

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 cedcc64d7..78b0def92 100644
--- 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 32d5c9443..49b8ca10a 100644
--- 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);