[PATCH v2 0/3] MR1070: Add more tests for \Device\NamedPipe, \Device\NamedPipe\, and exotic pipe names
-- v2: ntdll/tests: Add more tests for \Device\NamedPipe and \Device\NamedPipe\. ntdll/tests: Add tests for exotic pipe names. kernel32/tests: Add test for pipe name with a trailing backslash. https://gitlab.winehq.org/wine/wine/-/merge_requests/1070
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- 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"); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1070
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- dlls/ntdll/tests/pipe.c | 137 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 137 insertions(+) diff --git a/dlls/ntdll/tests/pipe.c b/dlls/ntdll/tests/pipe.c index 0ad09daaa82..d85678f1226 100644 --- a/dlls/ntdll/tests/pipe.c +++ b/dlls/ntdll/tests/pipe.c @@ -2658,6 +2658,142 @@ static void test_empty_name(void) CloseHandle(hdirectory); } +struct pipe_name_test { + const WCHAR *name; + NTSTATUS status; + const WCHAR *no_open_name; +}; + +struct pipe_name_test_modifier { + ULONG attributes; +}; + +static void subtest_exotic_pipe_name(PUNICODE_STRING name_prefix, + const struct pipe_name_test_modifier *pntm, + const struct pipe_name_test *pnt) +{ + OBJECT_ATTRIBUTES attr; + LARGE_INTEGER timeout; + IO_STATUS_BLOCK iosb; + HANDLE pipe, client; + UNICODE_STRING name; + WCHAR namebuf[512]; + NTSTATUS status; + + name.Length = 0; + name.MaximumLength = sizeof(namebuf); + name.Buffer = namebuf; + + status = RtlAppendUnicodeStringToString(&name, name_prefix); + ok(status == STATUS_SUCCESS, "Expected success, got %#lx\n", status); + + if (pnt->name) + { + status = RtlAppendUnicodeToString(&name, L"\\"); + ok(status == STATUS_SUCCESS, "Expected success, got %#lx\n", status); + + status = RtlAppendUnicodeToString(&name, pnt->name); + ok(status == STATUS_SUCCESS, "Expected success, got %#lx\n", status); + } + + InitializeObjectAttributes(&attr, &name, pntm->attributes, 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->name || !pnt->name[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; + WCHAR no_open_namebuf[512]; + + no_open_name.Length = 0; + no_open_name.MaximumLength = sizeof(namebuf); + no_open_name.Buffer = no_open_namebuf; + + status = RtlAppendUnicodeStringToString(&no_open_name, name_prefix); + ok(status == STATUS_SUCCESS, "Expected success, got %#lx\n", status); + + status = RtlAppendUnicodeToString(&no_open_name, L"\\"); + ok(status == STATUS_SUCCESS, "Expected success, got %#lx\n", status); + + status = RtlAppendUnicodeToString(&no_open_name, pnt->no_open_name); + ok(status == STATUS_SUCCESS, "Expected success, got %#lx\n", status); + + InitializeObjectAttributes(&no_open_attr, &no_open_name, pntm->attributes, 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_exotic_pipe_names(void) +{ + static const struct pipe_name_test_modifier modifiers[] = { + { OBJ_CASE_INSENSITIVE }, + { OBJ_CASE_INSENSITIVE | OBJ_OPENIF }, + { 0 }, + { OBJ_OPENIF }, + }; + static const struct pipe_name_test tests[] = { + { NULL , STATUS_OBJECT_NAME_INVALID }, + { L"" , STATUS_OBJECT_NAME_INVALID }, + { L"\\" , STATUS_SUCCESS }, + { L"wine-test\\" , STATUS_SUCCESS, L"wine-test" }, + { L"wine/test" , STATUS_SUCCESS, L"wine\\test" }, + { L"wine:test" , STATUS_SUCCESS }, + { L"wine\\.\\test" , STATUS_SUCCESS, L"wine\\test" }, + { L"wine\\..\\test" , STATUS_SUCCESS, L"test" }, + { L"..\\wine-test" , STATUS_SUCCESS }, + { L"!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~", STATUS_SUCCESS }, + }; + UNICODE_STRING name_prefix; + size_t i, j; + + RtlInitUnicodeString(&name_prefix, L"\\Device\\NamedPipe"); + + for (i = 0; i < ARRAY_SIZE(modifiers); i++) + { + const struct pipe_name_test_modifier *pntm = &modifiers[i]; + + for (j = 0; j < ARRAY_SIZE(tests); j++) + { + const struct pipe_name_test *pnt = &tests[j]; + + winetest_push_context("test %Iu/%Iu: %s", i, j, debugstr_w(pnt->name)); + subtest_exotic_pipe_name(&name_prefix, pntm, pnt); + winetest_pop_context(); + } + } +} + START_TEST(pipe) { if (!init_func_ptrs()) @@ -2712,6 +2848,7 @@ START_TEST(pipe) test_file_info(); test_security_info(); test_empty_name(); + test_exotic_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); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1070
From: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- dlls/ntdll/tests/pipe.c | 74 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 73 insertions(+), 1 deletion(-) diff --git a/dlls/ntdll/tests/pipe.c b/dlls/ntdll/tests/pipe.c index d85678f1226..456ec335a23 100644 --- a/dlls/ntdll/tests/pipe.c +++ b/dlls/ntdll/tests/pipe.c @@ -2473,8 +2473,27 @@ 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_TRANSCEIVE, STATUS_ACCESS_DENIED), + 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 +2504,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 +2533,29 @@ 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 && ft->status != STATUS_ACCESS_DENIED) + 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); + + status = NtWriteFile(hdirectory, event, NULL, NULL, &io, buffer, sizeof(buffer), NULL, NULL); + ok(status == STATUS_ACCESS_DENIED, "NtWriteFile on \\Device\\NamedPipe: got %#lx\n", status); + name.Buffer = NULL; name.Length = 0; name.MaximumLength = 0; @@ -2548,6 +2596,29 @@ 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 && ft->status != STATUS_ACCESS_DENIED) + 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); + + status = NtWriteFile(hdirectory, event, NULL, NULL, &io, buffer, sizeof(buffer), NULL, NULL); + ok(status == STATUS_ACCESS_DENIED, "NtWriteFile on \\Device\\NamedPipe\\: got %#lx\n", status); + name.Buffer = NULL; name.Length = 0; name.MaximumLength = 0; @@ -2656,6 +2727,7 @@ static void test_empty_name(void) CloseHandle(handle); CloseHandle(hpipe); CloseHandle(hdirectory); + CloseHandle(event); } struct pipe_name_test { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1070
Zebediah Figura (@zfigura) commented about dlls/ntdll/tests/pipe.c:
+ "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_exotic_pipe_names(void) +{ + static const struct pipe_name_test_modifier modifiers[] = { + { OBJ_CASE_INSENSITIVE }, + { OBJ_CASE_INSENSITIVE | OBJ_OPENIF }, + { 0 }, + { OBJ_OPENIF }, + }; Why test attributes? The behaviour doesn't vary, and it's not obvious to me why OBJ_OPENIF or OBJ_CASE_INSENSITIVE would even potentially change behaviour either.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1070#note_10909
Zebediah Figura (@zfigura) commented about dlls/ntdll/tests/pipe.c:
test_file_info(); test_security_info(); test_empty_name(); + test_exotic_pipe_names();
Not to bikeshed naming too much, but this doesn't seem to be about exotic names as much as testing whether there's any path resolution (there isn't). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1070#note_10911
Zebediah Figura (@zfigura) commented about dlls/ntdll/tests/pipe.c:
+ static const struct pipe_name_test tests[] = { + { NULL , STATUS_OBJECT_NAME_INVALID }, + { L"" , STATUS_OBJECT_NAME_INVALID }, + { L"\\" , STATUS_SUCCESS }, + { L"wine-test\\" , STATUS_SUCCESS, L"wine-test" }, + { L"wine/test" , STATUS_SUCCESS, L"wine\\test" }, + { L"wine:test" , STATUS_SUCCESS }, + { L"wine\\.\\test" , STATUS_SUCCESS, L"wine\\test" }, + { L"wine\\..\\test" , STATUS_SUCCESS, L"test" }, + { L"..\\wine-test" , STATUS_SUCCESS }, + { L"!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~", STATUS_SUCCESS }, + }; + UNICODE_STRING name_prefix; + size_t i, j; + + RtlInitUnicodeString(&name_prefix, L"\\Device\\NamedPipe"); It seems like it would be simpler and more readable just to put this in the test structure itself.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1070#note_10910
Zebediah Figura (@zfigura) commented about dlls/ntdll/tests/pipe.c:
+ { + WaitForSingleObject(event, INFINITE); + status = io.Status; + } + todo_wine_if(ft->status != STATUS_NOT_SUPPORTED && ft->status != STATUS_ACCESS_DENIED) + 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); + + status = NtWriteFile(hdirectory, event, NULL, NULL, &io, buffer, sizeof(buffer), NULL, NULL); + ok(status == STATUS_ACCESS_DENIED, "NtWriteFile on \\Device\\NamedPipe: got %#lx\n", status); This is not exactly interesting, because we're opening the directory without write access. The same applies to some of the ioctls above.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1070#note_10912
On Mon Oct 17 00:07:18 2022 +0000, Zebediah Figura wrote:
It seems like it would be simpler and more readable just to put this in the test structure itself. It was intended for future addition of tests with \Device\NamedPipe\ as root directory. well, it shouldn't matter though...
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1070#note_10964
participants (3)
-
Jinoh Kang -
Jinoh Kang (＠iamahuman)
-
Zebediah Figura (＠zfigura)