[PATCH 0/3] MR6882: secur32, urlmon, wininet: Fix some failing tests.

[Rémi Bernon (＠rbernon)]

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).

[PATCH 1/3] secur32/tests: Update the tests to expect HTTP/2 headers.

[Rémi Bernon]

From: Rémi Bernon

---
 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);

[PATCH 2/3] urlmon/tests: Expect "Upgrade, Keep-Alive" connection string.

[Rémi Bernon]

From: Rémi Bernon

---
 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);

[PATCH 3/3] wininet: Parse multi-token Connection strings for Keep-Alive.

[Rémi Bernon]

From: Rémi Bernon

---
 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");

[Rémi Bernon (＠rbernon)]

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.

[Jacek Caban (＠jacek)]

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.