From 256bd7c3fb28756c4ce4c175464426d218da575b Mon Sep 17 00:00:00 2001
From: Domenic Denicola <d@domenic.me>
Date: Fri, 22 May 2015 19:41:28 -0400
Subject: [PATCH] vm: fix property descriptors of sandbox properties

The GlobalPropertyQueryCallback was changed in 2010 to return an
integer instead of a boolean:

https://groups.google.com/forum/#!topic/v8-users/OOjHJrix-cU

This integer communicates the property descriptors of the property,
instead of just its presence or absence. However, the original
contextify code was probably written before this change, and it was
not updated when porting to Node.js.

Credit to @smikes for the test and the original PR of #885.

Fixes #885; fixes #864.
---
 src/node_contextify.cc                      | 14 ++++++++----
 test/parallel/test-vm-preserves-property.js | 25 +++++++++++++++++++++
 2 files changed, 35 insertions(+), 4 deletions(-)
 create mode 100644 test/parallel/test-vm-preserves-property.js

diff --git a/src/node_contextify.cc b/src/node_contextify.cc
index 81d0388a35f390..febcd4320a4aae 100644
--- a/src/node_contextify.cc
+++ b/src/node_contextify.cc
@@ -30,6 +30,7 @@ using v8::None;
 using v8::Object;
 using v8::ObjectTemplate;
 using v8::Persistent;
+using v8::PropertyAttribute;
 using v8::PropertyCallbackInfo;
 using v8::Script;
 using v8::ScriptCompiler;
@@ -409,10 +410,15 @@ class ContextifyContext {
     Local<Object> proxy_global = PersistentToLocal(isolate,
                                                    ctx->proxy_global_);
 
-    bool in_sandbox = sandbox->GetRealNamedProperty(property).IsEmpty();
-    bool in_proxy_global =
-        proxy_global->GetRealNamedProperty(property).IsEmpty();
-    if (!in_sandbox || !in_proxy_global) {
+    if (sandbox->HasRealNamedProperty(property)) {
+      PropertyAttribute propAttr =
+        sandbox->GetRealNamedPropertyAttributes(property).FromJust();
+      args.GetReturnValue().Set(propAttr);
+    } else if (proxy_global->HasRealNamedProperty(property)) {
+      PropertyAttribute propAttr =
+        proxy_global->GetRealNamedPropertyAttributes(property).FromJust();
+      args.GetReturnValue().Set(propAttr);
+    } else {
       args.GetReturnValue().Set(None);
     }
   }
diff --git a/test/parallel/test-vm-preserves-property.js b/test/parallel/test-vm-preserves-property.js
new file mode 100644
index 00000000000000..85ce2d6e8165c8
--- /dev/null
+++ b/test/parallel/test-vm-preserves-property.js
@@ -0,0 +1,25 @@
+'use strict';
+
+var common = require('../common');
+var assert = require('assert');
+
+var vm = require('vm');
+
+var x = {};
+Object.defineProperty(x, 'prop', {
+  configurable: false,
+  enumerable: false,
+  writable: false,
+  value: 'val'
+});
+var o = vm.createContext(x);
+
+var code = 'Object.getOwnPropertyDescriptor(this, "prop")';
+var res = vm.runInContext(code, o, 'test');
+
+assert(res);
+assert.equal(typeof res, 'object');
+assert.equal(res.value, 'val');
+assert.equal(res.configurable, false, 'should not be configurable');
+assert.equal(res.enumerable, false, 'should not be enumerable');
+assert.equal(res.writable, false, 'should not be writable');