vm: known issue with CopyProperties throw on empty code

CopyProperties() causes sandboxed Proxy to throw error
despite no code being run. The CopyProperties() function
will be removed shortly with the updates to the V8 API.

Refs: #11671

'use strict';

//Sandbox throws in CopyProperties() despite no code being run

require('../common');
const assert = require('assert');
const vm = require('vm');

const handler = {
    getOwnPropertyDescriptor: (target, prop) => {
      throw new Error('whoops');
    }
};
const sandbox = new Proxy({foo: 'bar'}, handler);
const context = vm.createContext(sandbox);


assert.doesNotThrow(() => vm.runInContext('', context));

Not sure if this is related (or even a problem), but the example below throws an error on v8.0.0-pre:

'use strict';

const handler = {
    getOwnPropertyDescriptor: function(target, prop) {
      throw new Error('whoops');
    }
};
const sandbox = new Proxy({foo: 'bar'}, handler);
console.log(sandbox);

/Users/ace/Desktop/node/test.js:5
      throw new Error('whoops');
      ^

Error: whoops
    at Object.get (/Users/ace/Desktop/node/test.js:5:13)
    at formatValue (util.js:311:38)
    at inspect (util.js:152:10)
    at exports.format (util.js:34:24)
    at Console.log (console.js:85:24)
    at Object.<anonymous> (/Users/ace/Desktop/node/test.js:9:9)
    at Module._compile (module.js:571:32)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:488:32)
    at tryModuleLoad (module.js:447:12)

AFAICT, console.log(sandbox) uses the getOwnPropertyDescriptor method as defined in the sandbox (i.e. in handler).

Since the console.log is running in the 'global' context, I'm guessing that it should be using the global getOwnPropertyDescriptor version (and not that of the sandbox).

Happy to investigate this further if this is indeed the/an issue and I'm on the right track.

Since the console.log is running in the 'global' context, I'm guessing that it should be using the global getOwnPropertyDescriptor version (and not that of the sandbox).

Can you elaborate? What did you mean by "'global' context"? If you mean Object.getOwnPropertyDescriptors(), that function uses the getOwnPropertyDescriptor trap of the proxy per spec.

@TimothyGu

@TimothyGu yeah - that is what I meant by the global context. What I'm not sure of is whether that trap is supposed to occur.

The error itself is triggered by calling Object.keys() on the sandbox object here.

It seems like it is, given that running the following example in Chrome's version of v8 gives this pattern:

/* sandbox code from previous comment */

console.log(sandbox); // no error
console.log(Object.keys(sandbox)); // raises error

To tie this back to the original issue: it seems like CopyProperties() calls Object.getOwnPropertyDescriptor() - but I don't know whether it should call the proxy's version or the default one.

@ace-n

@ace-n I’m not sure how CopyProperties() relates to the code that you are using, though? Unless you’re using the vm module in some way, I would say it’s unrelated, so I’d suggest opening a new issue.

@addaleax

@addaleax thanks - I've filed a new issue here and will continue the discussion there. Sorry for any confusion!

Should this stay open?

Yes, it's still an issue.

src: use new V8 API in vm module

fix(vm): cachedData fingerprints, proxy sandboxes, module link errors

Navigation Menu

Repository navigation

vm: known issue with CopyProperties throw on empty code #11902

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

vm: known issue with CopyProperties throw on empty code #11902

Description

Activity

ace-n commented on Apr 16, 2017

TimothyGu commented on Apr 16, 2017

ace-n commented on Apr 16, 2017

addaleax commented on Apr 16, 2017

ace-n commented on Apr 16, 2017

Trott commented on Aug 2, 2017

fhinkel commented on Oct 19, 2017

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions