[PATCH v3 0/2] MR4774: msvcrt: Compare environmental variable names case insensitively

[Zsolt Vadász (＠zsoltiv)]

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

[PATCH v3 1/2] msvcrt: Compare environmental variable names case insensitively

[Zsolt Vadasz]

From: Zsolt Vadasz

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;

[PATCH v3 2/2] msvcrt/tests: Test case insensitivity of getenv() and _wgetenv()

[Zsolt Vadasz]

From: Zsolt Vadasz

---
 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();
 }

[Zsolt Vadász (＠zsoltiv)]

> I think we can also add a test for putenv mainly to ensure the implementation is correct in this regard.

done

[Zebediah Figura (＠zfigura)]

What about locale? Do we know that wcsnicmp() is the right thing to use here?

[Zsolt Vadász (＠zsoltiv)]

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.

[Piotr Caban (＠piotr)]

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=");
```