From 5b79b1700fb324e4831621fd6a8b9d24d462e4fb Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 14 May 2019 02:01:09 +0200 Subject: [PATCH 1/3] process: mark process.env as side-effect-free Read-only access to `process.env` does not have side effects. Refs: https://github.com/nodejs/node/pull/27523 --- src/node_env_var.cc | 4 +++- test/parallel/test-process-env-sideeffects.js | 24 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-process-env-sideeffects.js diff --git a/src/node_env_var.cc b/src/node_env_var.cc index 4b398ce7cd877e..717c675d9a7d3c 100644 --- a/src/node_env_var.cc +++ b/src/node_env_var.cc @@ -28,6 +28,7 @@ using v8::Nothing; using v8::Object; using v8::ObjectTemplate; using v8::PropertyCallbackInfo; +using v8::PropertyHandlerFlags; using v8::String; using v8::Value; @@ -377,7 +378,8 @@ MaybeLocal CreateEnvVarProxy(Local context, EscapableHandleScope scope(isolate); Local env_proxy_template = ObjectTemplate::New(isolate); env_proxy_template->SetHandler(NamedPropertyHandlerConfiguration( - EnvGetter, EnvSetter, EnvQuery, EnvDeleter, EnvEnumerator, data)); + EnvGetter, EnvSetter, EnvQuery, EnvDeleter, EnvEnumerator, data, + PropertyHandlerFlags::kHasNoSideEffect)); return scope.EscapeMaybe(env_proxy_template->NewInstance(context)); } } // namespace node diff --git a/test/parallel/test-process-env-sideeffects.js b/test/parallel/test-process-env-sideeffects.js new file mode 100644 index 00000000000000..e907729c46b7e0 --- /dev/null +++ b/test/parallel/test-process-env-sideeffects.js @@ -0,0 +1,24 @@ +'use strict'; +const common = require('../common'); +common.skipIfInspectorDisabled(); + +// Test that read-only process.env access is considered to have no +// side-effects by the insepctor. + +const assert = require('assert'); +const inspector = require('inspector'); + +const session = new inspector.Session(); +session.connect(); + +process.env.TESTVAR = 'foobar'; + +session.post('Runtime.evaluate', { + expression: 'process.env.TESTVAR', + throwOnSideEffect: true +}, (error, res) => { + assert.ifError(error); + assert.deepStrictEqual(res, { + result: { type: 'string', value: 'foobar' } + }); +}); From 02eee2bfc30c251d3d3c2070da2f8a6ae1ef9d1e Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 14 May 2019 02:26:32 +0200 Subject: [PATCH 2/3] fixup! process: mark process.env as side-effect-free --- .../test-inspector-vm-global-accessors-sideeffects.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-inspector-vm-global-accessors-sideeffects.js b/test/parallel/test-inspector-vm-global-accessors-sideeffects.js index 31551a08cb569b..33545e14c7ae2b 100644 --- a/test/parallel/test-inspector-vm-global-accessors-sideeffects.js +++ b/test/parallel/test-inspector-vm-global-accessors-sideeffects.js @@ -19,7 +19,7 @@ session.post('Runtime.evaluate', { expression: 'a', throwOnSideEffect: true, contextId: 2 // context's id -}, (error, res) => { +}, common.mustCall((error, res) => { assert.ifError(error), assert.deepStrictEqual(res, { result: { @@ -28,4 +28,4 @@ session.post('Runtime.evaluate', { description: '100' } }); -}); +})); From bbd66ab4064b641c50d53fdf95eac9384132b5ef Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 14 May 2019 13:37:48 +0200 Subject: [PATCH 3/3] fixup! fixup! process: mark process.env as side-effect-free --- test/parallel/test-process-env-sideeffects.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-process-env-sideeffects.js b/test/parallel/test-process-env-sideeffects.js index e907729c46b7e0..ee05e40e579c80 100644 --- a/test/parallel/test-process-env-sideeffects.js +++ b/test/parallel/test-process-env-sideeffects.js @@ -3,7 +3,7 @@ const common = require('../common'); common.skipIfInspectorDisabled(); // Test that read-only process.env access is considered to have no -// side-effects by the insepctor. +// side-effects by the inspector. const assert = require('assert'); const inspector = require('inspector');