[PATCH v3 0/3] MR7735: wininet: Handle HTTP status code 308 (Permanent Redirect)
This implements handling of HTTP status code 308 ([Permanant Redirect](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Status/308)) in wininet/winhttp. I added handling at all places I could find, but could only test the one in `dlls/wininet/http.c` with the program where it failed (Marvel Heroes). I tried to keep the implementation parallel to 307, since they serve a similar purpose (redirect while keeping the request intact). PS: `case` labels are sometimes in different order, would it be a good idea to sort them in numerical HTTP status order while I'm at it? -- v3: urlmon: Handle HTTP status code 308 (Permanent Redirect) winhttp: Handle HTTP status code 308 (Permanent Redirect) wininet: Handle HTTP status code 308 (Permanent Redirect) https://gitlab.winehq.org/wine/wine/-/merge_requests/7735
From: Tobias Gruetzmacher <tobias-git(a)23.gs> --- dlls/wininet/http.c | 10 +++++++--- include/wininet.h | 1 + 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/dlls/wininet/http.c b/dlls/wininet/http.c index 48bfe35866a..497a582ff89 100644 --- a/dlls/wininet/http.c +++ b/dlls/wininet/http.c @@ -5206,12 +5206,14 @@ static DWORD HTTP_HttpSendRequestW(http_request_t *request, LPCWSTR lpszHeaders, case HTTP_STATUS_MOVED: case HTTP_STATUS_REDIRECT_KEEP_VERB: case HTTP_STATUS_REDIRECT_METHOD: + case HTTP_STATUS_PERMANENT_REDIRECT: new_url = get_redirect_url(request); if(!new_url) break; if (wcscmp(request->verb, L"GET") && wcscmp(request->verb, L"HEAD") && - request->status_code != HTTP_STATUS_REDIRECT_KEEP_VERB) + request->status_code != HTTP_STATUS_REDIRECT_KEEP_VERB && + request->status_code != HTTP_STATUS_PERMANENT_REDIRECT) { free(request->verb); request->verb = wcsdup(L"GET"); @@ -5401,7 +5403,8 @@ static DWORD HTTP_HttpEndRequestW(http_request_t *request, DWORD dwFlags, DWORD_ case HTTP_STATUS_REDIRECT: case HTTP_STATUS_MOVED: case HTTP_STATUS_REDIRECT_METHOD: - case HTTP_STATUS_REDIRECT_KEEP_VERB: { + case HTTP_STATUS_REDIRECT_KEEP_VERB: + case HTTP_STATUS_PERMANENT_REDIRECT: { WCHAR *new_url; new_url = get_redirect_url(request); @@ -5409,7 +5412,8 @@ static DWORD HTTP_HttpEndRequestW(http_request_t *request, DWORD dwFlags, DWORD_ break; if (wcscmp(request->verb, L"GET") && wcscmp(request->verb, L"HEAD") && - request->status_code != HTTP_STATUS_REDIRECT_KEEP_VERB) + request->status_code != HTTP_STATUS_REDIRECT_KEEP_VERB && + request->status_code != HTTP_STATUS_PERMANENT_REDIRECT) { free(request->verb); request->verb = wcsdup(L"GET"); diff --git a/include/wininet.h b/include/wininet.h index 0de2b830812..0537585adf8 100644 --- a/include/wininet.h +++ b/include/wininet.h @@ -1242,6 +1242,7 @@ BOOLAPI GopherGetAttributeW(HINTERNET ,LPCWSTR ,LPCWSTR ,LPBYTE , #define HTTP_STATUS_NOT_MODIFIED 304 #define HTTP_STATUS_USE_PROXY 305 #define HTTP_STATUS_REDIRECT_KEEP_VERB 307 +#define HTTP_STATUS_PERMANENT_REDIRECT 308 #define HTTP_STATUS_BAD_REQUEST 400 #define HTTP_STATUS_DENIED 401 #define HTTP_STATUS_PAYMENT_REQ 402 -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7735
From: Tobias Gruetzmacher <tobias-git(a)23.gs> --- dlls/winhttp/request.c | 6 ++++-- dlls/winhttp/tests/winhttp.c | 29 ++++++++++++++++++++--------- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/dlls/winhttp/request.c b/dlls/winhttp/request.c index 26022dba3b1..57a834bb07f 100644 --- a/dlls/winhttp/request.c +++ b/dlls/winhttp/request.c @@ -2872,7 +2872,8 @@ static DWORD handle_redirect( struct request *request, DWORD status ) else request->path = wcsdup( L"/" ); } - if (status != HTTP_STATUS_REDIRECT_KEEP_VERB && !wcscmp( request->verb, L"POST" )) + if (status != HTTP_STATUS_REDIRECT_KEEP_VERB && status != HTTP_STATUS_PERMANENT_REDIRECT && + !wcscmp( request->verb, L"POST" )) { free( request->verb ); request->verb = wcsdup( L"GET" ); @@ -3003,7 +3004,8 @@ static DWORD receive_response( struct request *request ) ret = handle_passport_redirect( request ); goto done; } - if (status == HTTP_STATUS_MOVED || status == HTTP_STATUS_REDIRECT || status == HTTP_STATUS_REDIRECT_KEEP_VERB) + if (status == HTTP_STATUS_MOVED || status == HTTP_STATUS_REDIRECT || + status == HTTP_STATUS_REDIRECT_KEEP_VERB || status == HTTP_STATUS_PERMANENT_REDIRECT) { if (request->hdr.disable_flags & WINHTTP_DISABLE_REDIRECTS || request->hdr.redirect_policy == WINHTTP_OPTION_REDIRECT_POLICY_NEVER) goto done; diff --git a/dlls/winhttp/tests/winhttp.c b/dlls/winhttp/tests/winhttp.c index 00c05caaa5a..12dbfdce663 100644 --- a/dlls/winhttp/tests/winhttp.c +++ b/dlls/winhttp/tests/winhttp.c @@ -2285,12 +2285,18 @@ static const char switchprotocols[] = "Upgrade: websocket\r\n" "Connection: Upgrade\r\n"; -static const char redirectmsg[] = +static const char temp_redirectmsg[] = "HTTP/1.1 307 Temporary Redirect\r\n" "Content-Length: 0\r\n" "Location: /temporary\r\n" "Connection: close\r\n\r\n"; +static const char perm_redirectmsg[] = +"HTTP/1.1 308 Permanent Redirect\r\n" +"Content-Length: 0\r\n" +"Location: /permanent\r\n" +"Connection: close\r\n\r\n"; + static const char badreplyheadermsg[] = "HTTP/1.1 200 OK\r\n" "Server: winetest\r\n" @@ -2546,11 +2552,15 @@ static DWORD CALLBACK server_thread(LPVOID param) } else send(c, notokmsg, sizeof(notokmsg) - 1, 0); } - else if (strstr(buffer, "POST /redirect")) + else if (strstr(buffer, "POST /redirect-temp")) + { + send(c, temp_redirectmsg, sizeof temp_redirectmsg - 1, 0); + } + else if (strstr(buffer, "POST /redirect-perm")) { - send(c, redirectmsg, sizeof redirectmsg - 1, 0); + send(c, perm_redirectmsg, sizeof perm_redirectmsg - 1, 0); } - else if (strstr(buffer, "POST /temporary")) + else if (strstr(buffer, "POST /temporary") || strstr(buffer, "POST /permanent")) { char buf[32]; recv(c, buf, sizeof(buf), 0); @@ -3327,7 +3337,7 @@ static void test_head_request(int port) WinHttpCloseHandle(ses); } -static void test_redirect(int port) +static void test_redirect(int port, const WCHAR *path, const WCHAR *target) { HINTERNET ses, con, req; char buf[128]; @@ -3341,14 +3351,14 @@ static void test_redirect(int port) con = WinHttpConnect(ses, L"localhost", port, 0); ok(con != NULL, "failed to open a connection %lu\n", GetLastError()); - req = WinHttpOpenRequest(con, L"POST", L"/redirect", NULL, NULL, NULL, 0); + req = WinHttpOpenRequest(con, L"POST", path, NULL, NULL, NULL, 0); ok(req != NULL, "failed to open a request %lu\n", GetLastError()); url[0] = 0; size = sizeof(url); ret = WinHttpQueryOption(req, WINHTTP_OPTION_URL, url, &size); ok(ret, "got %lu\n", GetLastError()); - swprintf(expected, ARRAY_SIZE(expected), L"http://localhost:%u/redirect", port); + swprintf(expected, ARRAY_SIZE(expected), L"http://localhost:%u%s", port, path); ok(!wcscmp(url, expected), "expected %s got %s\n", wine_dbgstr_w(expected), wine_dbgstr_w(url)); ret = WinHttpSendRequest(req, NULL, 0, (void *)"data", sizeof("data"), sizeof("data"), 0); @@ -3374,7 +3384,7 @@ static void test_redirect(int port) size = sizeof(url); ret = WinHttpQueryOption(req, WINHTTP_OPTION_URL, url, &size); ok(ret, "got %lu\n", GetLastError()); - swprintf(expected, ARRAY_SIZE(expected), L"http://localhost:%u/temporary", port); + swprintf(expected, ARRAY_SIZE(expected), L"http://localhost:%u/%s", port, target); ok(!wcscmp(url, expected), "expected %s got %s\n", wine_dbgstr_w(expected), wine_dbgstr_w(url)); status = 0xdeadbeef; @@ -6328,7 +6338,8 @@ START_TEST (winhttp) test_request_path_escapes(si.port); test_passport_auth(si.port); test_websocket(si.port); - test_redirect(si.port); + test_redirect(si.port, L"/redirect-temp", L"temporary"); + test_redirect(si.port, L"/redirect-perm", L"permanent"); test_WinHttpGetProxyForUrl(si.port); test_connection_cache(si.port); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7735
From: Tobias Gruetzmacher <tobias-git(a)23.gs> --- dlls/urlmon/http.c | 1 + 1 file changed, 1 insertion(+) diff --git a/dlls/urlmon/http.c b/dlls/urlmon/http.c index a24006ba08c..e592f697485 100644 --- a/dlls/urlmon/http.c +++ b/dlls/urlmon/http.c @@ -495,6 +495,7 @@ static BOOL is_redirect_response(DWORD status_code) case HTTP_STATUS_MOVED: case HTTP_STATUS_REDIRECT_KEEP_VERB: case HTTP_STATUS_REDIRECT_METHOD: + case HTTP_STATUS_PERMANENT_REDIRECT: return TRUE; } return FALSE; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7735
@hans: Doh! I'm stupid :grin: - I extended the redirect test to make sure that 307 & 308 work the same ways. Give me a shout if I should split the test changes into an additional commit. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7735#note_100003
This merge request was approved by Hans Leidekker. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7735
This merge request was approved by Jacek Caban. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7735
participants (4)
-
Hans Leidekker (＠hans) -
Jacek Caban (＠jacek)
-
Tobias Gruetzmacher
-
Tobias Gruetzmacher (＠TobiX)