Object.getOwnPropertyDescriptor operator in VM uses [[Get]] instead of [[GetOwnProperty]]Β #17481
Copy link
Copy link
Closed
Labels
v8 engineIssues and PRs related to the V8 dependency.Issues and PRs related to the V8 dependency.vmIssues and PRs related to the vm subsystem.Issues and PRs related to the vm subsystem.
Description
- Version: master
- Platform: all
- Subsystem: vm
Very similar to #17480, except the issue now is with Object.getOwnPropertyDescriptor rather than in operator.
const globals = {};
const handlers = {};
const realHandlers = Reflect.ownKeys(Reflect).reduce((handlers, p) => {
handlers[p] = (t, ...args) => {
// Avoid printing the Receiver argument, which can lead to an infinite loop.
console.log(p, ...(p === 'get' || p === 'set' ? args.slice(0, -1) : args));
return Reflect[p](t, ...args);
};
return handlers;
}, {});
const proxy = vm.createContext(new Proxy(globals, handlers));
// Indirection needed to mitigate against #17465
// https://github.com/nodejs/node/issues/17465
const globalProxy = vm.runInContext('this', proxy);
for (const k of Reflect.ownKeys(globalProxy)) {
Object.defineProperty(globals, k, Object.getOwnPropertyDescriptor(globalProxy, k));
}
Object.assign(handlers, realHandlers);
Object.getOwnPropertyDescriptor(proxy, "a")
// prints "getOwnPropertyDescriptor a"
// returns undefined
vm.runInContext('Object.getOwnPropertyDescriptor(this, "a")', proxy);
// prints "get Object"
// prints "getOwnPropertyDescriptor a"
// prints "get a"
// prints "get a"
// returns { value: undefined,
// writable: true,
// enumerable: false,
// configurable: true } (because of https://github.com/nodejs/node/issues/17465)Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
v8 engineIssues and PRs related to the V8 dependency.Issues and PRs related to the V8 dependency.vmIssues and PRs related to the vm subsystem.Issues and PRs related to the vm subsystem.
Activity
fhinkel commentedon Dec 21, 2017
See https://chromium.googlesource.com/v8/v8/+/d5fbf7c5c3f8f9b46b75f674771f3533c7e3e24d. Let's give it some canary coverage, then we can backport it. Thanks @TimothyGu
apapirovski commentedon Apr 12, 2018
@fhinkel @TimothyGu do you happen to have a status update on this? Still an issue? Has the fix landed in Node.js since then?
bnoordhuis commentedon Apr 12, 2018
The upstream commit was reverted again in https://chromium-review.googlesource.com/c/v8/v8/+/850355 because of performance issues.
TimothyGu commentedon Aug 24, 2018
Landed in fa543c0...85c356c.Ugh. Fixed in 85c356c / #22390.
deps: cherry-pick 9eb96bb from upstream V8
deps: cherry-pick e1a7699 from upstream V8
src: implement query callbacks for vm
src: implement query callbacks for vm