[PATCH v2 0/1] MR4045: kernelbase: Rewrite ExpandEnvironmentStringsA to return ansi string size

[Fabian Maurer (＠DarkShadow44)]

We can't return the unicode string size, this returns sizes to small
for multi-byte characters.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54295

--
  v2: kernelbase: Rewrite ExpandEnvironmentStringsA to return ansi string size
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4045

[PATCH v2 1/1] kernelbase: Rewrite ExpandEnvironmentStringsA to return ansi string size

[Fabian Maurer]

From: Fabian Maurer <dark.shadow4(a)web.de>

We can't return the unicode string size, this returns sizes to small
for multi-byte characters.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54295
---
 dlls/kernel32/tests/environ.c | 21 +++++++++++++++++++++
 dlls/kernelbase/process.c     | 26 +++++++++++++++++---------
 dlls/shlwapi/tests/shreg.c    |  7 ++-----
 3 files changed, 40 insertions(+), 14 deletions(-)

diff --git a/dlls/kernel32/tests/environ.c b/dlls/kernel32/tests/environ.c
index c1a49e175be..1e2287491cb 100644
--- a/dlls/kernel32/tests/environ.c
+++ b/dlls/kernel32/tests/environ.c
@@ -461,6 +461,27 @@ static void test_ExpandEnvironmentStringsA(void)
     SetEnvironmentVariableA("IndirectVar", NULL);
 
     SetEnvironmentVariableA("EnvVar", NULL);
+
+    if (GetACP() == 932) /* shift-jis */
+    {
+        const char* japanese_test = "\x88\xEA\x93\xF1\x8E\x4F\x8E\x6C\x8C\xDC\x98\x5A\x8E\xB5\x94\xAA\x8B\xE3"; /* Japanese Kanji for "one" to "nine", in shift-jis */
+        int japanese_len = strlen(japanese_test);
+
+        ret_size = ExpandEnvironmentStringsA(japanese_test, NULL, 0);
+        ok(ret_size >= japanese_len, "Needed at least %d, got %lu\n", japanese_len, ret_size);
+
+        ret_size = ExpandEnvironmentStringsA(japanese_test, buf, ret_size);
+        ok(ret_size >= japanese_len, "Needed at least %d, got %lu\n", japanese_len, ret_size);
+        ok(!strcmp(buf, japanese_test), "Got %s\n", debugstr_a(buf));
+
+        SetLastError(0xdeadbeef);
+        ret_size = ExpandEnvironmentStringsA(japanese_test, buf, japanese_len / 2); /* Buffer too small */
+        ok(ret_size >= japanese_len, "Needed at least %d, got %lu\n", japanese_len, ret_size);
+    }
+    else
+    {
+        skip("Skipping japanese language tests\n");
+    }
 }
 
 static void test_GetComputerName(void)
diff --git a/dlls/kernelbase/process.c b/dlls/kernelbase/process.c
index 4807e84594d..f9505d1e6ce 100644
--- a/dlls/kernelbase/process.c
+++ b/dlls/kernelbase/process.c
@@ -1400,20 +1400,28 @@ DWORD WINAPI DECLSPEC_HOTPATCH ExpandEnvironmentStringsA( LPCSTR src, LPSTR dst,
 {
     UNICODE_STRING us_src;
     PWSTR dstW = NULL;
-    DWORD ret;
+    DWORD count_neededW;
+    DWORD count_neededA;
 
     RtlCreateUnicodeStringFromAsciiz( &us_src, src );
-    if (count)
-    {
-        if (!(dstW = HeapAlloc(GetProcessHeap(), 0, count * sizeof(WCHAR)))) return 0;
-        ret = ExpandEnvironmentStringsW( us_src.Buffer, dstW, count);
-        if (ret) WideCharToMultiByte( CP_ACP, 0, dstW, ret, dst, count, NULL, NULL );
-    }
-    else ret = ExpandEnvironmentStringsW( us_src.Buffer, NULL, 0 );
+
+    /* We always need to call ExpandEnvironmentStringsW, since we need the result to calculate the needed buffer size */
+    count_neededW = ExpandEnvironmentStringsW( us_src.Buffer, NULL, 0 );
+    if (!(dstW = HeapAlloc(GetProcessHeap(), 0, count_neededW * sizeof(WCHAR)))) return 0;
+    count_neededW = ExpandEnvironmentStringsW( us_src.Buffer, dstW, count_neededW);
+
+    /* Calculate needed buffer */
+    count_neededA = WideCharToMultiByte( CP_ACP, 0, dstW, count_neededW, NULL, 0, NULL, NULL );
+    if (!count_neededA)
+           return 0;
+
+    /* If provided buffer is enough, do actual conversion */
+    if (count >= count_neededA)
+        count_neededA = WideCharToMultiByte( CP_ACP, 0, dstW, count_neededW, dst, count, NULL, NULL );
 
     RtlFreeUnicodeString( &us_src );
     HeapFree( GetProcessHeap(), 0, dstW );
-    return ret;
+    return count_neededA;
 }
 
 
diff --git a/dlls/shlwapi/tests/shreg.c b/dlls/shlwapi/tests/shreg.c
index bc411779c8f..75d4dfae7eb 100644
--- a/dlls/shlwapi/tests/shreg.c
+++ b/dlls/shlwapi/tests/shreg.c
@@ -290,11 +290,8 @@ static void test_SHQueryValueEx(void)
             broken(ERROR_SUCCESS == dwRet), /* < IE5.5*/
             "Expected ERROR_MORE_DATA, got (%lu)\n", dwRet);
 
-        todo_wine
-        {
-                ok( (0 == strcmp("", buf)) || (0 == strcmp(sTestpath2, buf)),
-                    "Expected empty or unexpanded string (win98), got (%s)\n", buf); 
-        }
+        ok( (0 == strcmp("", buf)) || (0 == strcmp(sTestpath2, buf)),
+            "Expected empty or unexpanded string (win98), got (%s)\n", buf);
 
         ok( dwSize >= nUsedBuffer2 ||
             broken(dwSize == (strlen("") + 1)), /* < IE 5.5 */
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/4045

[Fabian Maurer (＠DarkShadow44)]

Any news on this?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4045#note_50116