From 7c003215b0c4ea687dd940415186dcef53566ded Mon Sep 17 00:00:00 2001
From: Franziska Hinkelmann <franzih@chromium.org>
Date: Thu, 9 Feb 2017 17:46:13 +0100
Subject: [PATCH] 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.

Fixes https://github.com/nodejs/node/issues/6287
---
 src/node_contextify.cc                                    | 8 ++++++--
 .../test-vm-deleting-property.js                          | 2 +-
 2 files changed, 7 insertions(+), 3 deletions(-)
 rename test/{known_issues => parallel}/test-vm-deleting-property.js (88%)

diff --git a/src/node_contextify.cc b/src/node_contextify.cc
index 4efb420fa128b7..d9b6306954437b 100644
--- 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/test/known_issues/test-vm-deleting-property.js b/test/parallel/test-vm-deleting-property.js
similarity index 88%
rename from test/known_issues/test-vm-deleting-property.js
rename to test/parallel/test-vm-deleting-property.js
index cda3371a33bf16..994aa0aff94f2d 100644
--- a/test/known_issues/test-vm-deleting-property.js
+++ b/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);