[PATCH 0/1] MR7290: Draft: kernelbase: Handle correctly paths with forward slashes in ReplaceFileW.

[Roman Pišl (＠rpisl)]

PathRemoveFileSpecW keeps only the drive if the path contains all forward slashes as shown in tests.

But then the temporary file is created in the root folder which fails for drive Z:.

This fixes a corner case when using CMake in Wine. Another usage of PathRemoveFileSpecW is in PathRelativePathToW which should be also checked.

I'm leaving this as a draft until getting some feedback and adding tests.

[PATCH 1/1] kernelbase: Handle correctly paths with forward slashes in ReplaceFileW.

[Roman Pišl]

From: Roman Pišl

PathRemoveFileSpecW keeps only the drive if the path contains
all forward slashes as shown in tests.

But then the temporary file is created in the root folder
which fails for drive Z:.
---
 dlls/kernelbase/file.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/dlls/kernelbase/file.c b/dlls/kernelbase/file.c
index f0dedfe3b14..53286f1331a 100644
--- a/dlls/kernelbase/file.c
+++ b/dlls/kernelbase/file.c
@@ -2717,8 +2717,16 @@ BOOL WINAPI DECLSPEC_HOTPATCH ReplaceFileW( const WCHAR *replaced, const WCHAR *
          * it out of the way first. */
         WCHAR temp_path[MAX_PATH], temp_file[MAX_PATH];

-        lstrcpynW( temp_path, replaced, ARRAY_SIZE( temp_path ) );
-        PathRemoveFileSpecW( temp_path );
+        WCHAR* filePart;
+        DWORD cnt = GetFullPathNameW(replaced, ARRAY_SIZE( temp_path ), temp_path, &filePart);
+        if (!cnt) return FALSE;
+        if (cnt >= ARRAY_SIZE( temp_path ) || !filePart)
+        {
+            SetLastError( ERROR_PATH_NOT_FOUND );
+            return FALSE;
+        }
+        *filePart = 0;
+
         if (!GetTempFileNameW( temp_path, L"rf", 0, temp_file ) ||
             !MoveFileExW( replaced, temp_file, MOVEFILE_REPLACE_EXISTING ))
             return FALSE;

[PATCH 0/1] MR7290: kernelbase: Handle correctly paths with forward slashes in ReplaceFileW.

[Roman Pišl (＠rpisl)]

I didn't find a simple and reliable way how to test this internal behavior. I'm marking it as ready.

[PATCH 0/1] MR7290: kernelbase: Handle correctly paths with forward slashes in ReplaceFileW.

[Jinoh Kang (＠iamahuman)]

On Sat Mar  8 14:18:34 2025 +0000, Roman Pišl wrote:
> I didn't find a simple and reliable way how to test this internal
> behavior. I'm marking it as ready.

If CMake could trigger it, it surely isn't an internal behavior. Internal behavior is something that cannot be observed by an application.

You should add a test that demonstrates what CMake did that made it run into the bug, not test for the internals directly.

[PATCH 0/1] MR7290: kernelbase: Handle correctly paths with forward slashes in ReplaceFileW.

[Roman Pišl (＠rpisl)]

On Sat Mar  8 14:19:20 2025 +0000, Jinoh Kang wrote:
> If CMake could trigger it, it surely isn't an internal behavior.
> Internal behavior is something that cannot be observed by an application.
> You should add a test that demonstrates what CMake did that made it run
> into the bug, not test for the internals directly.

But how to do it in an elegant way? This can be triggered simply in Wine on driver Z: (as / is obviously not writable for a user in Linux) but is not triggered in Wine on C: which is writable in Wine but not on Windows and I expect is the only drive on testbots.

So idea - test on Z: and fallback to C: if Z: is not present?

[PATCH 0/1] MR7290: kernelbase: Handle correctly paths with forward slashes in ReplaceFileW.

[Roman Pišl (＠rpisl)]

On Sat Mar  8 16:09:36 2025 +0000, Roman Pišl wrote:
> But how to do it in an elegant way? This can be triggered simply in Wine
> on driver Z: (as / is obviously not writable for a user in Linux) but is
> not triggered in Wine on C: which is writable in Wine but not on Windows
> and I expect is the only drive on testbots.
> So idea - test on Z: and fallback to C: if Z: is not present?

Another option would be to create the same conditions locally in a temporary folder, but then I'm leaving this for the long winter evenings..