[PATCH v9 1/2] ntdll: Strip trailing slashes after looking up unix name.
As they are useless anyway, and have inconsistencies, plus not useful to new code that assumes there's not any on the unix side. See: https://www.winehq.org/pipermail/wine-devel/2021-April/185850.html Note that the root / is not stripped, which is still correct. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- winepath still reports multiple slashes, but I checked the trace from this, it doesn't return trailing slash anymore (except for / root component), so that's an unrelated problem in winepath, I think. dlls/ntdll/unix/file.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index 2f18325..ec0beaa 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -3155,6 +3155,8 @@ static NTSTATUS lookup_unix_name( const WCHAR *name, int name_len, char **buffer char *p; unix_name[pos + 1 + ret] = 0; for (p = unix_name + pos ; *p; p++) if (*p == '\\') *p = '/'; + while (p - 1 > unix_name && p[-1] == '/') p--; + *p = 0; if (!stat( unix_name, &st )) { if (disposition == FILE_CREATE) return STATUS_OBJECT_NAME_COLLISION; @@ -3221,6 +3223,12 @@ static NTSTATUS lookup_unix_name( const WCHAR *name, int name_len, char **buffer name = next; } + if (status == STATUS_SUCCESS || status == STATUS_NO_SUCH_FILE) + { + char *p = unix_name + strlen( unix_name ); + while (p - 1 > unix_name && p[-1] == '/') p--; + *p = 0; + } return status; } -- 2.30.0
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(a)gmail.com> --- Added special case for '/' root component, as it has no named component. Otherwise, \\?\unix\ for example would cause the case component to be 'unix' even though it refers to / dlls/kernel32/tests/file.c | 4 +- dlls/ntdll/tests/file.c | 4 +- dlls/ntdll/unix/file.c | 69 +++++++++++++++++++++++++-- server/fd.c | 98 +++++++++++++++++++++++++------------- server/protocol.def | 2 + 5 files changed, 137 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 ec0beaa..21d26be 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -3408,6 +3408,61 @@ 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; + } + + /* special case for '/' root itself, as it has no named components */ + if ((*name_ret)[0] == '/' && !(*name_ret)[1]) + actual_case[0] = 0; + else + { + 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 * @@ -4530,8 +4585,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; @@ -4539,7 +4594,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 ) @@ -4547,15 +4602,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 ); } @@ -4567,8 +4625,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; @@ -4576,7 +4634,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 ) @@ -4584,15 +4642,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 d061fca..77bfdaf 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -3519,9 +3519,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 -- 2.30.0
Gabriel Ivăncescu <gabrielopcode(a)gmail.com> writes:
As they are useless anyway, and have inconsistencies, plus not useful to new code that assumes there's not any on the unix side. See: https://www.winehq.org/pipermail/wine-devel/2021-April/185850.html
Actually, trailing slashes are in general not allowed on NT names, we have some todo tests about that. A single trailing slash on a directory name is OK, and I think your code should deal with that. -- Alexandre Julliard julliard(a)winehq.org
On 29/04/2021 19:58, Alexandre Julliard wrote:
Gabriel Ivăncescu <gabrielopcode(a)gmail.com> writes:
As they are useless anyway, and have inconsistencies, plus not useful to new code that assumes there's not any on the unix side. See: https://www.winehq.org/pipermail/wine-devel/2021-April/185850.html
Actually, trailing slashes are in general not allowed on NT names, we have some todo tests about that. A single trailing slash on a directory name is OK, and I think your code should deal with that.
What about unix names? I'm only stripping it from unix names. If so, should I handle any amount of trailing slashes on unix names, or just one?
Gabriel Ivăncescu <gabrielopcode(a)gmail.com> writes:
On 29/04/2021 19:58, Alexandre Julliard wrote:
Gabriel Ivăncescu <gabrielopcode(a)gmail.com> writes:
As they are useless anyway, and have inconsistencies, plus not useful to new code that assumes there's not any on the unix side. See: https://www.winehq.org/pipermail/wine-devel/2021-April/185850.html Actually, trailing slashes are in general not allowed on NT names, we have some todo tests about that. A single trailing slash on a directory name is OK, and I think your code should deal with that.
What about unix names? I'm only stripping it from unix names.
If so, should I handle any amount of trailing slashes on unix names, or just one?
If the checks on the NT side are done correctly there can be at most one. I'm not convinced that it's necessary to remove it. -- Alexandre Julliard julliard(a)winehq.org
participants (2)
-
Alexandre Julliard -
Gabriel Ivăncescu