Skip to content

Commit

Permalink
vm: fix symbol access
Browse files Browse the repository at this point in the history 
By using the new SetHandler API instead of SetNamedPropertyHandler, we can
intercept symbols now.

Fixes nodejs#884.
  • Loading branch information
@domenic
domenic committed May 23, 2015
1 parent ec3905b commit 976d494
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 20 deletions.
62 changes: 42 additions & 20 deletions src/node_contextify.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ using v8::HandleScope;
using v8::Integer;
using v8::Isolate;
using v8::Local;
using v8::MaybeLocal;
using v8::Name;
using v8::NamedPropertyHandlerConfiguration;
using v8::None;
using v8::Object;
using v8::ObjectTemplate;
Expand Down Expand Up @@ -202,12 +205,14 @@ class ContextifyContext {

Local<ObjectTemplate> object_template =
function_template->InstanceTemplate();
object_template->SetNamedPropertyHandler(GlobalPropertyGetterCallback,

NamedPropertyHandlerConfiguration config(GlobalPropertyGetterCallback,
GlobalPropertySetterCallback,
GlobalPropertyQueryCallback,
GlobalPropertyDeleterCallback,
GlobalPropertyEnumeratorCallback,
CreateDataWrapper(env));
object_template->SetHandler(config);

Local<Context> ctx = Context::New(env->isolate(), nullptr, object_template);
if (!ctx.IsEmpty())
Expand Down Expand Up @@ -342,7 +347,7 @@ class ContextifyContext {


static void GlobalPropertyGetterCallback(
Local<String> property,
Local<Name> property,
const PropertyCallbackInfo<Value>& args) {
Isolate* isolate = args.GetIsolate();
HandleScope scope(isolate);
Expand All @@ -351,22 +356,27 @@ class ContextifyContext {
Unwrap<ContextifyContext>(args.Data().As<Object>());

Local<Object> sandbox = PersistentToLocal(isolate, ctx->sandbox_);
Local<Value> rv = sandbox->GetRealNamedProperty(property);
MaybeLocal<Value> rv =
sandbox->GetRealNamedProperty(ctx->context(), property);
if (rv.IsEmpty()) {
Local<Object> proxy_global = PersistentToLocal(isolate,
ctx->proxy_global_);
rv = proxy_global->GetRealNamedProperty(property);
}
if (!rv.IsEmpty() && rv == ctx->sandbox_) {
rv = PersistentToLocal(isolate, ctx->proxy_global_);
rv = proxy_global->GetRealNamedProperty(ctx->context(), property);
}

args.GetReturnValue().Set(rv);
Local<Value> localRV;
if (rv.ToLocal(&localRV)) {
if (localRV == ctx->sandbox_) {
rv = PersistentToLocal(isolate, ctx->proxy_global_);
}

args.GetReturnValue().Set(localRV);
}
}


static void GlobalPropertySetterCallback(
Local<String> property,
Local<Name> property,
Local<Value> value,
const PropertyCallbackInfo<Value>& args) {
Isolate* isolate = args.GetIsolate();
Expand All @@ -380,7 +390,7 @@ class ContextifyContext {


static void GlobalPropertyQueryCallback(
Local<String> property,
Local<Name> property,
const PropertyCallbackInfo<Integer>& args) {
Isolate* isolate = args.GetIsolate();
HandleScope scope(isolate);
Expand All @@ -392,31 +402,43 @@ class ContextifyContext {
Local<Object> proxy_global = PersistentToLocal(isolate,
ctx->proxy_global_);

if (sandbox->HasRealNamedProperty(property)) {

This comment has been minimized.

Sign in to view

Copy link
@domenic

domenic May 23, 2015

Author Owner

This function is a friggin mess. The 80 char limit is killing me, and I'm sure my coding style isn't doing me any favors either. And it compounds my earlier frustrations about not knowing what to do with Maybes.

Would appreciate a rewrite from someone who knows how we should be using Maybes and MaybeLocals, and knows the style guide better.
PropertyAttribute propAttr =
sandbox->GetRealNamedPropertyAttributes(property).FromJust();
args.GetReturnValue().Set(propAttr);
} else if (proxy_global->HasRealNamedProperty(property)) {
bool in_sandbox =
sandbox->HasRealNamedProperty(ctx->context(), property).FromMaybe(false);

if (in_sandbox) {
PropertyAttribute propAttr =
proxy_global->GetRealNamedPropertyAttributes(property).FromJust();
sandbox->
GetRealNamedPropertyAttributes(ctx->context(), property).FromJust();
args.GetReturnValue().Set(propAttr);
} else {
args.GetReturnValue().Set(None);
bool in_proxy_global =
proxy_global->HasRealNamedProperty(ctx->context(), property)
.FromMaybe(false);
if (in_proxy_global) {
PropertyAttribute propAttr =
proxy_global->
GetRealNamedPropertyAttributes(ctx->context(), property)
.FromJust();
args.GetReturnValue().Set(propAttr);
}
else {
args.GetReturnValue().Set(None);
}
}
}


static void GlobalPropertyDeleterCallback(
Local<String> property,
Local<Name> property,
const PropertyCallbackInfo<Boolean>& args) {
Isolate* isolate = args.GetIsolate();
HandleScope scope(isolate);

ContextifyContext* ctx =
Unwrap<ContextifyContext>(args.Data().As<Object>());
Local<Object> sandbox = PersistentToLocal(isolate, ctx->sandbox_);

bool success = PersistentToLocal(isolate,
ctx->sandbox_)->Delete(property);
bool success = sandbox->Delete(ctx->context(), property).FromMaybe(false);
args.GetReturnValue().Set(success);
}

Expand Down
23 changes: 23 additions & 0 deletions test/parallel/test-vm-symbols.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
var common = require('../common');
var assert = require('assert');

var vm = require('vm');

var symbol = Symbol();

function Document() {
this[symbol] = 'foo';
}

Document.prototype.getSymbolValue = function () {
return this[symbol];
};

var context = new Document();
vm.createContext(context);

assert.equal(context.getSymbolValue(), 'foo',
'should return symbol-keyed value from the outside');

assert.equal(vm.runInContext('this.getSymbolValue()', context), 'foo',
'should return symbol-keyed value from the inside');

0 comments on commit 976d494

Please sign in to comment.