[PATCH 0/2] MR7540: kernel32/tests: Add tests for maximum path length limitation.
In CopyFileEx, and DeleteFile functions, by default, the file name and path are limited to MAX_PATH characters. To extend this limit to 32,767 wide characters, need prepend "\\\\?\\" to the path. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7540
From: yaoyongjie <yaoyongjie(a)uniontech.com> In CopyFileEx, and DeleteFile functions, by default, the file name and path are limited to MAX_PATH characters. To extend this limit to 32,767 wide characters, need prepend "\\\\?\\" to the path. --- dlls/kernel32/tests/file.c | 46 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/dlls/kernel32/tests/file.c b/dlls/kernel32/tests/file.c index 68b9f53700e..7743b408bed 100644 --- a/dlls/kernel32/tests/file.c +++ b/dlls/kernel32/tests/file.c @@ -910,6 +910,10 @@ static void test_CopyFileW(void) WCHAR source[MAX_PATH], dest[MAX_PATH]; static const WCHAR prefix[] = {'p','f','x',0}; DWORD ret; + // long file name, 247 charactors + const WCHAR *long_name = L"e2345678lsfalfielsfleflsfielfelfjellsfalfielsfleflsfielfelfjellsfalfielsfleflsfielfelfjellsfalfielsfleflsfielfelfjellsfalfielsfleflsfielfelfjellsfalfielsfleflsfielfelfjellsfalfielsfleflsfielfelfjellsfalfielsfleflsfielfelfjellsfalfielsfleflsfie.txt"; + // long file path is %TEMP%/%long_name% + WCHAR long_dest[MAX_PATH * 2] = { 0 }; ret = GetTempPathW(MAX_PATH, temp_path); if (ret == 0 && GetLastError() == ERROR_CALL_NOT_IMPLEMENTED) @@ -942,6 +946,25 @@ static void test_CopyFileW(void) ok(GetLastError() == ERROR_SUCCESS || broken(GetLastError() == ERROR_INVALID_PARAMETER) /* some win8 machines */, "Unexpected error %lu.\n", GetLastError()); + // test long file path, the length of the long_dest is larger than MAX_PATH. + wcscpy(long_dest, temp_path); + wcscat(long_dest, long_name); + SetLastError(0xdeadbeef); + ret = CopyFileExW(source, long_dest, NULL, NULL, NULL, 0); + ok(!ret && GetLastError() == ERROR_PATH_NOT_FOUND, "Expected CopyFileExW failed with ERROR_PATH_NOT_FOUND, but got %ld, copy %s -> %s\n", GetLastError(), wine_dbgstr_w(source), wine_dbgstr_w(long_dest)); + ret = DeleteFileW(long_dest); + ok(!ret, "Unexpected DeleteFileW successed\n"); + + // test long file name prepend "\\?\" + wcscpy(long_dest, L"\\\\?\\"); + wcscat(long_dest, temp_path); + wcscat(long_dest, long_name); + SetLastError(0xdeadbeef); + ret = CopyFileExW(source, long_dest, NULL, NULL, NULL, 0); + ok(ret, "CopyFileExW failed, got %ld, copy %s -> %s failed\n", GetLastError(), wine_dbgstr_w(source), wine_dbgstr_w(long_dest)); + + ret = DeleteFileW(long_dest); + ok(ret, "DeleteFileW: error %ld\n", GetLastError()); ret = DeleteFileW(source); ok(ret, "DeleteFileW: error %ld\n", GetLastError()); ret = DeleteFileW(dest); @@ -1194,6 +1217,10 @@ static void test_CopyFileEx(void) HANDLE hfile; DWORD ret; BOOL retok; + // long file name, 247 charactors + const char *long_name = "d2345678lsfalfielsfleflsfielfelfjellsfalfielsfleflsfielfelfjellsfalfielsfleflsfielfelfjellsfalfielsfleflsfielfelfjellsfalfielsfleflsfielfelfjellsfalfielsfleflsfielfelfjellsfalfielsfleflsfielfelfjellsfalfielsfleflsfielfelfjellsfalfielsfleflsfie.txt"; + // long file path is %TEMP%/%long_name% + char long_dest[MAX_PATH * 2]; ret = GetTempPathA(MAX_PATH, temp_path); ok(ret != 0, "GetTempPathA error %ld\n", GetLastError()); @@ -1235,6 +1262,25 @@ static void test_CopyFileEx(void) ok(!retok, "CopyFileExA unexpectedly succeeded\n"); ok(GetLastError() == ERROR_PATH_NOT_FOUND, "expected ERROR_PATH_NOT_FOUND, got %ld\n", GetLastError()); + // test long file path, the length of the long_dest is larger than MAX_PATH. + strcpy(long_dest, temp_path); + strcat(long_dest, long_name); + SetLastError(0xdeadbeef); + retok = CopyFileExA(source, long_dest, NULL, NULL, NULL, 0); + ok(!retok && GetLastError() == ERROR_PATH_NOT_FOUND, "Expected CopyFileExW failed with ERROR_PATH_NOT_FOUND, but got %ld, copy %s -> %s\n", GetLastError(), source, long_dest); + retok = DeleteFileA(long_dest); + ok(!retok, "Unexpected DeleteFileA successed\n"); + + // test long file name prepend "\\?\" + strcpy(long_dest, "\\\\?\\"); + strcat(long_dest, temp_path); + strcat(long_dest, long_name); + SetLastError(0xdeadbeef); + retok = CopyFileExA(source, long_dest, NULL, NULL, NULL, 0); + ok(retok, "CopyFileExA failed, got %ld, copy %s -> %s failed\n", GetLastError(), source, long_dest); + + ret = DeleteFileA(long_dest); + ok(ret, "DeleteFileA failed with error %ld\n", GetLastError()); ret = DeleteFileA(source); ok(ret, "DeleteFileA failed with error %ld\n", GetLastError()); ret = DeleteFileA(dest); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7540
From: yaoyongjie <yaoyongjie(a)uniontech.com> In CopyFileEx, and DeleteFile functions, by default, the file name and path are limited to MAX_PATH characters. To extend this limit to 32,767 wide characters, need prepend "\\\\?\\" to the path. If alloc is FALSE, file_name_AtoW will uses the TEB static buffer, but the buffer size is only MAX_PATH charactors. If DeleteFileA is to support long paths prefixed with "\\\\?\\", alloc must be set to TRUE. --- dlls/kernelbase/file.c | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/dlls/kernelbase/file.c b/dlls/kernelbase/file.c index f0dedfe3b14..1b254187601 100644 --- a/dlls/kernelbase/file.c +++ b/dlls/kernelbase/file.c @@ -512,6 +512,18 @@ static BOOL copy_file( const WCHAR *source, const WCHAR *dest, COPYFILE2_EXTENDE SetLastError( ERROR_INVALID_PARAMETER ); return FALSE; } + + if (wcsncmp(dest, L"\\\\?\\", 4) && wcslen(dest) >= MAX_PATH ) + { + SetLastError( ERROR_PATH_NOT_FOUND ); + return FALSE; + } + if (wcsncmp(dest, L"\\\\?\\", 4) && wcslen(dest) >= MAX_PATH ) + { + SetLastError( ERROR_PATH_NOT_FOUND ); + return FALSE; + } + if (!(buffer = HeapAlloc( GetProcessHeap(), 0, buffer_size ))) { SetLastError( ERROR_NOT_ENOUGH_MEMORY ); @@ -991,9 +1003,12 @@ BOOLEAN WINAPI /* DECLSPEC_HOTPATCH */ CreateSymbolicLinkW( LPCWSTR link, LPCWST BOOL WINAPI DECLSPEC_HOTPATCH DeleteFileA( LPCSTR path ) { WCHAR *pathW; + BOOL res; - if (!(pathW = file_name_AtoW( path, FALSE ))) return FALSE; - return DeleteFileW( pathW ); + if (!(pathW = file_name_AtoW( path, TRUE ))) return FALSE; + res = DeleteFileW( pathW ); + HeapFree( GetProcessHeap(), 0, pathW ); + return res; } @@ -1010,6 +1025,12 @@ BOOL WINAPI DECLSPEC_HOTPATCH DeleteFileW( LPCWSTR path ) TRACE( "%s\n", debugstr_w(path) ); + if (path && wcsncmp(path, L"\\\\?\\", 4) && wcslen(path) >= MAX_PATH ) + { + SetLastError( ERROR_PATH_NOT_FOUND ); + return FALSE; + } + if (!RtlDosPathNameToNtPathName_U( path, &nameW, NULL, NULL )) { SetLastError( ERROR_PATH_NOT_FOUND ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7540
Zhiyi Zhang (@zhiyi) commented about dlls/kernel32/tests/file.c:
ok(GetLastError() == ERROR_SUCCESS || broken(GetLastError() == ERROR_INVALID_PARAMETER) /* some win8 machines */, "Unexpected error %lu.\n", GetLastError());
+ // test long file path, the length of the long_dest is larger than MAX_PATH. + wcscpy(long_dest, temp_path); + wcscat(long_dest, long_name); + SetLastError(0xdeadbeef); + ret = CopyFileExW(source, long_dest, NULL, NULL, NULL, 0); + ok(!ret && GetLastError() == ERROR_PATH_NOT_FOUND, "Expected CopyFileExW failed with ERROR_PATH_NOT_FOUND, but got %ld, copy %s -> %s\n", GetLastError(), wine_dbgstr_w(source), wine_dbgstr_w(long_dest));
I don't see todo_wine in this patch. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7540#note_97365
Zhiyi Zhang (@zhiyi) commented about dlls/kernelbase/file.c:
SetLastError( ERROR_INVALID_PARAMETER ); return FALSE; } + + if (wcsncmp(dest, L"\\\\?\\", 4) && wcslen(dest) >= MAX_PATH ) + { + SetLastError( ERROR_PATH_NOT_FOUND ); + return FALSE; + } + if (wcsncmp(dest, L"\\\\?\\", 4) && wcslen(dest) >= MAX_PATH ) + { + SetLastError( ERROR_PATH_NOT_FOUND ); + return FALSE; + }
You're comparing the length twice. And I don't see removing todo_wine in this patch. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7540#note_97367
Zhiyi Zhang (@zhiyi) commented about dlls/kernel32/tests/file.c:
WCHAR source[MAX_PATH], dest[MAX_PATH]; static const WCHAR prefix[] = {'p','f','x',0}; DWORD ret; + // long file name, 247 charactors
Let's use /**/ instead of // -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7540#note_97366
Zhiyi Zhang (@zhiyi) commented about dlls/kernelbase/file.c:
BOOL WINAPI DECLSPEC_HOTPATCH DeleteFileA( LPCSTR path ) { WCHAR *pathW; + BOOL res;
- if (!(pathW = file_name_AtoW( path, FALSE ))) return FALSE; - return DeleteFileW( pathW ); + if (!(pathW = file_name_AtoW( path, TRUE ))) return FALSE; + res = DeleteFileW( pathW ); + HeapFree( GetProcessHeap(), 0, pathW ); This looks like it should be in a separate patch.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7540#note_97368
Zhiyi Zhang (@zhiyi) commented about dlls/kernelbase/file.c:
SetLastError( ERROR_INVALID_PARAMETER ); return FALSE; } + + if (wcsncmp(dest, L"\\\\?\\", 4) && wcslen(dest) >= MAX_PATH )
What if the process is long-path-aware? Please see https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-lim... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7540#note_97369
On Tue Mar 11 08:43:09 2025 +0000, Zhiyi Zhang wrote:
I don't see todo_wine in this patch. For long path, the length larger than MAX_PATH, CopyFileExW will be success by wine, but on windows 10 with internal version 19045.5487, it will be failed with ERROR_PATH_NOT_FOUND.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7540#note_97371
On Tue Mar 11 08:44:42 2025 +0000, Zhiyi Zhang wrote:
Let's use /**/ instead of // ok
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7540#note_97372
On Tue Mar 11 08:44:42 2025 +0000, Zhiyi Zhang wrote:
You're comparing the length twice. And I don't see removing todo_wine in this patch. I wrote it wrong. I originally intended to write one as source and one as dest, -\_-.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7540#note_97373
On Tue Mar 11 09:16:38 2025 +0000, Yongjie Yao wrote:
For long path, the length larger than MAX_PATH, CopyFileExW will be success by wine, but on windows 10 with internal version 19045.5487, it will be failed with ERROR_PATH_NOT_FOUND. So what you do is to add a todo_wine before the ok macro. Because Wine succeeds, so `ok(!ret && GetLastError() == ERROR_PATH_NOT_FOUND, ...)` should fail on Wine. Then after you fix the bug on Wine, you remove the todo_wine.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7540#note_97374
On Tue Mar 11 08:45:31 2025 +0000, Zhiyi Zhang wrote:
This looks like it should be in a separate patch. ok, thank you for your advice.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7540#note_97375
On Tue Mar 11 09:20:31 2025 +0000, Zhiyi Zhang wrote:
So what you do is to add a todo_wine before the ok macro. Because Wine succeeds, so `ok(!ret && GetLastError() == ERROR_PATH_NOT_FOUND, ...)` should fail on Wine. Then after you fix the bug on Wine, you remove the todo_wine. Ok, I see, thank you.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7540#note_97380
participants (3)
-
yaoyongjie -
Yongjie Yao (＠yaoyongjie)
-
Zhiyi Zhang (＠zhiyi)