Skip to content

Commit

Permalink
test: add vm module edge cases
Browse files Browse the repository at this point in the history
Add two, admittedly contrived, examples that test
edge cases of the vm module.
They demonstrate that the if statements `if (maybe_rv.IsEmpty())` and
`if (maybe_prop_attr.IsNothing())` in the GetterCallback
and the QueryCallback are observable.

Both GetterCallback and QueryCallback
explicitly check the global_proxy() if a property is
not found on the sandbox. In these tests, the explicit check
inside the callback yields different results than deferring the
check until after the callback. The check is deferred, if the
callbacks do not intercept, i.e., if args.GetReturnValue().Set() is
not called.

PR-URL: #11265
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
fhinkel authored and MylesBorins committed Mar 9, 2017
1 parent 342c3e2 commit cb81ae8
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 0 deletions.
25 changes: 25 additions & 0 deletions test/known_issues/test-vm-attributes-property-not-on-sandbox.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
'use strict';
require('../common');
const assert = require('assert');
const vm = require('vm');

// The QueryCallback explicitly calls GetRealNamedPropertyAttributes
// on the global proxy if the property is not found on the sandbox.
//
// foo is not defined on the sandbox until we call CopyProperties().
// In the QueryCallback, we do not find the property on the sandbox
// and look up its PropertyAttributes on the global_proxy().
// PropertyAttributes are always flattened to a value
// descriptor.
const sandbox = {};
vm.createContext(sandbox);
const code = `Object.defineProperty(
this,
'foo',
{ get: function() {return 17} }
);
var desc = Object.getOwnPropertyDescriptor(this, 'foo');`;

vm.runInContext(code, sandbox);
// The descriptor is flattened. We wrongly have typeof desc.value = 'number'.
assert.strictEqual(typeof sandbox.desc.get, 'function');
37 changes: 37 additions & 0 deletions test/parallel/test-vm-property-not-on-sandbox.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
'use strict';
require('../common');
const assert = require('assert');
const vm = require('vm');

// This, admittedly contrived, example tests an edge cases of the vm module.
//
// The GetterCallback explicitly checks the global_proxy() if a property is
// not found on the sandbox. In the following test, the explicit check
// inside the callback yields different results than deferring the
// check until after the callback. The check is deferred if the
// callback does not intercept, i.e., if args.GetReturnValue().Set() is
// not called.

// Check that the GetterCallback explicitly calls GetRealNamedProperty()
// on the global proxy if the property is not found on the sandbox.
//
// foo is not defined on the sandbox until we call CopyProperties().
// In the GetterCallback, we do not find the property on the sandbox and
// get the property from the global proxy. Since the return value is
// the sandbox, we replace it by
// the global_proxy to keep the correct identities.
//
// This test case is partially inspired by
// https://github.com/nodejs/node/issues/855
const sandbox = {console};
sandbox.document = {defaultView: sandbox};
vm.createContext(sandbox);
const code = `Object.defineProperty(
this,
'foo',
{ get: function() {return document.defaultView} }
);
var result = foo === this;`;

vm.runInContext(code, sandbox);
assert.strictEqual(sandbox.result, true);

0 comments on commit cb81ae8

Please sign in to comment.