[PATCH resend 1/2] ntdll: Allow renaming a file/directory to a different capitalization of itself.

[Zebediah Figura]

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 at 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))
>>> +                {
>>
>> 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
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20201026/a61f3765/attachment.sig>