[PATCH v8] ntdll: Allow renaming a file/directory to a different case of itself.
Rémi Bernon
rbernon at codeweavers.com
Thu Apr 29 09:33:36 CDT 2021
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?
--
Rémi Bernon <rbernon at codeweavers.com>
More information about the wine-devel
mailing list