Improve CefURLRequestJob redirect handling (issue #2542)

include/capi/cef_resource_handler_capi.h

@@ -33,7 +33,7 @@
 // by hand. See the translator.README.txt file in the tools directory for
 // more information.
 //
-// $hash=2d14f5cfe748ef0f34aade25cfd1e299cb0ae793$
+// $hash=706e3ebbdb4e3b01f3f3bb18eb85c1897c8b5ade$
 //
 
 #ifndef CEF_INCLUDE_CAPI_CEF_RESOURCE_HANDLER_CAPI_H_
@@ -80,8 +80,12 @@ typedef struct _cef_resource_handler_t {
   // (0) or the specified number of bytes have been read. Use the |response|
   // object to set the mime type, http status code and other optional header
   // values. To redirect the request to a new URL set |redirectUrl| to the new
-  // URL. If an error occured while setting up the request you can call
+  // URL. |redirectUrl| can be either a relative or fully qualified URL. It is
-  // set_error() on |response| to indicate the error condition.
+  // also possible to set |response| to a redirect http status code and pass the
+  // new URL via a Location header. Likewise with |redirectUrl| it is valid to
+  // set a relative or fully qualified URL as the Location header value. If an
+  // error occured while setting up the request you can call set_error() on
+  // |response| to indicate the error condition.
   ///
   void(CEF_CALLBACK* get_response_headers)(struct _cef_resource_handler_t* self,
                                            struct _cef_response_t* response,

include/cef_resource_handler.h

@@ -71,7 +71,11 @@ class CefResourceHandler : public virtual CefBaseRefCounted {
   // false or the specified number of bytes have been read. Use the |response|
   // object to set the mime type, http status code and other optional header
   // values. To redirect the request to a new URL set |redirectUrl| to the new
-  // URL. If an error occured while setting up the request you can call
+  // URL. |redirectUrl| can be either a relative or fully qualified URL.
+  // It is also possible to set |response| to a redirect http status code
+  // and pass the new URL via a Location header. Likewise with |redirectUrl| it
+  // is valid to set a relative or fully qualified URL as the Location header
+  // value. If an error occured while setting up the request you can call
   // SetError() on |response| to indicate the error condition.
   ///
   /*--cef()--*/

libcef/browser/net/resource_request_job.cc

@@ -15,6 +15,7 @@
 #include "libcef/common/response_impl.h"
 
 #include "base/logging.h"
+#include "base/strings/string_util.h"
 #include "net/base/io_buffer.h"
 #include "net/base/load_flags.h"
 #include "net/base/mime_util.h"
@@ -27,6 +28,21 @@ using net::URLRequestStatus;
 
 namespace {
 
+using HeaderMap = std::multimap<CefString, CefString>;
+
+struct CaseInsensitiveComparator {
+  base::StringPiece search;
+  CaseInsensitiveComparator(const std::string& s) : search(s) {}
+  bool operator()(const HeaderMap::value_type& p) const {
+    return base::EqualsCaseInsensitiveASCII(search, p.first.ToString());
+  }
+};
+
+HeaderMap::const_iterator FindHeader(const HeaderMap& m,
+                                     const std::string& name) {
+  return std::find_if(m.begin(), m.end(), CaseInsensitiveComparator(name));
+}
+
 bool SetHeaderIfMissing(CefRequest::HeaderMap& headerMap,
                         const std::string& name,
                         const std::string& value) {
@@ -299,15 +315,18 @@ bool CefResourceRequestJob::IsRedirectResponse(
   } else if (response_.get()) {
     // Check for HTTP 302 or HTTP 303 redirect.
     int status = response_->GetStatus();
-    if (status == 302 || status == 303) {
+    if (net::HttpResponseHeaders::IsRedirectResponseCode(status)) {
       CefResponse::HeaderMap headerMap;
       response_->GetHeaderMap(headerMap);
-      CefRequest::HeaderMap::iterator iter = headerMap.find("Location");
+      CefRequest::HeaderMap::const_iterator iter =
+          FindHeader(headerMap, "Location");
       if (iter != headerMap.end()) {
-        GURL new_url = GURL(std::string(iter->second));
+        GURL new_url = request_->url().Resolve(std::string(iter->second));
-        *http_status_code = status;
+        if (new_url.is_valid()) {
-        location->Swap(&new_url);
+          *http_status_code = status;
-        redirect = true;
+          *location = new_url;
+          redirect = true;
+        }
       }
     }
   }
@@ -392,7 +411,7 @@ void CefResourceRequestJob::SendHeaders() {
 
   if (!redirectUrl.empty()) {
     std::string redirectUrlStr = redirectUrl;
-    redirect_url_ = GURL(redirectUrlStr);
+    redirect_url_ = request_->url().Resolve(redirectUrlStr);
   }
 
   if (remaining_bytes_ > 0)

tests/ceftests/urlrequest_unittest.cc

@@ -58,6 +58,7 @@ enum RequestTestMode {
   REQTEST_GET_ALLOWCOOKIES,
   REQTEST_GET_REDIRECT,
   REQTEST_GET_REDIRECT_STOP,
+  REQTEST_GET_REDIRECT_LOCATION,
   REQTEST_GET_REFERRER,
   REQTEST_POST,
   REQTEST_POST_FILE,
@@ -1061,6 +1062,8 @@ class RequestTestRunner : public base::RefCountedThreadSafe<RequestTestRunner> {
     REGISTER_TEST(REQTEST_GET_REDIRECT, SetupGetRedirectTest, SingleRunTest);
     REGISTER_TEST(REQTEST_GET_REDIRECT_STOP, SetupGetRedirectStopTest,
                   SingleRunTest);
+    REGISTER_TEST(REQTEST_GET_REDIRECT_LOCATION, SetupGetRedirectLocationTest,
+                  SingleRunTest);
     REGISTER_TEST(REQTEST_GET_REFERRER, SetupGetReferrerTest, SingleRunTest);
     REGISTER_TEST(REQTEST_POST, SetupPostTest, SingleRunTest);
     REGISTER_TEST(REQTEST_POST_FILE, SetupPostFileTest, SingleRunTest);
@@ -1171,12 +1174,16 @@ class RequestTestRunner : public base::RefCountedThreadSafe<RequestTestRunner> {
     }
   }
 
+  std::string GetTestPath(const std::string& name) {
+    return std::string(run_in_browser_process_ ? "/Browser" : "/Renderer") +
+           name;
+  }
+
   std::string GetTestURL(const std::string& name) {
     // Avoid name duplication between tests running in different processes.
     // Otherwise we'll get unexpected state leakage (cache hits) when running
     // multiple tests.
-    return GetRequestOrigin(is_server_backend_) + "/" +
+    return GetRequestOrigin(is_server_backend_) + GetTestPath(name);
-           (run_in_browser_process_ ? "Browser" : "Renderer") + name;
   }
 
   void SetupGetTestShared() {
@@ -1278,6 +1285,27 @@ class RequestTestRunner : public base::RefCountedThreadSafe<RequestTestRunner> {
     complete_callback.Run();
   }
 
+  void SetupGetRedirectLocationTest(const base::Closure& complete_callback) {
+    // Start with the normal get test.
+    SetupGetTestShared();
+
+    // Add a redirect request.
+    settings_.redirect_request = CefRequest::Create();
+    settings_.redirect_request->SetURL(GetTestURL("redirect.html"));
+    settings_.redirect_request->SetMethod("GET");
+
+    settings_.redirect_response = CefResponse::Create();
+    settings_.redirect_response->SetMimeType("text/html");
+    settings_.redirect_response->SetStatus(302);
+    settings_.redirect_response->SetStatusText("Found");
+
+    CefResponse::HeaderMap headerMap;
+    headerMap.insert(std::make_pair("LoCaTioN", GetTestPath("GetTest.html")));
+    settings_.redirect_response->SetHeaderMap(headerMap);
+
+    complete_callback.Run();
+  }
+
   void SetupGetReferrerTest(const base::Closure& complete_callback) {
     settings_.request = CefRequest::Create();
     settings_.request->SetURL(GetTestURL("GetTest.html"));
@@ -2250,6 +2278,8 @@ void RegisterURLRequestCustomSchemes(
            true, test_server_backend);                                         \
   REQ_TEST(BrowserGETRedirectStop##suffix, REQTEST_GET_REDIRECT_STOP,          \
            context_mode, true, test_server_backend);                           \
+  REQ_TEST(BrowserGETRedirectLocation##suffix, REQTEST_GET_REDIRECT_LOCATION,  \
+           context_mode, true, test_server_backend);                           \
   REQ_TEST(BrowserGETReferrer##suffix, REQTEST_GET_REFERRER, context_mode,     \
            true, test_server_backend);                                         \
   REQ_TEST(BrowserPOST##suffix, REQTEST_POST, context_mode, true,              \
@@ -2270,6 +2300,8 @@ void RegisterURLRequestCustomSchemes(
            false, test_server_backend);                                        \
   REQ_TEST(RendererGETRedirectStop##suffix, REQTEST_GET_REDIRECT_STOP,         \
            context_mode, false, test_server_backend);                          \
+  REQ_TEST(RendererGETRedirectLocation##suffix, REQTEST_GET_REDIRECT_LOCATION, \
+           context_mode, false, test_server_backend);                          \
   REQ_TEST(RendererGETReferrer##suffix, REQTEST_GET_REFERRER, context_mode,    \
            false, test_server_backend);                                        \
   REQ_TEST(RendererPOST##suffix, REQTEST_POST, context_mode, false,            \