From 3b211d4bf5037224b223f7e3707eeb53724a94f9 Mon Sep 17 00:00:00 2001 From: Marshall Greenblatt Date: Thu, 11 Jul 2019 16:36:27 -0400 Subject: [PATCH] Remove POST data after redirect to GET (see issue #2707, see issue #2622). For 303 redirects all request methods except HEAD are converted to GET as per the latest http draft. For historical reasons the draft also allows POST requests to be converted to GETs when following 301/302 redirects. Most major browsers do this and so shall we. When a request is converted to GET any POST data should also be removed. Use 307 redirects instead if you want the request to be repeated using the same method and POST data. --- .../net_service/proxy_url_loader_factory.cc | 5 ++ tests/ceftests/urlrequest_unittest.cc | 65 +++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/libcef/browser/net_service/proxy_url_loader_factory.cc b/libcef/browser/net_service/proxy_url_loader_factory.cc index 34bbc2b45..a2eb4e8f2 100644 --- a/libcef/browser/net_service/proxy_url_loader_factory.cc +++ b/libcef/browser/net_service/proxy_url_loader_factory.cc @@ -831,6 +831,11 @@ void InterceptedRequest::ContinueToBeforeRedirect( request_.site_for_cookies = redirect_info.new_site_for_cookies; request_.referrer = GURL(redirect_info.new_referrer); request_.referrer_policy = redirect_info.new_referrer_policy; + + // The request method can be changed to "GET". In this case we need to + // reset the request body manually. + if (request_.method == net::HttpRequestHeaders::kGetMethod) + request_.request_body = nullptr; } void InterceptedRequest::ContinueToResponseStarted(int error_code) { diff --git a/tests/ceftests/urlrequest_unittest.cc b/tests/ceftests/urlrequest_unittest.cc index 1015bb263..ca68810e4 100644 --- a/tests/ceftests/urlrequest_unittest.cc +++ b/tests/ceftests/urlrequest_unittest.cc @@ -68,6 +68,8 @@ enum RequestTestMode { REQTEST_POST, REQTEST_POST_FILE, REQTEST_POST_WITHPROGRESS, + REQTEST_POST_REDIRECT, + REQTEST_POST_REDIRECT_TOGET, REQTEST_HEAD, REQTEST_CACHE_WITH_CONTROL, REQTEST_CACHE_WITHOUT_CONTROL, @@ -1251,6 +1253,9 @@ class RequestTestRunner : public base::RefCountedThreadSafe { REGISTER_TEST(REQTEST_POST_FILE, SetupPostFileTest, SingleRunTest); REGISTER_TEST(REQTEST_POST_WITHPROGRESS, SetupPostWithProgressTest, SingleRunTest); + REGISTER_TEST(REQTEST_POST_REDIRECT, SetupPostRedirectTest, SingleRunTest); + REGISTER_TEST(REQTEST_POST_REDIRECT_TOGET, SetupPostRedirectToGetTest, + SingleRunTest); REGISTER_TEST(REQTEST_HEAD, SetupHeadTest, SingleRunTest); REGISTER_TEST(REQTEST_CACHE_WITH_CONTROL, SetupCacheWithControlTest, MultipleRunTest); @@ -1589,6 +1594,58 @@ class RequestTestRunner : public base::RefCountedThreadSafe { complete_callback.Run(); } + void SetupPostRedirectTest(const base::Closure& complete_callback) { + // Start with the normal post test. + SetupPostTestShared(); + + // Add a redirect request. + settings_.redirect_request = CefRequest::Create(); + settings_.redirect_request->SetURL(GetTestURL("redirect.html")); + settings_.redirect_request->SetMethod("POST"); + SetUploadData(settings_.redirect_request, "the_post_data"); + + settings_.redirect_response = CefResponse::Create(); + settings_.redirect_response->SetMimeType("text/html"); + // Only 307 is supported for redirecting the same method and post data. + settings_.redirect_response->SetStatus(307); + settings_.redirect_response->SetStatusText("Found"); + + CefResponse::HeaderMap headerMap; + headerMap.insert(std::make_pair("Location", settings_.request->GetURL())); + settings_.redirect_response->SetHeaderMap(headerMap); + + complete_callback.Run(); + } + + void SetupPostRedirectToGetTest(const base::Closure& complete_callback) { + // Start with the normal post test. + SetupPostTestShared(); + + // The expected result after redirect is a GET request without POST data. + settings_.request = CefRequest::Create(); + settings_.request->SetURL(GetTestURL("PostTest.html")); + settings_.request->SetMethod("GET"); + + // Add a redirect request. + settings_.redirect_request = CefRequest::Create(); + settings_.redirect_request->SetURL(GetTestURL("redirect.html")); + settings_.redirect_request->SetMethod("POST"); + SetUploadData(settings_.redirect_request, "the_post_data"); + + settings_.redirect_response = CefResponse::Create(); + settings_.redirect_response->SetMimeType("text/html"); + // Redirect codes other than 307 will cause conversion to GET and removal + // of POST data. + settings_.redirect_response->SetStatus(302); + settings_.redirect_response->SetStatusText("Found"); + + CefResponse::HeaderMap headerMap; + headerMap.insert(std::make_pair("Location", settings_.request->GetURL())); + settings_.redirect_response->SetHeaderMap(headerMap); + + complete_callback.Run(); + } + void SetupHeadTest(const base::Closure& complete_callback) { settings_.request = CefRequest::Create(); settings_.request->SetURL(GetTestURL("HeadTest.html")); @@ -2873,6 +2930,10 @@ void RegisterURLRequestCustomSchemes( test_server_backend, test_frame_method) \ REQ_TEST(BrowserPOSTWithProgress##suffix, REQTEST_POST_WITHPROGRESS, \ context_mode, true, test_server_backend, test_frame_method) \ + REQ_TEST(BrowserPOSTRedirect##suffix, REQTEST_POST_REDIRECT, context_mode, \ + true, test_server_backend, test_frame_method) \ + REQ_TEST(BrowserPOSTRedirectToGET##suffix, REQTEST_POST_REDIRECT_TOGET, \ + context_mode, true, test_server_backend, test_frame_method) \ REQ_TEST(BrowserHEAD##suffix, REQTEST_HEAD, context_mode, true, \ test_server_backend, test_frame_method) \ REQ_TEST(RendererGET##suffix, REQTEST_GET, context_mode, false, \ @@ -2893,6 +2954,10 @@ void RegisterURLRequestCustomSchemes( test_server_backend, test_frame_method) \ REQ_TEST(RendererPOSTWithProgress##suffix, REQTEST_POST_WITHPROGRESS, \ context_mode, false, test_server_backend, test_frame_method) \ + REQ_TEST(RendererPOSTRedirect##suffix, REQTEST_POST_REDIRECT, context_mode, \ + false, test_server_backend, test_frame_method) \ + REQ_TEST(RendererPOSTRedirectToGET##suffix, REQTEST_POST_REDIRECT_TOGET, \ + context_mode, false, test_server_backend, test_frame_method) \ REQ_TEST(RendererHEAD##suffix, REQTEST_HEAD, context_mode, false, \ test_server_backend, test_frame_method)