Skip to content

Commit 9531d70

Browse files
committed
vm: fix symbol access
By using the new SetHandler API instead of SetNamedPropertyHandler, we can intercept symbols now. Fixes #884.
1 parent d119272 commit 9531d70

File tree

2 files changed

+65
-26
lines changed

2 files changed

+65
-26
lines changed

src/node_contextify.cc

Lines changed: 42 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@ using v8::HandleScope;
2626
using v8::Integer;
2727
using v8::Isolate;
2828
using v8::Local;
29+
using v8::Maybe;
30+
using v8::MaybeLocal;
31+
using v8::Name;
32+
using v8::NamedPropertyHandlerConfiguration;
2933
using v8::None;
3034
using v8::Object;
3135
using v8::ObjectTemplate;
@@ -202,12 +206,14 @@ class ContextifyContext {
202206

203207
Local<ObjectTemplate> object_template =
204208
function_template->InstanceTemplate();
205-
object_template->SetNamedPropertyHandler(GlobalPropertyGetterCallback,
209+
210+
NamedPropertyHandlerConfiguration config(GlobalPropertyGetterCallback,
206211
GlobalPropertySetterCallback,
207212
GlobalPropertyQueryCallback,
208213
GlobalPropertyDeleterCallback,
209214
GlobalPropertyEnumeratorCallback,
210215
CreateDataWrapper(env));
216+
object_template->SetHandler(config);
211217

212218
Local<Context> ctx = Context::New(env->isolate(), nullptr, object_template);
213219
if (!ctx.IsEmpty())
@@ -342,7 +348,7 @@ class ContextifyContext {
342348

343349

344350
static void GlobalPropertyGetterCallback(
345-
Local<String> property,
351+
Local<Name> property,
346352
const PropertyCallbackInfo<Value>& args) {
347353
Isolate* isolate = args.GetIsolate();
348354
HandleScope scope(isolate);
@@ -351,22 +357,27 @@ class ContextifyContext {
351357
Unwrap<ContextifyContext>(args.Data().As<Object>());
352358

353359
Local<Object> sandbox = PersistentToLocal(isolate, ctx->sandbox_);
354-
Local<Value> rv = sandbox->GetRealNamedProperty(property);
355-
if (rv.IsEmpty()) {
360+
MaybeLocal<Value> maybeRV =
361+
sandbox->GetRealNamedProperty(ctx->context(), property);
362+
if (maybeRV.IsEmpty()) {
356363
Local<Object> proxy_global = PersistentToLocal(isolate,
357364
ctx->proxy_global_);
358-
rv = proxy_global->GetRealNamedProperty(property);
359-
}
360-
if (!rv.IsEmpty() && rv == ctx->sandbox_) {
361-
rv = PersistentToLocal(isolate, ctx->proxy_global_);
365+
maybeRV = proxy_global->GetRealNamedProperty(ctx->context(), property);
362366
}
363367

364-
args.GetReturnValue().Set(rv);
368+
Local<Value> rv;
369+
if (maybeRV.ToLocal(&rv)) {
370+
if (rv == ctx->sandbox_) {
371+
rv = PersistentToLocal(isolate, ctx->proxy_global_);
372+
}
373+
374+
args.GetReturnValue().Set(rv);
375+
}
365376
}
366377

367378

368379
static void GlobalPropertySetterCallback(
369-
Local<String> property,
380+
Local<Name> property,
370381
Local<Value> value,
371382
const PropertyCallbackInfo<Value>& args) {
372383
Isolate* isolate = args.GetIsolate();
@@ -380,7 +391,7 @@ class ContextifyContext {
380391

381392

382393
static void GlobalPropertyQueryCallback(
383-
Local<String> property,
394+
Local<Name> property,
384395
const PropertyCallbackInfo<Integer>& args) {
385396
Isolate* isolate = args.GetIsolate();
386397
HandleScope scope(isolate);
@@ -389,35 +400,40 @@ class ContextifyContext {
389400
Unwrap<ContextifyContext>(args.Data().As<Object>());
390401

391402
Local<Object> sandbox = PersistentToLocal(isolate, ctx->sandbox_);
392-
Local<Object> proxy_global = PersistentToLocal(isolate,
393-
ctx->proxy_global_);
394403

395-
if (sandbox->HasRealNamedProperty(property)) {
396-
PropertyAttribute propAttr =
397-
sandbox->GetRealNamedPropertyAttributes(property).FromJust();
398-
args.GetReturnValue().Set(propAttr);
399-
} else if (proxy_global->HasRealNamedProperty(property)) {
400-
PropertyAttribute propAttr =
401-
proxy_global->GetRealNamedPropertyAttributes(property).FromJust();
404+
Maybe<PropertyAttribute> maybePropAttr =
405+
sandbox->GetRealNamedPropertyAttributes(ctx->context(), property);
406+
407+
if (maybePropAttr.IsNothing()) {
408+
Local<Object> proxy_global = PersistentToLocal(isolate,
409+
ctx->proxy_global_);
410+
411+
maybePropAttr =
412+
proxy_global->GetRealNamedPropertyAttributes(ctx->context(), property);
413+
}
414+
415+
if (maybePropAttr.IsJust()) {
416+
PropertyAttribute propAttr = maybePropAttr.FromJust();
402417
args.GetReturnValue().Set(propAttr);
403-
} else {
404-
args.GetReturnValue().Set(None);
405418
}
406419
}
407420

408421

409422
static void GlobalPropertyDeleterCallback(
410-
Local<String> property,
423+
Local<Name> property,
411424
const PropertyCallbackInfo<Boolean>& args) {
412425
Isolate* isolate = args.GetIsolate();
413426
HandleScope scope(isolate);
414427

415428
ContextifyContext* ctx =
416429
Unwrap<ContextifyContext>(args.Data().As<Object>());
430+
Local<Object> sandbox = PersistentToLocal(isolate, ctx->sandbox_);
431+
432+
Maybe<bool> success = sandbox->Delete(ctx->context(), property);
417433

418-
bool success = PersistentToLocal(isolate,
419-
ctx->sandbox_)->Delete(property);
420-
args.GetReturnValue().Set(success);
434+
if (success.IsJust()) {
435+
args.GetReturnValue().Set(success.FromJust());
436+
}
421437
}
422438

423439

test/parallel/test-vm-symbols.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
var common = require('../common');
2+
var assert = require('assert');
3+
4+
var vm = require('vm');
5+
6+
var symbol = Symbol();
7+
8+
function Document() {
9+
this[symbol] = 'foo';
10+
}
11+
12+
Document.prototype.getSymbolValue = function () {
13+
return this[symbol];
14+
};
15+
16+
var context = new Document();
17+
vm.createContext(context);
18+
19+
assert.equal(context.getSymbolValue(), 'foo',
20+
'should return symbol-keyed value from the outside');
21+
22+
assert.equal(vm.runInContext('this.getSymbolValue()', context), 'foo',
23+
'should return symbol-keyed value from the inside');

0 commit comments

Comments
 (0)