Fix Windows GetEnv mangling non-ASCII environment values (UTF-8) - #3885
Open
kai-ion wants to merge 1 commit into
Open
Fix Windows GetEnv mangling non-ASCII environment values (UTF-8)#3885kai-ion wants to merge 1 commit into
kai-ion wants to merge 1 commit into
Conversation
kai-ion
force-pushed
the
no-more-mojibake
branch
3 times, most recently
from
July 30, 2026 21:21
7ac9b1e to
8718685
Compare
On Windows, GetEnv read variables through the narrow _dupenv_s, which returns the value in the active ANSI code page (e.g. CP-1252). Non-ASCII values -- such as a home path with an accented username -- were then reinterpreted as UTF-8 and came out mangled, so ~/.aws/credentials and ~/.aws/config failed to load and the profile provider fell back to an anonymous request (GitHub issue #3865). Read env vars through the wide _wdupenv_s and convert explicitly to UTF-8, since Windows stores them natively as UTF-16. Mirrors the same fix in the CRT (awslabs/aws-c-common#1260). Non-MSVC compilers keep the existing std::getenv path. Adds a Windows-only regression test that injects a non-ASCII value via the wide CRT API and asserts GetEnv returns the correct UTF-8 bytes.
kai-ion
force-pushed
the
no-more-mojibake
branch
from
July 30, 2026 21:29
8718685 to
89408d2
Compare
sbiscigl
reviewed
Jul 31, 2026
|
|
||
| ASSERT_EQ(Aws::String("Jos\xC3\xA9"), value); // "José" in UTF-8 | ||
| #else | ||
| GTEST_SKIP() << "Non-ASCII environment encoding fix is Windows-specific."; |
Collaborator
There was a problem hiding this comment.
instead of skipping, just ifdef the whole test
#ifdef _MSC_VER
TEST_F(FileTest, GetEnvReturnsNonAsciiValueAsUtf8) {
....
}
#endifwe do that in several other places, i.e. http tests for libcurl only tests
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue #, if available:
#3520
Description of changes:
On Windows,
Aws::Environment::GetEnvread variables through the narrow_dupenv_s, which returns the value in the active ANSI code page (e.g.CP-1252). Any non-ASCII value — an accented username in a home path, a
region/proxy set to a localized string — was then reinterpreted elsewhere in
the SDK as UTF-8 and came out as mojibake (e.g.
José→Jos\xE9instead ofJos\xC3\xA9).This changes the Windows implementation to read through the wide
_wdupenv_sand convert explicitly to UTF-8 via
StringUtils::FromWString, since Windowsstores environment variables natively as UTF-16. This mirrors the same fix
made in the CRT (awslabs/aws-c-common#1260).
_wdupenv_sisMSVC-specific, so non-MSVC compilers keep the existing
std::getenvpath.Added a regression test (
FileSystemUtilsTest.GetEnvReturnsNonAsciiValueAsUtf8)that injects a non-ASCII value via the wide CRT API (
_wputenv_s, so the trueUnicode value is stored rather than a pre-mangled narrow one) and asserts
GetEnvreturns the correct UTF-8 bytes. The test is Windows-only and skipselsewhere.
Check all that applies:
Check which platforms you have built SDK on to verify the correctness of this PR.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.