Skip to content

Commit

Permalink
src: allow getting Symbols on process.env
Browse files Browse the repository at this point in the history
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 20, 2016
1 parent 6b86ecc commit 3295a7f
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 4 deletions.
3 changes: 3 additions & 0 deletions src/node.cc
Expand Up @@ -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);
Expand Down
9 changes: 5 additions & 4 deletions test/parallel/test-process-env-symbols.js
Expand Up @@ -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(() => {
Expand All @@ -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));
2 changes: 2 additions & 0 deletions test/parallel/test-util-inspect.js
Expand Up @@ -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.