-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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
Comments
in
operator not working correctly when using Proxy as VM sandboxin
operator not working correctly when using Proxy as VM context
This is a limitation of V8. See: #22390 I'd be interested in fixing this at some point. |
This also causes issues when using the |
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. |
See also: #17465 |
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.
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}
Still reproducible. Sorry for the mis-close. |
@redyetidev |
Oh yes, I had it backwards, my bad, sorry. |
The
in
operator does not work correctly when using a Proxy as a VM context.In the above code, the expected output is:
but the actual output is:
If we remove the
get(target, key, receiver)
function, the still incorrect output is:The text was updated successfully, but these errors were encountered: