Skip to content

Commit 74feb0b

Browse files
addaleaxtargos
authored andcommitted
process: mark process.env as side-effect-free
Read-only access to `process.env` does not have side effects. Refs: #27523 PR-URL: #27684 Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Aleksei Koziatinskii <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]>
1 parent e058900 commit 74feb0b

3 files changed

+29
-3
lines changed

src/node_env_var.cc

+3-1
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ using v8::Nothing;
2828
using v8::Object;
2929
using v8::ObjectTemplate;
3030
using v8::PropertyCallbackInfo;
31+
using v8::PropertyHandlerFlags;
3132
using v8::String;
3233
using v8::Value;
3334

@@ -377,7 +378,8 @@ MaybeLocal<Object> CreateEnvVarProxy(Local<Context> context,
377378
EscapableHandleScope scope(isolate);
378379
Local<ObjectTemplate> env_proxy_template = ObjectTemplate::New(isolate);
379380
env_proxy_template->SetHandler(NamedPropertyHandlerConfiguration(
380-
EnvGetter, EnvSetter, EnvQuery, EnvDeleter, EnvEnumerator, data));
381+
EnvGetter, EnvSetter, EnvQuery, EnvDeleter, EnvEnumerator, data,
382+
PropertyHandlerFlags::kHasNoSideEffect));
381383
return scope.EscapeMaybe(env_proxy_template->NewInstance(context));
382384
}
383385
} // namespace node

test/parallel/test-inspector-vm-global-accessors-sideeffects.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ session.post('Runtime.evaluate', {
1919
expression: 'a',
2020
throwOnSideEffect: true,
2121
contextId: 2 // context's id
22-
}, (error, res) => {
22+
}, common.mustCall((error, res) => {
2323
assert.ifError(error),
2424
assert.deepStrictEqual(res, {
2525
result: {
@@ -28,4 +28,4 @@ session.post('Runtime.evaluate', {
2828
description: '100'
2929
}
3030
});
31-
});
31+
}));
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
'use strict';
2+
const common = require('../common');
3+
common.skipIfInspectorDisabled();
4+
5+
// Test that read-only process.env access is considered to have no
6+
// side-effects by the inspector.
7+
8+
const assert = require('assert');
9+
const inspector = require('inspector');
10+
11+
const session = new inspector.Session();
12+
session.connect();
13+
14+
process.env.TESTVAR = 'foobar';
15+
16+
session.post('Runtime.evaluate', {
17+
expression: 'process.env.TESTVAR',
18+
throwOnSideEffect: true
19+
}, (error, res) => {
20+
assert.ifError(error);
21+
assert.deepStrictEqual(res, {
22+
result: { type: 'string', value: 'foobar' }
23+
});
24+
});

0 commit comments

Comments
 (0)