[PATCH v2 0/4] MR6759: kernelbase: Make trailing slashes in CreateFileW set ERROR_PATH_NOT_FOUND

[Fabian Maurer (＠DarkShadow44)]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57376

--
  v2: kernelbase: Make trailing slashes in CreateFileW set ERROR_PATH_NOT_FOUND
      kernel32/tests: Add another CreateFile test
      kernel32/tests: Only skip volume tests when access is denied
      kernel32/tests: Only skip volume tests on windows
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6759

[PATCH v2 1/4] kernel32/tests: Only skip volume tests on windows

[Fabian Maurer]

From: Fabian Maurer <dark.shadow4(a)web.de>

Otherwise todo tests that already succeed will be skipped,
we should always have access to volumes on Wine.
---
 dlls/kernel32/tests/file.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/dlls/kernel32/tests/file.c b/dlls/kernel32/tests/file.c
index c57edebb316..587d81f3a46 100644
--- a/dlls/kernel32/tests/file.c
+++ b/dlls/kernel32/tests/file.c
@@ -1287,7 +1287,7 @@ static void test_CreateFileA(void)
         {"removeme\\", ERROR_PATH_NOT_FOUND, -1, FILE_ATTRIBUTE_NORMAL, TRUE }, /* exst dir w \ */
         {"c:", ERROR_ACCESS_DENIED, ERROR_PATH_NOT_FOUND, FILE_ATTRIBUTE_NORMAL, FALSE }, /* device in file namespace */
         {"c:", ERROR_SUCCESS, ERROR_PATH_NOT_FOUND, FILE_FLAG_BACKUP_SEMANTICS, FALSE }, /* device in file namespace as dir */
-        {"c:\\", ERROR_PATH_NOT_FOUND, ERROR_ACCESS_DENIED, FILE_ATTRIBUTE_NORMAL, TRUE }, /* root dir w \ */
+        {"c:\\", ERROR_PATH_NOT_FOUND, ERROR_ACCESS_DENIED, FILE_ATTRIBUTE_NORMAL, FALSE }, /* root dir w \ */
         {"c:\\", ERROR_SUCCESS, ERROR_ACCESS_DENIED, FILE_FLAG_BACKUP_SEMANTICS, FALSE }, /* root dir w \ as dir */
         {"c:c:\\windows", ERROR_INVALID_NAME, -1, FILE_ATTRIBUTE_NORMAL, TRUE }, /* invalid path */
         {"\\\\?\\c:", ERROR_SUCCESS, ERROR_BAD_NETPATH, FILE_ATTRIBUTE_NORMAL,FALSE }, /* dev namespace drive */
@@ -1390,14 +1390,12 @@ static void test_CreateFileA(void)
         /* if we get ACCESS_DENIED when we do not expect it, assume
          * no access to the volume
          */
-        if (hFile == INVALID_HANDLE_VALUE &&
+        if (!strcmp(winetest_platform, "windows") &&
+            hFile == INVALID_HANDLE_VALUE &&
             GetLastError() == ERROR_ACCESS_DENIED &&
             p[i].err != ERROR_ACCESS_DENIED)
         {
-            if (p[i].todo_flag)
-                skip("Either no authority to volume, or is todo_wine for %s err=%ld should be %ld\n", filename, GetLastError(), p[i].err);
-            else
-                skip("Do not have authority to access volumes. Test for %s skipped\n", filename);
+            skip("Do not have authority to access volumes. Test for %s skipped\n", filename);
         }
         /* otherwise validate results with expectations */
         else
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/6759

[PATCH v2 2/4] kernel32/tests: Only skip volume tests when access is denied

[Fabian Maurer]

From: Fabian Maurer <dark.shadow4(a)web.de>

This prevents a mistaken skip in the next commit
---
 dlls/kernel32/tests/file.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/dlls/kernel32/tests/file.c b/dlls/kernel32/tests/file.c
index 587d81f3a46..51261cc1cf6 100644
--- a/dlls/kernel32/tests/file.c
+++ b/dlls/kernel32/tests/file.c
@@ -79,6 +79,7 @@ struct test_list {
     const LONG err2;            /* Win 9x & ME error code  or -1 */
     const DWORD options;        /* option flag to use for open */
     const BOOL todo_flag;       /* todo_wine indicator */
+    const BOOL needs_volume_access; /* If true, could fail without admin rights */
 } ;
 
 static void InitFunctionPointers(void)
@@ -1285,13 +1286,13 @@ static void test_CreateFileA(void)
         {"a\\", ERROR_FILE_NOT_FOUND, ERROR_PATH_NOT_FOUND, FILE_ATTRIBUTE_NORMAL, FALSE }, /* non-exist dir */
         {"removeme", ERROR_ACCESS_DENIED, -1, FILE_ATTRIBUTE_NORMAL, FALSE }, /* exist dir w/o \ */
         {"removeme\\", ERROR_PATH_NOT_FOUND, -1, FILE_ATTRIBUTE_NORMAL, TRUE }, /* exst dir w \ */
-        {"c:", ERROR_ACCESS_DENIED, ERROR_PATH_NOT_FOUND, FILE_ATTRIBUTE_NORMAL, FALSE }, /* device in file namespace */
-        {"c:", ERROR_SUCCESS, ERROR_PATH_NOT_FOUND, FILE_FLAG_BACKUP_SEMANTICS, FALSE }, /* device in file namespace as dir */
-        {"c:\\", ERROR_PATH_NOT_FOUND, ERROR_ACCESS_DENIED, FILE_ATTRIBUTE_NORMAL, FALSE }, /* root dir w \ */
-        {"c:\\", ERROR_SUCCESS, ERROR_ACCESS_DENIED, FILE_FLAG_BACKUP_SEMANTICS, FALSE }, /* root dir w \ as dir */
+        {"c:", ERROR_ACCESS_DENIED, ERROR_PATH_NOT_FOUND, FILE_ATTRIBUTE_NORMAL, FALSE, TRUE }, /* device in file namespace */
+        {"c:", ERROR_SUCCESS, ERROR_PATH_NOT_FOUND, FILE_FLAG_BACKUP_SEMANTICS, FALSE, TRUE }, /* device in file namespace as dir */
+        {"c:\\", ERROR_PATH_NOT_FOUND, ERROR_ACCESS_DENIED, FILE_ATTRIBUTE_NORMAL, FALSE, TRUE }, /* root dir w \ */
+        {"c:\\", ERROR_SUCCESS, ERROR_ACCESS_DENIED, FILE_FLAG_BACKUP_SEMANTICS, FALSE, TRUE }, /* root dir w \ as dir */
         {"c:c:\\windows", ERROR_INVALID_NAME, -1, FILE_ATTRIBUTE_NORMAL, TRUE }, /* invalid path */
-        {"\\\\?\\c:", ERROR_SUCCESS, ERROR_BAD_NETPATH, FILE_ATTRIBUTE_NORMAL,FALSE }, /* dev namespace drive */
-        {"\\\\?\\c:\\", ERROR_PATH_NOT_FOUND, ERROR_BAD_NETPATH, FILE_ATTRIBUTE_NORMAL, TRUE }, /* dev namespace drive w \ */
+        {"\\\\?\\c:", ERROR_SUCCESS, ERROR_BAD_NETPATH, FILE_ATTRIBUTE_NORMAL, FALSE, TRUE }, /* dev namespace drive */
+        {"\\\\?\\c:\\", ERROR_PATH_NOT_FOUND, ERROR_BAD_NETPATH, FILE_ATTRIBUTE_NORMAL, TRUE, TRUE }, /* dev namespace drive w \ */
         {NULL, 0, -1, 0, FALSE}
     };
     BY_HANDLE_FILE_INFORMATION  Finfo;
@@ -1390,7 +1391,7 @@ static void test_CreateFileA(void)
         /* if we get ACCESS_DENIED when we do not expect it, assume
          * no access to the volume
          */
-        if (!strcmp(winetest_platform, "windows") &&
+        if (!strcmp(winetest_platform, "windows") && p[i].needs_volume_access &&
             hFile == INVALID_HANDLE_VALUE &&
             GetLastError() == ERROR_ACCESS_DENIED &&
             p[i].err != ERROR_ACCESS_DENIED)
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/6759

[PATCH v2 3/4] kernel32/tests: Add another CreateFile test

[Fabian Maurer]

From: Fabian Maurer <dark.shadow4(a)web.de>

---
 dlls/kernel32/tests/file.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/dlls/kernel32/tests/file.c b/dlls/kernel32/tests/file.c
index 51261cc1cf6..c94976158c1 100644
--- a/dlls/kernel32/tests/file.c
+++ b/dlls/kernel32/tests/file.c
@@ -1285,7 +1285,8 @@ static void test_CreateFileA(void)
         {"a", ERROR_FILE_NOT_FOUND, -1, FILE_ATTRIBUTE_NORMAL, FALSE }, /* non-exist file */
         {"a\\", ERROR_FILE_NOT_FOUND, ERROR_PATH_NOT_FOUND, FILE_ATTRIBUTE_NORMAL, FALSE }, /* non-exist dir */
         {"removeme", ERROR_ACCESS_DENIED, -1, FILE_ATTRIBUTE_NORMAL, FALSE }, /* exist dir w/o \ */
-        {"removeme\\", ERROR_PATH_NOT_FOUND, -1, FILE_ATTRIBUTE_NORMAL, TRUE }, /* exst dir w \ */
+        {"removeme\\", ERROR_PATH_NOT_FOUND, -1, FILE_ATTRIBUTE_NORMAL, TRUE }, /* exist dir w \ */
+        {"removeme/", ERROR_ACCESS_DENIED, -1, FILE_ATTRIBUTE_NORMAL, FALSE }, /* exist dir w / */
         {"c:", ERROR_ACCESS_DENIED, ERROR_PATH_NOT_FOUND, FILE_ATTRIBUTE_NORMAL, FALSE, TRUE }, /* device in file namespace */
         {"c:", ERROR_SUCCESS, ERROR_PATH_NOT_FOUND, FILE_FLAG_BACKUP_SEMANTICS, FALSE, TRUE }, /* device in file namespace as dir */
         {"c:\\", ERROR_PATH_NOT_FOUND, ERROR_ACCESS_DENIED, FILE_ATTRIBUTE_NORMAL, FALSE, TRUE }, /* root dir w \ */
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/6759

[PATCH v2 4/4] kernelbase: Make trailing slashes in CreateFileW set ERROR_PATH_NOT_FOUND

[Fabian Maurer]

From: Fabian Maurer <dark.shadow4(a)web.de>

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57376
---
 dlls/kernel32/tests/file.c | 7 +++----
 dlls/kernelbase/file.c     | 5 +++++
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/dlls/kernel32/tests/file.c b/dlls/kernel32/tests/file.c
index c94976158c1..84e6c21d872 100644
--- a/dlls/kernel32/tests/file.c
+++ b/dlls/kernel32/tests/file.c
@@ -1280,12 +1280,12 @@ static void test_CreateFileA(void)
     DWORD i, ret, len;
     static const struct test_list p[] =
     {
-        {"", ERROR_PATH_NOT_FOUND, -1, FILE_ATTRIBUTE_NORMAL, TRUE }, /* dir as file w \ */
+        {"", ERROR_PATH_NOT_FOUND, -1, FILE_ATTRIBUTE_NORMAL, FALSE }, /* dir as file w \ */
         {"", ERROR_SUCCESS, ERROR_PATH_NOT_FOUND, FILE_FLAG_BACKUP_SEMANTICS, FALSE }, /* dir as dir w \ */
         {"a", ERROR_FILE_NOT_FOUND, -1, FILE_ATTRIBUTE_NORMAL, FALSE }, /* non-exist file */
         {"a\\", ERROR_FILE_NOT_FOUND, ERROR_PATH_NOT_FOUND, FILE_ATTRIBUTE_NORMAL, FALSE }, /* non-exist dir */
         {"removeme", ERROR_ACCESS_DENIED, -1, FILE_ATTRIBUTE_NORMAL, FALSE }, /* exist dir w/o \ */
-        {"removeme\\", ERROR_PATH_NOT_FOUND, -1, FILE_ATTRIBUTE_NORMAL, TRUE }, /* exist dir w \ */
+        {"removeme\\", ERROR_PATH_NOT_FOUND, -1, FILE_ATTRIBUTE_NORMAL, FALSE }, /* exist dir w \ */
         {"removeme/", ERROR_ACCESS_DENIED, -1, FILE_ATTRIBUTE_NORMAL, FALSE }, /* exist dir w / */
         {"c:", ERROR_ACCESS_DENIED, ERROR_PATH_NOT_FOUND, FILE_ATTRIBUTE_NORMAL, FALSE, TRUE }, /* device in file namespace */
         {"c:", ERROR_SUCCESS, ERROR_PATH_NOT_FOUND, FILE_FLAG_BACKUP_SEMANTICS, FALSE, TRUE }, /* device in file namespace as dir */
@@ -1293,7 +1293,7 @@ static void test_CreateFileA(void)
         {"c:\\", ERROR_SUCCESS, ERROR_ACCESS_DENIED, FILE_FLAG_BACKUP_SEMANTICS, FALSE, TRUE }, /* root dir w \ as dir */
         {"c:c:\\windows", ERROR_INVALID_NAME, -1, FILE_ATTRIBUTE_NORMAL, TRUE }, /* invalid path */
         {"\\\\?\\c:", ERROR_SUCCESS, ERROR_BAD_NETPATH, FILE_ATTRIBUTE_NORMAL, FALSE, TRUE }, /* dev namespace drive */
-        {"\\\\?\\c:\\", ERROR_PATH_NOT_FOUND, ERROR_BAD_NETPATH, FILE_ATTRIBUTE_NORMAL, TRUE, TRUE }, /* dev namespace drive w \ */
+        {"\\\\?\\c:\\", ERROR_PATH_NOT_FOUND, ERROR_BAD_NETPATH, FILE_ATTRIBUTE_NORMAL, FALSE, TRUE }, /* dev namespace drive w \ */
         {NULL, 0, -1, 0, FALSE}
     };
     BY_HANDLE_FILE_INFORMATION  Finfo;
@@ -1487,7 +1487,6 @@ static void test_CreateFileA(void)
                         FILE_SHARE_READ | FILE_SHARE_WRITE,
                         NULL, OPEN_EXISTING,
                         FILE_ATTRIBUTE_NORMAL | FILE_FLAG_NO_BUFFERING, NULL );
-        todo_wine
         ok(hFile == INVALID_HANDLE_VALUE && GetLastError() == ERROR_PATH_NOT_FOUND,
             "CreateFileA should have returned ERROR_PATH_NOT_FOUND on %s, but got %lu\n",
             filename, GetLastError());
diff --git a/dlls/kernelbase/file.c b/dlls/kernelbase/file.c
index 249f476eb7e..7f81208ce48 100644
--- a/dlls/kernelbase/file.c
+++ b/dlls/kernelbase/file.c
@@ -883,6 +883,11 @@ HANDLE WINAPI DECLSPEC_HOTPATCH CreateFileW( LPCWSTR filename, DWORD access, DWO
          */
         if (status == STATUS_OBJECT_NAME_COLLISION)
             SetLastError( ERROR_FILE_EXISTS );
+        else if (status == STATUS_FILE_IS_A_DIRECTORY)
+        {
+            WCHAR last = filename[wcslen(filename) - 1];
+            SetLastError(last == '\\' ? ERROR_PATH_NOT_FOUND : ERROR_ACCESS_DENIED);
+        }
         else
             SetLastError( RtlNtStatusToDosError(status) );
     }
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/6759

[PATCH v2 4/4] kernelbase: Make trailing slashes in CreateFileW set ERROR_PATH_NOT_FOUND

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=149437

Your paranoid android.


=== debian11b (64 bit WoW report) ===

kernel32:
profile.c:1013: Test failed: Expected ERROR_ACCESS_DENIED, got 3
time.c:329: Test failed: got -396, expected 123

winmm:
mci: Timeout

[Fabian Maurer (＠DarkShadow44)]

On Sat Nov  2 11:18:47 2024 +0000, Jinoh Kang wrote:
> I believe this causes regression for some drivers that handle
> `IRP_MJ_CREATE` and may return `STATUS_FILE_IS_A_DIRECTORY` from there.
> `NtCreateFile()` is ***not*** just a wrapper for `open()`. For custom
> devices like `\\.\MyCustomDriver`, it delegates to the third-party
> driver. The third-party driver might return `STATUS_FILE_IS_A_DIRECTORY`
> *but* still expect it to be translated to `ERROR_ACCESS_DENIED` even
> with the trailing slash.
> I suspect the real culprit is that `NtCreateFile()` is incorrectly
> translating Unix `EISDIR` to `STATUS_FILE_IS_A_DIRECTORY`
> *unconditionally*. In this case, the bug needs to be fixed in ntdll, not kernelbase.
> I created a testcase here: https://bugs.winehq.org/attachment.cgi?id=77355

On windows, NtCreateFile returns `STATUS_FILE_IS_A_DIRECTORY` in both cases, same as Wine.

Not 100% sure how to best approach that driver issue, maybe check if the path starts with `\\??\C:`?

Also, AFAIK we don't need to skip tests on wine, and on windows I got a false positive that I missed, so I tried improving the tests a bit.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6759#note_86472

[Jinoh Kang (＠iamahuman)]

On Sun Nov  3 15:40:18 2024 +0000, Fabian Maurer wrote:
> I created a testcase here: https://bugs.winehq.org/attachment.cgi?id=77355
> On windows, NtCreateFile returns `STATUS_FILE_IS_A_DIRECTORY` in both
> cases, same as Wine.
> Not 100% sure how to best approach that driver issue, maybe check if the
> path starts with `\\??\C:`?
> Also, AFAIK we don't need to skip tests on wine, and on windows I got a
> false positive that I missed, so I tried improving the tests a bit.
> Thanks. https://testbot.winehq.org/JobDetails.pl?Key=149442 seems to suggest that the status postprocessing is happening in kernelbase, not in ntdll.

Probably some compatibility logic for old DOS file API oddity.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6759#note_86474

[Fabian Maurer (＠DarkShadow44)]

Any news on this?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6759#note_88987

[Alexander Pickering (＠Apickx)]

Original reporter, just pinging to see where this is at.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6759#note_90401

[Fabian Maurer (＠DarkShadow44)]

Any news on this?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6759#note_99339

[PATCH v2 0/4] MR6759: kernelbase: Make trailing slashes in CreateFileW set ERROR_PATH_NOT_FOUND - closed

[Fabian Maurer (＠DarkShadow44)]

This merge request was closed by Fabian Maurer.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6759

[Fabian Maurer (＠DarkShadow44)]

Superseded by https://gitlab.winehq.org/wine/wine/-/commit/0fcc187de4117901e1c7c45b17e5f10...

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6759#note_119938