[PATCH v2 0/1] MR8528: ntdll: Remove trailing slashes from the NT names for directories.
Fixes a regression with af35741d. --- The EA Games launcher depends on `GetFinalPathNameByHandleW` returning a path *without* a trailing slash for directories, even if the handle was opened with one. After af35741d, we now use the server's notion of the fd path (via `NtQueryObject(ObjectNameInformation)`), which will include a trailing slash if that's how the path was specified when opened. A quick test on Windows show that `NtQueryObject(ObjectNameInformation)` for file handles returns a `\Device\...` path. (This is why I didn't add tests for its behavior.) For directories, `ObjectNameInformation` will only return a trailing slash for the root of a device - e.g. `\Device\HarddiskVolume3\` for `C:\`, but `\Device\HarddiskVolume3\Windows` for `C:\Windows\`. ~~Since our `NtQueryObject(ObjectNameInformation)` already doesn't match native (we return `\??\` paths), arguably this change should be made in `GetFinalPathNameByHandleW`. It seemed cleaner in the server though, as we immediately know whether a path points to a directory.~~ Now doing this client-side in `get_nt_and_unix_names`, which intercepts paths before they're handed to the server. -- v2: ntdll: Remove trailing slashes from the NT names for directories. https://gitlab.winehq.org/wine/wine/-/merge_requests/8528
From: Tim Clem <tclem(a)codeweavers.com> Fixes a regression with af35741d. --- dlls/kernel32/tests/file.c | 24 ++++++++++++++++++++++++ dlls/ntdll/unix/file.c | 22 ++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/dlls/kernel32/tests/file.c b/dlls/kernel32/tests/file.c index 83766dfaaa2..1a5981261a5 100644 --- a/dlls/kernel32/tests/file.c +++ b/dlls/kernel32/tests/file.c @@ -5436,6 +5436,30 @@ static void test_GetFinalPathNameByHandleW(void) wine_dbgstr_w(nt_path), wine_dbgstr_w(result_path)); CloseHandle(file); + + /* Roots of drives should come back with a trailing slash. */ + file = CreateFileW(L"C:\\", GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL, + OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, 0); + ok(file != INVALID_HANDLE_VALUE, "CreateFileW error %lu\n", GetLastError()); + memset(result_path, 0x11, sizeof(result_path)); + count = pGetFinalPathNameByHandleW(file, result_path, MAX_PATH, FILE_NAME_NORMALIZED | VOLUME_NAME_DOS); + lstrcpyW(dos_path, L"\\\\?\\C:\\"); + ok(count == lstrlenW(dos_path), "Expected length %u, got %lu\n", lstrlenW(dos_path), count); + ok(lstrcmpiW(dos_path, result_path) == 0, "Expected %s, got %s\n", + wine_dbgstr_w(dos_path), wine_dbgstr_w(result_path)); + CloseHandle(file); + + /* Other directories should not have a trailing slash. */ + file = CreateFileW(L"C:\\windows\\", GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL, + OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, 0); + ok(file != INVALID_HANDLE_VALUE, "CreateFileW error %lu\n", GetLastError()); + memset(result_path, 0x11, sizeof(result_path)); + count = pGetFinalPathNameByHandleW(file, result_path, MAX_PATH, FILE_NAME_NORMALIZED | VOLUME_NAME_DOS); + lstrcpyW(dos_path, L"\\\\?\\C:\\windows"); + ok(count == lstrlenW(dos_path), "Expected length %u, got %lu\n", lstrlenW(dos_path), count); + ok(lstrcmpiW(dos_path, result_path) == 0, "Expected %s, got %s\n", + wine_dbgstr_w(dos_path), wine_dbgstr_w(result_path)); + CloseHandle(file); } static void test_SetFileInformationByHandle(void) diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index 5bed090a45b..a6c60635e33 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -4045,8 +4045,30 @@ NTSTATUS get_nt_and_unix_names( OBJECT_ATTRIBUTES *attr, UNICODE_STRING *nt_name if (status && status != STATUS_NO_SUCH_FILE) TRACE( "%s -> ret %x\n", debugstr_us(orig), status ); else + { + /* Trim trailing slashes from anything other than roots of drives. */ + USHORT nt_len = attr->ObjectName->Length / sizeof(WCHAR); + if (nt_len > 7 /* "\??\X:\" */ && attr->ObjectName->Buffer[nt_len - 1] == '\\') + { + if (nt_name->Buffer) + { + nt_name->Buffer[nt_len - 1] = 0; + nt_name->Length -= sizeof(WCHAR); + } + else + { + WCHAR *buffer = malloc( nt_len * sizeof(WCHAR) ); + if (!buffer) return STATUS_NO_MEMORY; + memcpy( buffer, attr->ObjectName->Buffer, (nt_len - 1) * sizeof(WCHAR) ); + buffer[nt_len - 1] = 0; + init_unicode_string( nt_name, buffer ); + attr->ObjectName = nt_name; + } + } + TRACE( "%s -> ret %x nt %s unix %s\n", debugstr_us(orig), status, debugstr_us(attr->ObjectName), debugstr_a(*unix_name_ret) ); + } return status; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8528
On Wed Jul 9 16:36:21 2025 +0000, Alexandre Julliard wrote:
This should most likely be done client-side. In general the server doesn't interpret the NT path, it just passes it through. v2 does this in ntdll. `get_nt_and_unix_names` seemed like the most sensible place to me, but I'm open to suggestions obviously.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8528#note_109351
On Wed Jul 9 16:36:21 2025 +0000, Tim Clem wrote:
v2 does this in ntdll. `get_nt_and_unix_names` seemed like the most sensible place to me, but I'm open to suggestions obviously. You can't assume that you get a drive letter path, there are other possible formats. Also the tests should use NtCreateFile to confirm that this should be done on the kernel side.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8528#note_109364
On Wed Jul 9 20:13:08 2025 +0000, Alexandre Julliard wrote:
You can't assume that you get a drive letter path, there are other possible formats. Also the tests should use NtCreateFile to confirm that this should be done on the kernel side. Ah, true, didn't think that NtCreateFile behavior might differ. It doesn't, and I'll update the tests.
As for other possible formats, does it suffice to skip other prefixes using `get_dos_prefix_len`, like `nt_to_unix_file_name_no_root` does? After such a prefix, I should always have a drive letter path, right? Otherwise previous parts of `get_nt_and_unix_names` will have set a non-zero status, I think. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8528#note_109494
On Thu Jul 10 18:30:57 2025 +0000, Tim Clem wrote:
Ah, true, didn't think that NtCreateFile behavior might differ. It doesn't, and I'll update the tests. As for other possible formats, does it suffice to skip other prefixes using `get_dos_prefix_len`, like `nt_to_unix_file_name_no_root` does? After such a prefix, I should always have a drive letter path, right? Otherwise previous parts of `get_nt_and_unix_names` will have set a non-zero status, I think. No, you can also get other DOS devices, or relative path names.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8528#note_109519
On Thu Jul 10 19:59:52 2025 +0000, Alexandre Julliard wrote:
No, you can also get other DOS devices, or relative path names. Ah, I see - the Unix path gets fully resolved, but the NT path can be relative if there's a root in the OBJECT_ATTRIBUTES. That makes it very tricky to correct in `get_nt_and_unix_names`.
Perhaps we should just strip trailing slashes in GetFinalPathNameByHandle for now, since we're not going to match native on ObjectNameInformation anyway? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8528#note_109612
This merge request was closed by Tim Clem. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8528
Superseded by 8f799fb41615f2a54ccb963d2987ab2109ee61d4. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8528#note_110675
participants (3)
-
Alexandre Julliard (＠julliard) -
Tim Clem
-
Tim Clem (＠tclem)