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