[PATCH v3 0/3] MR1070: Add more tests for \Device\NamedPipe, \Device\NamedPipe\, and exotic pipe names

[Jinoh Kang (＠iamahuman)]

--
  v3: ntdll/tests: Add more tests for \Device\NamedPipe and \Device\NamedPipe.
      ntdll/tests: Add tests for pipe names.

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

[PATCH v3 1/3] kernel32/tests: Add test for pipe name with a trailing backslash.

[Jinoh Kang]

From: Jinoh Kang

---
 dlls/kernel32/tests/pipe.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/dlls/kernel32/tests/pipe.c b/dlls/kernel32/tests/pipe.c
index 7fa3c313d47..6a75e5cc62d 100644
--- a/dlls/kernel32/tests/pipe.c
+++ b/dlls/kernel32/tests/pipe.c
@@ -681,6 +681,17 @@ static void test_CreateNamedPipe(int pipemode)
     CloseHandle(hFile);
     CloseHandle(hnp);

+    hnp = CreateNamedPipeA("\\.\pipe\trailingslash\", PIPE_ACCESS_DUPLEX,
+            PIPE_TYPE_BYTE, 1, 1024, 1024, NMPWAIT_USE_DEFAULT_WAIT, NULL);
+    ok(hnp != INVALID_HANDLE_VALUE, "failed to create pipe, error %lu\n", GetLastError());
+    hFile = CreateFileA("\\.\pipe\trailingslash", 0, 0, NULL, OPEN_EXISTING, 0, 0);
+    ok(hFile == INVALID_HANDLE_VALUE, "expected opening pipe to fail\n");
+    ok(GetLastError() == ERROR_FILE_NOT_FOUND, "expected ERROR_FILE_NOT_FOUND, got %lu\n", GetLastError());
+    hFile = CreateFileA("\\.\pipe\trailingslash\", 0, 0, NULL, OPEN_EXISTING, 0, 0);
+    ok(hFile != INVALID_HANDLE_VALUE, "failed to open pipe, error %lu\n", GetLastError());
+    CloseHandle(hFile);
+    CloseHandle(hnp);
+
     if (winetest_debug > 1) trace("test_CreateNamedPipe returning\n");
 }

[PATCH v3 2/3] ntdll/tests: Add tests for pipe names.

[Jinoh Kang]

From: Jinoh Kang

---
 dlls/ntdll/tests/pipe.c | 93 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 93 insertions(+)

diff --git a/dlls/ntdll/tests/pipe.c b/dlls/ntdll/tests/pipe.c
index 0ad09daaa82..5fe273a2c94 100644
--- a/dlls/ntdll/tests/pipe.c
+++ b/dlls/ntdll/tests/pipe.c
@@ -2658,6 +2658,98 @@ static void test_empty_name(void)
     CloseHandle(hdirectory);
 }

+enum {
+    TestTodoWine = 1,
+};
+
+struct pipe_name_test {
+    const WCHAR *name;
+    NTSTATUS status;
+    int flags;
+    const WCHAR *no_open_name;
+};
+
+static void subtest_pipe_name(const struct pipe_name_test *pnt)
+{
+    OBJECT_ATTRIBUTES attr;
+    LARGE_INTEGER timeout;
+    IO_STATUS_BLOCK iosb;
+    HANDLE pipe, client;
+    UNICODE_STRING name;
+    NTSTATUS status;
+
+    pRtlInitUnicodeString(&name, pnt->name);
+    InitializeObjectAttributes(&attr, &name, OBJ_CASE_INSENSITIVE, NULL, NULL);
+    timeout.QuadPart = -100000000;
+    pipe = NULL;
+    status = pNtCreateNamedPipeFile(&pipe,
+                                    GENERIC_READ | FILE_WRITE_ATTRIBUTES | SYNCHRONIZE,
+                                    &attr, &iosb, FILE_SHARE_READ | FILE_SHARE_WRITE,
+                                    FILE_CREATE, FILE_SYNCHRONOUS_IO_NONALERT,
+                                    0, 0, 0, 3, 4096, 4096, &timeout);
+    todo_wine_if((pnt->flags & TestTodoWine) != 0)
+    ok(status == pnt->status, "Expected status %#lx, got %#lx\n", pnt->status, status);
+
+    if (!NT_SUCCESS(status))
+    {
+        ok(pipe == NULL, "expected NULL handle, got %p\n", pipe);
+        return;
+    }
+
+    ok(pipe != NULL, "expected non-NULL handle, got %p\n", client);
+
+    client = NULL;
+    status = NtCreateFile(&client, SYNCHRONIZE, &attr, &iosb, NULL, 0,
+                          FILE_SHARE_READ | FILE_SHARE_WRITE, FILE_OPEN, 0, NULL, 0);
+    ok(status == STATUS_SUCCESS, "Expected success, got %#lx\n", status);
+    ok(client != NULL, "expected non-NULL handle, got %p\n", client);
+    NtClose(client);
+
+    if (pnt->no_open_name)
+    {
+        OBJECT_ATTRIBUTES no_open_attr;
+        UNICODE_STRING no_open_name;
+
+        pRtlInitUnicodeString(&no_open_name, pnt->no_open_name);
+        InitializeObjectAttributes(&no_open_attr, &no_open_name, OBJ_CASE_INSENSITIVE, NULL, NULL);
+        client = NULL;
+        status = NtCreateFile(&client, SYNCHRONIZE, &no_open_attr, &iosb, NULL, 0,
+                              FILE_SHARE_READ | FILE_SHARE_WRITE, FILE_OPEN, 0, NULL, 0);
+        ok(status == STATUS_OBJECT_NAME_NOT_FOUND,
+           "Expected STATUS_OBJECT_NAME_NOT_FOUND opening %s, got %#lx\n",
+           debugstr_wn(no_open_name.Buffer, no_open_name.Length / sizeof(WCHAR)), status);
+        ok(client == NULL, "expected NULL handle, got %p\n", client);
+    }
+
+    NtClose(pipe);
+}
+
+static void test_pipe_names(void)
+{
+    static const struct pipe_name_test tests[] = {
+        { L"\Device\NamedPipe"                 , STATUS_OBJECT_NAME_INVALID, TestTodoWine },
+        { L"\Device\NamedPipe\"               , STATUS_OBJECT_NAME_INVALID, TestTodoWine },
+        { L"\Device\NamedPipe\\"             , STATUS_SUCCESS },
+        { L"\Device\NamedPipe\wine-test\"    , STATUS_SUCCESS, 0, L"\Device\NamedPipe\wine-test" },
+        { L"\Device\NamedPipe\wine/test"      , STATUS_SUCCESS, 0, L"\Device\NamedPipe\wine\test" },
+        { L"\Device\NamedPipe\wine:test"      , STATUS_SUCCESS },
+        { L"\Device\NamedPipe\wine\.\test"  , STATUS_SUCCESS, 0, L"\Device\NamedPipe\wine\test" },
+        { L"\Device\NamedPipe\wine\..\test" , STATUS_SUCCESS, 0, L"\Device\NamedPipe\test" },
+        { L"\Device\NamedPipe\..\wine-test"  , STATUS_SUCCESS },
+        { L"\Device\NamedPipe\!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~", STATUS_SUCCESS },
+    };
+    size_t i;
+
+    for (i = 0; i < ARRAY_SIZE(tests); i++)
+    {
+        const struct pipe_name_test *pnt = &tests[i];
+
+        winetest_push_context("test %Iu: %s", i, debugstr_w(pnt->name));
+        subtest_pipe_name(pnt);
+        winetest_pop_context();
+    }
+}
+
 START_TEST(pipe)
 {
     if (!init_func_ptrs())
@@ -2712,6 +2804,7 @@ START_TEST(pipe)
     test_file_info();
     test_security_info();
     test_empty_name();
+    test_pipe_names();

pipe_for_each_state(create_pipe_server, connect_pipe, test_pipe_state);
     pipe_for_each_state(create_pipe_server, connect_and_write_pipe, test_pipe_with_data_state);

[PATCH v3 3/3] ntdll/tests: Add more tests for \Device\NamedPipe and \Device\NamedPipe\.

[Jinoh Kang]

From: Jinoh Kang

---
 dlls/ntdll/tests/pipe.c | 67 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 66 insertions(+), 1 deletion(-)

diff --git a/dlls/ntdll/tests/pipe.c b/dlls/ntdll/tests/pipe.c
index 5fe273a2c94..3669d1d93c2 100644
--- a/dlls/ntdll/tests/pipe.c
+++ b/dlls/ntdll/tests/pipe.c
@@ -2473,8 +2473,26 @@ static void test_security_info(void)

static void test_empty_name(void)
 {
+    static const struct fsctl_test {
+        const char *name;
+        ULONG code;
+        NTSTATUS status;
+        NTSTATUS status_broken;
+    } fsctl_tests[] = {
+#define FSCTL_TEST(code, ...) { #code, code, __VA_ARGS__ }
+        FSCTL_TEST(FSCTL_PIPE_ASSIGN_EVENT,             STATUS_NOT_SUPPORTED),
+        FSCTL_TEST(FSCTL_PIPE_DISCONNECT,               STATUS_PIPE_DISCONNECTED),
+        FSCTL_TEST(FSCTL_PIPE_LISTEN,                   STATUS_ILLEGAL_FUNCTION),
+        FSCTL_TEST(FSCTL_PIPE_PEEK,                     STATUS_INVALID_PARAMETER),
+        FSCTL_TEST(FSCTL_PIPE_QUERY_EVENT,              STATUS_NOT_SUPPORTED),
+        FSCTL_TEST(FSCTL_PIPE_IMPERSONATE,              STATUS_ILLEGAL_FUNCTION),
+        FSCTL_TEST(FSCTL_PIPE_SET_CLIENT_PROCESS,       STATUS_NOT_SUPPORTED),
+        FSCTL_TEST(FSCTL_PIPE_QUERY_CLIENT_PROCESS,     STATUS_INVALID_PARAMETER, /* win10 1507 */ STATUS_PIPE_DISCONNECTED),
+        FSCTL_TEST(FSCTL_PIPE_GET_CONNECTION_ATTRIBUTE, STATUS_INVALID_PARAMETER),
+#undef FSCTL_TEST
+    };
     static const LARGE_INTEGER zero_timeout = {{ 0 }};
-    HANDLE hdirectory, hpipe, hpipe2, hwrite, hwrite2, handle;
+    HANDLE hdirectory, hpipe, hpipe2, hwrite, hwrite2, handle, event;
     OBJECT_TYPE_INFORMATION *type_info;
     OBJECT_NAME_INFORMATION *name_info;
     OBJECT_ATTRIBUTES attr;
@@ -2485,12 +2503,18 @@ static void test_empty_name(void)
     char buffer[1024];
     NTSTATUS status;
     BOOL ret;
+    size_t i;

type_info = (OBJECT_TYPE_INFORMATION *)buffer;
     name_info = (OBJECT_NAME_INFORMATION *)buffer;

hpipe = hwrite = NULL;

+    InitializeObjectAttributes(&attr, NULL, 0, 0, NULL);
+    event = NULL;
+    status = NtCreateEvent(&event, GENERIC_ALL, &attr, NotificationEvent, FALSE);
+    ok(status == STATUS_SUCCESS, "NtCreateEvent returned %#lx\n", status);
+
     attr.Length                   = sizeof(attr);
     attr.Attributes               = OBJ_CASE_INSENSITIVE;
     attr.SecurityDescriptor       = NULL;
@@ -2508,6 +2532,26 @@ static void test_empty_name(void)
     status = wait_pipe(hdirectory, &name, &zero_timeout);
     todo_wine ok(status == STATUS_ILLEGAL_FUNCTION, "unexpected status for FSCTL_PIPE_WAIT on \Device\NamedPipe: %#lx\n", status);

+    for (i = 0; i < ARRAY_SIZE(fsctl_tests); i++)
+    {
+        const struct fsctl_test *ft = &fsctl_tests[i];
+
+        status = NtFsControlFile(hdirectory, event, NULL, NULL, &io, ft->code, 0, 0, 0, 0);
+        if (status == STATUS_PENDING)
+        {
+            WaitForSingleObject(event, INFINITE);
+            status = io.Status;
+        }
+        todo_wine_if(ft->status != STATUS_NOT_SUPPORTED)
+        ok(status == ft->status || (ft->status_broken && broken(status == ft->status_broken)),
+           "NtFsControlFile(%s) on \Device\NamedPipe: expected %#lx, got %#lx\n",
+           ft->name, ft->status, status);
+    }
+
+    status = NtReadFile(hdirectory, event, NULL, NULL, &io, buffer, sizeof(buffer), NULL, NULL);
+    todo_wine
+    ok(status == STATUS_INVALID_PARAMETER, "NtReadFile on \Device\NamedPipe: got %#lx\n", status);
+
     name.Buffer = NULL;
     name.Length = 0;
     name.MaximumLength = 0;
@@ -2548,6 +2592,26 @@ static void test_empty_name(void)
     status = wait_pipe(hdirectory, &name, &zero_timeout);
     ok(status == STATUS_OBJECT_NAME_NOT_FOUND, "unexpected status for FSCTL_PIPE_WAIT on \Device\NamedPipe\: %#lx\n", status);

+    for (i = 0; i < ARRAY_SIZE(fsctl_tests); i++)
+    {
+        const struct fsctl_test *ft = &fsctl_tests[i];
+
+        status = NtFsControlFile(hdirectory, event, NULL, NULL, &io, ft->code, 0, 0, 0, 0);
+        if (status == STATUS_PENDING)
+        {
+            WaitForSingleObject(event, INFINITE);
+            status = io.Status;
+        }
+        todo_wine_if(ft->status != STATUS_NOT_SUPPORTED)
+        ok(status == ft->status || (ft->status_broken && broken(status == ft->status_broken)),
+           "NtFsControlFile(%s) on \Device\NamedPipe\: expected %#lx, got %#lx\n",
+           ft->name, ft->status, status);
+    }
+
+    status = NtReadFile(hdirectory, event, NULL, NULL, &io, buffer, sizeof(buffer), NULL, NULL);
+    todo_wine
+    ok(status == STATUS_INVALID_PARAMETER, "NtReadFile on \Device\NamedPipe: got %#lx\n", status);
+
     name.Buffer = NULL;
     name.Length = 0;
     name.MaximumLength = 0;
@@ -2656,6 +2720,7 @@ static void test_empty_name(void)
     CloseHandle(handle);
     CloseHandle(hpipe);
     CloseHandle(hdirectory);
+    CloseHandle(event);
 }

enum {

[Zebediah Figura (＠zfigura)]

Zebediah Figura (@zfigura) commented about dlls/ntdll/tests/pipe.c:
>  CloseHandle(hdirectory);
>
> }
>
> +enum {
> - TestTodoWine = 1,
>
> +};
> -
>
> +struct pipe_name_test {
> - const WCHAR *name;
> - NTSTATUS status;
> - int flags;
> - const WCHAR *no_open_name;
>
> +};

No need for this flags enum; just use a bool variable.

(Also, usual Wine style has braces on a new line.)

[Zebediah Figura (＠zfigura)]

On Mon Oct 17 17:07:58 2022 +0000, Jinoh Kang wrote:
> changed this line in [version 3 of the diff](/wine/wine/-/merge_requests/1070/diffs?diff_id=14138&start_sha=61f0943aae67210e1346b6155c2cde52751fbc10#80905fa57573d6165fc3fb5ada289086a8eef97a_2557_2555)

I was thinking along the lines of adding write access to the handle, not removing the tests.

The ioctl tests are also uninteresting because most of them pass invalid parameters. I forgot to mention that in review last time.

[Jinoh Kang (＠iamahuman)]

On Thu Oct 20 18:22:32 2022 +0000, Zebediah Figura wrote:
> No need for this flags enum; just use a bool variable.
> (Also, usual Wine style has braces on a new line.)

Well, I thought named flags enhance the readability.