diff --git a/src/node_contextify.cc b/src/node_contextify.cc index d74b01ea0da371..5ed7d5faf0932e 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -383,19 +383,28 @@ class ContextifyContext { if (ctx->context_.IsEmpty()) return; + auto attributes = PropertyAttribute::None; bool is_declared = - ctx->global_proxy()->HasRealNamedProperty(ctx->context(), - property).FromJust(); + ctx->global_proxy()->GetRealNamedPropertyAttributes(ctx->context(), + property) + .To(&attributes); + bool read_only = + static_cast(attributes) & + static_cast(PropertyAttribute::ReadOnly); + + if (is_declared && read_only) + return; + + // true for x = 5 + // false for this.x = 5 + // false for Object.defineProperty(this, 'foo', ...) + // false for vmResult.x = 5 where vmResult = vm.runInContext(); bool is_contextual_store = ctx->global_proxy() != args.This(); - bool set_property_will_throw = - args.ShouldThrowOnError() && - !is_declared && - is_contextual_store; + if (!is_declared && args.ShouldThrowOnError() && is_contextual_store) + return; - if (!set_property_will_throw) { - ctx->sandbox()->Set(property, value); - } + ctx->sandbox()->Set(property, value); } diff --git a/test/parallel/test-vm-context.js b/test/parallel/test-vm-context.js index 75d91ce8129dcb..c58bdc29daba66 100644 --- a/test/parallel/test-vm-context.js +++ b/test/parallel/test-vm-context.js @@ -75,3 +75,16 @@ assert.throws(function() { // https://github.com/nodejs/node/issues/6158 ctx = new Proxy({}, {}); assert.strictEqual(typeof vm.runInNewContext('String', ctx), 'function'); + +// https://github.com/nodejs/node/issues/10223 +ctx = vm.createContext(); +vm.runInContext('Object.defineProperty(this, "x", { value: 42 })', ctx); +assert.strictEqual(ctx.x, 42); +assert.strictEqual(vm.runInContext('x', ctx), 42); + +vm.runInContext('x = 0', ctx); // Does not throw but x... +assert.strictEqual(vm.runInContext('x', ctx), 42); // ...should be unaltered. + +assert.throws(() => vm.runInContext('"use strict"; x = 0', ctx), + /Cannot assign to read only property 'x'/); +assert.strictEqual(vm.runInContext('x', ctx), 42);