Skip to content

Commit 4527de8

Browse files
indutnyry
authored andcommitted
vm context with accessors
true copy of sandbox properties catch sealed errors, pass global's prototype to CloneObject Fixes nodejs#1673
1 parent bb3a1d5 commit 4527de8

File tree

3 files changed

+74
-22
lines changed

3 files changed

+74
-22
lines changed

lib/vm.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,19 @@
2121

2222
var binding = process.binding('evals');
2323

24+
binding.NodeScript._setCloneMethod(function(source, target) {
25+
Object.getOwnPropertyNames(source).forEach(function(key) {
26+
try {
27+
var desc = Object.getOwnPropertyDescriptor(source, key);
28+
if (desc.value === source) desc.value = target;
29+
30+
Object.defineProperty(target, key, desc);
31+
} catch (e) {
32+
// Catch sealed properties errors
33+
}
34+
});
35+
});
36+
2437
exports.Script = binding.NodeScript;
2538
exports.createScript = function(code, ctx, name) {
2639
return new exports.Script(code, ctx, name);

src/node_script.cc

Lines changed: 35 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,11 @@ using v8::TryCatch;
3737
using v8::String;
3838
using v8::Exception;
3939
using v8::Local;
40+
using v8::Null;
4041
using v8::Array;
4142
using v8::Persistent;
4243
using v8::Integer;
44+
using v8::Function;
4345
using v8::FunctionTemplate;
4446

4547

@@ -94,10 +96,23 @@ class WrappedScript : ObjectWrap {
9496
static Handle<Value> CompileRunInThisContext(const Arguments& args);
9597
static Handle<Value> CompileRunInNewContext(const Arguments& args);
9698

99+
static Handle<Value> SetCloneMethod(const Arguments& args);
100+
97101
Persistent<Script> script_;
98102
};
99103

100104

105+
Persistent<Function> cloneObjectMethod;
106+
107+
void CloneObject(Handle<Object> recv,
108+
Handle<Value> source, Handle<Value> target) {
109+
HandleScope scope;
110+
111+
Handle<Value> args[] = {source, target};
112+
cloneObjectMethod->Call(recv, 2, args);
113+
}
114+
115+
101116
void WrappedContext::Initialize(Handle<Object> target) {
102117
HandleScope scope;
103118

@@ -177,6 +192,10 @@ void WrappedScript::Initialize(Handle<Object> target) {
177192
"runInNewContext",
178193
WrappedScript::RunInNewContext);
179194

195+
NODE_SET_PROTOTYPE_METHOD(constructor_template,
196+
"_setCloneMethod",
197+
WrappedScript::SetCloneMethod);
198+
180199
NODE_SET_METHOD(constructor_template,
181200
"createContext",
182201
WrappedScript::CreateContext);
@@ -193,6 +212,10 @@ void WrappedScript::Initialize(Handle<Object> target) {
193212
"runInNewContext",
194213
WrappedScript::CompileRunInNewContext);
195214

215+
NODE_SET_METHOD(constructor_template,
216+
"_setCloneMethod",
217+
WrappedScript::SetCloneMethod);
218+
196219
target->Set(String::NewSymbol("NodeScript"),
197220
constructor_template->GetFunction());
198221
}
@@ -225,14 +248,8 @@ Handle<Value> WrappedScript::CreateContext(const Arguments& args) {
225248

226249
if (args.Length() > 0) {
227250
Local<Object> sandbox = args[0]->ToObject();
228-
Local<Array> keys = sandbox->GetPropertyNames();
229251

230-
for (uint32_t i = 0; i < keys->Length(); i++) {
231-
Handle<String> key = keys->Get(Integer::New(i))->ToString();
232-
Handle<Value> value = sandbox->Get(key);
233-
if(value == sandbox) { value = context; }
234-
context->Set(key, value);
235-
}
252+
CloneObject(args.This(), sandbox, context);
236253
}
237254

238255

@@ -275,6 +292,15 @@ Handle<Value> WrappedScript::CompileRunInNewContext(const Arguments& args) {
275292
WrappedScript::EvalMachine<compileCode, newContext, returnResult>(args);
276293
}
277294

295+
Handle<Value> WrappedScript::SetCloneMethod(const Arguments& args) {
296+
HandleScope scope;
297+
298+
Local<Function> cloneObjectMethod_ = Local<Function>::Cast(args[0]);
299+
cloneObjectMethod = Persistent<Function>::New(cloneObjectMethod_);
300+
301+
return scope.Close(Null());
302+
}
303+
278304

279305
template <WrappedScript::EvalInputFlags input_flag,
280306
WrappedScript::EvalContextFlags context_flag,
@@ -343,14 +369,7 @@ Handle<Value> WrappedScript::EvalMachine(const Arguments& args) {
343369

344370
// Copy everything from the passed in sandbox (either the persistent
345371
// context for runInContext(), or the sandbox arg to runInNewContext()).
346-
keys = sandbox->GetPropertyNames();
347-
348-
for (i = 0; i < keys->Length(); i++) {
349-
Handle<String> key = keys->Get(Integer::New(i))->ToString();
350-
Handle<Value> value = sandbox->Get(key);
351-
if (value == sandbox) { value = context->Global(); }
352-
context->Global()->Set(key, value);
353-
}
372+
CloneObject(args.This(), sandbox, context->Global()->GetPrototype());
354373
}
355374

356375
// Catch errors
@@ -408,13 +427,7 @@ Handle<Value> WrappedScript::EvalMachine(const Arguments& args) {
408427

409428
if (context_flag == userContext || context_flag == newContext) {
410429
// success! copy changes back onto the sandbox object.
411-
keys = context->Global()->GetPropertyNames();
412-
for (i = 0; i < keys->Length(); i++) {
413-
Handle<String> key = keys->Get(Integer::New(i))->ToString();
414-
Handle<Value> value = context->Global()->Get(key);
415-
if (value == context->Global()) { value = sandbox; }
416-
sandbox->Set(key, value);
417-
}
430+
CloneObject(args.This(), context->Global()->GetPrototype(), sandbox);
418431
}
419432

420433
if (context_flag == newContext) {
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
var common = require('../common');
2+
var assert = require('assert');
3+
var vm = require('vm');
4+
5+
var ctx = {};
6+
7+
Object.defineProperty(ctx, 'getter', {
8+
get: function() {
9+
return 'ok';
10+
}
11+
});
12+
13+
var val;
14+
Object.defineProperty(ctx, 'setter', {
15+
set: function(_val) {
16+
val = _val;
17+
},
18+
get: function() {
19+
return 'ok=' + val;
20+
}
21+
});
22+
23+
ctx = vm.createContext(ctx);
24+
25+
var result = vm.runInContext('setter = "test";[getter,setter]', ctx);
26+
assert.deepEqual(result, ['ok', 'ok=test']);

0 commit comments

Comments
 (0)