[PATCH 0/3] MR6882: secur32, urlmon, wininet: Fix some failing tests.
Probably caused by some winehq.org HTTP server update, it now supports HTTP/2 and sends "Upgrade, Keep-Alive" connection strings (was "Keep-Alive" before). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6882
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/secur32/tests/schannel.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/secur32/tests/schannel.c b/dlls/secur32/tests/schannel.c index eecbc41415e..85f0f0a8560 100644 --- a/dlls/secur32/tests/schannel.c +++ b/dlls/secur32/tests/schannel.c @@ -1592,8 +1592,8 @@ static void test_application_protocol_negotiation(void) { ok(protocol.ProtoNegoStatus == SecApplicationProtocolNegotiationStatus_Success, "got %u\n", protocol.ProtoNegoStatus); ok(protocol.ProtoNegoExt == SecApplicationProtocolNegotiationExt_ALPN, "got %u\n", protocol.ProtoNegoExt); - ok(protocol.ProtocolIdSize == 8, "got %u\n", protocol.ProtocolIdSize); - ok(!memcmp(protocol.ProtocolId, "http/1.1", 8), "wrong protocol id\n"); + ok(protocol.ProtocolIdSize == 2, "got %u\n", protocol.ProtocolIdSize); + ok(!memcmp(protocol.ProtocolId, "h2", 2), "wrong protocol id\n"); } DeleteSecurityContext(&context); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6882
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/urlmon/tests/protocol.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/urlmon/tests/protocol.c b/dlls/urlmon/tests/protocol.c index 01e564b2cf7..2affa2ef12a 100644 --- a/dlls/urlmon/tests/protocol.c +++ b/dlls/urlmon/tests/protocol.c @@ -981,7 +981,7 @@ static void test_http_info(IInternetProtocol *protocol) if(tested_protocol != FTP_TEST) { ok(hres == S_OK, "QueryInfo failed: %08lx\n", hres); - ok(!strcmp(buf, "Keep-Alive"), "buf = %s\n", buf); + ok(!strcmp(buf, "Keep-Alive") || !strcmp(buf, "Upgrade, Keep-Alive"), "buf = %s\n", buf); len = strlen(buf); ok(size == len, "size = %lu, expected %lu\n", size, len); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6882
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/wininet/http.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/dlls/wininet/http.c b/dlls/wininet/http.c index 92417714fac..7eed106b17c 100644 --- a/dlls/wininet/http.c +++ b/dlls/wininet/http.c @@ -1915,6 +1915,21 @@ static void http_release_netconn(http_request_t *req, BOOL reuse) INTERNET_STATUS_CONNECTION_CLOSED, 0, 0); } +static BOOL has_keep_alive(const WCHAR *str) +{ + const WCHAR *next; + + for (; *str; str = next + wcsspn(next, L" ,")) + { + next = str + wcscspn(str, L" ,"); + if (next - str != 10) continue; + if (wcsnicmp(str, L"Keep-Alive", 10)) continue; + return TRUE; + } + + return FALSE; +} + static BOOL HTTP_KeepAlive(http_request_t *request) { WCHAR szVersion[10]; @@ -1934,7 +1949,7 @@ static BOOL HTTP_KeepAlive(http_request_t *request) if (HTTP_HttpQueryInfoW(request, HTTP_QUERY_PROXY_CONNECTION, szConnectionResponse, &dwBufferSize, NULL) == ERROR_SUCCESS || HTTP_HttpQueryInfoW(request, HTTP_QUERY_CONNECTION, szConnectionResponse, &dwBufferSize, NULL) == ERROR_SUCCESS) { - keepalive = !wcsicmp(szConnectionResponse, L"Keep-Alive"); + keepalive = has_keep_alive(szConnectionResponse); } return keepalive; @@ -4862,9 +4877,9 @@ static void http_process_keep_alive(http_request_t *req) EnterCriticalSection( &req->headers_section ); if ((index = HTTP_GetCustomHeaderIndex(req, L"Connection", 0, FALSE)) != -1) - req->netconn->keep_alive = !wcsicmp(req->custHeaders[index].lpszValue, L"Keep-Alive"); + req->netconn->keep_alive = has_keep_alive(req->custHeaders[index].lpszValue); else if ((index = HTTP_GetCustomHeaderIndex(req, L"Proxy-Connection", 0, FALSE)) != -1) - req->netconn->keep_alive = !wcsicmp(req->custHeaders[index].lpszValue, L"Keep-Alive"); + req->netconn->keep_alive = has_keep_alive(req->custHeaders[index].lpszValue); else req->netconn->keep_alive = !wcsicmp(req->version, L"HTTP/1.1"); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6882
Not sure about the windows failures but as https://test.winehq.org/data/patterns.html#urlmon:protocol shows timeout failures on Gitlab runners too I'd say it's unrelated to the changes here. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6882#note_88491
Jacek Caban (@jacek) commented about dlls/wininet/http.c:
if (HTTP_HttpQueryInfoW(request, HTTP_QUERY_PROXY_CONNECTION, szConnectionResponse, &dwBufferSize, NULL) == ERROR_SUCCESS || HTTP_HttpQueryInfoW(request, HTTP_QUERY_CONNECTION, szConnectionResponse, &dwBufferSize, NULL) == ERROR_SUCCESS) { - keepalive = !wcsicmp(szConnectionResponse, L"Keep-Alive"); + keepalive = has_keep_alive(szConnectionResponse);
This doesn't seem right (although I'm not sure without tests, which would be nice to have). HTTP/1.1 should default to keep-alive, which it would not do for "connection: upgrade" with this implementation. Previous implementation assumed that header's value is either "close" or "keep-alive". If we're allowing other values, I guess the logic should be something like `is_http11 ? !have_value("close") : have_value("keep-alive")`. `http_process_keep_alive` has the same problem. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6882#note_88496
participants (2)
-
Jacek Caban (＠jacek) -
Rémi Bernon