Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VM treats Proxy objects with/without get trap differently #17465

Closed
TimothyGu opened this issue Dec 5, 2017 · 1 comment
Closed

VM treats Proxy objects with/without get trap differently #17465

TimothyGu opened this issue Dec 5, 2017 · 1 comment
Labels
v8 engine Issues and PRs related to the V8 dependency. vm Issues and PRs related to the vm subsystem.

Comments

@TimothyGu
Copy link
Member

TimothyGu commented Dec 5, 2017

  • Version: master
  • Platform: all
  • Subsystem: vm

This is a case of "I understand why it acts this way but it still feels weird." Maybe we could put this in the documentation, but filing it here so it's recorded somewhere at least.

> vm.runInNewContext('this', new Proxy({}, {})).Object
[Function: Object]
> vm.runInNewContext('this', new Proxy({}, { get: Reflect.get })).Object
undefined

One might expect them to both return the Object function.

This is probably because of the fact that VM checks whether or not the sandbox object has the requested property. In the first case, it can just check the target object, since V8 knows nothing funny is going on with [[Get]]. In the second, even though Reflect.get should act the same way as not specifying a trap, V8 doesn't know that, since unlike [[GetOwnProperty]]/getOwnPropertyDescriptor [[Get]]/get has no way of differentiating between an undefined-valued property and a nonexistent property.

Code-wise,

node/deps/v8/src/objects.cc

Lines 1064 to 1116 in 35c01d8

// static
MaybeHandle<Object> JSProxy::GetProperty(Isolate* isolate,
Handle<JSProxy> proxy,
Handle<Name> name,
Handle<Object> receiver,
bool* was_found) {
*was_found = true;
DCHECK(!name->IsPrivate());
STACK_CHECK(isolate, MaybeHandle<Object>());
Handle<Name> trap_name = isolate->factory()->get_string();
// 1. Assert: IsPropertyKey(P) is true.
// 2. Let handler be the value of the [[ProxyHandler]] internal slot of O.
Handle<Object> handler(proxy->handler(), isolate);
// 3. If handler is null, throw a TypeError exception.
// 4. Assert: Type(handler) is Object.
if (proxy->IsRevoked()) {
THROW_NEW_ERROR(isolate,
NewTypeError(MessageTemplate::kProxyRevoked, trap_name),
Object);
}
// 5. Let target be the value of the [[ProxyTarget]] internal slot of O.
Handle<JSReceiver> target(proxy->target(), isolate);
// 6. Let trap be ? GetMethod(handler, "get").
Handle<Object> trap;
ASSIGN_RETURN_ON_EXCEPTION(
isolate, trap,
Object::GetMethod(Handle<JSReceiver>::cast(handler), trap_name), Object);
// 7. If trap is undefined, then
if (trap->IsUndefined(isolate)) {
// 7.a Return target.[[Get]](P, Receiver).
LookupIterator it =
LookupIterator::PropertyOrElement(isolate, receiver, name, target);
MaybeHandle<Object> result = Object::GetProperty(&it);
*was_found = it.IsFound();
return result;
}
// 8. Let trapResult be ? Call(trap, handler, «target, P, Receiver»).
Handle<Object> trap_result;
Handle<Object> args[] = {target, name, receiver};
ASSIGN_RETURN_ON_EXCEPTION(
isolate, trap_result,
Execution::Call(isolate, trap, handler, arraysize(args), args), Object);
MaybeHandle<Object> result =
JSProxy::CheckGetTrapResult(isolate, name, target, trap_result);
if (result.is_null()) {
return result;
}
// 11. Return trap_result
return trap_result;
}

after being initialized to true, was_found can only be set to false if the get trap is undefined, and prototype lookup for the Proxy target reveals nothing with that property name.

@TimothyGu TimothyGu added the vm Issues and PRs related to the vm subsystem. label Dec 5, 2017
@maclover7 maclover7 added the v8 engine Issues and PRs related to the V8 dependency. label Dec 25, 2017
@TimothyGu
Copy link
Member Author

I'm going to call this a can't fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v8 engine Issues and PRs related to the V8 dependency. vm Issues and PRs related to the vm subsystem.
Projects
None yet
Development

No branches or pull requests

2 participants