Skip to content

Commit 587857e

Browse files
fhinkelMylesBorins
authored andcommitted
src: fix delete operator on vm context
In the implementation of the vm module, if a property is successfully deleted on the sandbox, we also need to delete it on the global_proxy object. Therefore, we must not call args.GetReturnValue().Set(). We only intercept, i.e., call args.GetReturnValue().Set(), in the DeleterCallback, if Delete() failed, e.g. because the property was read only. PR-URL: #11266 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
1 parent 2db4c3c commit 587857e

File tree

2 files changed

+7
-3
lines changed

2 files changed

+7
-3
lines changed

src/node_contextify.cc

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -441,8 +441,12 @@ class ContextifyContext {
441441

442442
Maybe<bool> success = ctx->sandbox()->Delete(ctx->context(), property);
443443

444-
if (success.IsJust())
445-
args.GetReturnValue().Set(success.FromJust());
444+
if (success.FromMaybe(false))
445+
return;
446+
447+
// Delete failed on the sandbox, intercept and do not delete on
448+
// the global object.
449+
args.GetReturnValue().Set(false);
446450
}
447451

448452

test/known_issues/test-vm-deleting-property.js renamed to test/parallel/test-vm-deleting-property.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,4 @@ const res = vm.runInContext(`
1212
Object.getOwnPropertyDescriptor(this, 'x');
1313
`, context);
1414

15-
assert.strictEqual(res.value, undefined);
15+
assert.strictEqual(res, undefined);

0 commit comments

Comments
 (0)