Skip to content

Fix Windows GetEnv mangling non-ASCII environment values (UTF-8) #3885

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kai-ion wants to merge 1 commit into
base: main
Choose a base branch
from
+27 −12
Open

Fix Windows GetEnv mangling non-ASCII environment values (UTF-8) #3885

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 7 additions & 12 deletions src/aws-cpp-sdk-core/source/platform/windows/Environment.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/

#include <aws/core/platform/Environment.h>
#include <aws/core/utils/StringUtils.h>

#include <stdio.h>
#include <utility>
Expand All @@ -13,25 +14,19 @@ namespace Aws
namespace Environment
{

/*
using std::getenv generates a warning on windows so we use _dupenv_s instead. The character array returned by this function is our responsibility to clean up, so rather than returning raw strings
that would need to be manually freed in all the client functions, just copy it into a Aws::String instead, freeing it here.

since _dupenv_s is a non-standard function, on non-Microsoft compilers we will fall back to using std::getenv instead.
*/
Aws::String GetEnv(const char *variableName)
{
#ifdef _MSC_VER
char* variableValue = nullptr;
#ifdef _MSC_VER
wchar_t* variableValue = nullptr;
std::size_t valueSize = 0;
auto queryResult = _dupenv_s(&variableValue, &valueSize, variableName);
const auto queryResult = _wdupenv_s(&variableValue, &valueSize, Aws::Utils::StringUtils::ToWString(variableName).c_str());

Aws::String result;
if(queryResult == 0 && variableValue != nullptr && valueSize > 0)
if(queryResult == 0 && variableValue != nullptr)
{
result.assign(variableValue, valueSize - 1); // don't copy the c-string terminator byte
free(variableValue);
result = Aws::Utils::StringUtils::FromWString(variableValue);
}
free(variableValue);

return result;
#else
Expand Down
20 changes: 20 additions & 0 deletions tests/aws-cpp-sdk-core-tests/utils/FileSystemUtilsTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,15 @@
* SPDX-License-Identifier: Apache-2.0.
*/

#include <aws/core/platform/Environment.h>
#include <aws/core/platform/FileSystem.h>
#include <aws/core/utils/FileSystemUtils.h>
#include <aws/core/utils/memory/stl/AWSSet.h>
#include <aws/testing/AwsCppSdkGTestSuite.h>
#include <fstream>
#ifdef _MSC_VER
#include <stdlib.h> // _wputenv_s
#endif
#if defined(HAS_PATHCONF)
#include <unistd.h>
#include <climits>
Expand All @@ -32,6 +36,22 @@ TEST_F(FileTest, HomeDirectory)
ASSERT_EQ(Aws::FileSystem::PATH_DELIM, homeDirectory.back());
}

TEST_F(FileTest, GetEnvReturnsNonAsciiValueAsUtf8)
{
#ifdef _MSC_VER
const char* varName = "AWS_SDK_TEST_NONASCII";
// Set via the wide CRT API so the true Unicode value is stored, not a pre-mangled narrow one.
ASSERT_EQ(0, _wputenv_s(L"AWS_SDK_TEST_NONASCII", L"José"));

const auto value = Aws::Environment::GetEnv(varName);
_wputenv_s(L"AWS_SDK_TEST_NONASCII", L"");

ASSERT_EQ(Aws::String("Jos\xC3\xA9"), value); // "José" in UTF-8
#else
GTEST_SKIP() << "Non-ASCII environment encoding fix is Windows-specific.";

@sbiscigl sbiscigl Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of skipping, just ifdef the whole test

#ifdef _MSC_VER
TEST_F(FileTest, GetEnvReturnsNonAsciiValueAsUtf8) {
  ....
}
#endif

we do that in several other places, i.e. http tests for libcurl only tests

#endif
}

TEST_F(FileTest, TestInvalidDirectoryPath)
{
auto badDir = Aws::FileSystem::OpenDirectory("boogieMan");
Expand Down
Loading