Permalink
Browse files

src: allow getting Symbols on process.env

1aa595e introduced a `throw` for accessing `Symbol` properties of
`process.env`. However, this breaks `util.inspect(process)` and
things like `Object.prototype.toString.call(process.env)`, so this
patch changes the behaviour for the getter to just always return
`undefined`.

Ref: #9446
Fixes: #9641
PR-URL: #9631
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information...
addaleax committed Nov 16, 2016
1 parent 6b86ecc commit 3295a7febad0502dd79ad9fdae4c0975d1fb271c
Showing with 10 additions and 4 deletions.
  1. +3 −0 src/node.cc
  2. +5 −4 test/parallel/test-process-env-symbols.js
  3. +2 −0 test/parallel/test-util-inspect.js
View
@@ -2681,6 +2681,9 @@ static void ProcessTitleSetter(Local<Name> property,
static void EnvGetter(Local<Name> property,
const PropertyCallbackInfo<Value>& info) {
Isolate* isolate = info.GetIsolate();
if (property->IsSymbol()) {
return info.GetReturnValue().SetUndefined();
}
#ifdef __POSIX__
node::Utf8Value key(isolate, property);
const char* val = getenv(*key);
@@ -5,10 +5,8 @@ const assert = require('assert');
const symbol = Symbol('sym');
const errRegExp = /^TypeError: Cannot convert a Symbol value to a string$/;
// Verify that getting via a symbol key throws.
assert.throws(() => {
process.env[symbol];
}, errRegExp);
// Verify that getting via a symbol key returns undefined.
assert.strictEqual(process.env[symbol], undefined);
// Verify that assigning via a symbol key throws.
assert.throws(() => {
@@ -24,3 +22,6 @@ assert.throws(() => {
assert.throws(() => {
symbol in process.env;
}, errRegExp);
// Checks that well-known symbols like `Symbol.toStringTag` won’t throw.
assert.doesNotThrow(() => Object.prototype.toString.call(process.env));
@@ -925,3 +925,5 @@ checkAlignment(new Map(big_array.map(function(y) { return [y, null]; })));
util.inspect.defaultOptions = 'bad';
}, /"options" must be an object/);
}
assert.doesNotThrow(() => util.inspect(process));

0 comments on commit 3295a7f

Please sign in to comment.