[PATCH 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

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

[PATCH 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 +++++++++++++++++---------
 2 files changed, 38 insertions(+), 9 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;
 }
 
 
-- 
GitLab

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

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

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=138511

Your paranoid android.


=== debian11b (64 bit WoW report) ===

shlwapi:
shreg.c:295: Test succeeded inside todo block: Expected empty or unexpanded string (win98), got (%FOO%\subdir1)