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@gmail.com ---
We just prepend the unix casing filename (without path) to the wineserver data before the actual unix filename, and a field `caselen` which is the length of this data.
dlls/ntdll/unix/file.c | 56 ++++++++++++++++++++++-- server/fd.c | 97 ++++++++++++++++++++++++++++-------------- server/protocol.def | 2 + 3 files changed, 118 insertions(+), 37 deletions(-)
diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index 488f748..d8fdb27 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -4520,9 +4520,11 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, if (len >= sizeof(FILE_RENAME_INFORMATION)) { FILE_RENAME_INFORMATION *info = ptr; + int nt_filenamelen, casinglen = 0; UNICODE_STRING name_str, redir; + const WCHAR *nt_filename; + char *unix_name, *casing; OBJECT_ATTRIBUTES attr; - char *unix_name;
name_str.Buffer = info->FileName; name_str.Length = info->FileNameLength; @@ -4530,17 +4532,38 @@ 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 ); + nt_filename = attr.ObjectName->Buffer + attr.ObjectName->Length / sizeof(WCHAR); + for (; nt_filename != attr.ObjectName->Buffer; nt_filename--) + if (nt_filename[-1] == '\') + break; + nt_filenamelen = attr.ObjectName->Buffer + attr.ObjectName->Length / sizeof(WCHAR) - nt_filename; + + if ((casing = malloc( nt_filenamelen * 3 ))) + { + casinglen = ntdll_wcstoumbs( nt_filename, nt_filenamelen, casing, nt_filenamelen * 3, TRUE ); + io->u.Status = nt_to_unix_file_name( &attr, &unix_name, FILE_OPEN_IF ); + } + else + io->u.Status = STATUS_NO_MEMORY; + if (io->u.Status == STATUS_SUCCESS || io->u.Status == STATUS_NO_SUCH_FILE) { + if (casinglen <= 0) + { + casing = strrchr( unix_name, '/' ); + casing = casing ? casing + 1 : unix_name; + casinglen = strlen( 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->caselen = 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 ); } @@ -4548,6 +4571,7 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io,
free( unix_name ); } + free( casing ); free( redir.Buffer ); } else io->u.Status = STATUS_INVALID_PARAMETER_3; @@ -4557,9 +4581,11 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, if (len >= sizeof(FILE_LINK_INFORMATION)) { FILE_LINK_INFORMATION *info = ptr; + int nt_filenamelen, casinglen = 0; UNICODE_STRING name_str, redir; + const WCHAR *nt_filename; + char *unix_name, *casing; OBJECT_ATTRIBUTES attr; - char *unix_name;
name_str.Buffer = info->FileName; name_str.Length = info->FileNameLength; @@ -4567,17 +4593,38 @@ 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 ); + nt_filename = attr.ObjectName->Buffer + attr.ObjectName->Length / sizeof(WCHAR); + for (; nt_filename != attr.ObjectName->Buffer; nt_filename--) + if (nt_filename[-1] == '\') + break; + nt_filenamelen = attr.ObjectName->Buffer + attr.ObjectName->Length / sizeof(WCHAR) - nt_filename; + + if ((casing = malloc( nt_filenamelen * 3 ))) + { + casinglen = ntdll_wcstoumbs( nt_filename, nt_filenamelen, casing, nt_filenamelen * 3, TRUE ); + io->u.Status = nt_to_unix_file_name( &attr, &unix_name, FILE_OPEN_IF ); + } + else + io->u.Status = STATUS_NO_MEMORY; + if (io->u.Status == STATUS_SUCCESS || io->u.Status == STATUS_NO_SUCH_FILE) { + if (casinglen <= 0) + { + casing = strrchr( unix_name, '/' ); + casing = casing ? casing + 1 : unix_name; + casinglen = strlen( 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->caselen = 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 ); } @@ -4585,6 +4632,7 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io,
free( unix_name ); } + free( casing ); free( redir.Buffer ); } else io->u.Status = STATUS_INVALID_PARAMETER_3; diff --git a/server/fd.c b/server/fd.c index 481e9a8..47fab1e 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 ) { + size_t pathlen, filenamelen; + int different_casing; struct inode *inode; struct stat st, st2; - char *name; + 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_casing = (filenamelen != casinglen || memcmp( tmp, casing, casinglen )); + + if (filenamelen < casinglen) + { + tmp = realloc( name, pathlen + casinglen + 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,55 @@ 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 (!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; } - }
- /* 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_casing) + { + if (unlink( name )) + { + file_set_error(); + goto failed; + } } } }
+ memcpy( name + pathlen, casing, casinglen ); + name[pathlen + casinglen] = 0; + if (create_link) { if (link( fd->unix_name, name )) @@ -2879,16 +2907,19 @@ DECL_HANDLER(set_fd_disp_info) /* set fd name information */ DECL_HANDLER(set_fd_name_info) { + const char *fullname, *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->caselen) { 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; + fullname = casing + req->caselen;
if (req->rootdir) { @@ -2902,8 +2933,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, + casing, 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 9ea6967..02e5ab0 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 caselen; /* 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,caselen);/* new file name's actual casing (without path) */ VARARG(filename,string); /* new file name */ @END
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/kernel32/tests/file.c | 64 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+)
diff --git a/dlls/kernel32/tests/file.c b/dlls/kernel32/tests/file.c index 5deed96..dfa69db 100644 --- a/dlls/kernel32/tests/file.c +++ b/dlls/kernel32/tests/file.c @@ -1954,6 +1954,7 @@ static void test_MoveFileA(void) char tempdir[MAX_PATH]; char source[MAX_PATH], dest[MAX_PATH]; static const char prefix[] = "pfx"; + WIN32_FIND_DATAA find_data; HANDLE hfile; HANDLE hmapfile; DWORD ret; @@ -2023,9 +2024,72 @@ static void test_MoveFileA(void) ok(ret, "MoveFileA: failed, error %d\n", GetLastError());
lstrcatA(tempdir, "Remove Me"); + + /* test renaming a file "Remove Me" to itself but in lowercase "me" */ + lstrcpyA(source, tempdir); + tempdir[lstrlenA(tempdir) - 2] = 'm'; + + hfile = CreateFileA(source, GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, CREATE_ALWAYS, 0, 0); + ok(hfile != INVALID_HANDLE_VALUE, "failed to create %s\n", source); + CloseHandle(hfile); + + ret = MoveFileA(source, tempdir); + ok(ret, "MoveFileA: failed, error %d\n", GetLastError()); + + hfile = FindFirstFileA(tempdir, &find_data); + ok(hfile != INVALID_HANDLE_VALUE, "FindFirstFileA: failed, error %d\n", GetLastError()); + if (hfile != INVALID_HANDLE_VALUE) + { + ok(!lstrcmpA(strrchr(tempdir, '\') + 1, find_data.cFileName), + "MoveFile failed to change caps on same file: got %s\n", find_data.cFileName); + } + CloseHandle(hfile); + + /* test renaming another file "Remove Be" to "Remove Me", which replaces the existing "Remove me" */ + tempdir[lstrlenA(tempdir) - 2] = 'B'; + + hfile = CreateFileA(tempdir, GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, CREATE_ALWAYS, 0, 0); + ok(hfile != INVALID_HANDLE_VALUE, "failed to create %s\n", tempdir); + CloseHandle(hfile); + + ret = MoveFileA(tempdir, source); + ok(!ret, "MoveFileA: expected failure\n"); + ok(GetLastError() == ERROR_ALREADY_EXISTS, "MoveFileA: expected ERROR_ALREADY_EXISTS, got %d\n", GetLastError()); + ret = MoveFileExA(tempdir, source, MOVEFILE_REPLACE_EXISTING); + ok(ret, "MoveFileExA: failed, error %d\n", GetLastError()); + + tempdir[lstrlenA(tempdir) - 2] = 'm'; + + hfile = FindFirstFileA(tempdir, &find_data); + ok(hfile != INVALID_HANDLE_VALUE, "FindFirstFileA: failed, error %d\n", GetLastError()); + if (hfile != INVALID_HANDLE_VALUE) + { + ok(!lstrcmpA(strrchr(source, '\') + 1, find_data.cFileName), + "MoveFile failed to change caps on existing target file: got %s\n", find_data.cFileName); + } + CloseHandle(hfile); + + ret = DeleteFileA(tempdir); + ok(ret, "DeleteFileA: error %d\n", GetLastError()); + + /* now test a directory from "Remove me" to uppercase "Me" */ ret = CreateDirectoryA(tempdir, NULL); ok(ret == TRUE, "CreateDirectoryA failed\n");
+ lstrcpyA(source, tempdir); + tempdir[lstrlenA(tempdir) - 2] = 'M'; + ret = MoveFileA(source, tempdir); + ok(ret, "MoveFileA: failed, error %d\n", GetLastError()); + + hfile = FindFirstFileA(tempdir, &find_data); + ok(hfile != INVALID_HANDLE_VALUE, "FindFirstFileA: failed, error %d\n", GetLastError()); + if (hfile != INVALID_HANDLE_VALUE) + { + ok(!lstrcmpA(strrchr(tempdir, '\') + 1, find_data.cFileName), + "MoveFile failed to change caps on same directory: got %s\n", find_data.cFileName); + } + CloseHandle(hfile); + lstrcpyA(source, dest); lstrcpyA(dest, tempdir); lstrcatA(dest, "\wild?.*");
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=89144
Your paranoid android.
=== debiant2 (32 bit Chinese:China report) ===
ntdll: threadpool.c:1960: Test failed: WaitForSingleObject returned 258
=== debiant2 (64 bit WoW report) ===
ntdll: om.c:2320: Test failed: got 84
On 4/21/21 2:42 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@gmail.com
We just prepend the unix casing filename (without path) to the wineserver data before the actual unix filename, and a field `caselen` which is the length of this data.
dlls/ntdll/unix/file.c | 56 ++++++++++++++++++++++-- server/fd.c | 97 ++++++++++++++++++++++++++++-------------- server/protocol.def | 2 + 3 files changed, 118 insertions(+), 37 deletions(-)
diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index 488f748..d8fdb27 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -4520,9 +4520,11 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, if (len >= sizeof(FILE_RENAME_INFORMATION)) { FILE_RENAME_INFORMATION *info = ptr;
int nt_filenamelen, casinglen = 0; UNICODE_STRING name_str, redir;const WCHAR *nt_filename;char *unix_name, *casing; OBJECT_ATTRIBUTES attr;
char *unix_name; name_str.Buffer = info->FileName; name_str.Length = info->FileNameLength;@@ -4530,17 +4532,38 @@ 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 );
nt_filename = attr.ObjectName->Buffer + attr.ObjectName->Length / sizeof(WCHAR);for (; nt_filename != attr.ObjectName->Buffer; nt_filename--)if (nt_filename[-1] == '\\')break;nt_filenamelen = attr.ObjectName->Buffer + attr.ObjectName->Length / sizeof(WCHAR) - nt_filename;if ((casing = malloc( nt_filenamelen * 3 ))){casinglen = ntdll_wcstoumbs( nt_filename, nt_filenamelen, casing, nt_filenamelen * 3, TRUE );io->u.Status = nt_to_unix_file_name( &attr, &unix_name, FILE_OPEN_IF );}elseio->u.Status = STATUS_NO_MEMORY;if (io->u.Status == STATUS_SUCCESS || io->u.Status == STATUS_NO_SUCH_FILE) {if (casinglen <= 0){casing = strrchr( unix_name, '/' );casing = casing ? casing + 1 : unix_name;casinglen = strlen( 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->caselen = 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 ); }@@ -4548,6 +4571,7 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io,
free( unix_name ); }
free( casing ); free( redir.Buffer ); } else io->u.Status = STATUS_INVALID_PARAMETER_3;
This shouldn't free if the fallback path was taken.
@@ -4557,9 +4581,11 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, if (len >= sizeof(FILE_LINK_INFORMATION)) { FILE_LINK_INFORMATION *info = ptr;
int nt_filenamelen, casinglen = 0; UNICODE_STRING name_str, redir;const WCHAR *nt_filename;char *unix_name, *casing; OBJECT_ATTRIBUTES attr;
char *unix_name; name_str.Buffer = info->FileName; name_str.Length = info->FileNameLength;@@ -4567,17 +4593,38 @@ 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 );
nt_filename = attr.ObjectName->Buffer + attr.ObjectName->Length / sizeof(WCHAR);for (; nt_filename != attr.ObjectName->Buffer; nt_filename--)if (nt_filename[-1] == '\\')break;nt_filenamelen = attr.ObjectName->Buffer + attr.ObjectName->Length / sizeof(WCHAR) - nt_filename;if ((casing = malloc( nt_filenamelen * 3 ))){casinglen = ntdll_wcstoumbs( nt_filename, nt_filenamelen, casing, nt_filenamelen * 3, TRUE );io->u.Status = nt_to_unix_file_name( &attr, &unix_name, FILE_OPEN_IF );}elseio->u.Status = STATUS_NO_MEMORY;if (io->u.Status == STATUS_SUCCESS || io->u.Status == STATUS_NO_SUCH_FILE) {if (casinglen <= 0){casing = strrchr( unix_name, '/' );casing = casing ? casing + 1 : unix_name;casinglen = strlen( 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->caselen = 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 ); }@@ -4585,6 +4632,7 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io,
free( unix_name ); }
free( casing ); free( redir.Buffer ); } else io->u.Status = STATUS_INVALID_PARAMETER_3;
I'm not sure we want to support a different casing here, I think there are some additional consideration to have on the server side code:
We don't want to create link of some file to another casing of itself, but instead probably rename the link to another casing if it already exists?
I think we should just use the strrchr fallback all the time here.
However, if we want to support proper casing for FileLinkInformation too, it would probably deserve a few tests first to see what this means for links.
And the implementation could then come in a different patch to make it clearer. In which case, assuming we need the same kind of logic to send the proper casing, a separate helper would indeed be useful.
In any case, I think it would also be nice (if possible?) to have the tests first in the series with todo_wine, removed later in the patch that fixes it.
On 21/04/2021 21:41, Rémi Bernon wrote:
On 4/21/21 2:42 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@gmail.com
We just prepend the unix casing filename (without path) to the wineserver data before the actual unix filename, and a field `caselen` which is the length of this data.
dlls/ntdll/unix/file.c | 56 ++++++++++++++++++++++-- server/fd.c | 97 ++++++++++++++++++++++++++++-------------- server/protocol.def | 2 + 3 files changed, 118 insertions(+), 37 deletions(-)
diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index 488f748..d8fdb27 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -4520,9 +4520,11 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, if (len >= sizeof(FILE_RENAME_INFORMATION)) { FILE_RENAME_INFORMATION *info = ptr; + int nt_filenamelen, casinglen = 0; UNICODE_STRING name_str, redir; + const WCHAR *nt_filename; + char *unix_name, *casing; OBJECT_ATTRIBUTES attr; - char *unix_name; name_str.Buffer = info->FileName; name_str.Length = info->FileNameLength; @@ -4530,17 +4532,38 @@ 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 ); + nt_filename = attr.ObjectName->Buffer + attr.ObjectName->Length / sizeof(WCHAR); + for (; nt_filename != attr.ObjectName->Buffer; nt_filename--) + if (nt_filename[-1] == '\') + break; + nt_filenamelen = attr.ObjectName->Buffer + attr.ObjectName->Length / sizeof(WCHAR) - nt_filename;
+ if ((casing = malloc( nt_filenamelen * 3 ))) + { + casinglen = ntdll_wcstoumbs( nt_filename, nt_filenamelen, casing, nt_filenamelen * 3, TRUE ); + io->u.Status = nt_to_unix_file_name( &attr, &unix_name, FILE_OPEN_IF ); + } + else + io->u.Status = STATUS_NO_MEMORY;
if (io->u.Status == STATUS_SUCCESS || io->u.Status == STATUS_NO_SUCH_FILE) { + if (casinglen <= 0) + { + casing = strrchr( unix_name, '/' ); + casing = casing ? casing + 1 : unix_name; + casinglen = strlen( 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->caselen = 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 ); } @@ -4548,6 +4571,7 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, free( unix_name ); } + free( casing ); free( redir.Buffer ); } else io->u.Status = STATUS_INVALID_PARAMETER_3;
This shouldn't free if the fallback path was taken.
@@ -4557,9 +4581,11 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, if (len >= sizeof(FILE_LINK_INFORMATION)) { FILE_LINK_INFORMATION *info = ptr; + int nt_filenamelen, casinglen = 0; UNICODE_STRING name_str, redir; + const WCHAR *nt_filename; + char *unix_name, *casing; OBJECT_ATTRIBUTES attr; - char *unix_name; name_str.Buffer = info->FileName; name_str.Length = info->FileNameLength; @@ -4567,17 +4593,38 @@ 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 ); + nt_filename = attr.ObjectName->Buffer + attr.ObjectName->Length / sizeof(WCHAR); + for (; nt_filename != attr.ObjectName->Buffer; nt_filename--) + if (nt_filename[-1] == '\') + break; + nt_filenamelen = attr.ObjectName->Buffer + attr.ObjectName->Length / sizeof(WCHAR) - nt_filename;
+ if ((casing = malloc( nt_filenamelen * 3 ))) + { + casinglen = ntdll_wcstoumbs( nt_filename, nt_filenamelen, casing, nt_filenamelen * 3, TRUE ); + io->u.Status = nt_to_unix_file_name( &attr, &unix_name, FILE_OPEN_IF ); + } + else + io->u.Status = STATUS_NO_MEMORY;
if (io->u.Status == STATUS_SUCCESS || io->u.Status == STATUS_NO_SUCH_FILE) { + if (casinglen <= 0) + { + casing = strrchr( unix_name, '/' ); + casing = casing ? casing + 1 : unix_name; + casinglen = strlen( 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->caselen = 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 ); } @@ -4585,6 +4632,7 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, free( unix_name ); } + free( casing ); free( redir.Buffer ); } else io->u.Status = STATUS_INVALID_PARAMETER_3;
I'm not sure we want to support a different casing here, I think there are some additional consideration to have on the server side code:
We don't want to create link of some file to another casing of itself, but instead probably rename the link to another casing if it already exists?
I think we should just use the strrchr fallback all the time here.
However, if we want to support proper casing for FileLinkInformation too, it would probably deserve a few tests first to see what this means for links.
And the implementation could then come in a different patch to make it clearer. In which case, assuming we need the same kind of logic to send the proper casing, a separate helper would indeed be useful.
In any case, I think it would also be nice (if possible?) to have the tests first in the series with todo_wine, removed later in the patch that fixes it.
Yeah, I'm doubtful it matters for links, but I'll see about the tests in any case—and with todo_wine should be cleaner I hope.
But honestly, wouldn't it just be easier to send caselen == 0 and consider that as the fallback in the wineserver? If links even turn out to not be affected by this, it would add no change to the link case at all or any complications with freeing (as you can see, I messed up above).
Basically it boils down to: is it really worth complicating the ntdll code so much just to avoid 2 lines of code on the wineserver side? I just feel like it's not adding much value to make it non-optional, but perhaps I'm missing the bigger picture.
On 22/04/2021 13:34, Gabriel Ivăncescu wrote:
On 21/04/2021 21:41, Rémi Bernon wrote:
On 4/21/21 2:42 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@gmail.com
We just prepend the unix casing filename (without path) to the wineserver data before the actual unix filename, and a field `caselen` which is the length of this data.
dlls/ntdll/unix/file.c | 56 ++++++++++++++++++++++-- server/fd.c | 97 ++++++++++++++++++++++++++++-------------- server/protocol.def | 2 + 3 files changed, 118 insertions(+), 37 deletions(-)
diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index 488f748..d8fdb27 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -4520,9 +4520,11 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, if (len >= sizeof(FILE_RENAME_INFORMATION)) { FILE_RENAME_INFORMATION *info = ptr; + int nt_filenamelen, casinglen = 0; UNICODE_STRING name_str, redir; + const WCHAR *nt_filename; + char *unix_name, *casing; OBJECT_ATTRIBUTES attr; - char *unix_name; name_str.Buffer = info->FileName; name_str.Length = info->FileNameLength; @@ -4530,17 +4532,38 @@ 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 ); + nt_filename = attr.ObjectName->Buffer + attr.ObjectName->Length / sizeof(WCHAR); + for (; nt_filename != attr.ObjectName->Buffer; nt_filename--) + if (nt_filename[-1] == '\') + break; + nt_filenamelen = attr.ObjectName->Buffer + attr.ObjectName->Length / sizeof(WCHAR) - nt_filename;
+ if ((casing = malloc( nt_filenamelen * 3 ))) + { + casinglen = ntdll_wcstoumbs( nt_filename, nt_filenamelen, casing, nt_filenamelen * 3, TRUE ); + io->u.Status = nt_to_unix_file_name( &attr, &unix_name, FILE_OPEN_IF ); + } + else + io->u.Status = STATUS_NO_MEMORY;
if (io->u.Status == STATUS_SUCCESS || io->u.Status == STATUS_NO_SUCH_FILE) { + if (casinglen <= 0) + { + casing = strrchr( unix_name, '/' ); + casing = casing ? casing + 1 : unix_name; + casinglen = strlen( 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->caselen = 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 ); } @@ -4548,6 +4571,7 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, free( unix_name ); } + free( casing ); free( redir.Buffer ); } else io->u.Status = STATUS_INVALID_PARAMETER_3;
This shouldn't free if the fallback path was taken.
@@ -4557,9 +4581,11 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, if (len >= sizeof(FILE_LINK_INFORMATION)) { FILE_LINK_INFORMATION *info = ptr; + int nt_filenamelen, casinglen = 0; UNICODE_STRING name_str, redir; + const WCHAR *nt_filename; + char *unix_name, *casing; OBJECT_ATTRIBUTES attr; - char *unix_name; name_str.Buffer = info->FileName; name_str.Length = info->FileNameLength; @@ -4567,17 +4593,38 @@ 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 ); + nt_filename = attr.ObjectName->Buffer + attr.ObjectName->Length / sizeof(WCHAR); + for (; nt_filename != attr.ObjectName->Buffer; nt_filename--) + if (nt_filename[-1] == '\') + break; + nt_filenamelen = attr.ObjectName->Buffer + attr.ObjectName->Length / sizeof(WCHAR) - nt_filename;
+ if ((casing = malloc( nt_filenamelen * 3 ))) + { + casinglen = ntdll_wcstoumbs( nt_filename, nt_filenamelen, casing, nt_filenamelen * 3, TRUE ); + io->u.Status = nt_to_unix_file_name( &attr, &unix_name, FILE_OPEN_IF ); + } + else + io->u.Status = STATUS_NO_MEMORY;
if (io->u.Status == STATUS_SUCCESS || io->u.Status == STATUS_NO_SUCH_FILE) { + if (casinglen <= 0) + { + casing = strrchr( unix_name, '/' ); + casing = casing ? casing + 1 : unix_name; + casinglen = strlen( 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->caselen = 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 ); } @@ -4585,6 +4632,7 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, free( unix_name ); } + free( casing ); free( redir.Buffer ); } else io->u.Status = STATUS_INVALID_PARAMETER_3;
I'm not sure we want to support a different casing here, I think there are some additional consideration to have on the server side code:
We don't want to create link of some file to another casing of itself, but instead probably rename the link to another casing if it already exists?
I think we should just use the strrchr fallback all the time here.
However, if we want to support proper casing for FileLinkInformation too, it would probably deserve a few tests first to see what this means for links.
And the implementation could then come in a different patch to make it clearer. In which case, assuming we need the same kind of logic to send the proper casing, a separate helper would indeed be useful.
In any case, I think it would also be nice (if possible?) to have the tests first in the series with todo_wine, removed later in the patch that fixes it.
Yeah, I'm doubtful it matters for links, but I'll see about the tests in any case—and with todo_wine should be cleaner I hope.
But honestly, wouldn't it just be easier to send caselen == 0 and consider that as the fallback in the wineserver? If links even turn out to not be affected by this, it would add no change to the link case at all or any complications with freeing (as you can see, I messed up above).
Basically it boils down to: is it really worth complicating the ntdll code so much just to avoid 2 lines of code on the wineserver side? I just feel like it's not adding much value to make it non-optional, but perhaps I'm missing the bigger picture.
So actually, I found out a way to simplify it and also handle links properly without sending caselen == 0, I hope it will be fine I'll send it shortly. If this is still too complicated or has some edge cases I'm out of ideas without complicating the ntdll code too much...
-
Gabriel Ivăncescu -
Marvin -
Rémi Bernon