[PATCH v4 1/2] ntdll: Allow renaming a file/directory to a different casing of itself.
Rémi Bernon
rbernon at codeweavers.com
Tue Apr 20 16:02:11 CDT 2021
On 4/20/21 5:04 PM, Gabriel Ivăncescu wrote:
> Renaming a file or directory from e.g. foobar to FooBar (or any other casing
> change) should work, like on Windows, instead of being a no-op. Clobbering
> an existing file must also respect the new casing.
>
> 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 casing filename (without path) to the wineserver
> data before the actual unix filename, and a field `casinglen` which is the
> length of this data.
>
> dlls/ntdll/unix/file.c | 59 ++++++++++++++++++++------
> server/fd.c | 95 +++++++++++++++++++++++++++++-------------
> server/protocol.def | 2 +
> 3 files changed, 114 insertions(+), 42 deletions(-)
>
> diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c
> index 488f748..0cf1a7f 100644
> --- a/dlls/ntdll/unix/file.c
> +++ b/dlls/ntdll/unix/file.c
> @@ -2800,6 +2800,31 @@ void init_files(void)
> }
>
>
> +/***********************************************************************
> + * get_nt_file_name_unix_casing
> + *
> + * Get the Unix file name with the same casing as the NT file name (without path).
> + */
> +static unsigned int get_nt_file_name_unix_casing(const OBJECT_ATTRIBUTES *attr, char **casing)
> +{
> + const WCHAR *filename = attr->ObjectName->Buffer + attr->ObjectName->Length / sizeof(WCHAR);
> + int len, casinglen = 0;
> +
> + /* NT name may not be NUL terminated; look for last \ character */
> + for (; filename != attr->ObjectName->Buffer; filename--)
> + if (filename[-1] == '\\')
> + break;
> + len = attr->ObjectName->Buffer + attr->ObjectName->Length / sizeof(WCHAR) - filename;
> +
> + if ((*casing = malloc( len * 3 )))
> + {
> + casinglen = ntdll_wcstoumbs( filename, len, *casing, len * 3, TRUE );
> + if (casinglen < 0) casinglen = 0;
> + }
> + return casinglen;
> +}
> +
> +
> /******************************************************************************
> * get_dos_device
> *
> @@ -4521,8 +4546,8 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io,
> {
> FILE_RENAME_INFORMATION *info = ptr;
> UNICODE_STRING name_str, redir;
> + char *unix_name, *casing;
> OBJECT_ATTRIBUTES attr;
> - char *unix_name;
>
> name_str.Buffer = info->FileName;
> name_str.Length = info->FileNameLength;
> @@ -4533,20 +4558,25 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io,
> io->u.Status = nt_to_unix_file_name( &attr, &unix_name, FILE_OPEN_IF );
> if (io->u.Status == STATUS_SUCCESS || io->u.Status == STATUS_NO_SUCH_FILE)
> {
> + unsigned int casinglen = get_nt_file_name_unix_casing( &attr, &casing );
> +
> SERVER_START_REQ( set_fd_name_info )
> {
> - req->handle = wine_server_obj_handle( handle );
> - req->rootdir = wine_server_obj_handle( attr.RootDirectory );
> - req->namelen = attr.ObjectName->Length;
> - req->link = FALSE;
> - req->replace = info->ReplaceIfExists;
> + req->handle = wine_server_obj_handle( handle );
> + req->rootdir = wine_server_obj_handle( attr.RootDirectory );
> + req->namelen = attr.ObjectName->Length;
> + req->casinglen = casinglen;
> + req->link = FALSE;
> + req->replace = info->ReplaceIfExists;
> wine_server_add_data( req, attr.ObjectName->Buffer, attr.ObjectName->Length );
> + wine_server_add_data( req, casing, casinglen );
> wine_server_add_data( req, unix_name, strlen(unix_name) );
> io->u.Status = wine_server_call( req );
> }
> SERVER_END_REQ;
>
> free( unix_name );
> + free( casing );
> }
> free( redir.Buffer );
> }
I think it'd be better to handle the casing allocation even before
nt_to_unix_file_name, and set Status in case of failure.
Then I'm not sure how likely ntdll_wcstoumbs is expected to fail, but in
that case it should probably just use strrchr( unix_name, '/' ) or
unix_name, to have a nicer fallback.
> @@ -4558,8 +4588,8 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io,
> {
> FILE_LINK_INFORMATION *info = ptr;
> UNICODE_STRING name_str, redir;
> + char *unix_name, *casing;
> OBJECT_ATTRIBUTES attr;
> - char *unix_name;
>
> name_str.Buffer = info->FileName;
> name_str.Length = info->FileNameLength;
> @@ -4570,20 +4600,25 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io,
> io->u.Status = nt_to_unix_file_name( &attr, &unix_name, FILE_OPEN_IF );
> if (io->u.Status == STATUS_SUCCESS || io->u.Status == STATUS_NO_SUCH_FILE)
> {
> + unsigned int casinglen = get_nt_file_name_unix_casing( &attr, &casing );
> +
> SERVER_START_REQ( set_fd_name_info )
> {
> - req->handle = wine_server_obj_handle( handle );
> - req->rootdir = wine_server_obj_handle( attr.RootDirectory );
> - req->namelen = attr.ObjectName->Length;
> - req->link = TRUE;
> - req->replace = info->ReplaceIfExists;
> + req->handle = wine_server_obj_handle( handle );
> + req->rootdir = wine_server_obj_handle( attr.RootDirectory );
> + req->namelen = attr.ObjectName->Length;
> + req->casinglen = casinglen;
> + req->link = TRUE;
> + req->replace = info->ReplaceIfExists;
> wine_server_add_data( req, attr.ObjectName->Buffer, attr.ObjectName->Length );
> + wine_server_add_data( req, casing, casinglen );
> wine_server_add_data( req, unix_name, strlen(unix_name) );
> io->u.Status = wine_server_call( req );
> }
> SERVER_END_REQ;
>
> free( unix_name );
> + free( casing );
> }
> free( redir.Buffer );
> }
Here too you could set casing to be strrchr( unix_name, '/' ) + 1 or
unix_name. So it wouldn't need an allocation. It's also probably not
worth keeping the separate helper.
(My OCD also tells me that using "caselen" instead of "casinglen" in the
request would keep things aligned and save you the re-identation, but
that probably doesn't matter much ;))
> diff --git a/server/fd.c b/server/fd.c
> index 481e9a8..0b4c0a1 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 *casing, data_size_t casinglen, struct unicode_str nt_name,
> + int create_link, int replace )
> {
> + char *name, *filename;
> + int different_casing;
> struct inode *inode;
> struct stat st, st2;
> - char *name;
> + size_t filenamelen;
>
> if (!fd->inode || !fd->unix_name)
> {
> @@ -2533,50 +2536,79 @@ static void set_fd_name( struct fd *fd, struct fd *root, const char *nameptr, da
> goto failed;
> }
>
> + filename = strrchr( name, '/' );
> + filename = filename ? filename + 1 : name;
> + filenamelen = strlen( filename );
> + different_casing = casinglen && (filenamelen != casinglen || memcmp( filename, casing, casinglen ));
> +
I think with the fallback above the casinglen == 0 check is probably not
very useful here?
Also I would do the name realloc here, or even earlier, right after
combined_name, simply checking if casinglen > filenamelen, to make sure
we can simply copy casing to filename later.
> if (!stat( name, &st ))
> {
> 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 (!replace)
> - {
> - set_error( STATUS_OBJECT_NAME_COLLISION );
> - goto failed;
> + if (!different_casing)
> + {
> + free( name );
> + return;
> + }
> }
> -
> - /* 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;
> }
> +
> + /* 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)
> + {
> + 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_casing)
> + {
> + if (unlink( name ))
> + {
> + file_set_error();
> + 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 (different_casing)
> + {
> + size_t pathlen = filename - name;
> + if (filenamelen < casinglen)
> {
> - if (unlink( name ))
> + char *new_name = mem_alloc( pathlen + casinglen + 1 );
> + if (!new_name)
> {
> - file_set_error();
> + set_error( STATUS_NO_MEMORY );
> goto failed;
> }
> + memcpy( new_name, name, pathlen );
> + free( name );
> + name = new_name;
> }
> + memcpy(name + pathlen, casing, casinglen);
> + name[pathlen + casinglen] = 0;
> }
And so this whole if could be something like that instead:
memcpy( filename, casing, caselen );
filename[caselen] = 0;
>
> if (create_link)
> @@ -2879,16 +2911,19 @@ DECL_HANDLER(set_fd_disp_info)
> /* set fd name information */
> DECL_HANDLER(set_fd_name_info)
> {
> + const char *filename, *casing;
> 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->casinglen)
> {
> set_error( STATUS_INVALID_PARAMETER );
> return;
> }
> nt_name.str = get_req_data();
> nt_name.len = (req->namelen / sizeof(WCHAR)) * sizeof(WCHAR);
> + casing = (const char *)get_req_data() + req->namelen;
> + filename = casing + req->casinglen;
>
> if (req->rootdir)
> {
> @@ -2902,8 +2937,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, filename, (const char *)get_req_data() + get_req_data_size() - filename,
> + casing, req->casinglen, 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 9ea6967..8560469 100644
> --- a/server/protocol.def
> +++ b/server/protocol.def
> @@ -3531,9 +3531,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 casinglen; /* length of the casing filename */
> int link; /* link instead of renaming */
> int replace; /* replace an existing file? */
> VARARG(name,unicode_str,namelen); /* NT name */
> + VARARG(casing,string,casinglen); /* new file name's actual casing (without path) */
> VARARG(filename,string); /* new file name */
> @END
>
>
--
Rémi Bernon <rbernon at codeweavers.com>
More information about the wine-devel
mailing list