vm: mark global proxy as side-effect-free

addaleax · addaleax · commit 178dbb850333 · 2019-05-01T23:23:50.000+02:00
Fixes: #27518
diff --git a/src/node_contextify.cc b/src/node_contextify.cc
@@ -60,6 +60,7 @@ using v8::PrimitiveArray;
 using v8::PropertyAttribute;
 using v8::PropertyCallbackInfo;
 using v8::PropertyDescriptor;
+using v8::PropertyHandlerFlags;
 using v8::Script;
 using v8::ScriptCompiler;
 using v8::ScriptOrigin;
@@ -148,13 +149,15 @@ MaybeLocal<Context> ContextifyContext::CreateV8Context(
   if (!CreateDataWrapper(env).ToLocal(&data_wrapper))
     return MaybeLocal<Context>();
 
-  NamedPropertyHandlerConfiguration config(PropertyGetterCallback,
-                                           PropertySetterCallback,
-                                           PropertyDescriptorCallback,
-                                           PropertyDeleterCallback,
-                                           PropertyEnumeratorCallback,
-                                           PropertyDefinerCallback,
-                                           data_wrapper);
+  NamedPropertyHandlerConfiguration config(
+      PropertyGetterCallback,
+      PropertySetterCallback,
+      PropertyDescriptorCallback,
+      PropertyDeleterCallback,
+      PropertyEnumeratorCallback,
+      PropertyDefinerCallback,
+      data_wrapper,
+      PropertyHandlerFlags::kHasNoSideEffect);
 
   IndexedPropertyHandlerConfiguration indexed_config(
       IndexedPropertyGetterCallback,
@@ -163,7 +166,8 @@ MaybeLocal<Context> ContextifyContext::CreateV8Context(
       IndexedPropertyDeleterCallback,
       PropertyEnumeratorCallback,
       IndexedPropertyDefinerCallback,
-      data_wrapper);
+      data_wrapper,
+      PropertyHandlerFlags::kHasNoSideEffect);
 
   object_template->SetHandler(config);
   object_template->SetHandler(indexed_config);
diff --git a/test/parallel/test-inspector-vm-global-accessors-sideeffects.js b/test/parallel/test-inspector-vm-global-accessors-sideeffects.js
@@ -0,0 +1,31 @@
+'use strict';
+const common = require('../common');
+common.skipIfInspectorDisabled();
+
+// Regression test for https://github.com/nodejs/node/issues/27518.
+
+const assert = require('assert');
+const inspector = require('inspector');
+const vm = require('vm');
+
+const session = new inspector.Session();
+session.connect();
+
+const context = vm.createContext({
+  a: 100
+});
+
+session.post('Runtime.evaluate', {
+  expression: 'a',
+  throwOnSideEffect: true,
+  contextId: 2 // context's id
+}, (error, res) => {
+  assert.ifError(error),
+  assert.deepStrictEqual(res, {
+    result: {
+      type: 'number',
+      value: context.a,
+      description: '100'
+    }
+  });
+});
