June 2022 - Wine-Devel - list.winehq.org

[PATCH v2 2/4] httpapi/tests: Add tests for when URLs fail to be added to a queue.

by Julian Klemann

If a queue is associated to a URL group and the URL cannot be added to the queue, the URL should not be set in the group. In other cases, it should be. Signed-off-by: Julian Klemann <jklemann(a)codeweavers.com> --- v2: No change. --- dlls/httpapi/tests/httpapi.c | 118 +++++++++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+) diff --git a/dlls/httpapi/tests/httpapi.c b/dlls/httpapi/tests/httpapi.c index 9d344f26d02..d231fd1d8ed 100644 --- a/dlls/httpapi/tests/httpapi.c +++ b/dlls/httpapi/tests/httpapi.c @@ -1218,6 +1218,122 @@ static void test_HttpCreateUrlGroup(void) ok(!ret, "Unexpected return value %u.\n", ret); } +static void test_v2_bound_port(void) +{ + static const HTTPAPI_VERSION version = {2, 0}; + struct sockaddr_in sockaddr; + HTTP_SERVER_SESSION_ID session; + HTTP_BINDING_INFO binding; + HTTP_URL_GROUP_ID group; + unsigned short port; + WCHAR url[50]; + HANDLE queue, dummy_queue; + int ret; + SOCKET s, s2; + + ret = pHttpCreateServerSession(version, &session, 0); + ok(!ret, "Failed to create session, error %u.\n", ret); + ret = pHttpCreateUrlGroup(session, &group, 0); + ok(!ret, "Failed to create URL group, error %u.\n", ret); + + ret = pHttpCreateRequestQueue(version, NULL, NULL, 0, &queue); + ok(!ret, "Failed to create request queue, error %u.\n", ret); + ret = pHttpCreateRequestQueue(version, NULL, NULL, 0, &dummy_queue); + ok(!ret, "Failed to create request queue, error %u.\n", ret); + binding.Flags.Present = 1; + binding.RequestQueueHandle = queue; + ret = pHttpSetUrlGroupProperty(group, HttpServerBindingProperty, &binding, sizeof(binding)); + ok(!ret, "Failed to bind request queue, error %u.\n", ret); + + s2 = socket(AF_INET, SOCK_STREAM, 0); + sockaddr.sin_family = AF_INET; + sockaddr.sin_addr.S_un.S_addr = inet_addr("127.0.0.1"); + for (port = 50000; port < 51000; ++port) + { + sockaddr.sin_port = htons(port); + ret = bind(s2, (struct sockaddr *)&sockaddr, sizeof(sockaddr)); + if (!ret) + break; + } + ok(!ret, "Failed to bind to port\n"); + swprintf(url, ARRAY_SIZE(url), L"http://localhost:%u/", port); + ret = pHttpAddUrlToUrlGroup(group, url, 0xdeadbeef, 0); + ok(ret == ERROR_SHARING_VIOLATION, "Unexpected failure adding %s, error %u.\n", debugstr_w(url), ret); + shutdown(s2, SD_BOTH); + closesocket(s2); + + binding.RequestQueueHandle = dummy_queue; + ret = pHttpSetUrlGroupProperty(group, HttpServerBindingProperty, &binding, sizeof(binding)); + ok(!ret, "Failed to rebind request queue, error %u.\n", ret); + + s = socket(AF_INET, SOCK_STREAM, 0); + ret = connect(s, (struct sockaddr *)&sockaddr, sizeof(sockaddr)); + todo_wine ok(ret, "Connecting to socket succeeded, %lu.\n", GetLastError()); + todo_wine ok(GetLastError() == WSAECONNREFUSED, "Unexpected error connecting to socket, %lu.\n", GetLastError()); + + closesocket(s); + ret = pHttpCloseRequestQueue(dummy_queue); + ok(!ret, "Failed to close queue handle, error %u.\n", ret); + ret = pHttpCloseRequestQueue(queue); + ok(!ret, "Failed to close queue handle, error %u.\n", ret); + ret = pHttpCloseUrlGroup(group); + ok(!ret, "Failed to close group, error %u.\n", ret); + ret = pHttpCloseServerSession(session); + ok(!ret, "Failed to close group, error %u.\n", ret); +} + +static void test_v2_queue_after_url(void) +{ + char DECLSPEC_ALIGN(8) req_buffer[2048]; + HTTP_REQUEST_V2 *reqv2 = (HTTP_REQUEST_V2 *)req_buffer; + static const HTTPAPI_VERSION version = {2, 0}; + HTTP_REQUEST_V1 *req = &reqv2->s; + HTTP_SERVER_SESSION_ID session; + HTTP_BINDING_INFO binding; + HTTP_URL_GROUP_ID group; + unsigned short port; + char req_text[100]; + HANDLE queue; + int ret; + SOCKET s; + + ret = pHttpCreateServerSession(version, &session, 0); + ok(!ret, "Failed to create session, error %u.\n", ret); + ret = pHttpCreateUrlGroup(session, &group, 0); + ok(!ret, "Failed to create URL group, error %u.\n", ret); + + port = add_url_v2(group); + + ret = pHttpCreateRequestQueue(version, NULL, NULL, 0, &queue); + ok(!ret, "Failed to create request queue, error %u.\n", ret); + binding.Flags.Present = 1; + binding.RequestQueueHandle = queue; + ret = pHttpSetUrlGroupProperty(group, HttpServerBindingProperty, &binding, sizeof(binding)); + ok(!ret, "Failed to bind request queue, error %u.\n", ret); + + s = create_client_socket(port); + + sprintf(req_text, simple_req, port); + ret = send(s, req_text, strlen(req_text), 0); + ok(ret == strlen(req_text), "send() returned %d.\n", ret); + + ret = HttpReceiveHttpRequest(queue, HTTP_NULL_ID, 0, (HTTP_REQUEST *)req, sizeof(req_buffer), NULL, NULL); + ok(!ret, "Got error %u.\n", ret); + + ok(req->BytesReceived == strlen(req_text), "Got %s bytes.\n", wine_dbgstr_longlong(req->BytesReceived)); + + ret = remove_url_v2(group, port); + ok(!ret, "Got error %u.\n", ret); + + closesocket(s); + ret = pHttpCloseRequestQueue(queue); + ok(!ret, "Failed to close queue handle, error %u.\n", ret); + ret = pHttpCloseUrlGroup(group); + ok(!ret, "Failed to close group, error %u.\n", ret); + ret = pHttpCloseServerSession(session); + ok(!ret, "Failed to close group, error %u.\n", ret); +} + static void test_v2_server(void) { char DECLSPEC_ALIGN(8) req_buffer[2048], response_buffer[2048]; @@ -1538,6 +1654,8 @@ START_TEST(httpapi) test_HttpCreateServerSession(); test_HttpCreateUrlGroup(); test_v2_server(); + test_v2_queue_after_url(); + test_v2_bound_port(); test_v2_completion_port(); ret = HttpTerminate(HTTP_INITIALIZE_SERVER, NULL); -- 2.36.1

June 16, 2022

[PATCH v2 1/4] httpapi/tests: Move sleep statement.

by Julian Klemann

The mentioned bug randomly occurred after some subsequent patches, so the sleep should happen before GetOverlappedResult is called. Signed-off-by: Julian Klemann <jklemann(a)codeweavers.com> --- v2: No change. --- dlls/httpapi/tests/httpapi.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dlls/httpapi/tests/httpapi.c b/dlls/httpapi/tests/httpapi.c index be1d31c12ad..9d344f26d02 100644 --- a/dlls/httpapi/tests/httpapi.c +++ b/dlls/httpapi/tests/httpapi.c @@ -228,10 +228,6 @@ static void test_v1_server(void) ret = send(s, req_text, strlen(req_text), 0); ok(ret == strlen(req_text), "send() returned %d.\n", ret); - ret = GetOverlappedResult(queue, &ovl, &ret_size, TRUE); - ok(ret, "Got error %lu.\n", GetLastError()); - ok(ret_size > sizeof(*req), "Got size %lu.\n", ret_size); - /* Various versions of Windows (observed on 64-bit Windows 8 and Windows 10 * version 1507, but probably affecting others) suffer from a bug where the * kernel will report success before completely filling the buffer or @@ -239,6 +235,10 @@ static void test_v1_server(void) * around this. */ Sleep(100); + ret = GetOverlappedResult(queue, &ovl, &ret_size, TRUE); + ok(ret, "Got error %lu.\n", GetLastError()); + ok(ret_size > sizeof(*req), "Got size %lu.\n", ret_size); + ok(!req->Flags, "Got flags %#lx.\n", req->Flags); ok(req->ConnectionId, "Expected nonzero connection ID.\n"); ok(req->RequestId, "Expected nonzero connection ID.\n"); -- 2.36.1

June 16, 2022

Re: [PATCH v2 0/1] MR246: Allow renaming a file/directory to a different case of itself

by Gabriel Ivăncescu

June 16, 2022

Re: Gitlab experiment wrapup

by Alexandre Julliard

Alex Henrie <alexhenrie24(a)gmail.com> writes: > On Thu, Jun 16, 2022 at 10:04 AM Gabriel Ivăncescu > <gabrielopcode(a)gmail.com> wrote: >> >> There's one other (pretty big, for me) problem I can't seem to find how >> to replicate with MRs compared to sending patches: how do I add notes >> for each commit that shouldn't actually be committed? For patches I used >> to add below the --- line, and these are super useful when you just want >> to tell the information to the reviewer, which wouldn't make much sense >> to have in the codebase itself. >> >> These seem to get lost when I push. I have to admit `git notes` seems >> pretty convoluted to me and I've no idea how it gets "stored" especially >> for merge requests. Patches were much easier to comprehend. >> >> I mean, I guess I can add it to the MR description but that's not >> pointing out to a specific commit/patch... sigh. > > In a GitLab merge request, I can click on a specific commit to see its > diff, then click on the commit hash again, and GitLab gives me a box > to leave a comment either about a specific line of the commit or about > the commit as a whole. Would that work for you? Unfortunately comments tied to a specific commit are no longer visible when the branch is rebased, for instance when a reviewer pushes fixups. There's a issue filed with Gitlab about that, but until this is fixed it's better to avoid commit-specific comments. -- Alexandre Julliard julliard(a)winehq.org

June 16, 2022

Re: [PATCH v2 0/1] MR246: Allow renaming a file/directory to a different case of itself

by Huw Davies (＠huw)

Since this has been around for a while and been through several iterations, could you add links to the previous email threads so I can refresh my memory? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/246#note_2201

June 16, 2022

Re: [PATCH v3 0/1] MR256: ntdll: Do not open directory file when setting the same directory path.

by Paul Gofman (＠gofman)

v3: - use wcsnicmp(). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/256#note_2200

June 16, 2022

[PATCH v3 1/1] ntdll: Do not open directory file when setting the same directory path.

by Paul Gofman

From: Paul Gofman <pgofman(a)codeweavers.com> Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/path.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/dlls/ntdll/path.c b/dlls/ntdll/path.c index cccd000a6c6..889d020bd8e 100644 --- a/dlls/ntdll/path.c +++ b/dlls/ntdll/path.c @@ -905,13 +905,13 @@ ULONG WINAPI RtlGetCurrentDirectory_U(ULONG buflen, LPWSTR buf) NTSTATUS WINAPI RtlSetCurrentDirectory_U(const UNICODE_STRING* dir) { FILE_FS_DEVICE_INFORMATION device_info; + ULONG size, compare_size; OBJECT_ATTRIBUTES attr; UNICODE_STRING newdir; IO_STATUS_BLOCK io; CURDIR *curdir; HANDLE handle; NTSTATUS nts; - ULONG size; PWSTR ptr; newdir.Buffer = NULL; @@ -929,6 +929,22 @@ NTSTATUS WINAPI RtlSetCurrentDirectory_U(const UNICODE_STRING* dir) goto out; } + size = newdir.Length / sizeof(WCHAR); + ptr = newdir.Buffer; + ptr += 4; /* skip \??\ prefix */ + size -= 4; + + if (size && ptr[size - 1] == '\\') compare_size = size - 1; + else compare_size = size; + + if (curdir->DosPath.Length == (compare_size + 1) * sizeof(WCHAR) + && !wcsnicmp( curdir->DosPath.Buffer, ptr, compare_size )) + { + TRACE( "dir %s is the same as current.\n", debugstr_us(dir) ); + nts = STATUS_SUCCESS; + goto out; + } + attr.Length = sizeof(attr); attr.RootDirectory = 0; attr.Attributes = OBJ_CASE_INSENSITIVE; @@ -953,10 +969,6 @@ NTSTATUS WINAPI RtlSetCurrentDirectory_U(const UNICODE_STRING* dir) curdir->Handle = handle; /* append trailing \ if missing */ - size = newdir.Length / sizeof(WCHAR); - ptr = newdir.Buffer; - ptr += 4; /* skip \??\ prefix */ - size -= 4; if (size && ptr[size - 1] != '\\') ptr[size++] = '\\'; /* convert \??\UNC\ path to \\ prefix */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/256

June 16, 2022

[PATCH v3 0/1] MR256: ntdll: Do not open directory file when setting the same directory path.

by Paul Gofman (＠gofman)

Fixes The Rising of The Shield Hero: Relieve the Animation hangs on start (and maybe something else with ENIGMA protector). The protector implements virtual file system which hotpatches a lot of file related functions, in particular, NtOpenFile(). When NtOpenFile() is called from RtlSetCurrentDirectory_U() with peb lock held the hotpatcher takes its own lock and that can deadlock with other threads performing file / path operations ending up in get_full_path_helper() which also takes PEB lock. Once the hotpatcher is initialized the game only sets "." or the same full path as current and may do that rather often. It seems that reopening the file handle for the same directory is redundant anyway, so probably makes sense to just avoid that part. -- v3: ntdll: Do not open directory file when setting the same directory path. https://gitlab.winehq.org/wine/wine/-/merge_requests/256

June 16, 2022

[PATCH v2 1/1] ntdll: Do not open directory file when setting the same directory path.

by Paul Gofman

From: Paul Gofman <pgofman(a)codeweavers.com> Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/ntdll/path.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/dlls/ntdll/path.c b/dlls/ntdll/path.c index cccd000a6c6..519263c2c6a 100644 --- a/dlls/ntdll/path.c +++ b/dlls/ntdll/path.c @@ -905,13 +905,13 @@ ULONG WINAPI RtlGetCurrentDirectory_U(ULONG buflen, LPWSTR buf) NTSTATUS WINAPI RtlSetCurrentDirectory_U(const UNICODE_STRING* dir) { FILE_FS_DEVICE_INFORMATION device_info; + ULONG size, compare_size; OBJECT_ATTRIBUTES attr; UNICODE_STRING newdir; IO_STATUS_BLOCK io; CURDIR *curdir; HANDLE handle; NTSTATUS nts; - ULONG size; PWSTR ptr; newdir.Buffer = NULL; @@ -929,6 +929,22 @@ NTSTATUS WINAPI RtlSetCurrentDirectory_U(const UNICODE_STRING* dir) goto out; } + size = newdir.Length / sizeof(WCHAR); + ptr = newdir.Buffer; + ptr += 4; /* skip \??\ prefix */ + size -= 4; + + if (size && ptr[size - 1] == '\\') compare_size = size - 1; + else compare_size = size; + + if (curdir->DosPath.Length == (compare_size + 1) * sizeof(WCHAR) + && !memcmp( curdir->DosPath.Buffer, ptr, compare_size * sizeof(WCHAR) )) + { + TRACE( "dir %s is the same as current.\n", debugstr_us(dir) ); + nts = STATUS_SUCCESS; + goto out; + } + attr.Length = sizeof(attr); attr.RootDirectory = 0; attr.Attributes = OBJ_CASE_INSENSITIVE; @@ -953,10 +969,6 @@ NTSTATUS WINAPI RtlSetCurrentDirectory_U(const UNICODE_STRING* dir) curdir->Handle = handle; /* append trailing \ if missing */ - size = newdir.Length / sizeof(WCHAR); - ptr = newdir.Buffer; - ptr += 4; /* skip \??\ prefix */ - size -= 4; if (size && ptr[size - 1] != '\\') ptr[size++] = '\\'; /* convert \??\UNC\ path to \\ prefix */ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/256

June 16, 2022

[PATCH v2 0/1] MR256: ntdll: Do not open directory file when setting the same directory path.

by Paul Gofman (＠gofman)

Fixes The Rising of The Shield Hero: Relieve the Animation hangs on start (and maybe something else with ENIGMA protector). The protector implements virtual file system which hotpatches a lot of file related functions, in particular, NtOpenFile(). When NtOpenFile() is called from RtlSetCurrentDirectory_U() with peb lock held the hotpatcher takes its own lock and that can deadlock with other threads performing file / path operations ending up in get_full_path_helper() which also takes PEB lock. Once the hotpatcher is initialized the game only sets "." or the same full path as current and may do that rather often. It seems that reopening the file handle for the same directory is redundant anyway, so probably makes sense to just avoid that part. -- v2: ntdll: Do not open directory file when setting the same directory path. https://gitlab.winehq.org/wine/wine/-/merge_requests/256

June 16, 2022