Skip to content

Commit 2dfd002

Browse files
committed
vm: pass attributes from sandbox
GlobalPropertyQueryCallback() was returning `None` as attributes for all properties, without regard to actual properties settings. This patch looks up attributes for sandbox-defined properties and returns them. Partial fix for nodejsgh-864 Add test for nodejsgh-864, non-default settings of object properties (e.g., `enumerable: false`) are not visible when object is used as a sandbox
1 parent 9b6b055 commit 2dfd002

File tree

2 files changed

+32
-4
lines changed

2 files changed

+32
-4
lines changed

src/node_contextify.cc

+8-4
Original file line numberDiff line numberDiff line change
@@ -387,11 +387,15 @@ class ContextifyContext {
387387
Local<Object> proxy_global = PersistentToLocal(isolate,
388388
ctx->proxy_global_);
389389

390-
bool in_sandbox = sandbox->GetRealNamedProperty(property).IsEmpty();
390+
bool in_sandbox = !(sandbox->GetRealNamedProperty(property).IsEmpty());
391391
bool in_proxy_global =
392-
proxy_global->GetRealNamedProperty(property).IsEmpty();
393-
if (!in_sandbox || !in_proxy_global) {
394-
args.GetReturnValue().Set(None);
392+
!(proxy_global->GetRealNamedProperty(property).IsEmpty());
393+
if (in_sandbox || in_proxy_global) {
394+
v8::PropertyAttribute attr = None;
395+
if (in_sandbox) {
396+
attr = sandbox->GetPropertyAttributes(property);
397+
}
398+
args.GetReturnValue().Set(attr);
395399
}
396400
}
397401

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
var common = require('../common');
2+
var assert = require('assert');
3+
4+
var vm = require('vm');
5+
6+
var code = 'Object.getOwnPropertyDescriptor(this, "prop")';
7+
8+
var x = {};
9+
Object.defineProperty(x, "prop", {
10+
configurable: false,
11+
enumerable: false,
12+
writable: false,
13+
value: "val"
14+
});
15+
var o = vm.createContext(x);
16+
17+
var res = vm.runInContext(code, o, 'test');
18+
19+
assert(res);
20+
assert.equal(typeof res, 'object');
21+
assert.equal(res.value, "val");
22+
assert.equal(res.configurable, false, "should not be configurable");
23+
assert.equal(res.enumerable, false, "should not be enumerable");
24+
assert.equal(res.writable, false, "should not be writable");

0 commit comments

Comments
 (0)