src: delete proccess.env values set to undefined

targos · targos · commit f88476ab6fac · 2018-01-23T13:12:01.000+01:00
Before this commit, setting a property of process.env to undefined would set the value to the string "undefined". This changes the behavior to instead remove the value, working like the delete operator. Refs: #15089
diff --git a/doc/api/process.md b/doc/api/process.md
@@ -836,6 +836,11 @@ emitMyWarning();
 ## process.env
 <!-- YAML
 added: v0.1.27
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/18158
+    description: Assigning a property to `undefined` now deletes it from the
+                 environment.
 -->
 
 * {Object}
@@ -885,12 +890,13 @@ Example:
 process.env.test = null;
 console.log(process.env.test);
 // => 'null'
-process.env.test = undefined;
+process.env.test = 42;
 console.log(process.env.test);
-// => 'undefined'
+// => '42'
 ```
 
-Use `delete` to delete a property from `process.env`.
+Use `delete` to delete a property from `process.env` or assign it to
+`undefined`.
 
 Example:
 
@@ -899,6 +905,10 @@ process.env.TEST = 1;
 delete process.env.TEST;
 console.log(process.env.TEST);
 // => undefined
+process.env.TEST = 1;
+process.env.TEST = undefined;
+console.log('TEST' in process.env);
+// => false
 ```
 
 On Windows operating systems, environment variables are case-insensitive.
diff --git a/src/node.cc b/src/node.cc
@@ -2697,6 +2697,18 @@ static void ProcessTitleSetter(Local<Name> property,
 }
 
 
+inline void RemovePropertyFromEnv(Isolate* isolate, Local<Name> property) {
+#ifdef __POSIX__
+  node::Utf8Value key(isolate, property);
+  unsetenv(*key);
+#else
+  node::TwoByteValue key(isolate, property);
+  WCHAR* key_ptr = reinterpret_cast<WCHAR*>(*key);
+  SetEnvironmentVariableW(key_ptr, nullptr);
+#endif
+}
+
+
 static void EnvGetter(Local<Name> property,
                       const PropertyCallbackInfo<Value>& info) {
   Isolate* isolate = info.GetIsolate();
@@ -2731,6 +2743,11 @@ static void EnvGetter(Local<Name> property,
 static void EnvSetter(Local<Name> property,
                       Local<Value> value,
                       const PropertyCallbackInfo<Value>& info) {
+  if (value->IsUndefined()) {
+    RemovePropertyFromEnv(info.GetIsolate(), property);
+    info.GetReturnValue().Set(value);
+    return;
+  }
 #ifdef __POSIX__
   node::Utf8Value key(info.GetIsolate(), property);
   node::Utf8Value val(info.GetIsolate(), value);
@@ -2780,14 +2797,7 @@ 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);
-#else
-    node::TwoByteValue key(info.GetIsolate(), property);
-    WCHAR* key_ptr = reinterpret_cast<WCHAR*>(*key);
-    SetEnvironmentVariableW(key_ptr, nullptr);
-#endif
+    RemovePropertyFromEnv(info.GetIsolate(), property);
   }
 
   // process.env never has non-configurable properties, so always
diff --git a/test/parallel/test-process-env.js b/test/parallel/test-process-env.js
@@ -100,3 +100,12 @@ if (common.isWindows) {
   assert.strictEqual(process.env.test, undefined);
   assert.strictEqual(process.env.teST, undefined);
 }
+
+{
+  // Setting an environment variable to undefined should act like a delete.
+  process.env.UNDEF = 'test';
+  assert.strictEqual(process.env.UNDEF, 'test');
+  process.env.UNDEF = undefined;
+  assert.strictEqual(process.env.UNDEF, undefined);
+  assert.strictEqual('UNDEF' in process.env, false);
+}
