Skip to content

Commit

Permalink
src: throw on process.env symbol usage
Browse files Browse the repository at this point in the history
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 7, 2016
1 parent b315e24 commit 1aa595e
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 42 deletions.
14 changes: 6 additions & 8 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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'=') {
Expand All @@ -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) {
Expand All @@ -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
Expand Down Expand Up @@ -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();
Expand Down
26 changes: 26 additions & 0 deletions test/parallel/test-process-env-symbols.js
Original file line number Diff line number Diff line change
@@ -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);
34 changes: 0 additions & 34 deletions test/parallel/test-v8-interceptStrings-not-Symbols.js

This file was deleted.

0 comments on commit 1aa595e

Please sign in to comment.