[PATCH v3 0/2] MR4774: msvcrt: Compare environmental variable names case insensitively
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56134 -- v3: msvcrt/tests: Test case insensitivity of getenv() and _wgetenv() https://gitlab.winehq.org/wine/wine/-/merge_requests/4774
From: Zsolt Vadasz <zsolt_vadasz(a)protonmail.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56134 --- dlls/msvcrt/environ.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/msvcrt/environ.c b/dlls/msvcrt/environ.c index 063a9254c67..aa857afa615 100644 --- a/dlls/msvcrt/environ.c +++ b/dlls/msvcrt/environ.c @@ -125,7 +125,7 @@ static int env_get_index(const char *name) len = strlen(name); for (i = 0; MSVCRT__environ[i]; i++) { - if (!strncmp(name, MSVCRT__environ[i], len) && MSVCRT__environ[i][len] == '=') + if (!strnicmp(name, MSVCRT__environ[i], len) && MSVCRT__environ[i][len] == '=') return i; } return i; @@ -138,7 +138,7 @@ static int wenv_get_index(const wchar_t *name) len = wcslen(name); for (i = 0; MSVCRT__wenviron[i]; i++) { - if (!wcsncmp(name, MSVCRT__wenviron[i], len) && MSVCRT__wenviron[i][len] == '=') + if (!wcsnicmp(name, MSVCRT__wenviron[i], len) && MSVCRT__wenviron[i][len] == '=') return i; } return i; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4774
From: Zsolt Vadasz <zsolt_vadasz(a)protonmail.com> --- dlls/msvcrt/tests/environ.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/dlls/msvcrt/tests/environ.c b/dlls/msvcrt/tests/environ.c index aab14a422b3..777bc447b66 100644 --- a/dlls/msvcrt/tests/environ.c +++ b/dlls/msvcrt/tests/environ.c @@ -421,6 +421,28 @@ static void test_child_env(char** argv) free( env ); } +static void test_case_insensitive(void) +{ + const char *uppercase_env = getenv("APPDATA"); + const char *lowercase_env = getenv("appdata"); + const wchar_t *uppercase_wenv = _wgetenv(L"APPDATA"); + const wchar_t *lowercase_wenv = _wgetenv(L"appdata"); + char *bar; + wchar_t *foo; + ok (uppercase_env == lowercase_env, "getenv() must be case insensitive, %p should be %p\n", + lowercase_env, uppercase_env); + ok (uppercase_wenv == lowercase_wenv, "_wgetenv() must be case insensitive, %p should be %p\n", + lowercase_wenv, uppercase_wenv); + ok (!_putenv_s("fOo", "bar"), "Failed to set FOO=bar\n"); + ok (!p_wputenv_s(L"BaR", L"foo"), "Failed to set BAR=foo\n"); + ok (!_putenv_s("FOO", "BAR"), "Failed to set FOO=BAR\n"); + ok (!p_wputenv_s(L"BAR", L"FOO"), "Failed to set BAR=FOO\n"); + foo = _wgetenv(L"BaR"); + bar = getenv("fOo"); + ok (!strnicmp(bar, "BAR", strlen(bar)), "_putenv_s() must be case insensitive\n"); + ok (!_wcsnicmp(foo, L"FOO", wcslen(foo)), "_wputenv_s() must be case insensitive\n"); +} + START_TEST(environ) { char **argv; @@ -443,4 +465,5 @@ START_TEST(environ) test_environment_manipulation(); test_child_env(argv); test_system(); + test_case_insensitive(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4774
I think we can also add a test for putenv mainly to ensure the implementation is correct in this regard.
done -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4774#note_56587
What about locale? Do we know that wcsnicmp() is the right thing to use here? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4774#note_56588
On Tue Jan 2 22:04:45 2024 +0000, Zebediah Figura wrote:
What about locale? Do we know that wcsnicmp() is the right thing to use here? i'm not familiar with either wide character related functions nor locale shenanigans to be honest. but msdn docs said this is the wide character equivalent of strnicmp() so i used it. i am open to suggestions though.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4774#note_56589
Piotr Caban (@piotr) commented about dlls/msvcrt/tests/environ.c:
+ const wchar_t *uppercase_wenv = _wgetenv(L"APPDATA"); + const wchar_t *lowercase_wenv = _wgetenv(L"appdata"); + char *bar; + wchar_t *foo; + ok (uppercase_env == lowercase_env, "getenv() must be case insensitive, %p should be %p\n", + lowercase_env, uppercase_env); + ok (uppercase_wenv == lowercase_wenv, "_wgetenv() must be case insensitive, %p should be %p\n", + lowercase_wenv, uppercase_wenv); + ok (!_putenv_s("fOo", "bar"), "Failed to set FOO=bar\n"); + ok (!p_wputenv_s(L"BaR", L"foo"), "Failed to set BAR=foo\n"); + ok (!_putenv_s("FOO", "BAR"), "Failed to set FOO=BAR\n"); + ok (!p_wputenv_s(L"BAR", L"FOO"), "Failed to set BAR=FOO\n"); + foo = _wgetenv(L"BaR"); + bar = getenv("fOo"); + ok (!strnicmp(bar, "BAR", strlen(bar)), "_putenv_s() must be case insensitive\n"); + ok (!_wcsnicmp(foo, L"FOO", wcslen(foo)), "_wputenv_s() must be case insensitive\n"); It's better to avoid _putenv_s function since it may be unavailable in older versions of msvcrt. I have also used strcmp/wcscmp to compare environment variable value since this part is case sensitive. I have also added some white-space changes and cleaned up the environment after the test.
```suggestion:-17+0 const char *uppercase_env = getenv("APPDATA"); const char *lowercase_env = getenv("appdata"); const wchar_t *uppercase_wenv = _wgetenv(L"APPDATA"); const wchar_t *lowercase_wenv = _wgetenv(L"appdata"); ok( uppercase_env == lowercase_env, "getenv() must be case insensitive, %p should be %p\n", lowercase_env, uppercase_env ); ok( uppercase_wenv == lowercase_wenv, "_wgetenv() must be case insensitive, %p should be %p\n", lowercase_wenv, uppercase_wenv ); ok( !_putenv("cAt=bar"), "Failed to set CAT=bar\n" ); ok( !_putenv("CAT=BAR"), "Failed to set CAT=BAR\n" ); ok( !strcmp(getenv("cAt"), "BAR"), "_putenv() must be case insensitive\n" ); ok( !_wputenv(L"cAt=bar"), "Failed to set CAT=bar\n" ); ok( !_wputenv_s(L"CAT=BAR"), "Failed to set CAT=BAR\n" ); ok( !wcscmp(_wgetenv(L"cAt"), L"BAR"), "_wputenv() must be case insensitive\n" ); _putenv("cat="); ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4774#note_56623
participants (3)
-
Piotr Caban (＠piotr) -
Zebediah Figura (＠zfigura)
-
Zsolt Vadasz