Repository navigation
Object.defineProperty called inside the vm context fails to copy data properties onto the sandbox #10977
Description
- Version: current master
- Platform: OS X
- Subsystem: vm
Data properties defined with the Object.defineProperty call inside the vm context are not copied onto the sandbox in the current master.
Test:
'use strict';
require('../common');
var vm = require('vm');
const util = require('util');
const sandbox = {};
const context = vm.createContext(sandbox);
const code = `
Object.defineProperty(this, "foo", {value: 5});
`;
const res = vm.runInContext(code, context);
console.log(util.inspect(sandbox)); // returns: {}
In v6.2.0 (homebrew installation):
> node test_setter.js
{ foo: 5 }
Debugging the core shows failure in GlobalPropertySetterCallback.
ctx->sandbox()->Set(property, value);
does not set 'foo' on the sandbox (property and value are as expected). 'foo' is present on the global object, but also not returned by
Local<Array> names = global->GetOwnPropertyNames(context).ToLocalChecked();
in CopyProperties.
Activity
AnnaMag commentedon Jan 24, 2017
I think it is a good idea to add the test to the known_issues.
@fhinkel
jasnell commentedon Jan 24, 2017
Agree. A PR would be wonderful :-)
fhinkel commentedon Jan 24, 2017
On a side note, if you want to test different node versions, you probably want nvm.
bnoordhuis commentedon Jan 24, 2017
#10920 fixes this.
AnnaMag commentedon Jan 24, 2017
No need for a PR then, am I right?
bnoordhuis commentedon Jan 24, 2017
Not for this issue but #10920 will reintroduce #10223 and that will need to be fixed eventually. It's yours if you want it.
AnnaMag commentedon Jan 24, 2017
I'm working with the V8 5.5. API (removing CopyProperties). #10223 should be fixed once 5.5. lands in master and we can send PRs with the refactored code. Do you agree @fhinkel?
fhinkel commentedon Jan 24, 2017
I think it's good to have the test. Either as known issue or as a regression test.