Skip to content

Commit

Permalink
process: mark process.env as side-effect-free
Browse files Browse the repository at this point in the history
Read-only access to `process.env` does not have side effects.

Refs: nodejs#27523

PR-URL: nodejs#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>
  • Loading branch information
addaleax authored and ZYSzys committed May 18, 2019
1 parent 2a24aa2 commit abe8211
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 3 deletions.
4 changes: 3 additions & 1 deletion src/node_env_var.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -28,4 +28,4 @@ session.post('Runtime.evaluate', {
description: '100'
}
});
});
}));
24 changes: 24 additions & 0 deletions test/parallel/test-process-env-sideeffects.js
Original file line number Diff line number Diff line change
@@ -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' }
});
});

0 comments on commit abe8211

Please sign in to comment.