vm: fix symbol access

By using the new SetHandler API instead of SetNamedPropertyHandler, we can intercept symbols now. This forces us to use Maybes and MaybeLocals more, since this new API does not have a non-maybe variant. Fixes: #884 PR-URL: #1773 Reviewed-By: Fedor Indutny <fedor@indutny.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
nodejs · Aug 4, 2015 · 7767b9d · 7767b9d
1 parent 5ca9579
commit 7767b9d
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 28 deletions.
diff --git a/src/node_contextify.cc b/src/node_contextify.cc
@@ -26,6 +26,10 @@ using v8::HandleScope;
 using v8::Integer;
 using v8::Isolate;
 using v8::Local;
+using v8::Maybe;
+using v8::MaybeLocal;
+using v8::Name;
+using v8::NamedPropertyHandlerConfiguration;
 using v8::None;
 using v8::Object;
 using v8::ObjectTemplate;
@@ -202,12 +206,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())
@@ -342,30 +348,34 @@ class ContextifyContext {
 
 
   static void GlobalPropertyGetterCallback(
-      Local<String> property,
+      Local<Name> property,
       const PropertyCallbackInfo<Value>& args) {
     Isolate* isolate = args.GetIsolate();
 
     ContextifyContext* ctx =
         Unwrap<ContextifyContext>(args.Data().As<Object>());
 
     Local<Object> sandbox = PersistentToLocal(isolate, ctx->sandbox_);
-    Local<Value> rv = sandbox->GetRealNamedProperty(property);
-    if (rv.IsEmpty()) {
+    MaybeLocal<Value> maybe_rv =
+        sandbox->GetRealNamedProperty(ctx->context(), property);
+    if (maybe_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_);
+      maybe_rv = proxy_global->GetRealNamedProperty(ctx->context(), property);
     }
 
-    args.GetReturnValue().Set(rv);
+    Local<Value> rv;
+    if (maybe_rv.ToLocal(&rv)) {
+      if (rv == ctx->sandbox_)
+        rv = PersistentToLocal(isolate, ctx->proxy_global_);
+
+      args.GetReturnValue().Set(rv);
+    }
   }
 
 
   static void GlobalPropertySetterCallback(
-      Local<String> property,
+      Local<Name> property,
       Local<Value> value,
       const PropertyCallbackInfo<Value>& args) {
     Isolate* isolate = args.GetIsolate();
@@ -378,42 +388,46 @@ class ContextifyContext {
 
 
   static void GlobalPropertyQueryCallback(
-      Local<String> property,
+      Local<Name> property,
       const PropertyCallbackInfo<Integer>& args) {
     Isolate* isolate = args.GetIsolate();
 
     ContextifyContext* ctx =
         Unwrap<ContextifyContext>(args.Data().As<Object>());
 
     Local<Object> sandbox = PersistentToLocal(isolate, ctx->sandbox_);
-    Local<Object> proxy_global = PersistentToLocal(isolate,
-                                                   ctx->proxy_global_);
-
-    if (sandbox->HasRealNamedProperty(property)) {
-      PropertyAttribute propAttr =
-          sandbox->GetRealNamedPropertyAttributes(property).FromJust();
-      args.GetReturnValue().Set(propAttr);
-    } else if (proxy_global->HasRealNamedProperty(property)) {
-      PropertyAttribute propAttr =
-          proxy_global->GetRealNamedPropertyAttributes(property).FromJust();
-      args.GetReturnValue().Set(propAttr);
-    } else {
-      args.GetReturnValue().Set(None);
+    Maybe<PropertyAttribute> maybe_prop_attr =
+        sandbox->GetRealNamedPropertyAttributes(ctx->context(), property);
+
+    if (maybe_prop_attr.IsNothing()) {
+      Local<Object> proxy_global = PersistentToLocal(isolate,
+          ctx->proxy_global_);
+
+      maybe_prop_attr =
+          proxy_global->GetRealNamedPropertyAttributes(ctx->context(),
+              property);
+    }
+
+    if (maybe_prop_attr.IsJust()) {
+      PropertyAttribute prop_attr = maybe_prop_attr.FromJust();
+      args.GetReturnValue().Set(prop_attr);
     }
   }
 
 
   static void GlobalPropertyDeleterCallback(
-      Local<String> property,
+      Local<Name> property,
       const PropertyCallbackInfo<Boolean>& args) {
     Isolate* isolate = args.GetIsolate();
 
     ContextifyContext* ctx =
         Unwrap<ContextifyContext>(args.Data().As<Object>());
+    Local<Object> sandbox = PersistentToLocal(isolate, ctx->sandbox_);
+
+    Maybe<bool> success = sandbox->Delete(ctx->context(), property);
 
-    bool success = PersistentToLocal(isolate,
-                                     ctx->sandbox_)->Delete(property);
-    args.GetReturnValue().Set(success);
+    if (success.IsJust())
+      args.GetReturnValue().Set(success.FromJust());
   }
 
 

diff --git a/test/parallel/test-vm-symbols.js b/test/parallel/test-vm-symbols.js
@@ -0,0 +1,25 @@
+'use strict';
+
+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');