Skip to content

Commit

Permalink
Implement setenv correctly and support setting values to empty strings
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Orvid authored and Facebook Github Bot 1 committed Aug 9, 2016
1 parent 4d77729 commit 5d8ca09
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 3 deletions.
5 changes: 5 additions & 0 deletions folly/experimental/test/TestUtilTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)});
Expand Down
48 changes: 45 additions & 3 deletions folly/portability/Environment.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down

0 comments on commit 5d8ca09

Please sign in to comment.