Skip to content

Commit 179ea5e

Browse files
committed
Revert "src: don't overwrite non-writable vm globals"
This reverts commit 524f693. Fixes: nodejs#10806 Fixes: nodejs#10492 Ref: nodejs#10227
1 parent ba776b3 commit 179ea5e

File tree

2 files changed

+10
-24
lines changed

2 files changed

+10
-24
lines changed

src/node_contextify.cc

+10-13
Original file line numberDiff line numberDiff line change
@@ -383,22 +383,19 @@ class ContextifyContext {
383383
if (ctx->context_.IsEmpty())
384384
return;
385385

386-
auto attributes = PropertyAttribute::None;
387386
bool is_declared =
388-
ctx->global_proxy()->GetRealNamedPropertyAttributes(ctx->context(),
389-
property)
390-
.To(&attributes);
391-
bool read_only =
392-
static_cast<int>(attributes) &
393-
static_cast<int>(PropertyAttribute::ReadOnly);
394-
395-
if (is_declared && read_only)
396-
return;
387+
ctx->global_proxy()->HasRealNamedProperty(ctx->context(),
388+
property).FromJust();
389+
bool is_contextual_store = ctx->global_proxy() != args.This();
397390

398-
if (!is_declared && args.ShouldThrowOnError())
399-
return;
391+
bool set_property_will_throw =
392+
args.ShouldThrowOnError() &&
393+
!is_declared &&
394+
is_contextual_store;
400395

401-
ctx->sandbox()->Set(property, value);
396+
if (!set_property_will_throw) {
397+
ctx->sandbox()->Set(property, value);
398+
}
402399
}
403400

404401

test/parallel/test-vm-context.js

-11
Original file line numberDiff line numberDiff line change
@@ -75,14 +75,3 @@ assert.throws(function() {
7575
// https://github.com/nodejs/node/issues/6158
7676
ctx = new Proxy({}, {});
7777
assert.strictEqual(typeof vm.runInNewContext('String', ctx), 'function');
78-
79-
// https://github.com/nodejs/node/issues/10223
80-
ctx = vm.createContext();
81-
vm.runInContext('Object.defineProperty(this, "x", { value: 42 })', ctx);
82-
assert.strictEqual(ctx.x, undefined); // Not copied out by cloneProperty().
83-
assert.strictEqual(vm.runInContext('x', ctx), 42);
84-
vm.runInContext('x = 0', ctx); // Does not throw but x...
85-
assert.strictEqual(vm.runInContext('x', ctx), 42); // ...should be unaltered.
86-
assert.throws(() => vm.runInContext('"use strict"; x = 0', ctx),
87-
/Cannot assign to read only property 'x'/);
88-
assert.strictEqual(vm.runInContext('x', ctx), 42);

0 commit comments

Comments
 (0)