On 20/05/2021 18:39, Rémi Bernon wrote:
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):
- For unix root, e.g.: \?\unix\
- 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.
Yeah, it still fills it right now. The only time it doesn't, is in weird cases when it maps to a device name (or the root / dir), or when the allocation fails, which makes sense to me as it uses it as fallback. Note that there's not really any special casing done for this in ntdll, it's implicit based on the code already. Just the server needs to check for it.
It's more along the lines of "don't apply the case check at all" rather than "this has a case but I don't want to send it" thing, if that makes sense.
For the leak: good point, I'll see if a buffer is better, or can just free the buffer if it's easier.