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. :-)
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,
Yeah, that's actually really clever, I like it!