[PATCH 0/3] MR5474: winhttp: Receive response timeout and connection caching.
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/winhttp/request.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c index 9fe8e1c4782..1b6cb1e2a45 100644 --- a/dlls/winhttp/request.c +++ b/dlls/winhttp/request.c @@ -43,6 +43,14 @@ WINE_DEFAULT_DEBUG_CHANNEL(winhttp); #define DEFAULT_KEEP_ALIVE_TIMEOUT 30000 +#define ACTUAL_DEFAULT_RECEIVE_RESPONSE_TIMEOUT 21000 + +static int request_receive_response_timeout( struct request *req ) +{ + if (req->receive_response_timeout == -1) return ACTUAL_DEFAULT_RECEIVE_RESPONSE_TIMEOUT; + return req->receive_response_timeout; +} + static const WCHAR *attribute_table[] = { L"Mime-Version", /* WINHTTP_QUERY_MIME_VERSION = 0 */ @@ -1658,7 +1666,7 @@ static DWORD open_connection( struct request *request ) return ret; } netconn_set_timeout( netconn, TRUE, request->send_timeout ); - netconn_set_timeout( netconn, FALSE, request->receive_response_timeout ); + netconn_set_timeout( netconn, FALSE, request_receive_response_timeout( request )); request->netconn = netconn; @@ -1696,7 +1704,7 @@ static DWORD open_connection( struct request *request ) TRACE("using connection %p\n", netconn); netconn_set_timeout( netconn, TRUE, request->send_timeout ); - netconn_set_timeout( netconn, FALSE, request->receive_response_timeout ); + netconn_set_timeout( netconn, FALSE, request_receive_response_timeout( request )); request->netconn = netconn; } @@ -2329,7 +2337,7 @@ static DWORD send_request( struct request *request, const WCHAR *headers, DWORD if (!chunked && content_length <= optional_len) { - netconn_set_timeout( request->netconn, FALSE, request->receive_response_timeout ); + netconn_set_timeout( request->netconn, FALSE, request_receive_response_timeout( request )); request->read_reply_status = read_reply( request ); if (request->state == REQUEST_RESPONSE_STATE_READ_RESPONSE_QUEUED) request->state = REQUEST_RESPONSE_STATE_READ_RESPONSE_QUEUED_REPLY_RECEIVED; @@ -2932,7 +2940,7 @@ static DWORD receive_response( struct request *request ) } /* fallthrough */ case REQUEST_RESPONSE_STATE_READ_RESPONSE_QUEUED_REQUEST_SENT: - netconn_set_timeout( request->netconn, FALSE, request->receive_response_timeout ); + netconn_set_timeout( request->netconn, FALSE, request_receive_response_timeout( request )); request->read_reply_status = read_reply( request ); request->state = REQUEST_RESPONSE_STATE_REPLY_RECEIVED; break; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5474
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/winhttp/tests/winhttp.c | 114 +++++++++++++++++++++++++++++++---- 1 file changed, 102 insertions(+), 12 deletions(-) diff --git a/dlls/winhttp/tests/winhttp.c b/dlls/winhttp/tests/winhttp.c index 1724612202a..5d0ccd11f52 100644 --- a/dlls/winhttp/tests/winhttp.c +++ b/dlls/winhttp/tests/winhttp.c @@ -2197,6 +2197,12 @@ static const char okmsg[] = "Server: winetest\r\n" "\r\n"; +static const char okmsg_length0[] = +"HTTP/1.1 200 OK\r\n" +"Server: winetest\r\n" +"Content-length: 0\r\n" +"\r\n"; + static const char notokmsg[] = "HTTP/1.1 400 Bad Request\r\n" "\r\n"; @@ -2324,6 +2330,24 @@ static void create_websocket_accept(const char *key, char *buf, unsigned int buf CryptBinaryToStringA( (BYTE *)sha1, sizeof(sha1), CRYPT_STRING_BASE64, buf, &len); } +static int server_receive_request(int c, char *buffer, size_t buffer_size) +{ + int i, r; + + memset(buffer, 0, buffer_size); + for(i = 0; i < buffer_size - 1; i++) + { + r = recv(c, &buffer[i], 1, 0); + if (r != 1) + break; + if (i < 4) continue; + if (buffer[i - 2] == '\n' && buffer[i] == '\n' && + buffer[i - 3] == '\r' && buffer[i - 1] == '\r') + break; + } + return r; +} + static DWORD CALLBACK server_thread(LPVOID param) { struct server_info *si = param; @@ -2357,18 +2381,7 @@ static DWORD CALLBACK server_thread(LPVOID param) do { if (c == -1) c = accept(s, NULL, NULL); - - memset(buffer, 0, sizeof buffer); - for(i = 0; i < sizeof buffer - 1; i++) - { - r = recv(c, &buffer[i], 1, 0); - if (r != 1) - break; - if (i < 4) continue; - if (buffer[i - 2] == '\n' && buffer[i] == '\n' && - buffer[i - 3] == '\r' && buffer[i - 1] == '\r') - break; - } + server_receive_request(c, buffer, sizeof(buffer)); if (strstr(buffer, "GET /basic")) { send(c, okmsg, sizeof okmsg - 1, 0); @@ -2569,6 +2582,23 @@ static DWORD CALLBACK server_thread(LPVOID param) } send(c, okmsg, sizeof(okmsg) - 1, 0); } + + if (strstr(buffer, "GET /cached")) + { + send(c, okmsg_length0, sizeof okmsg_length0 - 1, 0); + r = server_receive_request(c, buffer, sizeof(buffer)); + ok(r > 0, "got %d.\n", r); + ok(!!strstr(buffer, "GET /cached"), "request not found.\n"); + send(c, okmsg_length0, sizeof okmsg_length0 - 1, 0); + r = server_receive_request(c, buffer, sizeof(buffer)); + todo_wine ok(!r, "got %d, buffer[0] %d.\n", r, buffer[0]); + } + if (strstr(buffer, "GET /notcached")) + { + send(c, okmsg, sizeof okmsg - 1, 0); + r = server_receive_request(c, buffer, sizeof(buffer)); + ok(!r, "got %d, buffer[0] %d.\n", r, buffer[0] ); + } shutdown(c, 2); closesocket(c); c = -1; @@ -5884,6 +5914,65 @@ static void test_client_cert_authentication(void) WinHttpCloseHandle( ses ); } +static void test_connection_cache(int port) +{ + HINTERNET ses, con, req; + DWORD status, size; + char buffer[256]; + BOOL ret; + + ses = WinHttpOpen(L"winetest", WINHTTP_ACCESS_TYPE_NO_PROXY, NULL, NULL, 0); + ok(ses != NULL, "failed to open session %lu\n", GetLastError()); + + con = WinHttpConnect(ses, L"localhost", port, 0); + ok(con != NULL, "failed to open a connection %lu\n", GetLastError()); + + req = WinHttpOpenRequest(con, L"GET", L"/cached", NULL, NULL, NULL, 0); + ok(req != NULL, "failed to open a request %lu\n", GetLastError()); + ret = WinHttpSendRequest(req, NULL, 0, NULL, 0, 0, 0); + ok(ret, "failed to send request %lu\n", GetLastError()); + ret = WinHttpReceiveResponse(req, NULL); + ok(ret, "failed to receive response %lu\n", GetLastError()); + size = sizeof(status); + ret = WinHttpQueryHeaders(req, WINHTTP_QUERY_STATUS_CODE|WINHTTP_QUERY_FLAG_NUMBER, NULL, &status, &size, NULL); + ok(ret, "failed to query status code %lu\n", GetLastError()); + ok(status == HTTP_STATUS_OK, "request failed unexpectedly %lu\n", status); + ret = WinHttpReadData(req, buffer, sizeof buffer, &size); + ok(ret, "failed to read data %lu\n", GetLastError()); + ok(!size, "got size %lu.\n", size); + WinHttpCloseHandle(req); + + req = WinHttpOpenRequest(con, L"GET", L"/cached", NULL, NULL, NULL, 0); + ok(req != NULL, "failed to open a request %lu\n", GetLastError()); + ret = WinHttpSendRequest(req, L"Connection: close", ~0ul, NULL, 0, 0, 0); + ok(ret, "failed to send request %lu\n", GetLastError()); + ret = WinHttpReceiveResponse(req, NULL); + ok(ret, "failed to receive response %lu\n", GetLastError()); + size = sizeof(status); + ret = WinHttpQueryHeaders(req, WINHTTP_QUERY_STATUS_CODE|WINHTTP_QUERY_FLAG_NUMBER, NULL, &status, &size, NULL); + ok(ret, "failed to query status code %lu\n", GetLastError()); + ok(status == HTTP_STATUS_OK, "request failed unexpectedly %lu\n", status); + ret = WinHttpReadData(req, buffer, sizeof buffer, &size); + ok(ret, "failed to read data %lu\n", GetLastError()); + ok(!size, "got size %lu.\n", size); + WinHttpCloseHandle(req); + + req = WinHttpOpenRequest(con, L"GET", L"/notcached", NULL, NULL, NULL, 0); + ok(req != NULL, "failed to open a request %lu\n", GetLastError()); + ret = WinHttpSendRequest(req, L"Connection: close", ~0ul, NULL, 0, 0, 0); + ok(ret, "failed to send request %lu\n", GetLastError()); + ret = WinHttpReceiveResponse(req, NULL); + ok(ret, "failed to receive response %lu\n", GetLastError()); + size = sizeof(status); + ret = WinHttpQueryHeaders(req, WINHTTP_QUERY_STATUS_CODE|WINHTTP_QUERY_FLAG_NUMBER, NULL, &status, &size, NULL); + ok(ret, "failed to query status code %lu\n", GetLastError()); + ok(status == HTTP_STATUS_OK, "request failed unexpectedly %lu\n", status); + WinHttpCloseHandle(req); + + WinHttpCloseHandle(con); + WinHttpCloseHandle(ses); +} + START_TEST (winhttp) { struct server_info si; @@ -5951,6 +6040,7 @@ START_TEST (winhttp) test_websocket(si.port); test_redirect(si.port); test_WinHttpGetProxyForUrl(si.port); + test_connection_cache(si.port); /* send the basic request again to shutdown the server thread */ test_basic_request(si.port, NULL, L"/quit"); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5474
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/winhttp/request.c | 20 +++++++++----------- dlls/winhttp/tests/winhttp.c | 2 +- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c index 1b6cb1e2a45..207f1ec5c61 100644 --- a/dlls/winhttp/request.c +++ b/dlls/winhttp/request.c @@ -1922,7 +1922,7 @@ static DWORD refill_buffer( struct request *request, BOOL notify ) static void finished_reading( struct request *request ) { - BOOL close = FALSE, notify; + BOOL close = FALSE, close_request_headers; WCHAR connection[20]; DWORD size = sizeof(connection); @@ -1937,18 +1937,16 @@ static void finished_reading( struct request *request ) } else if (!wcscmp( request->version, L"HTTP/1.0" )) close = TRUE; - if (close) + size = sizeof(connection); + close_request_headers = + (!query_headers( request, WINHTTP_QUERY_CONNECTION | WINHTTP_QUERY_FLAG_REQUEST_HEADERS, NULL, connection, &size, NULL ) + || !query_headers( request, WINHTTP_QUERY_PROXY_CONNECTION | WINHTTP_QUERY_FLAG_REQUEST_HEADERS, NULL, connection, &size, NULL )) + && !wcsicmp( connection, L"close" ); + if (close || close_request_headers) { - size = sizeof(connection); - notify = (!query_headers( request, WINHTTP_QUERY_CONNECTION | WINHTTP_QUERY_FLAG_REQUEST_HEADERS, - NULL, connection, &size, NULL ) - || !query_headers( request, WINHTTP_QUERY_PROXY_CONNECTION | WINHTTP_QUERY_FLAG_REQUEST_HEADERS, - NULL, connection, &size, NULL )) - && !wcsicmp( connection, L"close" ); - - if (notify) send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_CLOSING_CONNECTION, 0, 0 ); + if (close_request_headers) send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_CLOSING_CONNECTION, 0, 0 ); netconn_release( request->netconn ); - if (notify) send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_CONNECTION_CLOSED, 0, 0 ); + if (close_request_headers) send_callback( &request->hdr, WINHTTP_CALLBACK_STATUS_CONNECTION_CLOSED, 0, 0 ); } else cache_connection( request->netconn ); diff --git a/dlls/winhttp/tests/winhttp.c b/dlls/winhttp/tests/winhttp.c index 5d0ccd11f52..ee01fd5ec72 100644 --- a/dlls/winhttp/tests/winhttp.c +++ b/dlls/winhttp/tests/winhttp.c @@ -2591,7 +2591,7 @@ static DWORD CALLBACK server_thread(LPVOID param) ok(!!strstr(buffer, "GET /cached"), "request not found.\n"); send(c, okmsg_length0, sizeof okmsg_length0 - 1, 0); r = server_receive_request(c, buffer, sizeof(buffer)); - todo_wine ok(!r, "got %d, buffer[0] %d.\n", r, buffer[0]); + ok(!r, "got %d, buffer[0] %d.\n", r, buffer[0]); } if (strstr(buffer, "GET /notcached")) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5474
Fixes Shatterline hanging while loading in game (into the second match). The game sets 'Connection: close' in additional headers in WinHttpSendRequest(). Server reply does not have any Connection: header and Wine ends up with cached connection. The game does two similar POST requests to the same server one after another. For some reason the second request never gets a reply if reusing the connection. Then, the game never sets WINHTTP_OPTION_RECEIVE_RESPONSE_TIMEOUT and we end up with default '-1' value, so request waits forever for reply on an open connection which never arrives. As far as my testing goes, both parts are erroneous. Fixing any of those fix the game but it probably makes sense to fix both. WRT response timeout, MS docs suggest that default WINHTTP_OPTION_RECEIVE_RESPONSE_TIMEOUT is 90sec [1]. The existing test which query default timeout get -1, that's probably why we've got such a default. However, my ad-hoc testing of the actual timeout using a trivial Python webserver from [2] shows that the actual timeout is different: ``` # Python 3 server example from http.server import BaseHTTPRequestHandler, HTTPServer import time hostName = "0.0.0.0" serverPort = 8080 class MyServer(BaseHTTPRequestHandler): def do_GET(self): time.sleep(100) self.send_response(200) self.send_header("Content-type", "text/html") self.end_headers() self.wfile.write(bytes("<html><head><title>https://pythonbasics.org</title></head>", "utf-8")) self.wfile.write(bytes("<p>Request: %s</p>" % self.path, "utf-8")) self.wfile.write(bytes("<body>", "utf-8")) self.wfile.write(bytes("<p>This is an example web server.</p>", "utf-8")) self.wfile.write(bytes("</body></html>", "utf-8")) if __name__ == "__main__": webServer = HTTPServer((hostName, serverPort), MyServer) print("Server started http://%s:%s" % (hostName, serverPort)) try: webServer.serve_forever() except KeyboardInterrupt: pass webServer.server_close() print("Server stopped.") ``` I added sleep(100) before sending response and made an ad-hoc test (attache) which shows that the actual timeout is 21sec, at least on Windows 10. The test is apparently too long to run to be included. WRT closing connection, I added a test which explicitly checks if the connection is closed. 1. https://learn.microsoft.com/en-us/windows/win32/winhttp/option-flags 2. https://pythonbasics.org/webserver/ [test.patch](/uploads/f0a7cce2961a668716ce4db78461442c/test.patch) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5474#note_67315
This merge request was approved by Hans Leidekker. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5474
participants (3)
-
Hans Leidekker (＠hans) -
Paul Gofman
-
Paul Gofman (＠gofman)