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]>
nodejs · Feb 11, 2017 · 0237198 · 0237198
1 parent a33f601
commit 0237198
Show file tree

Hide file tree

Showing 2 changed files with 7 additions and 3 deletions.
diff --git a/src/node_contextify.cc b/src/node_contextify.cc
@@ -456,8 +456,12 @@ class ContextifyContext {
 
     Maybe<bool> success = ctx->sandbox()->Delete(ctx->context(), property);
 
-    if (success.IsJust())
-      args.GetReturnValue().Set(success.FromJust());
+    if (success.FromMaybe(false))
+      return;
+
+    // Delete failed on the sandbox, intercept and do not delete on
+    // the global object.
+    args.GetReturnValue().Set(false);
   }
 
 

diff --git a/...known_issues/test-vm-deleting-property.js → test/parallel/test-vm-deleting-property.js b/...known_issues/test-vm-deleting-property.js → test/parallel/test-vm-deleting-property.js
@@ -12,4 +12,4 @@ const res = vm.runInContext(`
   Object.getOwnPropertyDescriptor(this, 'x');
 `, context);
 
-assert.strictEqual(res.value, undefined);
+assert.strictEqual(res, undefined);