[PATCH 0/2] MR6541: wininet: Only include non-empty session user agent string in request headers.
The WordSmith application downloads an update information file with wininet, but it fails to receive the correct data because the remote HTTP server returns a 503 error page, as it does not tolerate receiving a User-Agent header with a blank value in the request. Tests show that when a blank user agent string is passed to InternetOpen, a request created from the resulting session handle should not include a User-Agent header at all. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56875 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6541
From: Andrew Nguyen <arethusa26(a)gmail.com> --- dlls/wininet/tests/http.c | 128 +++++++++++++++++++++++++++++++++++++- 1 file changed, 125 insertions(+), 3 deletions(-) diff --git a/dlls/wininet/tests/http.c b/dlls/wininet/tests/http.c index c63e44a6841..7e1a229a9d3 100644 --- a/dlls/wininet/tests/http.c +++ b/dlls/wininet/tests/http.c @@ -7523,35 +7523,150 @@ static void test_user_agent_header(void) char buffer[64]; BOOL ret; - ses = InternetOpenA("Gizmo5", INTERNET_OPEN_TYPE_PRECONFIG, NULL, NULL, 0); + ses = InternetOpenA("", INTERNET_OPEN_TYPE_PRECONFIG, NULL, NULL, 0); ok(ses != NULL, "InternetOpen failed\n"); con = InternetConnectA(ses, "test.winehq.org", 80, NULL, NULL, INTERNET_SERVICE_HTTP, 0, 0); ok(con != NULL, "InternetConnect failed\n"); + /* If the user agent string provided by the session handle is empty and none + * is explicitly set for the request handle, then there should be no user + * agent header present in the request when it is sent. */ req = HttpOpenRequestA(con, "GET", "/tests/hello.html", "HTTP/1.0", NULL, NULL, 0, 0); ok(req != NULL, "HttpOpenRequest failed\n"); size = sizeof(buffer); + SetLastError(0xdeadbeef); ret = HttpQueryInfoA(req, HTTP_QUERY_USER_AGENT | HTTP_QUERY_FLAG_REQUEST_HEADERS, buffer, &size, NULL); err = GetLastError(); ok(!ret, "HttpQueryInfo succeeded\n"); ok(err == ERROR_HTTP_HEADER_NOT_FOUND, "expected ERROR_HTTP_HEADER_NOT_FOUND, got %lu\n", err); - ret = HttpAddRequestHeadersA(req, "User-Agent: Gizmo Project\r\n", ~0u, HTTP_ADDREQ_FLAG_ADD_IF_NEW); - ok(ret, "HttpAddRequestHeaders succeeded\n"); + ret = HttpSendRequestA(req, NULL, 0, NULL, 0); + ok(ret, "HttpSendRequest failed\n"); size = sizeof(buffer); + SetLastError(0xdeadbeef); + ret = HttpQueryInfoA(req, HTTP_QUERY_USER_AGENT | HTTP_QUERY_FLAG_REQUEST_HEADERS, buffer, &size, NULL); + err = GetLastError(); + todo_wine + ok(!ret, "HttpQueryInfo succeeded\n"); + todo_wine + ok(err == ERROR_HTTP_HEADER_NOT_FOUND, "expected ERROR_HTTP_HEADER_NOT_FOUND, got %lu\n", err); + + InternetCloseHandle(req); + + /* Set the user agent string in the session handle to something non-empty. */ + strcpy(buffer, "Gizmo5"); + ret = InternetSetOptionA(ses, INTERNET_OPTION_USER_AGENT, buffer, sizeof("Gizmo5") - 1); + ok(ret, "InternetSetOptionA failed to set user agent for session\n"); + + /* The connection handle still retains the previous user agent string, so + * the change to the user agent string in the session handle will not be + * visible in requests created from the existing connection. */ + req = HttpOpenRequestA(con, "GET", "/tests/hello.html", "HTTP/1.0", NULL, NULL, 0, 0); + ok(req != NULL, "HttpOpenRequest failed\n"); + + size = sizeof(buffer); + SetLastError(0xdeadbeef); + ret = HttpQueryInfoA(req, HTTP_QUERY_USER_AGENT | HTTP_QUERY_FLAG_REQUEST_HEADERS, buffer, &size, NULL); + err = GetLastError(); + ok(!ret, "HttpQueryInfo succeeded\n"); + ok(err == ERROR_HTTP_HEADER_NOT_FOUND, "expected ERROR_HTTP_HEADER_NOT_FOUND, got %lu\n", err); + + ret = HttpSendRequestA(req, NULL, 0, NULL, 0); + ok(ret, "HttpSendRequest failed: %lu\n", GetLastError()); + + size = sizeof(buffer); + SetLastError(0xdeadbeef); + ret = HttpQueryInfoA(req, HTTP_QUERY_USER_AGENT | HTTP_QUERY_FLAG_REQUEST_HEADERS, buffer, &size, NULL); + err = GetLastError(); + todo_wine + ok(!ret, "HttpQueryInfo succeeded\n"); + todo_wine + ok(err == ERROR_HTTP_HEADER_NOT_FOUND, "expected ERROR_HTTP_HEADER_NOT_FOUND, got %lu\n", err); + + InternetCloseHandle(req); + InternetCloseHandle(con); + + /* Recreate the connection so the updated user agent string from the session + * handle is picked up by new requests for subsequent tests. */ + con = InternetConnectA(ses, "test.winehq.org", 80, NULL, NULL, INTERNET_SERVICE_HTTP, 0, 0); + ok(con != NULL, "InternetConnect failed\n"); + + /* If a user agent string is not explicitly set for the request handle, then + * the user agent string from the session handle will be used for the header + * when the request is sent. */ + req = HttpOpenRequestA(con, "GET", "/tests/hello.html", "HTTP/1.0", NULL, NULL, 0, 0); + ok(req != NULL, "HttpOpenRequest failed\n"); + + size = sizeof(buffer); + SetLastError(0xdeadbeef); ret = HttpQueryInfoA(req, HTTP_QUERY_USER_AGENT | HTTP_QUERY_FLAG_REQUEST_HEADERS, buffer, &size, NULL); err = GetLastError(); + ok(!ret, "HttpQueryInfo succeeded\n"); + ok(err == ERROR_HTTP_HEADER_NOT_FOUND, "expected ERROR_HTTP_HEADER_NOT_FOUND, got %lu\n", err); + + ret = HttpSendRequestA(req, NULL, 0, NULL, 0); + ok(ret, "HttpSendRequest failed: %lu\n", GetLastError()); + + + buffer[0] = 0; + size = sizeof(buffer); + ret = HttpQueryInfoA(req, HTTP_QUERY_USER_AGENT | HTTP_QUERY_FLAG_REQUEST_HEADERS, buffer, &size, NULL); + ok(ret, "HttpQueryInfo failed\n"); + ok(!strcmp(buffer, "Gizmo5"), "got %s\n", buffer); + ok(size == strlen("Gizmo5"), "got %lu\n", size); + + InternetCloseHandle(req); + + /* Adding a User-Agent header with an empty value is not permitted. */ + req = HttpOpenRequestA(con, "GET", "/tests/hello.html", "HTTP/1.0", NULL, NULL, 0, 0); + ok(req != NULL, "HttpOpenRequest failed\n"); + + SetLastError(0xdeadbeef); + ret = HttpAddRequestHeadersA(req, "User-Agent: \r\n", ~0u, HTTP_ADDREQ_FLAG_ADD_IF_NEW); + err = GetLastError(); + todo_wine + ok(!ret, "HttpAddRequestHeaders succeeded\n"); + todo_wine + ok(err == ERROR_HTTP_HEADER_NOT_FOUND, "expected ERROR_HTTP_HEADER_NOT_FOUND, got %lu\n", err); + + InternetCloseHandle(req); + + /* A User-Agent header explicitly added to a request overrides the user + * agent string from the session handle. */ + req = HttpOpenRequestA(con, "GET", "/tests/hello.html", "HTTP/1.0", NULL, NULL, 0, 0); + ok(req != NULL, "HttpOpenRequest failed\n"); + + ret = HttpAddRequestHeadersA(req, "User-Agent: Gizmo Project\r\n", ~0u, HTTP_ADDREQ_FLAG_ADD_IF_NEW); + ok(ret, "HttpAddRequestHeaders failed\n"); + + buffer[0] = 0; + size = sizeof(buffer); + ret = HttpQueryInfoA(req, HTTP_QUERY_USER_AGENT | HTTP_QUERY_FLAG_REQUEST_HEADERS, buffer, &size, NULL); + ok(ret, "HttpQueryInfo failed\n"); + ok(!strcmp(buffer, "Gizmo Project"), "got %s\n", buffer); + ok(size == strlen("Gizmo Project"), "got %lu\n", size); + + ret = HttpSendRequestA(req, NULL, 0, NULL, 0); + ok(ret, "HttpSendRequest failed: %lu\n", GetLastError()); + + buffer[0] = 0; + size = sizeof(buffer); + ret = HttpQueryInfoA(req, HTTP_QUERY_USER_AGENT | HTTP_QUERY_FLAG_REQUEST_HEADERS, buffer, &size, NULL); ok(ret, "HttpQueryInfo failed\n"); + ok(!strcmp(buffer, "Gizmo Project"), "got %s\n", buffer); + ok(size == strlen("Gizmo Project"), "got %lu\n", size); InternetCloseHandle(req); + /* Explicitly adding both Accept and User-Agent headers at once is permitted. */ req = HttpOpenRequestA(con, "GET", "/", "HTTP/1.0", NULL, NULL, 0, 0); ok(req != NULL, "HttpOpenRequest failed\n"); size = sizeof(buffer); + SetLastError(0xdeadbeef); ret = HttpQueryInfoA(req, HTTP_QUERY_ACCEPT | HTTP_QUERY_FLAG_REQUEST_HEADERS, buffer, &size, NULL); err = GetLastError(); ok(!ret, "HttpQueryInfo succeeded\n"); @@ -7566,6 +7681,13 @@ static void test_user_agent_header(void) ok(ret, "HttpQueryInfo failed: %lu\n", GetLastError()); ok(!strcmp(buffer, "audio/*, image/*, text/*"), "got '%s' expected 'audio/*, image/*, text/*'\n", buffer); + buffer[0] = 0; + size = sizeof(buffer); + ret = HttpQueryInfoA(req, HTTP_QUERY_USER_AGENT | HTTP_QUERY_FLAG_REQUEST_HEADERS, buffer, &size, NULL); + ok(ret, "HttpQueryInfo failed: %lu\n", GetLastError()); + ok(!strcmp(buffer, "Gizmo Project"), "got %s\n", buffer); + ok(size == strlen("Gizmo Project"), "got %lu\n", size); + InternetCloseHandle(req); InternetCloseHandle(con); InternetCloseHandle(ses); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6541
From: Andrew Nguyen <arethusa26(a)gmail.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=56875 --- dlls/wininet/http.c | 10 +++++++--- dlls/wininet/tests/http.c | 2 -- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/dlls/wininet/http.c b/dlls/wininet/http.c index c0f1e13bfa8..38f6575c1fd 100644 --- a/dlls/wininet/http.c +++ b/dlls/wininet/http.c @@ -4988,6 +4988,7 @@ static DWORD HTTP_HttpSendRequestW(http_request_t *request, LPCWSTR lpszHeaders, WCHAR *request_header = NULL; INT responseLen, cnt; DWORD res; + const WCHAR *appinfo_agent; TRACE("--> %p\n", request); @@ -5005,14 +5006,17 @@ static DWORD HTTP_HttpSendRequestW(http_request_t *request, LPCWSTR lpszHeaders, set_content_length_header(request, dwContentLength, HTTP_ADDREQ_FLAG_ADD_IF_NEW); request->bytesToWrite = dwContentLength; } - if (request->session->appInfo->agent) + + appinfo_agent = request->session->appInfo->agent; + + if (appinfo_agent && *appinfo_agent) { WCHAR *agent_header; int len; - len = lstrlenW(request->session->appInfo->agent) + lstrlenW(L"User-Agent: %s\r\n"); + len = lstrlenW(appinfo_agent) + lstrlenW(L"User-Agent: %s\r\n"); agent_header = malloc(len * sizeof(WCHAR)); - swprintf(agent_header, len, L"User-Agent: %s\r\n", request->session->appInfo->agent); + swprintf(agent_header, len, L"User-Agent: %s\r\n", appinfo_agent); HTTP_HttpAddRequestHeadersW(request, agent_header, lstrlenW(agent_header), HTTP_ADDREQ_FLAG_ADD_IF_NEW); free(agent_header); diff --git a/dlls/wininet/tests/http.c b/dlls/wininet/tests/http.c index 7e1a229a9d3..14c08e0c0ad 100644 --- a/dlls/wininet/tests/http.c +++ b/dlls/wininet/tests/http.c @@ -7549,9 +7549,7 @@ static void test_user_agent_header(void) SetLastError(0xdeadbeef); ret = HttpQueryInfoA(req, HTTP_QUERY_USER_AGENT | HTTP_QUERY_FLAG_REQUEST_HEADERS, buffer, &size, NULL); err = GetLastError(); - todo_wine ok(!ret, "HttpQueryInfo succeeded\n"); - todo_wine ok(err == ERROR_HTTP_HEADER_NOT_FOUND, "expected ERROR_HTTP_HEADER_NOT_FOUND, got %lu\n", err); InternetCloseHandle(req); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6541
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=148571 Your paranoid android. === debian11b (64 bit WoW report) === user32: input.c:4305: Test succeeded inside todo block: button_down_hwnd_todo 1: got MSG_TEST_WIN hwnd 0000000000AD00EA, msg WM_LBUTTONDOWN, wparam 0x1, lparam 0x320032
This merge request was approved by Jacek Caban. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6541
participants (4)
-
Andrew Nguyen -
Andrew Nguyen (＠arethusa)
-
Jacek Caban (＠jacek)
-
Marvin