From 854a715db79b9f3772f4c5553094f40744e74e03 Mon Sep 17 00:00:00 2001
From: Timothy Gu <timothygu99@gmail.com>
Date: Sun, 5 Mar 2017 22:45:19 -0800
Subject: [PATCH] src: make process.env work with symbols for in/delete

The getter for process.env already allows symbols to be used, and `in`
operator as a read-only operator can do the same.

`delete a[b]` operator in ES always returns `true` without doing
anything when `b in a === false`. Allow symbols in the deleter
accordingly.
---
 src/node.cc                               | 42 +++++++++++++----------
 test/parallel/test-process-env-symbols.js |  9 ++---
 2 files changed, 28 insertions(+), 23 deletions(-)

diff --git a/src/node.cc b/src/node.cc
index b3e2bbe53aa1db..b7253afed1d604 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -2787,24 +2787,26 @@ static void EnvSetter(Local<Name> property,
 static void EnvQuery(Local<Name> property,
                      const PropertyCallbackInfo<Integer>& info) {
   int32_t rc = -1;  // Not found unless proven otherwise.
+  if (property->IsString()) {
 #ifdef __POSIX__
-  node::Utf8Value key(info.GetIsolate(), property);
-  if (getenv(*key))
-    rc = 0;
+    node::Utf8Value key(info.GetIsolate(), property);
+    if (getenv(*key))
+      rc = 0;
 #else  // _WIN32
-  node::TwoByteValue key(info.GetIsolate(), property);
-  WCHAR* key_ptr = reinterpret_cast<WCHAR*>(*key);
-  if (GetEnvironmentVariableW(key_ptr, nullptr, 0) > 0 ||
-      GetLastError() == ERROR_SUCCESS) {
-    rc = 0;
-    if (key_ptr[0] == L'=') {
-      // Environment variables that start with '=' are hidden and read-only.
-      rc = static_cast<int32_t>(v8::ReadOnly) |
-           static_cast<int32_t>(v8::DontDelete) |
-           static_cast<int32_t>(v8::DontEnum);
+    node::TwoByteValue key(info.GetIsolate(), property);
+    WCHAR* key_ptr = reinterpret_cast<WCHAR*>(*key);
+    if (GetEnvironmentVariableW(key_ptr, nullptr, 0) > 0 ||
+        GetLastError() == ERROR_SUCCESS) {
+      rc = 0;
+      if (key_ptr[0] == L'=') {
+        // Environment variables that start with '=' are hidden and read-only.
+        rc = static_cast<int32_t>(v8::ReadOnly) |
+             static_cast<int32_t>(v8::DontDelete) |
+             static_cast<int32_t>(v8::DontEnum);
+      }
     }
-  }
 #endif
+  }
   if (rc != -1)
     info.GetReturnValue().Set(rc);
 }
@@ -2812,14 +2814,16 @@ static void EnvQuery(Local<Name> property,
 
 static void EnvDeleter(Local<Name> property,
                        const PropertyCallbackInfo<Boolean>& info) {
+  if (property->IsString()) {
 #ifdef __POSIX__
-  node::Utf8Value key(info.GetIsolate(), property);
-  unsetenv(*key);
+    node::Utf8Value key(info.GetIsolate(), property);
+    unsetenv(*key);
 #else
-  node::TwoByteValue key(info.GetIsolate(), property);
-  WCHAR* key_ptr = reinterpret_cast<WCHAR*>(*key);
-  SetEnvironmentVariableW(key_ptr, nullptr);
+    node::TwoByteValue key(info.GetIsolate(), property);
+    WCHAR* key_ptr = reinterpret_cast<WCHAR*>(*key);
+    SetEnvironmentVariableW(key_ptr, nullptr);
 #endif
+  }
 
   // process.env never has non-configurable properties, so always
   // return true like the tc39 delete operator.
diff --git a/test/parallel/test-process-env-symbols.js b/test/parallel/test-process-env-symbols.js
index a8798fc457d935..b07d835599d870 100644
--- a/test/parallel/test-process-env-symbols.js
+++ b/test/parallel/test-process-env-symbols.js
@@ -18,10 +18,11 @@ assert.throws(() => {
   process.env.foo = symbol;
 }, errRegExp);
 
-// Verify that using a symbol with the in operator throws.
-assert.throws(() => {
-  symbol in process.env;
-}, errRegExp);
+// Verify that using a symbol with the in operator returns false.
+assert.strictEqual(symbol in process.env, false);
+
+// Verify that deleting a symbol value returns true.
+assert.strictEqual(delete process.env[symbol], true);
 
 // Checks that well-known symbols like `Symbol.toStringTag` won’t throw.
 assert.doesNotThrow(() => Object.prototype.toString.call(process.env));