From 74feb0b81ece02f66c6d209c129f7357ed6a6940 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Tue, 14 May 2019 02:01:09 +0200
Subject: [PATCH] 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

PR-URL: https://github.com/nodejs/node/pull/27684
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Aleksei Koziatinskii <ak239spb@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
---
 src/node_env_var.cc                           |  4 +++-
 ...spector-vm-global-accessors-sideeffects.js |  4 ++--
 test/parallel/test-process-env-sideeffects.js | 24 +++++++++++++++++++
 3 files changed, 29 insertions(+), 3 deletions(-)
 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<Object> CreateEnvVarProxy(Local<Context> context,
   EscapableHandleScope scope(isolate);
   Local<ObjectTemplate> 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-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'
     }
   });
-});
+}));
diff --git a/test/parallel/test-process-env-sideeffects.js b/test/parallel/test-process-env-sideeffects.js
new file mode 100644
index 00000000000000..ee05e40e579c80
--- /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 inspector.
+
+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' }
+  });
+});