Permalink
Browse files

src: throw on process.env symbol usage

This commit causes process.env to throw when a symbol is used as
either a key or a value.

Fixes: #9429
PR-URL: #9446
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
  • Loading branch information...
cjihrig committed Nov 3, 2016
1 parent b315e24 commit 1aa595e5bd64241451b3884d3029b9b9aa97c42d
Showing with 32 additions and 42 deletions.
  1. +6 −8 src/node.cc
  2. +26 −0 test/parallel/test-process-env-symbols.js
  3. +0 −34 test/parallel/test-v8-interceptStrings-not-Symbols.js
View
@@ -122,7 +122,6 @@ using v8::ObjectTemplate;
using v8::Promise;
using v8::PromiseRejectMessage;
using v8::PropertyCallbackInfo;
using v8::PropertyHandlerFlags;
using v8::ScriptOrigin;
using v8::SealHandleScope;
using v8::String;
@@ -2685,7 +2684,7 @@ static void EnvGetter(Local<Name> property,
return info.GetReturnValue().Set(String::NewFromUtf8(isolate, val));
}
#else // _WIN32
String::Value key(property);
node::TwoByteValue key(isolate, property);
WCHAR buffer[32767]; // The maximum size allowed for environment variables.
DWORD result = GetEnvironmentVariableW(reinterpret_cast<WCHAR*>(*key),
buffer,
@@ -2711,8 +2710,8 @@ static void EnvSetter(Local<Name> property,
node::Utf8Value val(info.GetIsolate(), value);
setenv(*key, *val, 1);
#else // _WIN32
String::Value key(property);
String::Value val(value);
node::TwoByteValue key(info.GetIsolate(), property);
node::TwoByteValue val(info.GetIsolate(), value);
WCHAR* key_ptr = reinterpret_cast<WCHAR*>(*key);
// Environment variables that start with '=' are read-only.
if (key_ptr[0] != L'=') {
@@ -2732,7 +2731,7 @@ static void EnvQuery(Local<Name> property,
if (getenv(*key))
rc = 0;
#else // _WIN32
String::Value key(property);
node::TwoByteValue key(info.GetIsolate(), property);
WCHAR* key_ptr = reinterpret_cast<WCHAR*>(*key);
if (GetEnvironmentVariableW(key_ptr, nullptr, 0) > 0 ||
GetLastError() == ERROR_SUCCESS) {
@@ -2756,7 +2755,7 @@ static void EnvDeleter(Local<Name> property,
node::Utf8Value key(info.GetIsolate(), property);
unsetenv(*key);
#else
String::Value key(property);
node::TwoByteValue key(info.GetIsolate(), property);
WCHAR* key_ptr = reinterpret_cast<WCHAR*>(*key);
SetEnvironmentVariableW(key_ptr, nullptr);
#endif
@@ -3155,8 +3154,7 @@ void SetupProcessObject(Environment* env,
EnvQuery,
EnvDeleter,
EnvEnumerator,
env->as_external(),
PropertyHandlerFlags::kOnlyInterceptStrings));
env->as_external()));
Local<Object> process_env =
process_env_template->NewInstance(env->context()).ToLocalChecked();
@@ -0,0 +1,26 @@
'use strict';
require('../common');
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 assigning via a symbol key throws.
assert.throws(() => {
process.env[symbol] = 42;
}, errRegExp);
// Verify that assigning a symbol value throws.
assert.throws(() => {
process.env.foo = symbol;
}, errRegExp);
// Verify that using a symbol with the in operator throws.
assert.throws(() => {
symbol in process.env;
}, errRegExp);
@@ -1,34 +0,0 @@
'use strict';
require('../common');
const assert = require('assert');
// Test that the v8 named property handler intercepts callbacks
// when properties are defined as Strings and NOT for Symbols.
//
// With the kOnlyInterceptStrings flag, manipulating properties via
// Strings is intercepted by the callbacks, while Symbols adopt
// the default global behaviour.
// Removing the kOnlyInterceptStrings flag, adds intercepting to Symbols,
// which causes Type Error at process.env[symbol]=42 due to process.env being
// strongly typed for Strings
// (node::Utf8Value key(info.GetIsolate(), property);).
const symbol = Symbol('sym');
// check if its undefined
assert.strictEqual(process.env[symbol], undefined);
// set a value using a Symbol
process.env[symbol] = 42;
// set a value using a String (call to EnvSetter, node.cc)
process.env['s'] = 42;
//check the values after substitutions
assert.strictEqual(42, process.env[symbol]);
assert.strictEqual('42', process.env['s']);
delete process.env[symbol];
assert.strictEqual(undefined, process.env[symbol]);

0 comments on commit 1aa595e

Please sign in to comment.