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

in operator not working correctly when using Proxy as VM context #30985

Open
underflow00 opened this issue Dec 16, 2019 · 7 comments
Open

in operator not working correctly when using Proxy as VM context #30985

underflow00 opened this issue Dec 16, 2019 · 7 comments
Labels
v8 engine Issues and PRs related to the V8 dependency. vm Issues and PRs related to the vm subsystem.

Comments

@underflow00
Copy link

underflow00 commented Dec 16, 2019

  • Version: v12.13.1
  • Platform: Windows 10

The in operator does not work correctly when using a Proxy as a VM context.

var o = {};
var p = new Proxy(o, {
    has(target, key) {
        console.log('has', key);
        return Reflect.has(target, key);
    },
    get(target, key, receiver) {
        console.log('get', key);
        return Reflect.get(target, key, receiver);
    },
});
vm.createContext(p);
vm.runInContext(`this.abcInThis = 'abc' in this`, p);
console.log(JSON.stringify(o)); // Prints {"abcInThis":false}

In the above code, the expected output is:

has abc
{"abcInThis":false}

but the actual output is:

get abc
{"abcInThis":true}

If we remove the get(target, key, receiver) function, the still incorrect output is:

{"abcInThis":false}
@underflow00 underflow00 changed the title in operator not working correctly when using Proxy as VM sandbox in operator not working correctly when using Proxy as VM context Dec 16, 2019
@devsnek
Copy link
Member

devsnek commented Dec 16, 2019

This is a limitation of V8. See: #22390

I'd be interested in fixing this at some point.

@addaleax addaleax added v8 engine Issues and PRs related to the V8 dependency. vm Issues and PRs related to the vm subsystem. labels Dec 18, 2019
@ExE-Boss
Copy link
Contributor

This also causes issues when using the with operator, which prevents merging jsdom/webidl2js#167.

@devsnek
Copy link
Member

devsnek commented Feb 15, 2020

I recently took a pretty deep dive into V8 internals to see what it would take to make fix this issue, and the quick answer is quite a lot. The good news is that V8 will need to update the internal global proxy infrastructure for the proposed realms api when they implement it, which will make it easier to fix this issue.

@ExE-Boss
Copy link
Contributor

See also: #17465

domenic pushed a commit to jsdom/jsdom that referenced this issue Mar 28, 2021
This works around a bug (nodejs/node#30985) which would prevent eventually migrating the global object to using a Proxy. It also prevents a global variable named "element" or "formOwner" from breaking the inner with statements.
@avivkeller avivkeller closed this as not planned Won't fix, can't repro, duplicate, stale Jul 11, 2024
@avivkeller
Copy link
Member

avivkeller commented Jul 11, 2024

const vm = require('vm');
var o = {};
var p = new Proxy(o, {
    has(target, key) {
        console.log('has', key);
        return Reflect.has(target, key);
    },
    get(target, key, receiver) {
        console.log('get', key);
        return Reflect.get(target, key, receiver);
    },
});
vm.createContext(p);
vm.runInContext(`this.abcInThis = 'abc' in this`, p);
console.log(JSON.stringify(o)); // Prints {"abcInThis":false}
get abc
{"abcInThis":true}

Still reproducible.

Sorry for the mis-close.

@ExE-Boss
Copy link
Contributor

@redyetidev
abcInThis should be false, so this is still reproducible.

@avivkeller
Copy link
Member

avivkeller commented Jul 12, 2024

Oh yes, I had it backwards, my bad, sorry.

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

5 participants