[PATCH resend 1/2] ntdll: Allow renaming a file/directory to a different capitalization of itself.
Renaming a file or directory from e.g. foobar to FooBar (or any other caps-only change) should work and capitalize it, like on Windows, instead of being a no-op.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46203 Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/ntdll/unix/file.c | 48 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-)
diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index afb552b..639ea6c 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -4326,9 +4326,10 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, if (len >= sizeof(FILE_RENAME_INFORMATION)) { FILE_RENAME_INFORMATION *info = ptr; + char *unix_name, *src_unix; UNICODE_STRING name_str; OBJECT_ATTRIBUTES attr; - char *unix_name; + size_t unix_name_len;
name_str.Buffer = info->FileName; name_str.Length = info->FileNameLength; @@ -4343,13 +4344,56 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, if (io->u.Status != STATUS_SUCCESS && io->u.Status != STATUS_NO_SUCH_FILE) break;
+ unix_name_len = strlen(unix_name); + + /* When it's renaming to the same file, preserve the case sensitivity of the last + component, so that renaming a\b\c\foobar to a\b\c\Foobar works correctly. */ + if (io->u.Status != STATUS_NO_SUCH_FILE && !server_get_unix_name(handle, &src_unix)) + { + char *name = realpath(unix_name, NULL); + + if (name && !strcmp(name, src_unix)) + { + size_t nt_filename_len, pathlen; + const WCHAR *nt_filename; + char *tmp; + INT maxsz; + + for (pathlen = name_str.Length / sizeof(WCHAR); pathlen; pathlen--) + if (name_str.Buffer[pathlen - 1] == '\') + break; + + nt_filename = name_str.Buffer + pathlen; + nt_filename_len = name_str.Length / sizeof(WCHAR) - pathlen; + + /* Build it from path + filename (the latter converted from nt_filename) */ + for (pathlen = unix_name_len; pathlen; pathlen--) + if (unix_name[pathlen - 1] == '/') + break; + + tmp = unix_name; + maxsz = pathlen + nt_filename_len * 3 + 1; + if (unix_name_len + 1 < maxsz) tmp = realloc(tmp, maxsz); + if (tmp) + { + unix_name = tmp; + unix_name_len = pathlen; + unix_name_len += ntdll_wcstoumbs(nt_filename, nt_filename_len, + unix_name + pathlen, maxsz - pathlen, TRUE); + unix_name[unix_name_len] = '\0'; + } + } + free(src_unix); + free(name); + } + SERVER_START_REQ( set_fd_name_info ) { req->handle = wine_server_obj_handle( handle ); req->rootdir = wine_server_obj_handle( attr.RootDirectory ); req->link = FALSE; req->replace = info->ReplaceIfExists; - wine_server_add_data( req, unix_name, strlen(unix_name) ); + wine_server_add_data( req, unix_name, unix_name_len ); io->u.Status = wine_server_call( req ); } SERVER_END_REQ;
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/kernel32/tests/file.c | 40 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)
diff --git a/dlls/kernel32/tests/file.c b/dlls/kernel32/tests/file.c index 9327d03..06d59cb 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,48 @@ 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); + + 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?.*");
On 10/8/20 9:55 AM, Gabriel Ivăncescu wrote:
Renaming a file or directory from e.g. foobar to FooBar (or any other caps-only change) should work and capitalize it, like on Windows, instead of being a no-op.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46203 Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com
dlls/ntdll/unix/file.c | 48 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-)
diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index afb552b..639ea6c 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -4326,9 +4326,10 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, if (len >= sizeof(FILE_RENAME_INFORMATION)) { FILE_RENAME_INFORMATION *info = ptr;
char *unix_name, *src_unix; UNICODE_STRING name_str; OBJECT_ATTRIBUTES attr;
char *unix_name;
size_t unix_name_len; name_str.Buffer = info->FileName; name_str.Length = info->FileNameLength;@@ -4343,13 +4344,56 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, if (io->u.Status != STATUS_SUCCESS && io->u.Status != STATUS_NO_SUCH_FILE) break;
unix_name_len = strlen(unix_name);/* When it's renaming to the same file, preserve the case sensitivity of the lastcomponent, so that renaming a\b\c\foobar to a\b\c\Foobar works correctly. */if (io->u.Status != STATUS_NO_SUCH_FILE && !server_get_unix_name(handle, &src_unix)){char *name = realpath(unix_name, NULL);if (name && !strcmp(name, src_unix)){
Do we even need to make either of these checks?
(What happens if you try to clobber an existing file with different casing?)
size_t nt_filename_len, pathlen;const WCHAR *nt_filename;char *tmp;INT maxsz;for (pathlen = name_str.Length / sizeof(WCHAR); pathlen; pathlen--)if (name_str.Buffer[pathlen - 1] == '\\')break;
wcsrchr()?
nt_filename = name_str.Buffer + pathlen;nt_filename_len = name_str.Length / sizeof(WCHAR) - pathlen;/* Build it from path + filename (the latter converted from nt_filename) */for (pathlen = unix_name_len; pathlen; pathlen--)if (unix_name[pathlen - 1] == '/')break;tmp = unix_name;maxsz = pathlen + nt_filename_len * 3 + 1;if (unix_name_len + 1 < maxsz) tmp = realloc(tmp, maxsz);
This reallocation strategy is very weird.
if (tmp){unix_name = tmp;unix_name_len = pathlen;unix_name_len += ntdll_wcstoumbs(nt_filename, nt_filename_len,unix_name + pathlen, maxsz - pathlen, TRUE);unix_name[unix_name_len] = '\0';}}free(src_unix);free(name);}SERVER_START_REQ( set_fd_name_info ) { req->handle = wine_server_obj_handle( handle ); req->rootdir = wine_server_obj_handle( attr.RootDirectory ); req->link = FALSE; req->replace = info->ReplaceIfExists;
wine_server_add_data( req, unix_name, strlen(unix_name) );
wine_server_add_data( req, unix_name, unix_name_len ); io->u.Status = wine_server_call( req ); } SERVER_END_REQ;
Hi Zeb,
Thanks for the review!
On 25/10/2020 00:36, Zebediah Figura wrote:
On 10/8/20 9:55 AM, Gabriel Ivăncescu wrote:
Renaming a file or directory from e.g. foobar to FooBar (or any other caps-only change) should work and capitalize it, like on Windows, instead of being a no-op.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46203 Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com
dlls/ntdll/unix/file.c | 48 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-)
diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index afb552b..639ea6c 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -4326,9 +4326,10 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, if (len >= sizeof(FILE_RENAME_INFORMATION)) { FILE_RENAME_INFORMATION *info = ptr;
char *unix_name, *src_unix; UNICODE_STRING name_str; OBJECT_ATTRIBUTES attr;
char *unix_name;
size_t unix_name_len; name_str.Buffer = info->FileName; name_str.Length = info->FileNameLength;@@ -4343,13 +4344,56 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, if (io->u.Status != STATUS_SUCCESS && io->u.Status != STATUS_NO_SUCH_FILE) break;
unix_name_len = strlen(unix_name);/* When it's renaming to the same file, preserve the case sensitivity of the lastcomponent, so that renaming a\b\c\foobar to a\b\c\Foobar works correctly. */if (io->u.Status != STATUS_NO_SUCH_FILE && !server_get_unix_name(handle, &src_unix)){char *name = realpath(unix_name, NULL);if (name && !strcmp(name, src_unix)){Do we even need to make either of these checks?
(What happens if you try to clobber an existing file with different casing?)
Interesting. It looks like Windows keeps the casing of the rename operation even when clobbering an existing file.
e.g.: `Abc` is an existing file. Rename `def` into `ABC` and overwrite, we get `ABC` with the contents of `def`, and `Abc` is no more.
This means the patch is incomplete.
However, this also means I'd have to pass both the existing unix filename (as we do now) and, on top of that, the new casing filename (without path) to the wineserver.
What would be the best way to do that? Currently we just send the unix filename path without a NUL terminator.
Would something like the following be acceptable, or do you have a better idea? Perhaps using length-prefix so it's faster to decode?
unix filename path, NUL, actual casing filename
So with the above example, renaming `def` would look like:
C:\Abc<NUL>ABC
Obviously it will require a bit more work to decode this.
size_t nt_filename_len, pathlen;const WCHAR *nt_filename;char *tmp;INT maxsz;for (pathlen = name_str.Length / sizeof(WCHAR); pathlen; pathlen--)if (name_str.Buffer[pathlen - 1] == '\\')break;wcsrchr()?
For some reason, it looks like the string is not NUL terminated. I assumed it was intended and not a bug. I can add a comment, though.
nt_filename = name_str.Buffer + pathlen;nt_filename_len = name_str.Length / sizeof(WCHAR) - pathlen;/* Build it from path + filename (the latter converted from nt_filename) */for (pathlen = unix_name_len; pathlen; pathlen--)if (unix_name[pathlen - 1] == '/')break;tmp = unix_name;maxsz = pathlen + nt_filename_len * 3 + 1;if (unix_name_len + 1 < maxsz) tmp = realloc(tmp, maxsz);This reallocation strategy is very weird.
Basically I'm trying to make room for the "converted" nt_filename into unix. It might take more room when converted to UTF-8 for example, depending on the character.
I noticed in other parts of the code that deals with such conversions, such as nt_to_unix_file_name_attr, that it uses something like:
name_len * 3 + MAX_DIR_ENTRY_LEN + 3;
so I assumed the * 3 is due to wide -> UTF-8 conversion, and used something similar.
That said, it probably won't be needed to reallocate if I go with the new approach to pass it to the wineserver. I'll probably still need the `name_len * 3` thing though, before I copy it, just to have enough room.
Thanks, Gabriel
Ehm, the C:\Abc part is supposed to be the actual unix path, sorry for the noise. But the idea is the same.
On 10/26/20 8:30 AM, Gabriel Ivăncescu wrote:
Hi Zeb,
Thanks for the review!
On 25/10/2020 00:36, Zebediah Figura wrote:
On 10/8/20 9:55 AM, Gabriel Ivăncescu wrote:
Renaming a file or directory from e.g. foobar to FooBar (or any other caps-only change) should work and capitalize it, like on Windows, instead of being a no-op.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46203 Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com
dlls/ntdll/unix/file.c | 48 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-)
diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index afb552b..639ea6c 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -4326,9 +4326,10 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, if (len >= sizeof(FILE_RENAME_INFORMATION)) { FILE_RENAME_INFORMATION *info = ptr;
char *unix_name, *src_unix; UNICODE_STRING name_str; OBJECT_ATTRIBUTES attr;
char *unix_name;
size_t unix_name_len; name_str.Buffer = info->FileName; name_str.Length = info->FileNameLength;@@ -4343,13 +4344,56 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, if (io->u.Status != STATUS_SUCCESS && io->u.Status != STATUS_NO_SUCH_FILE) break;
unix_name_len = strlen(unix_name);/* When it's renaming to the same file, preserve the case sensitivity of the lastcomponent, so that renaming a\b\c\foobar to a\b\c\Foobar works correctly. */if (io->u.Status != STATUS_NO_SUCH_FILE && !server_get_unix_name(handle, &src_unix)){char *name = realpath(unix_name, NULL);if (name && !strcmp(name, src_unix)){Do we even need to make either of these checks?
(What happens if you try to clobber an existing file with different casing?)
Interesting. It looks like Windows keeps the casing of the rename operation even when clobbering an existing file.
e.g.: `Abc` is an existing file. Rename `def` into `ABC` and overwrite, we get `ABC` with the contents of `def`, and `Abc` is no more.
This means the patch is incomplete.
However, this also means I'd have to pass both the existing unix filename (as we do now) and, on top of that, the new casing filename (without path) to the wineserver.
What would be the best way to do that? Currently we just send the unix filename path without a NUL terminator.
Would something like the following be acceptable, or do you have a better idea? Perhaps using length-prefix so it's faster to decode?
unix filename path, NUL, actual casing filename
So with the above example, renaming `def` would look like:
C:\Abc<NUL>ABC
Obviously it will require a bit more work to decode this.
I don't really know, but honestly, I'm inclined to think that without an application known to depend on this, the extra complexity just isn't worth having. That likely goes for the patch in general as well.
size_t nt_filename_len, pathlen;const WCHAR *nt_filename;char *tmp;INT maxsz;for (pathlen = name_str.Length / sizeof(WCHAR); pathlen; pathlen--)if (name_str.Buffer[pathlen - 1] == '\\')break;wcsrchr()?
For some reason, it looks like the string is not NUL terminated. I assumed it was intended and not a bug. I can add a comment, though.
Never mind, you're right; we can't assume the string is NULL terminated.
nt_filename = name_str.Buffer + pathlen;nt_filename_len = name_str.Length / sizeof(WCHAR) - pathlen;/* Build it from path + filename (the latter converted from nt_filename) */for (pathlen = unix_name_len; pathlen; pathlen--)if (unix_name[pathlen - 1] == '/')break;tmp = unix_name;maxsz = pathlen + nt_filename_len * 3 + 1;if (unix_name_len + 1 < maxsz) tmp = realloc(tmp, maxsz);This reallocation strategy is very weird.
Basically I'm trying to make room for the "converted" nt_filename into unix. It might take more room when converted to UTF-8 for example, depending on the character.
I noticed in other parts of the code that deals with such conversions, such as nt_to_unix_file_name_attr, that it uses something like:
name_len * 3 + MAX_DIR_ENTRY_LEN + 3;so I assumed the * 3 is due to wide -> UTF-8 conversion, and used something similar.
That said, it probably won't be needed to reallocate if I go with the new approach to pass it to the wineserver. I'll probably still need the `name_len * 3` thing though, before I copy it, just to have enough room.
Thanks, Gabriel
On 26/10/2020 17:01, Zebediah Figura wrote:
On 10/26/20 8:30 AM, Gabriel Ivăncescu wrote:
Hi Zeb,
Thanks for the review!
On 25/10/2020 00:36, Zebediah Figura wrote:
On 10/8/20 9:55 AM, Gabriel Ivăncescu wrote:
Renaming a file or directory from e.g. foobar to FooBar (or any other caps-only change) should work and capitalize it, like on Windows, instead of being a no-op.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46203 Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com
dlls/ntdll/unix/file.c | 48 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-)
diff --git a/dlls/ntdll/unix/file.c b/dlls/ntdll/unix/file.c index afb552b..639ea6c 100644 --- a/dlls/ntdll/unix/file.c +++ b/dlls/ntdll/unix/file.c @@ -4326,9 +4326,10 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, if (len >= sizeof(FILE_RENAME_INFORMATION)) { FILE_RENAME_INFORMATION *info = ptr;
char *unix_name, *src_unix; UNICODE_STRING name_str; OBJECT_ATTRIBUTES attr;
char *unix_name;
size_t unix_name_len; name_str.Buffer = info->FileName; name_str.Length = info->FileNameLength;@@ -4343,13 +4344,56 @@ NTSTATUS WINAPI NtSetInformationFile( HANDLE handle, IO_STATUS_BLOCK *io, if (io->u.Status != STATUS_SUCCESS && io->u.Status != STATUS_NO_SUCH_FILE) break;
unix_name_len = strlen(unix_name);/* When it's renaming to the same file, preserve the case sensitivity of the lastcomponent, so that renaming a\b\c\foobar to a\b\c\Foobar works correctly. */if (io->u.Status != STATUS_NO_SUCH_FILE && !server_get_unix_name(handle, &src_unix)){char *name = realpath(unix_name, NULL);if (name && !strcmp(name, src_unix)){Do we even need to make either of these checks?
(What happens if you try to clobber an existing file with different casing?)
Interesting. It looks like Windows keeps the casing of the rename operation even when clobbering an existing file.
e.g.: `Abc` is an existing file. Rename `def` into `ABC` and overwrite, we get `ABC` with the contents of `def`, and `Abc` is no more.
This means the patch is incomplete.
However, this also means I'd have to pass both the existing unix filename (as we do now) and, on top of that, the new casing filename (without path) to the wineserver.
What would be the best way to do that? Currently we just send the unix filename path without a NUL terminator.
Would something like the following be acceptable, or do you have a better idea? Perhaps using length-prefix so it's faster to decode?
unix filename path, NUL, actual casing filename
So with the above example, renaming `def` would look like:
C:\Abc<NUL>ABC
Obviously it will require a bit more work to decode this.
I don't really know, but honestly, I'm inclined to think that without an application known to depend on this, the extra complexity just isn't worth having. That likely goes for the patch in general as well.
Well there are applications that "depend" on this, not by crashing or giving a failure error, but simply unable to rename such files.
Most notably, any file managers under Wine won't be able to rename a file to some other casing of itself. So it's more like a missing feature, but IMO it's still a bug that should be solved somehow.
size_t nt_filename_len, pathlen;const WCHAR *nt_filename;char *tmp;INT maxsz;for (pathlen = name_str.Length / sizeof(WCHAR); pathlen; pathlen--)if (name_str.Buffer[pathlen - 1] == '\\')break;wcsrchr()?
For some reason, it looks like the string is not NUL terminated. I assumed it was intended and not a bug. I can add a comment, though.
Never mind, you're right; we can't assume the string is NULL terminated.
nt_filename = name_str.Buffer + pathlen;nt_filename_len = name_str.Length / sizeof(WCHAR) - pathlen;/* Build it from path + filename (the latter converted from nt_filename) */for (pathlen = unix_name_len; pathlen; pathlen--)if (unix_name[pathlen - 1] == '/')break;tmp = unix_name;maxsz = pathlen + nt_filename_len * 3 + 1;if (unix_name_len + 1 < maxsz) tmp = realloc(tmp, maxsz);This reallocation strategy is very weird.
Basically I'm trying to make room for the "converted" nt_filename into unix. It might take more room when converted to UTF-8 for example, depending on the character.
I noticed in other parts of the code that deals with such conversions, such as nt_to_unix_file_name_attr, that it uses something like:
name_len * 3 + MAX_DIR_ENTRY_LEN + 3;so I assumed the * 3 is due to wide -> UTF-8 conversion, and used something similar.
That said, it probably won't be needed to reallocate if I go with the new approach to pass it to the wineserver. I'll probably still need the `name_len * 3` thing though, before I copy it, just to have enough room.
Thanks, Gabriel
-
Gabriel Ivăncescu -
Zebediah Figura