1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

src: fix vm module for strict mode

Browse Source 
This patch fixes the problem with variables that
are declared only on the sandbox but not on the
global proxy.

PR-URL: https://github.com/nodejs/node/pull/16487
Fixes: https://github.com/nodejs/node/issues/12300
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
This commit is contained in:
Franziska Hinkelmann 2017-10-25 13:49:58 +02:00
parent fa939f0cf5
commit 5856c836ea
2 changed files with 21 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
src/node_contextify.cc
View File

@ -346,14 +346,21 @@ class ContextifyContext {
return;
auto attributes = PropertyAttribute::None;
bool is_declared = ctx->global_proxy()
bool is_declared_on_global_proxy = ctx->global_proxy()
->GetRealNamedPropertyAttributes(ctx->context(), property)
.To(&attributes);
bool read_only =
static_cast<int>(attributes) &
static_cast<int>(PropertyAttribute::ReadOnly);
if (is_declared && read_only)
bool is_declared_on_sandbox = ctx->sandbox()
->GetRealNamedPropertyAttributes(ctx->context(), property)
.To(&attributes);
read_only = read_only ||
(static_cast<int>(attributes) &
static_cast<int>(PropertyAttribute::ReadOnly));
if (read_only)
return;
// true for x = 5
@ -371,10 +378,20 @@ class ContextifyContext {
// this.f = function() {}, is_contextual_store = false.
bool is_function = value->IsFunction();
bool is_declared = is_declared_on_global_proxy || is_declared_on_sandbox;
if (!is_declared && args.ShouldThrowOnError() && is_contextual_store &&
!is_function)
return;
if (!is_declared_on_global_proxy && is_declared_on_sandbox &&
args.ShouldThrowOnError() && is_contextual_store && !is_function) {
// The property exists on the sandbox but not on the global
// proxy. Setting it would throw because we are in strict mode.
// Don't attempt to set it by signaling that the call was
// intercepted. Only change the value on the sandbox.
args.GetReturnValue().Set(false);
}
ctx->sandbox()->Set(property, value);
}

7
test/known_issues/test-vm-strict-mode.js → test/parallel/test-vm-strict-mode.js
View File

@ -7,11 +7,8 @@ const vm = require('vm');
const ctx = vm.createContext({ x: 42 });
// The following line wrongly throws an
// error because GlobalPropertySetterCallback()
// does not check if the property exists
// on the sandbox. It should just set x to 1
// instead of throwing an error.
// This might look as if x has not been declared, but x is defined on the
// sandbox and the assignment should not throw.
vm.runInContext('"use strict"; x = 1', ctx);
assert.strictEqual(ctx.x, 1);