diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 5239c411500431..b68fe9ad215771 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -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(attributes) & static_cast(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(attributes) & + static_cast(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); } diff --git a/test/known_issues/test-vm-strict-mode.js b/test/parallel/test-vm-strict-mode.js similarity index 55% rename from test/known_issues/test-vm-strict-mode.js rename to test/parallel/test-vm-strict-mode.js index 9528944732930c..b1b233664dab9b 100644 --- a/test/known_issues/test-vm-strict-mode.js +++ b/test/parallel/test-vm-strict-mode.js @@ -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);