[PATCH v10] ntdll: Allow renaming a file/directory to a different case of itself.

[Rémi Bernon]

On 5/20/21 4:57 PM, Gabriel Ivăncescu wrote:
> Hi Rémi,
> 
> On 20/05/2021 14:49, Rémi Bernon wrote:
>> On 4/30/21 3:58 PM, Gabriel Ivăncescu wrote:
>>> +/*********************************************************************** 
>>>
>>> + *           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, *p;
>>> +    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 */
>>> +    for (p = *name_ret; *p == '/'; p++) { }
>>> +    if (!*p)
>>> +        strcpy( actual_case, "/" );
>>> +    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;
>>> +}
>>> +
>>> +
>>
>> I feel like this is more complicated than it needs to be and that it 
>> will still miss some cases. For instance, the strrchr is incorrect if 
>> name_ret has trailing slashes.
>>
>> Since 405666b736f7e471e301f051cfbe68bcbef7e0f6 there's checks in 
>> nt_to_unix_file_name to prevent names to have more than one trailing 
>> slash. However, when the unix name shortcut matches, it may still 
>> return it with a trailing slash, while when it does it component by 
>> component, it won't.
>>
>> So in my opinion, to get the case of the last path component in a way 
>> that is consistent with the matched unix path, it all should be done 
>> in lookup_unix_name instead, as in the attached patch (although I did 
>> it quickly and I'm not sure it's completely correct).
>>
>> The patch doesn't try to normalize the returned paths (stripping the 
>> last / when the shortcut is taken), and maybe it would be better to do 
>> so (for the part below).
>>
> 
> Thanks, on retrospect, that does look a lot better. Although it seems 
> slightly off in some corner cases (when the shortcut's ret == 0):
> 
> 1) For unix root, e.g.: \\?\unix\
> 2) For a DOS device path with a backslash, e.g.: \\?\C:\
> 
> In those cases the full path maps to the underlying path with an actual 
> component (/ for root), while the file_case is an empty string.
> 
> The simplest fix, IMO, is to simply treat an empty file_case as "the 
> same case" as I had originally. It's literally just a couple extra 
> checks on the server and avoids a lot of such problems. Such situations 
> would simply avoid the case thing altogether.
> 
> Without that, we'd have to special case the unix root again (treat it as 
> filename) but also the other root DOS device paths by walking backwards 
> or something. Really not worth the trouble.
> 
> It also avoids the string copy when the case fails, removing an extra 
> line of code in ntdll. :-)
> 
I don't know, I feel like having file_case sometimes set and sometimes 
not, confusing. Especially if it's sometimes set but with the same case 
for some reason. And checking if the case it the same to discard the 
result feels also a bit ad-hoc.

Regarding the helper itself, as it takes a case_ret parameter, I would 
say it's expected to fill it when name_ret it, with the last component 
original case (and an eventual trailing slash I guess, if name_ret has).

Also note that I added a new helper because I was lazy and didn't want 
to change everywhere nt_to_unix_file_name is used, but maybe it's not 
worth a new function.
-- 
Rémi Bernon <rbernon at codeweavers.com>