[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:29:31 CDT 2021
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?
More information about the wine-devel
mailing list