Commit 587857e
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 <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>1 parent 2db4c3c commit 587857e
2 files changed
Lines changed: 7 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -441,8 +441,12 @@ class ContextifyContext { | |||
| 441 | 441 | ||
| 442 | 442 | Maybe<bool> success = ctx->sandbox()->Delete(ctx->context(), property); | |
| 443 | 443 | ||
| 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); | ||
| 446 | 450 | } | |
| 447 | 451 | ||
| 448 | 452 | ||
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 number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -12,4 +12,4 @@ const res = vm.runInContext(` | |||
| 12 | 12 | Object.getOwnPropertyDescriptor(this, 'x'); | |
| 13 | 13 | `, context); | |
| 14 | 14 | ||
| 15 | - assert.strictEqual(res.value, undefined); | ||
| 15 | + assert.strictEqual(res, undefined); | ||
0 commit comments