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

[Rémi Bernon]

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).

> diff --git a/server/fd.c b/server/fd.c
> index 481e9a8..d73ea56 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, *p;
>   
>       if (!fd->inode || !fd->unix_name)
>       {
> @@ -2526,6 +2529,29 @@ static void set_fd_name( struct fd *fd, struct fd *root, const char *nameptr, da
>           name = combined_name;
>       }
>   
> +    if (!(p = strrchr( name, '/' ))) p = name;
> +    else if (!*(++p))
> +    {
> +        /* get the last slash that's not trailing, but treat '/' root as a filename */
> +        while (p > name && p[-1] == '/') p--;
> +        p[p <= name] = 0;
> +        while (p > name && p[-1] != '/') p--;
> +    }
> +    pathlen = p - name;
> +    filenamelen = strlen( p );
> +    different_case = (filenamelen != caselen || memcmp( p, file_case, caselen ));
> +

Here I'm not completely sure. Looking for the last path component could 
be made simpler with something like that:

     p = name; pathlen = 0;
     while (*p) if (*p++ == '/' && *p) pathlen = p - name;
     filenamelen = p - name - pathlen;
     p = name + pathlen;

But then I don't know what we can assume, and whether we can trust 
client input to have both name and file_case be consistent together.

Cheers,
-- 
Rémi Bernon <rbernon at codeweavers.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ntdll-Introduce-new-nt_to_unix_file_name_with_case-h.patch
Type: text/x-patch
Size: 7733 bytes
Desc: not available
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20210520/06283878/attachment.bin>