diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 6590da62c5b497..77622f8cbd6d2b 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -61,6 +61,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; @@ -149,13 +150,15 @@ MaybeLocal ContextifyContext::CreateV8Context( if (!CreateDataWrapper(env).ToLocal(&data_wrapper)) return MaybeLocal(); - 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, @@ -164,7 +167,8 @@ MaybeLocal 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-getter-sideeffect.js b/test/parallel/test-inspector-vm-global-accessors-getter-sideeffect.js new file mode 100644 index 00000000000000..5dc9c9bb2dff3a --- /dev/null +++ b/test/parallel/test-inspector-vm-global-accessors-getter-sideeffect.js @@ -0,0 +1,33 @@ +'use strict'; +const common = require('../common'); +common.skipIfInspectorDisabled(); + +// Test that if there is a side effect in a getter invoked through the vm +// global proxy, Runtime.evaluate recognizes that. + +const assert = require('assert'); +const inspector = require('inspector'); +const vm = require('vm'); + +const session = new inspector.Session(); +session.connect(); + +const context = vm.createContext({ + get a() { + global.foo = '1'; + return 100; + } +}); + +session.post('Runtime.evaluate', { + expression: 'a', + throwOnSideEffect: true, + contextId: 2 // context's id +}, (error, res) => { + assert.ifError(error); + const { exception } = res.exceptionDetails; + assert.strictEqual(exception.className, 'EvalError'); + assert(/Possible side-effect/.test(exception.description)); + + assert(context); // Keep 'context' alive and make linter happy. +}); diff --git a/test/parallel/test-inspector-vm-global-accessors-sideeffects.js b/test/parallel/test-inspector-vm-global-accessors-sideeffects.js new file mode 100644 index 00000000000000..31551a08cb569b --- /dev/null +++ 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' + } + }); +});