From 31fce45fbe586d8c46e61ce674b77bbf7cfb379c Mon Sep 17 00:00:00 2001
From: Fedor Indutny <fedor.indutny@gmail.com>
Date: Thu, 8 Sep 2011 18:42:44 +0700
Subject: [PATCH 1/6] [script] true copy of sandbox properties

---
 lib/vm.js          | 11 ++++++++++
 src/node_script.cc | 55 +++++++++++++++++++++++++++-------------------
 2 files changed, 44 insertions(+), 22 deletions(-)

diff --git a/lib/vm.js b/lib/vm.js
index d3348afac50..0d395f47341 100644
--- a/lib/vm.js
+++ b/lib/vm.js
@@ -21,6 +21,17 @@
 
 var binding = process.binding('evals');
 
+binding.NodeScript._setCloneMethod(function(source, target) {
+  Object.getOwnPropertyNames(source).forEach(function(key) {
+    delete target[key];
+
+    var desc = Object.getOwnPropertyDescriptor(source, key);
+    if (desc.value === source) desc.value = target;
+
+    Object.defineProperty(target, key, desc);
+  });
+});
+
 exports.Script = binding.NodeScript;
 exports.createScript = function(code, ctx, name) {
   return new exports.Script(code, ctx, name);
diff --git a/src/node_script.cc b/src/node_script.cc
index 4f4153caa98..65a9484bdd2 100644
--- a/src/node_script.cc
+++ b/src/node_script.cc
@@ -37,9 +37,11 @@ using v8::TryCatch;
 using v8::String;
 using v8::Exception;
 using v8::Local;
+using v8::Null;
 using v8::Array;
 using v8::Persistent;
 using v8::Integer;
+using v8::Function;
 using v8::FunctionTemplate;
 
 
@@ -94,10 +96,21 @@ class WrappedScript : ObjectWrap {
   static Handle<Value> CompileRunInThisContext(const Arguments& args);
   static Handle<Value> CompileRunInNewContext(const Arguments& args);
 
+  static Handle<Value> SetCloneMethod(const Arguments& args);
+
   Persistent<Script> script_;
 };
 
 
+Persistent<Function> cloneObjectMethod;
+
+void CloneObject(Handle<Object> recv,
+                 Handle<Object> source, Handle<Object> target) {
+  Handle<Value> args[] = {source, target};
+  cloneObjectMethod->Call(recv, 2, args);
+}
+
+
 void WrappedContext::Initialize(Handle<Object> target) {
   HandleScope scope;
 
@@ -177,6 +190,10 @@ void WrappedScript::Initialize(Handle<Object> target) {
                             "runInNewContext",
                             WrappedScript::RunInNewContext);
 
+  NODE_SET_PROTOTYPE_METHOD(constructor_template,
+                            "_setCloneMethod",
+                            WrappedScript::SetCloneMethod);
+
   NODE_SET_METHOD(constructor_template,
                   "createContext",
                   WrappedScript::CreateContext);
@@ -193,6 +210,10 @@ void WrappedScript::Initialize(Handle<Object> target) {
                   "runInNewContext",
                   WrappedScript::CompileRunInNewContext);
 
+  NODE_SET_METHOD(constructor_template,
+                  "_setCloneMethod",
+                  WrappedScript::SetCloneMethod);
+
   target->Set(String::NewSymbol("NodeScript"),
               constructor_template->GetFunction());
 }
@@ -225,14 +246,8 @@ Handle<Value> WrappedScript::CreateContext(const Arguments& args) {
 
   if (args.Length() > 0) {
     Local<Object> sandbox = args[0]->ToObject();
-    Local<Array> keys = sandbox->GetPropertyNames();
 
-    for (uint32_t i = 0; i < keys->Length(); i++) {
-      Handle<String> key = keys->Get(Integer::New(i))->ToString();
-      Handle<Value> value = sandbox->Get(key);
-      if(value == sandbox) { value = context; }
-      context->Set(key, value);
-    }
+    CloneObject(args.This(), sandbox, context);
   }
 
 
@@ -275,6 +290,15 @@ Handle<Value> WrappedScript::CompileRunInNewContext(const Arguments& args) {
     WrappedScript::EvalMachine<compileCode, newContext, returnResult>(args);
 }
 
+Handle<Value> WrappedScript::SetCloneMethod(const Arguments& args) {
+  HandleScope scope;
+
+  Local<Function> cloneObjectMethod_ = Local<Function>::Cast(args[0]);
+  cloneObjectMethod = Persistent<Function>::New(cloneObjectMethod_);
+
+  return scope.Close(Null());
+}
+
 
 template <WrappedScript::EvalInputFlags input_flag,
           WrappedScript::EvalContextFlags context_flag,
@@ -343,14 +367,7 @@ Handle<Value> WrappedScript::EvalMachine(const Arguments& args) {
 
     // Copy everything from the passed in sandbox (either the persistent
     // context for runInContext(), or the sandbox arg to runInNewContext()).
-    keys = sandbox->GetPropertyNames();
-
-    for (i = 0; i < keys->Length(); i++) {
-      Handle<String> key = keys->Get(Integer::New(i))->ToString();
-      Handle<Value> value = sandbox->Get(key);
-      if (value == sandbox) { value = context->Global(); }
-      context->Global()->Set(key, value);
-    }
+    CloneObject(args.This(), sandbox, context->Global());
   }
 
   // Catch errors
@@ -408,13 +425,7 @@ Handle<Value> WrappedScript::EvalMachine(const Arguments& args) {
 
   if (context_flag == userContext || context_flag == newContext) {
     // 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 == context->Global()) { value = sandbox; }
-      sandbox->Set(key, value);
-    }
+    CloneObject(args.This(), context->Global(), sandbox);
   }
 
   if (context_flag == newContext) {

From b5ae3bc863308ca3909abb4fe43c3405fa658b9d Mon Sep 17 00:00:00 2001
From: Fedor Indutny <fedor.indutny@gmail.com>
Date: Thu, 8 Sep 2011 18:47:05 +0700
Subject: [PATCH 2/6] [script] don't forget to construct scope for CloneObject

---
 src/node_script.cc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/node_script.cc b/src/node_script.cc
index 65a9484bdd2..1c137bfb0f5 100644
--- a/src/node_script.cc
+++ b/src/node_script.cc
@@ -106,6 +106,8 @@ Persistent<Function> cloneObjectMethod;
 
 void CloneObject(Handle<Object> recv,
                  Handle<Object> source, Handle<Object> target) {
+  HandleScope scope;
+
   Handle<Value> args[] = {source, target};
   cloneObjectMethod->Call(recv, 2, args);
 }

From 574fb41fe5157c9a9a280ca48747950f51c70e7e Mon Sep 17 00:00:00 2001
From: Fedor Indutny <fedor.indutny@gmail.com>
Date: Thu, 8 Sep 2011 19:17:06 +0700
Subject: [PATCH 3/6] [vm, script] catch sealed errors, pass global's prototype
 to CloneObject

---
 lib/vm.js          | 12 +++++++-----
 src/node_script.cc |  6 +++---
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/lib/vm.js b/lib/vm.js
index 0d395f47341..45f0f59150b 100644
--- a/lib/vm.js
+++ b/lib/vm.js
@@ -23,12 +23,14 @@ var binding = process.binding('evals');
 
 binding.NodeScript._setCloneMethod(function(source, target) {
   Object.getOwnPropertyNames(source).forEach(function(key) {
-    delete target[key];
+    try {
+      var desc = Object.getOwnPropertyDescriptor(source, key);
+      if (desc.value === source) desc.value = target;
 
-    var desc = Object.getOwnPropertyDescriptor(source, key);
-    if (desc.value === source) desc.value = target;
-
-    Object.defineProperty(target, key, desc);
+      Object.defineProperty(target, key, desc);
+    } catch (e) {
+      // Catch sealed properties errors
+    }
   });
 });
 
diff --git a/src/node_script.cc b/src/node_script.cc
index 1c137bfb0f5..019597485f2 100644
--- a/src/node_script.cc
+++ b/src/node_script.cc
@@ -105,7 +105,7 @@ class WrappedScript : ObjectWrap {
 Persistent<Function> cloneObjectMethod;
 
 void CloneObject(Handle<Object> recv,
-                 Handle<Object> source, Handle<Object> target) {
+                 Handle<Value> source, Handle<Value> target) {
   HandleScope scope;
 
   Handle<Value> args[] = {source, target};
@@ -369,7 +369,7 @@ Handle<Value> WrappedScript::EvalMachine(const Arguments& args) {
 
     // 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());
+    CloneObject(args.This(), sandbox, context->Global()->GetPrototype());
   }
 
   // Catch errors
@@ -427,7 +427,7 @@ Handle<Value> WrappedScript::EvalMachine(const Arguments& args) {
 
   if (context_flag == userContext || context_flag == newContext) {
     // success! copy changes back onto the sandbox object.
-    CloneObject(args.This(), context->Global(), sandbox);
+    CloneObject(args.This(), context->Global()->GetPrototype(), sandbox);
   }
 
   if (context_flag == newContext) {

From b6afd9080e5c7a5703a8f41ca821798188f62a7b Mon Sep 17 00:00:00 2001
From: Fedor Indutny <fedor.indutny@gmail.com>
Date: Fri, 9 Sep 2011 00:25:41 +0700
Subject: [PATCH 4/6] [tests] added test for vm context with accessors

---
 .../test-vm-create-context-accessors.js       | 26 +++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 test/simple/test-vm-create-context-accessors.js

diff --git a/test/simple/test-vm-create-context-accessors.js b/test/simple/test-vm-create-context-accessors.js
new file mode 100644
index 00000000000..23fc935389d
--- /dev/null
+++ b/test/simple/test-vm-create-context-accessors.js
@@ -0,0 +1,26 @@
+var common = require('../common');
+var assert = require('assert');
+var vm = require('vm');
+
+var ctx = {};
+
+Object.defineProperty(ctx, 'getter', {
+  get: function() {
+    return 'ok';
+  }
+});
+
+var val;
+Object.defineProperty(ctx, 'setter', {
+  set: function(_val) {
+    val = _val;
+  },
+  get: function() {
+    return 'ok=' + val;
+  }
+});
+
+ctx = vm.createContext(ctx);
+
+var result = vm.runInContext('setter = "test";[getter,setter]', ctx);
+assert.deepEqual(result, ['ok', 'ok=test']);

From 5e75a6a8acc28a6c90a5c1e34381fd20d2daa271 Mon Sep 17 00:00:00 2001
From: Fedor Indutny <fedor.indutny@gmail.com>
Date: Fri, 9 Sep 2011 03:53:36 +0700
Subject: [PATCH 5/6] [script, vm] move cloneObjectMethod initialization inside
 script binding

---
 lib/vm.js          | 13 -------------
 src/node_script.cc | 40 +++++++++++++++++++++-------------------
 2 files changed, 21 insertions(+), 32 deletions(-)

diff --git a/lib/vm.js b/lib/vm.js
index 45f0f59150b..d3348afac50 100644
--- a/lib/vm.js
+++ b/lib/vm.js
@@ -21,19 +21,6 @@
 
 var binding = process.binding('evals');
 
-binding.NodeScript._setCloneMethod(function(source, target) {
-  Object.getOwnPropertyNames(source).forEach(function(key) {
-    try {
-      var desc = Object.getOwnPropertyDescriptor(source, key);
-      if (desc.value === source) desc.value = target;
-
-      Object.defineProperty(target, key, desc);
-    } catch (e) {
-      // Catch sealed properties errors
-    }
-  });
-});
-
 exports.Script = binding.NodeScript;
 exports.createScript = function(code, ctx, name) {
   return new exports.Script(code, ctx, name);
diff --git a/src/node_script.cc b/src/node_script.cc
index 019597485f2..f4f22f81807 100644
--- a/src/node_script.cc
+++ b/src/node_script.cc
@@ -96,8 +96,6 @@ class WrappedScript : ObjectWrap {
   static Handle<Value> CompileRunInThisContext(const Arguments& args);
   static Handle<Value> CompileRunInNewContext(const Arguments& args);
 
-  static Handle<Value> SetCloneMethod(const Arguments& args);
-
   Persistent<Script> script_;
 };
 
@@ -109,6 +107,27 @@ void CloneObject(Handle<Object> recv,
   HandleScope scope;
 
   Handle<Value> args[] = {source, target};
+
+  // Init
+  if (cloneObjectMethod.IsEmpty()) {
+    Local<Function> cloneObjectMethod_ = Local<Function>::Cast(
+      Script::Compile(String::New(
+        "(function(source, target) {\
+           Object.getOwnPropertyNames(source).forEach(function(key) {\
+           try {\
+             var desc = Object.getOwnPropertyDescriptor(source, key);\
+             if (desc.value === source) desc.value = target;\
+             Object.defineProperty(target, key, desc);\
+           } catch (e) {\
+            // Catch sealed properties errors\n\
+           }\
+         });\
+        })"
+      ), String::New("binding:script"))->Run()
+    );
+    cloneObjectMethod = Persistent<Function>::New(cloneObjectMethod_);
+  }
+
   cloneObjectMethod->Call(recv, 2, args);
 }
 
@@ -192,10 +211,6 @@ void WrappedScript::Initialize(Handle<Object> target) {
                             "runInNewContext",
                             WrappedScript::RunInNewContext);
 
-  NODE_SET_PROTOTYPE_METHOD(constructor_template,
-                            "_setCloneMethod",
-                            WrappedScript::SetCloneMethod);
-
   NODE_SET_METHOD(constructor_template,
                   "createContext",
                   WrappedScript::CreateContext);
@@ -212,10 +227,6 @@ void WrappedScript::Initialize(Handle<Object> target) {
                   "runInNewContext",
                   WrappedScript::CompileRunInNewContext);
 
-  NODE_SET_METHOD(constructor_template,
-                  "_setCloneMethod",
-                  WrappedScript::SetCloneMethod);
-
   target->Set(String::NewSymbol("NodeScript"),
               constructor_template->GetFunction());
 }
@@ -292,15 +303,6 @@ Handle<Value> WrappedScript::CompileRunInNewContext(const Arguments& args) {
     WrappedScript::EvalMachine<compileCode, newContext, returnResult>(args);
 }
 
-Handle<Value> WrappedScript::SetCloneMethod(const Arguments& args) {
-  HandleScope scope;
-
-  Local<Function> cloneObjectMethod_ = Local<Function>::Cast(args[0]);
-  cloneObjectMethod = Persistent<Function>::New(cloneObjectMethod_);
-
-  return scope.Close(Null());
-}
-
 
 template <WrappedScript::EvalInputFlags input_flag,
           WrappedScript::EvalContextFlags context_flag,

From 4457de376083c16382e563c6fde7e1723654db29 Mon Sep 17 00:00:00 2001
From: Fedor Indutny <fedor.indutny@gmail.com>
Date: Fri, 9 Sep 2011 16:24:04 +0700
Subject: [PATCH 6/6] [script] added formatting for inline js function

---
 src/node_script.cc | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/node_script.cc b/src/node_script.cc
index f4f22f81807..79f93fec341 100644
--- a/src/node_script.cc
+++ b/src/node_script.cc
@@ -112,16 +112,16 @@ void CloneObject(Handle<Object> recv,
   if (cloneObjectMethod.IsEmpty()) {
     Local<Function> cloneObjectMethod_ = Local<Function>::Cast(
       Script::Compile(String::New(
-        "(function(source, target) {\
-           Object.getOwnPropertyNames(source).forEach(function(key) {\
-           try {\
-             var desc = Object.getOwnPropertyDescriptor(source, key);\
-             if (desc.value === source) desc.value = target;\
-             Object.defineProperty(target, key, desc);\
-           } catch (e) {\
+        "(function(source, target) {\n\
+           Object.getOwnPropertyNames(source).forEach(function(key) {\n\
+           try {\n\
+             var desc = Object.getOwnPropertyDescriptor(source, key);\n\
+             if (desc.value === source) desc.value = target;\n\
+             Object.defineProperty(target, key, desc);\n\
+           } catch (e) {\n\
             // Catch sealed properties errors\n\
-           }\
-         });\
+           }\n\
+         });\n\
         })"
       ), String::New("binding:script"))->Run()
     );