Implement setenv correctly and support setting values to empty strings

Summary: Just calling `SetEnvironmentVariableA` wasn't updating `_environ`, which meant that calls to `getenv` weren't reflecting the changes made via `setenv`. The correct way to implement it is using `_putenv_s`, but there's one problem with that: passing an empty string as the value to `_putenv_s` results in the environment variable being unset. To make it possible to set the environment variable to an empty string, we shall dive head-first into the implementation details of the CRT and emerge victorious by blatently ignoring the documentation of `getenv` and modifying the string it returns to terminate it early.

Reviewed By: yfeldblum

Differential Revision: D3691007

fbshipit-source-id: 350c2ec72ec90b9178a9a45b2c2ed2659b788e37

diff --git a/folly/experimental/test/TestUtilTest.cpp b/folly/experimental/test/TestUtilTest.cpp
index 65af5ea8630072e18069c8a4b5ebaf1264772e06..8c5329a7b85f3546953b763a729cfd0b3808d84c 100644 (file)
--- a/folly/experimental/test/TestUtilTest.cpp
+++ b/folly/experimental/test/TestUtilTest.cpp
@@ -196,6 +196,11 @@ TEST_F(EnvVarSaverTest, ExampleNew) {
   auto key = "hahahahaha";
   EXPECT_EQ(nullptr, getenv(key));
 
+  PCHECK(0 == setenv(key, "", true));
+  EXPECT_STREQ("", getenv(key));
+  PCHECK(0 == unsetenv(key));
+  EXPECT_EQ(nullptr, getenv(key));
+
   auto saver = make_unique<EnvVarSaver>();
   PCHECK(0 == setenv(key, "blah", true));
   EXPECT_EQ("blah", std::string{getenv(key)});

diff --git a/folly/portability/Environment.cpp b/folly/portability/Environment.cpp
index 34f91217883e2f1dbe26a7699507ed73433c2261..247b7603b83cb6acf87d5a2945b3013983ac4e15 100755 (executable)
--- a/folly/portability/Environment.cpp
+++ b/folly/portability/Environment.cpp
@@ -26,10 +26,52 @@ int setenv(const char* name, const char* value, int overwrite) {
     return 0;
   }
 
+  if (*value != '\0') {
+    auto e = _putenv_s(name, value);
+    if (e != 0) {
+      errno = e;
+      return -1;
+    }
+    return 0;
+  }
+
+  // We are trying to set the value to an empty string, but
   // _putenv_s deletes entries if the value is an empty string,
-  // so we have to call the windows API function to safely assign
-  // these.
-  if (SetEnvironmentVariableA(name, value) != 0) {
+  // and just calling SetEnvironmentVariableA doesn't update
+  // _environ, so we have to do these terrible things.
+  if (_putenv_s(name, "  ") != 0) {
+    errno = EINVAL;
+    return -1;
+  }
+
+  // Here lies the documentation we blatently ignore to make
+  // this work >_>...
+  *getenv(name) = '\0';
+  // This would result in a double null termination, which
+  // normally signifies the end of the environment variable
+  // list, so we stick a completely empty environment variable
+  // into the list instead.
+  *(getenv(name) + 1) = '=';
+
+  // If _wenviron is null, the wide environment has not been initialized
+  // yet, and we don't need to try to update it.
+  // We have to do this otherwise we'd be forcing the initialization and
+  // maintenance of the wide environment even though it's never actually
+  // used in most programs.
+  if (_wenviron != nullptr) {
+    wchar_t buf[_MAX_ENV + 1];
+    size_t len;
+    if (mbstowcs_s(&len, buf, _MAX_ENV + 1, name, _MAX_ENV) != 0) {
+      errno = EINVAL;
+      return -1;
+    }
+    *_wgetenv(buf) = u'\0';
+    *(_wgetenv(buf) + 1) = u'=';
+  }
+
+  // And now, we have to update the outer environment to have
+  // a proper empty value.
+  if (!SetEnvironmentVariableA(name, value)) {
     errno = EINVAL;
     return -1;
   }