[PATCH v8] ntdll: Allow renaming a file/directory to a different case of itself.
Gabriel Ivăncescu
gabrielopcode at gmail.com
Thu Apr 29 09:58:37 CDT 2021
On 29/04/2021 17:33, Rémi Bernon wrote:
> On 4/29/21 4:29 PM, Gabriel Ivăncescu wrote:
>> On 28/04/2021 22:03, Rémi Bernon wrote:
>>> On 4/26/21 7:38 PM, Gabriel Ivăncescu wrote:
>>>> Renaming a file or directory from e.g. foobar to FooBar (or any
>>>> other case
>>>> change) should work, like on Windows, instead of being a no-op.
>>>> Clobbering
>>>> an existing file must also respect the new case.
>>>>
>>>> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46203
>>>> Signed-off-by: Gabriel Ivăncescu <gabrielopcode at gmail.com>
>>>> ---
>>>>
>>>> We just prepend the unix original case filename (without path) to the
>>>> wineserver data before the actual unix filename, and a field `caselen`
>>>> which is the length of this data.
>>>>
>>>> dlls/kernel32/tests/file.c | 4 +-
>>>> dlls/ntdll/tests/file.c | 4 +-
>>>> dlls/ntdll/unix/file.c | 63 ++++++++++++++++++++++--
>>>> server/fd.c | 98
>>>> +++++++++++++++++++++++++-------------
>>>> server/protocol.def | 2 +
>>>> 5 files changed, 131 insertions(+), 40 deletions(-)
>>>>
>>>> diff --git a/dlls/kernel32/tests/file.c b/dlls/kernel32/tests/file.c
>>>> index 8560524..9ca56e1 100644
>>>> --- a/dlls/kernel32/tests/file.c
>>>> +++ b/dlls/kernel32/tests/file.c
>>>> @@ -2040,7 +2040,7 @@ static void test_MoveFileA(void)
>>>> ok(hfile != INVALID_HANDLE_VALUE, "FindFirstFileA: failed,
>>>> error %d\n", GetLastError());
>>>> if (hfile != INVALID_HANDLE_VALUE)
>>>> {
>>>> - todo_wine ok(!lstrcmpA(strrchr(tempdir, '\\') + 1,
>>>> find_data.cFileName),
>>>> + ok(!lstrcmpA(strrchr(tempdir, '\\') + 1, find_data.cFileName),
>>>> "MoveFile failed to change casing on same file: got
>>>> %s\n", find_data.cFileName);
>>>> }
>>>> CloseHandle(hfile);
>>>> @@ -2085,7 +2085,7 @@ static void test_MoveFileA(void)
>>>> ok(hfile != INVALID_HANDLE_VALUE, "FindFirstFileA: failed,
>>>> error %d\n", GetLastError());
>>>> if (hfile != INVALID_HANDLE_VALUE)
>>>> {
>>>> - todo_wine ok(!lstrcmpA(strrchr(tempdir, '\\') + 1,
>>>> find_data.cFileName),
>>>> + ok(!lstrcmpA(strrchr(tempdir, '\\') + 1, find_data.cFileName),
>>>> "MoveFile failed to change casing on same directory:
>>>> got %s\n", find_data.cFileName);
>>>> }
>>>> CloseHandle(hfile);
>>>> diff --git a/dlls/ntdll/tests/file.c b/dlls/ntdll/tests/file.c
>>>> index d469b44..49f4e34 100644
>>>> --- a/dlls/ntdll/tests/file.c
>>>> +++ b/dlls/ntdll/tests/file.c
>>>> @@ -2315,7 +2315,7 @@ static void test_file_link_information(void)
>>>> ok(handle != INVALID_HANDLE_VALUE, "FindFirstFileW: failed,
>>>> error %d\n", GetLastError());
>>>> if (handle != INVALID_HANDLE_VALUE)
>>>> {
>>>> - todo_wine ok(!lstrcmpW(wcsrchr(newpath, '\\') + 1,
>>>> find_data.cFileName),
>>>> + ok(!lstrcmpW(wcsrchr(newpath, '\\') + 1, find_data.cFileName),
>>>> "Link did not change casing on existing target file:
>>>> got %s\n", wine_dbgstr_w(find_data.cFileName));
>>>> }
>>>> @@ -2900,7 +2900,7 @@ static void test_file_link_information(void)
>>>> ok(handle != INVALID_HANDLE_VALUE, "FindFirstFileW: failed,
>>>> error %d\n", GetLastError());
>>>> if (handle != INVALID_HANDLE_VALUE)
>>>> {
>>>> - todo_wine ok(!lstrcmpW(wcsrchr(oldpath, '\\') + 1,
>>>> find_data.cFileName),
>>>> + ok(!lstrcmpW(wcsrchr(oldpath, '\\') + 1, find_data.cFileName),
>>>> "Link did not change casing on same file: got %s\n",
>>>> wine_dbgstr_w(find_data.cFileName));
>>>> }
>>>> diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c
>>>> index e32de57..a3af5bb 100644
>>>> --- a/dlls/ntdll/unix/file.c
>>>> +++ b/dlls/ntdll/unix/file.c
>>>> @@ -3400,6 +3400,55 @@ NTSTATUS nt_to_unix_file_name( const
>>>> OBJECT_ATTRIBUTES *attr, char **name_ret, U
>>>> }
>>>> +/***********************************************************************
>>>>
>>>> + * nt_to_unix_file_name_with_actual_case
>>>> + *
>>>> + * Same as nt_to_unix_file_name, but additionally return unix file
>>>> name
>>>> + * without path, with the actual case from the NT file name.
>>>> + */
>>>> +static NTSTATUS nt_to_unix_file_name_with_actual_case( const
>>>> OBJECT_ATTRIBUTES *attr, char **name_ret,
>>>> + char
>>>> **actual_case_ret, UINT disposition )
>>>> +{
>>>> + const WCHAR *nt_filename = attr->ObjectName->Buffer;
>>>> + char *actual_case;
>>>> + NTSTATUS status;
>>>> + int len;
>>>> +
>>>> + /* strip off trailing backslashes; we also accept '/' for unix
>>>> namespaces */
>>>> + for (len = attr->ObjectName->Length / sizeof(WCHAR); len != 0;
>>>> len--)
>>>> + if (nt_filename[len - 1] != '\\' && nt_filename[len - 1] !=
>>>> '/')
>>>> + break;
>>>> +
>>>> + /* get the last component */
>>>> + for (nt_filename += len; nt_filename !=
>>>> attr->ObjectName->Buffer; nt_filename--)
>>>> + if (nt_filename[-1] == '\\' || nt_filename[-1] == '/')
>>>> + break;
>>>> + len = attr->ObjectName->Buffer + len - nt_filename;
>>>> +
>>>> + if (!(actual_case = malloc( len * 3 + 1 ))) return
>>>> STATUS_NO_MEMORY;
>>>> +
>>>> + status = nt_to_unix_file_name( attr, name_ret, disposition );
>>>> + if (status != STATUS_SUCCESS && status != STATUS_NO_SUCH_FILE)
>>>> + {
>>>> + free( actual_case );
>>>> + return status;
>>>> + }
>>>> +
>>>> + len = ntdll_wcstoumbs( nt_filename, len, actual_case, len * 3,
>>>> TRUE );
>>>> + if (len > 0)
>>>> + actual_case[len] = 0;
>>>> + else
>>>> + {
>>>> + char *p = strrchr( *name_ret, '/' );
>>>> + p = p ? p + 1 : *name_ret;
>>>> + strcpy( actual_case, p );
>>>> + }
>>>> +
>>>> + *actual_case_ret = actual_case;
>>>> + return status;
>>>> +}
>>>> +
>>>> +
>>>> /******************************************************************************
>>>>
>>>> * wine_nt_to_unix_file_name
>>>> *
>>>> @@ -4522,8 +4571,8 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE
>>>> handle, IO_STATUS_BLOCK *io,
>>>> {
>>>> FILE_RENAME_INFORMATION *info = ptr;
>>>> UNICODE_STRING name_str, redir;
>>>> + char *unix_name, *file_case;
>>>> OBJECT_ATTRIBUTES attr;
>>>> - char *unix_name;
>>>> name_str.Buffer = info->FileName;
>>>> name_str.Length = info->FileNameLength;
>>>> @@ -4531,7 +4580,7 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE
>>>> handle, IO_STATUS_BLOCK *io,
>>>> InitializeObjectAttributes( &attr, &name_str,
>>>> OBJ_CASE_INSENSITIVE, info->RootDirectory, NULL );
>>>> get_redirect( &attr, &redir );
>>>> - io->u.Status = nt_to_unix_file_name( &attr, &unix_name,
>>>> FILE_OPEN_IF );
>>>> + io->u.Status = nt_to_unix_file_name_with_actual_case(
>>>> &attr, &unix_name, &file_case, FILE_OPEN_IF );
>>>> if (io->u.Status == STATUS_SUCCESS || io->u.Status ==
>>>> STATUS_NO_SUCH_FILE)
>>>> {
>>>> SERVER_START_REQ( set_fd_name_info )
>>>> @@ -4539,15 +4588,18 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE
>>>> handle, IO_STATUS_BLOCK *io,
>>>> req->handle = wine_server_obj_handle( handle );
>>>> req->rootdir = wine_server_obj_handle(
>>>> attr.RootDirectory );
>>>> req->namelen = attr.ObjectName->Length;
>>>> + req->caselen = strlen( file_case );
>>>> req->link = FALSE;
>>>> req->replace = info->ReplaceIfExists;
>>>> wine_server_add_data( req,
>>>> attr.ObjectName->Buffer, attr.ObjectName->Length );
>>>> + wine_server_add_data( req, file_case,
>>>> req->caselen );
>>>> wine_server_add_data( req, unix_name,
>>>> strlen(unix_name) );
>>>> io->u.Status = wine_server_call( req );
>>>> }
>>>> SERVER_END_REQ;
>>>> free( unix_name );
>>>> + free( file_case );
>>>> }
>>>> free( redir.Buffer );
>>>> }
>>>> @@ -4559,8 +4611,8 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE
>>>> handle, IO_STATUS_BLOCK *io,
>>>> {
>>>> FILE_LINK_INFORMATION *info = ptr;
>>>> UNICODE_STRING name_str, redir;
>>>> + char *unix_name, *file_case;
>>>> OBJECT_ATTRIBUTES attr;
>>>> - char *unix_name;
>>>> name_str.Buffer = info->FileName;
>>>> name_str.Length = info->FileNameLength;
>>>> @@ -4568,7 +4620,7 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE
>>>> handle, IO_STATUS_BLOCK *io,
>>>> InitializeObjectAttributes( &attr, &name_str,
>>>> OBJ_CASE_INSENSITIVE, info->RootDirectory, NULL );
>>>> get_redirect( &attr, &redir );
>>>> - io->u.Status = nt_to_unix_file_name( &attr, &unix_name,
>>>> FILE_OPEN_IF );
>>>> + io->u.Status = nt_to_unix_file_name_with_actual_case(
>>>> &attr, &unix_name, &file_case, FILE_OPEN_IF );
>>>> if (io->u.Status == STATUS_SUCCESS || io->u.Status ==
>>>> STATUS_NO_SUCH_FILE)
>>>> {
>>>> SERVER_START_REQ( set_fd_name_info )
>>>> @@ -4576,15 +4628,18 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE
>>>> handle, IO_STATUS_BLOCK *io,
>>>> req->handle = wine_server_obj_handle( handle );
>>>> req->rootdir = wine_server_obj_handle(
>>>> attr.RootDirectory );
>>>> req->namelen = attr.ObjectName->Length;
>>>> + req->caselen = strlen( file_case );
>>>> req->link = TRUE;
>>>> req->replace = info->ReplaceIfExists;
>>>> wine_server_add_data( req,
>>>> attr.ObjectName->Buffer, attr.ObjectName->Length );
>>>> + wine_server_add_data( req, file_case,
>>>> req->caselen );
>>>> wine_server_add_data( req, unix_name,
>>>> strlen(unix_name) );
>>>> io->u.Status = wine_server_call( req );
>>>> }
>>>> SERVER_END_REQ;
>>>> free( unix_name );
>>>> + free( file_case );
>>>> }
>>>> free( redir.Buffer );
>>>> }
>>>> diff --git a/server/fd.c b/server/fd.c
>>>> index 481e9a8..3e51528 100644
>>>> --- a/server/fd.c
>>>> +++ b/server/fd.c
>>>> @@ -2488,11 +2488,14 @@ static void set_fd_disposition( struct fd
>>>> *fd, int unlink )
>>>> /* set new name for the fd */
>>>> static void set_fd_name( struct fd *fd, struct fd *root, const
>>>> char *nameptr, data_size_t len,
>>>> - struct unicode_str nt_name, int
>>>> create_link, int replace )
>>>> + const char *file_case, data_size_t
>>>> caselen, struct unicode_str nt_name,
>>>> + int create_link, int replace )
>>>> {
>>>> + size_t pathlen, filenamelen;
>>>> struct inode *inode;
>>>> struct stat st, st2;
>>>> - char *name;
>>>> + int different_case;
>>>> + char *name, *tmp;
>>>> if (!fd->inode || !fd->unix_name)
>>>> {
>>>> @@ -2526,6 +2529,23 @@ static void set_fd_name( struct fd *fd,
>>>> struct fd *root, const char *nameptr, da
>>>> name = combined_name;
>>>> }
>>>> + tmp = strrchr( name, '/' );
>>>> + tmp = tmp ? tmp + 1 : name;
>>>> + pathlen = tmp - name;
>>>> + filenamelen = strlen( tmp );
>>>> + different_case = (filenamelen != caselen || memcmp( tmp,
>>>> file_case, caselen ));
>>>> +
>>>> + if (filenamelen < caselen)
>>>> + {
>>>> + tmp = realloc( name, pathlen + caselen + 1 );
>>>> + if (!tmp)
>>>> + {
>>>> + set_error( STATUS_NO_MEMORY );
>>>> + goto failed;
>>>> + }
>>>> + name = tmp;
>>>> + }
>>>> +
>>>> /* when creating a hard link, source cannot be a dir */
>>>> if (create_link && !fstat( fd->unix_fd, &st ) && S_ISDIR(
>>>> st.st_mode ))
>>>> {
>>>> @@ -2538,47 +2558,58 @@ static void set_fd_name( struct fd *fd,
>>>> struct fd *root, const char *nameptr, da
>>>> if (!fstat( fd->unix_fd, &st2 ) && st.st_ino == st2.st_ino
>>>> && st.st_dev == st2.st_dev)
>>>> {
>>>> if (create_link && !replace) set_error(
>>>> STATUS_OBJECT_NAME_COLLISION );
>>>> - free( name );
>>>> - return;
>>>> - }
>>>> + if (!different_case)
>>>> + {
>>>> + free( name );
>>>> + return;
>>>> + }
>>>> - if (!replace)
>>>> - {
>>>> - set_error( STATUS_OBJECT_NAME_COLLISION );
>>>> - goto failed;
>>>> + /* creating a link with a different case on itself
>>>> renames the file */
>>>> + create_link = 0;
>>>> }
>>>> -
>>>> - /* can't replace directories or special files */
>>>> - if (!S_ISREG( st.st_mode ))
>>>> + else
>>>> {
>>>> - set_error( STATUS_ACCESS_DENIED );
>>>> - goto failed;
>>>> - }
>>>> + if (!replace)
>>>> + {
>>>> + set_error( STATUS_OBJECT_NAME_COLLISION );
>>>> + goto failed;
>>>> + }
>>>> - /* can't replace an opened file */
>>>> - if ((inode = get_inode( st.st_dev, st.st_ino, -1 )))
>>>> - {
>>>> - int is_empty = list_empty( &inode->open );
>>>> - release_object( inode );
>>>> - if (!is_empty)
>>>> + /* can't replace directories or special files */
>>>> + if (!S_ISREG( st.st_mode ))
>>>> {
>>>> set_error( STATUS_ACCESS_DENIED );
>>>> goto failed;
>>>> }
>>>> - }
>>>> - /* link() expects that the target doesn't exist */
>>>> - /* rename() cannot replace files with directories */
>>>> - if (create_link || S_ISDIR( st2.st_mode ))
>>>> - {
>>>> - if (unlink( name ))
>>>> + /* can't replace an opened file */
>>>> + if ((inode = get_inode( st.st_dev, st.st_ino, -1 )))
>>>> {
>>>> - file_set_error();
>>>> - goto failed;
>>>> + int is_empty = list_empty( &inode->open );
>>>> + release_object( inode );
>>>> + if (!is_empty)
>>>> + {
>>>> + set_error( STATUS_ACCESS_DENIED );
>>>> + goto failed;
>>>> + }
>>>> + }
>>>> +
>>>> + /* link() expects that the target doesn't exist */
>>>> + /* rename() cannot replace files with directories */
>>>> + if (create_link || S_ISDIR( st2.st_mode ) ||
>>>> different_case)
>>>> + {
>>>> + if (unlink( name ))
>>>> + {
>>>> + file_set_error();
>>>> + goto failed;
>>>> + }
>>>> }
>>>> }
>>>> }
>>>> + memcpy( name + pathlen, file_case, caselen );
>>>> + name[pathlen + caselen] = 0;
>>>> +
>>>> if (create_link)
>>>> {
>>>> if (link( fd->unix_name, name ))
>>>> @@ -2879,16 +2910,19 @@ DECL_HANDLER(set_fd_disp_info)
>>>> /* set fd name information */
>>>> DECL_HANDLER(set_fd_name_info)
>>>> {
>>>> + const char *fullname, *file_case;
>>>> struct fd *fd, *root_fd = NULL;
>>>> struct unicode_str nt_name;
>>>> - if (req->namelen > get_req_data_size())
>>>> + if (req->namelen > get_req_data_size() || get_req_data_size() -
>>>> req->namelen < req->caselen)
>>>> {
>>>> set_error( STATUS_INVALID_PARAMETER );
>>>> return;
>>>> }
>>>> nt_name.str = get_req_data();
>>>> nt_name.len = (req->namelen / sizeof(WCHAR)) * sizeof(WCHAR);
>>>> + file_case = (const char *)get_req_data() + req->namelen;
>>>> + fullname = file_case + req->caselen;
>>>> if (req->rootdir)
>>>> {
>>>> @@ -2902,8 +2936,8 @@ DECL_HANDLER(set_fd_name_info)
>>>> if ((fd = get_handle_fd_obj( current->process, req->handle, 0 )))
>>>> {
>>>> - set_fd_name( fd, root_fd, (const char *)get_req_data() +
>>>> req->namelen,
>>>> - get_req_data_size() - req->namelen, nt_name,
>>>> req->link, req->replace );
>>>> + set_fd_name( fd, root_fd, fullname, (const char
>>>> *)get_req_data() + get_req_data_size() - fullname,
>>>> + file_case, req->caselen, nt_name, req->link,
>>>> req->replace );
>>>> release_object( fd );
>>>> }
>>>> if (root_fd) release_object( root_fd );
>>>> diff --git a/server/protocol.def b/server/protocol.def
>>>> index 7c22fae..a4517e6 100644
>>>> --- a/server/protocol.def
>>>> +++ b/server/protocol.def
>>>> @@ -3527,9 +3527,11 @@ struct handle_info
>>>> obj_handle_t handle; /* handle to a file or directory */
>>>> obj_handle_t rootdir; /* root directory */
>>>> data_size_t namelen; /* length of NT name in bytes */
>>>> + data_size_t caselen; /* length of the actual case
>>>> filename */
>>>> int link; /* link instead of renaming */
>>>> int replace; /* replace an existing file? */
>>>> VARARG(name,unicode_str,namelen); /* NT name */
>>>> + VARARG(actual_case,string,caselen); /* new file name's actual
>>>> case (without path) */
>>>> VARARG(filename,string); /* new file name */
>>>> @END
>>>>
>>>
>>> I was a bit confused by the "unix namespaces" thing and the mixed
>>> slashes. So I spent a bit of time playing with that, and I think it
>>> could cause trouble, in some cases, although the problem arguably
>>> comes from nt_to_unix_file_name itself.
>>>
>>> It turns out that nt_to_unix_file_name does not consistently
>>> canonicalize the names, and leaves trailing slashes in a few cases.
>>> This can be verified with "winepath -u":
>>>
>>> For instance, it looks like only one trailing \\ or / is stripped,
>>> and only if the path, ignoring the last component, exists. More
>>> trailing back or forward slashes are kept.
>>>
>>> We'll be in trouble because of that, with the strrchr call done in
>>> the server code to figure the last component to replace with file_case.
>>>
>>> So either we fix nt_to_unix_file_name to canonicalize paths a bit (at
>>> least the trailing slashes part), or this will have to skip trailing
>>> '/' in the server as well.
>>
>> Ah, good point. AFAIK the unix namespace is case-sensitive, and
>> doesn't do much lookup, since well it's supposed to be "unix" so it's
>> more direct.
>>
>> If I skip the '/' in the server should I also copy the trailing
>> slashes over after the new case to preserve the behavior?
>
> I guess so, but maybe we could strip all trailing slashes in
> nt_to_unix_file_name instead? Is there a point keeping them?
Tbh it looks like an unintended corner case to me, so we should strip
them regardless imo. Doing something like '\\?\unix/foo/bar\\' strips
only one backslash, while '\\?\unix/foo/bar//' doesn't strip any.
'\\?\unix\foo\bar\' doesn't strip either (just replaced backslashes with
slashes *before* the end), but '\\?\unix\tmp\bar\' does, because tmp
exists... so for me, it's a bug, unless I'm unaware of something.
More information about the wine-devel
mailing list