Support CORS preflight requests with OutOfBlinkCors (fixes issue #3006)

A CORS preflight request is an "OPTIONS" request sent to a server prior to a
cross-origin XMLHttpRequest or Fetch request. The server's response determines
which HTTP request methods are allowed and supported, and whether credentials
such as Cookies and HTTP Authentication should be sent with requests.

A CORS preflight request will only be sent if certain conditions are met. For
example, it will be sent for requests that have potentially unsafe HTTP
methods [1] or request headers [2]. See the NeedsPreflight function in
services/network/cors/cors_url_loader.cc for full details.

CORS preflight functionality is implemented in the network service and will not
be triggered if the client handles the request instead of allowing it to proceed
over the network. Since the preflight request itself also runs in the network
service it cannot be intercepted by the client.

[1] https://fetch.spec.whatwg.org/#cors-safelisted-method
[2] https://fetch.spec.whatwg.org/#cors-safelisted-request-header

libcef/browser/net_service/proxy_url_loader_factory.cc

@@ -131,6 +131,43 @@ class ResourceContextData : public base::SupportsUserData::Data {
   DISALLOW_COPY_AND_ASSIGN(ResourceContextData);
 };
 
+// CORS preflight requests are handled in the network process, so we just need
+// to continue all of the callbacks and then delete ourself.
+class CorsPreflightRequest : public network::mojom::TrustedHeaderClient {
+ public:
+  explicit CorsPreflightRequest(
+      mojo::PendingReceiver<network::mojom::TrustedHeaderClient> receiver)
+      : weak_factory_(this) {
+    header_client_receiver_.Bind(std::move(receiver));
+
+    header_client_receiver_.set_disconnect_handler(base::BindOnce(
+        &CorsPreflightRequest::OnDestroy, weak_factory_.GetWeakPtr()));
+  }
+
+  // mojom::TrustedHeaderClient methods:
+  void OnBeforeSendHeaders(const net::HttpRequestHeaders& headers,
+                           OnBeforeSendHeadersCallback callback) override {
+    std::move(callback).Run(net::OK, base::nullopt);
+  }
+
+  void OnHeadersReceived(const std::string& headers,
+                         const net::IPEndPoint& remote_endpoint,
+                         OnHeadersReceivedCallback callback) override {
+    std::move(callback).Run(net::OK, base::nullopt, GURL());
+    OnDestroy();
+  }
+
+ private:
+  void OnDestroy() { delete this; }
+
+  mojo::Receiver<network::mojom::TrustedHeaderClient> header_client_receiver_{
+      this};
+
+  base::WeakPtrFactory<CorsPreflightRequest> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(CorsPreflightRequest);
+};
+
 //==============================
 // InterceptedRequest
 //=============================
@@ -1302,10 +1339,10 @@ void ProxyURLLoaderFactory::OnLoaderCreated(
 }
 
 void ProxyURLLoaderFactory::OnLoaderForCorsPreflightCreated(
-    const ::network::ResourceRequest& request,
+    const network::ResourceRequest& request,
-    mojo::PendingReceiver<network::mojom::TrustedHeaderClient> header_client) {
+    mojo::PendingReceiver<network::mojom::TrustedHeaderClient> receiver) {
   CEF_REQUIRE_IOT();
-  // TODO(cef): Should we do something here?
+  new CorsPreflightRequest(std::move(receiver));
 }
 
 void ProxyURLLoaderFactory::OnTargetFactoryError() {

libcef/browser/net_service/proxy_url_loader_factory.h

@@ -168,8 +168,8 @@ class ProxyURLLoaderFactory
       mojo::PendingReceiver<network::mojom::TrustedHeaderClient> receiver)
       override;
   void OnLoaderForCorsPreflightCreated(
-      const ::network::ResourceRequest& request,
+      const network::ResourceRequest& request,
-      mojo::PendingReceiver<network::mojom::TrustedHeaderClient> header_client)
+      mojo::PendingReceiver<network::mojom::TrustedHeaderClient> receiver)
       override;
 
  private:

tests/ceftests/cors_unittest.cc

@@ -3,6 +3,7 @@
 // can be found in the LICENSE file.
 
 #include <algorithm>
+#include <set>
 #include <vector>
 
 #include "include/base/cef_bind.h"
@@ -62,6 +63,8 @@ struct Resource {
   // Uniquely identifies the resource.
   HandlerType handler = HandlerType::SERVER;
   std::string path;
+  // If non-empty the method value must match.
+  std::string method;
 
   // Response information that will be returned.
   CefRefPtr<CefResponse> response;
@@ -159,22 +162,35 @@ struct TestSetup {
     console_messages.push_back(message);
   }
 
-  Resource* GetResource(const std::string& url) const {
+  Resource* GetResource(const std::string& url,
+                        const std::string& method = std::string()) const {
     if (resources.empty())
       return nullptr;
 
+    std::set<std::string> matching_methods;
+    if (method.empty()) {
+      // Match standard HTTP methods.
+      matching_methods.insert("GET");
+      matching_methods.insert("POST");
+    } else {
+      matching_methods.insert(method);
+    }
+
     const std::string& path_url = test_request::GetPathURL(url);
     ResourceList::const_iterator it = resources.begin();
     for (; it != resources.end(); ++it) {
       Resource* resource = *it;
-      if (resource->GetPathURL() == path_url)
+      if (resource->GetPathURL() == path_url &&
+          (resource->method.empty() ||
+           matching_methods.find(resource->method) != matching_methods.end())) {
         return resource;
       }
+    }
     return nullptr;
   }
 
   Resource* GetResource(CefRefPtr<CefRequest> request) const {
-    return GetResource(request->GetURL());
+    return GetResource(request->GetURL(), request->GetMethod());
   }
 
   // Validate expected initial state.
@@ -319,11 +335,17 @@ class CorsTestHandler : public RoutingTestHandler {
       CefRefPtr<CefFrame> frame,
       CefRefPtr<CefRequest> request) override {
     CEF_REQUIRE_IO_THREAD();
+    const std::string& url = request->GetURL();
+    const std::string& method = request->GetMethod();
+    if (method == "OPTIONS") {
+      // We should never see the CORS preflight request.
+      ADD_FAILURE() << "Unexpected CORS preflight for " << url;
+    }
+
     Resource* resource = setup_->GetResource(request);
     if (resource && resource->handler != HandlerType::SERVER) {
       resource->response_ct++;
-      EXPECT_TRUE(resource->VerifyRequest(request))
+      EXPECT_TRUE(resource->VerifyRequest(request)) << url;
-          << request->GetURL().ToString();
       return test_request::CreateResourceHandler(resource->response,
                                                  resource->response_data);
     }
@@ -812,6 +834,10 @@ CORS_TEST_IFRAME_ALL(AllowScriptsAndSameOrigin,
 
 namespace {
 
+const char kSubRequestMethod[] = "GET";
+const char kSubUnsafeHeaderName[] = "x-unsafe-header";
+const char kSubUnsafeHeaderValue[] = "not-safe";
+
 struct SubResource : CookieResource {
   SubResource() {}
 
@@ -820,6 +846,9 @@ struct SubResource : CookieResource {
   bool is_cross_origin = false;
 
   void InitCors(HandlerType main_handler, bool add_header) {
+    // Must specify the method to differentiate from the preflight request.
+    method = kSubRequestMethod;
+
     // Origin is always "null" for non-standard schemes.
     main_origin = main_handler == HandlerType::CUSTOM_NONSTANDARD_SCHEME
                       ? "null"
@@ -850,12 +879,76 @@ struct SubResource : CookieResource {
     if (!CookieResource::VerifyRequest(request))
       return false;
 
+    const std::string& request_method = request->GetMethod();
+    EXPECT_STREQ(method.c_str(), request_method.c_str()) << GetPathURL();
+    if (request_method != method)
+      return false;
+
     // Verify that the "Origin" header contains the expected value.
     const std::string& origin = request->GetHeaderByName("Origin");
     const std::string& expected_origin =
         is_cross_origin ? main_origin : std::string();
     EXPECT_STREQ(expected_origin.c_str(), origin.c_str()) << GetPathURL();
-    return expected_origin == origin;
+    if (expected_origin != origin)
+      return false;
+
+    // Verify that the "X-Unsafe-Header" header contains the expected value.
+    const std::string& unsafe_header =
+        request->GetHeaderByName(kSubUnsafeHeaderName);
+    EXPECT_STREQ(kSubUnsafeHeaderValue, unsafe_header.c_str()) << GetPathURL();
+    return unsafe_header == kSubUnsafeHeaderValue;
+  }
+};
+
+// See https://developer.mozilla.org/en-US/docs/Glossary/Preflight_request
+// for details of CORS preflight behavior.
+struct PreflightResource : Resource {
+  std::string main_origin;
+
+  void InitPreflight(HandlerType main_handler) {
+    // CORS preflight requests originate from PreflightController in the network
+    // process, so we only expect them for server requests.
+    EXPECT_EQ(HandlerType::SERVER, handler);
+
+    // Origin is always "null" for non-standard schemes.
+    main_origin = main_handler == HandlerType::CUSTOM_NONSTANDARD_SCHEME
+                      ? "null"
+                      : GetOrigin(main_handler);
+
+    method = "OPTIONS";
+    response->SetHeaderByName("Access-Control-Allow-Methods",
+                              "GET,HEAD,OPTIONS,POST", false);
+    response->SetHeaderByName("Access-Control-Allow-Headers",
+                              kSubUnsafeHeaderName, false);
+    response->SetHeaderByName("Access-Control-Allow-Origin", main_origin,
+                              false);
+  }
+
+  bool VerifyRequest(CefRefPtr<CefRequest> request) const override {
+    const std::string& request_method = request->GetMethod();
+    EXPECT_STREQ(method.c_str(), request_method.c_str()) << GetPathURL();
+    if (request_method != method)
+      return false;
+
+    const std::string& origin = request->GetHeaderByName("Origin");
+    EXPECT_STREQ(main_origin.c_str(), origin.c_str()) << GetPathURL();
+    if (main_origin != origin)
+      return false;
+
+    const std::string& ac_request_method =
+        request->GetHeaderByName("Access-Control-Request-Method");
+    EXPECT_STREQ(kSubRequestMethod, ac_request_method.c_str()) << GetPathURL();
+    if (ac_request_method != kSubRequestMethod)
+      return false;
+
+    const std::string& ac_request_headers =
+        request->GetHeaderByName("Access-Control-Request-Headers");
+    EXPECT_STREQ(kSubUnsafeHeaderName, ac_request_headers.c_str())
+        << GetPathURL();
+    if (ac_request_headers != kSubUnsafeHeaderName)
+      return false;
+
+    return true;
   }
 };
 
@@ -865,10 +958,15 @@ enum class ExecMode {
 };
 
 std::string GetXhrExecJS(const std::string& sub_url) {
+  // Inclusion of an unsafe header triggers CORS preflight for cross-origin
+  // requests to the server.
   return "xhr = new XMLHttpRequest();\n"
          "xhr.open(\"GET\", \"" +
          sub_url +
          "\", true)\n;"
+         "xhr.setRequestHeader('" +
+         kSubUnsafeHeaderName + "', '" + kSubUnsafeHeaderValue +
+         "');\n"
          "xhr.onload = function(e) {\n"
          "  if (xhr.readyState === 4) {\n"
          "    if (xhr.status === 200) {\n"
@@ -887,8 +985,16 @@ std::string GetXhrExecJS(const std::string& sub_url) {
 }
 
 std::string GetFetchExecJS(const std::string& sub_url) {
-  return "fetch('" + sub_url +
+  // Inclusion of an unsafe header triggers CORS preflight for cross-origin
-         "')\n"
+  // requests to the server.
+  return std::string() +
+         "let h = new Headers();\n"
+         "h.append('" +
+         kSubUnsafeHeaderName + "', '" + kSubUnsafeHeaderValue +
+         "');\n"
+         "fetch('" +
+         sub_url +
+         "', {headers: h})\n"
          ".then(function(response) {\n"
          "  if (response.status === 200) {\n"
          "      response.text().then(function(text) {\n"
@@ -923,6 +1029,10 @@ std::string GetExecMainHtml(ExecMode mode, const std::string& sub_url) {
 }
 
 // XHR and fetch requests behave the same, except for console message contents.
+// In addition to basic CORS header behaviors and request blocking, this test
+// verifies that CORS preflight requests are sent and received when expected.
+// Since preflight behavior is implemented in the network process we expect it
+// to already have substantial test coverage in Chromium.
 void SetupExecRequest(ExecMode mode,
                       CookieTestSetup* setup,
                       const std::string& test_name,
@@ -930,12 +1040,13 @@ void SetupExecRequest(ExecMode mode,
                       CookieResource* main_resource,
                       HandlerType sub_handler,
                       SubResource* sub_resource,
+                      PreflightResource* preflight_resource,
                       bool add_header) {
   const std::string& base_path = "/" + test_name;
 
   // Expect a single xhr request.
-  sub_resource->Init(sub_handler, base_path + ".sub.txt", kMimeTypeText,
+  const std::string& sub_path = base_path + ".sub.txt";
-                     kDefaultText);
+  sub_resource->Init(sub_handler, sub_path, kMimeTypeText, kDefaultText);
   sub_resource->InitCors(main_handler, add_header);
 
   // Expect a single main frame request.
@@ -945,13 +1056,24 @@ void SetupExecRequest(ExecMode mode,
 
   SetupCookieExpectations(setup, main_resource, sub_resource);
 
+  // Cross-origin requests to a server sub-resource will receive a CORS
+  // preflight request because we add an unsafe header.
+  const bool expect_cors_preflight =
+      sub_resource->is_cross_origin && sub_handler == HandlerType::SERVER;
+
   if (sub_resource->is_cross_origin &&
       (!sub_resource->supports_cors || !add_header)) {
     // Expect the cross-origin XHR to be blocked.
     main_resource->expected_failure_query_ct = 1;
 
     if (sub_resource->supports_cors && !add_header) {
-      // The request supports CORS, but we didn't add the header.
+      // The request supports CORS, but we didn't add the
+      // "Access-Control-Allow-Origin" header.
+      if (!expect_cors_preflight || preflight_resource != nullptr) {
+        // This is the error message when not expecting a CORS preflight
+        // request, or when the preflight request is handled by the server.
+        // Unhandled preflight requests will output a different error message
+        // (see below).
         if (mode == ExecMode::XHR) {
           setup->AddConsoleMessage(
               "Access to XMLHttpRequest at '" + sub_url + "' from origin '" +
@@ -964,11 +1086,13 @@ void SetupExecRequest(ExecMode mode,
               "Access to fetch at '" + sub_url + "' from origin '" +
               sub_resource->main_origin +
               "' has been blocked by CORS policy: No "
-            "'Access-Control-Allow-Origin' header is present on the requested "
+              "'Access-Control-Allow-Origin' header is present on the "
+              "requested "
               "resource. If an opaque response serves your needs, set the "
               "request's mode to 'no-cors' to fetch the resource with CORS "
               "disabled.");
         }
+      }
     } else if (mode == ExecMode::XHR) {
       setup->AddConsoleMessage(
           "Access to XMLHttpRequest at '" + sub_url + "' from origin '" +
@@ -987,6 +1111,45 @@ void SetupExecRequest(ExecMode mode,
 
   setup->AddResource(main_resource);
   setup->AddResource(sub_resource);
+
+  if (expect_cors_preflight) {
+    // Expect a CORS preflight request.
+    if (preflight_resource) {
+      // The server will handle the preflight request. The cross-origin XHR may
+      // still be blocked if the "Access-Control-Allow-Origin" header is missing
+      // (see above).
+      preflight_resource->Init(sub_handler, sub_path, kMimeTypeText,
+                               std::string());
+      preflight_resource->InitPreflight(main_handler);
+      setup->AddResource(preflight_resource);
+    } else {
+      // The server will not handle the preflight request. Expect the
+      // cross-origin XHR to be blocked.
+      main_resource->expected_failure_query_ct = 1;
+      main_resource->expected_success_query_ct = 0;
+      sub_resource->expected_response_ct = 0;
+
+      if (mode == ExecMode::XHR) {
+        setup->AddConsoleMessage(
+            "Access to XMLHttpRequest at '" + sub_url + "' from origin '" +
+            sub_resource->main_origin +
+            "' has been blocked by CORS policy: Response to preflight request "
+            "doesn't pass access control check: No "
+            "'Access-Control-Allow-Origin' header is present on the requested "
+            "resource.");
+      } else {
+        setup->AddConsoleMessage(
+            "Access to fetch at '" + sub_url + "' from origin '" +
+            sub_resource->main_origin +
+            "' has been blocked by CORS policy: Response to preflight request "
+            "doesn't pass access control check: No "
+            "'Access-Control-Allow-Origin' header is present on the requested "
+            "resource. If an opaque response serves your needs, set the "
+            "request's mode to 'no-cors' to fetch the resource with CORS "
+            "disabled.");
+      }
+    }
+  }
 }
 
 }  // namespace
@@ -997,9 +1160,11 @@ void SetupExecRequest(ExecMode mode,
     CookieTestSetup setup;                                              \
     CookieResource resource_main;                                       \
     SubResource resource_sub;                                           \
+    PreflightResource resource_preflight;                               \
     SetupExecRequest(ExecMode::XHR, &setup, "CorsTest.Xhr" #test_name,  \
                      HandlerType::handler_main, &resource_main,         \
-                     HandlerType::handler_sub, &resource_sub, add_header); \
+                     HandlerType::handler_sub, &resource_sub,           \
+                     &resource_preflight, add_header);                  \
     CefRefPtr<CorsTestHandler> handler = new CorsTestHandler(&setup);   \
     handler->ExecuteTest();                                             \
     ReleaseAndWaitForDestructor(handler);                               \
@@ -1044,15 +1209,48 @@ CORS_TEST_XHR_ALL(NoHeader, false)
 // XHR requests with the "Access-Control-Allow-Origin" header.
 CORS_TEST_XHR_ALL(WithHeader, true)
 
+// Like above, but without handling CORS preflight requests.
+#define CORS_TEST_XHR_NO_PREFLIGHT(test_name, handler_main, handler_sub, \
+                                   add_header)                           \
+  TEST(CorsTest, Xhr##test_name) {                                       \
+    CookieTestSetup setup;                                               \
+    CookieResource resource_main;                                        \
+    SubResource resource_sub;                                            \
+    SetupExecRequest(ExecMode::XHR, &setup, "CorsTest.Xhr" #test_name,   \
+                     HandlerType::handler_main, &resource_main,          \
+                     HandlerType::handler_sub, &resource_sub, nullptr,   \
+                     add_header);                                        \
+    CefRefPtr<CorsTestHandler> handler = new CorsTestHandler(&setup);    \
+    handler->ExecuteTest();                                              \
+    ReleaseAndWaitForDestructor(handler);                                \
+  }
+
+#define CORS_TEST_XHR_NO_PREFLIGHT_SERVER(name, add_header)                    \
+  CORS_TEST_XHR_NO_PREFLIGHT(name##ServerToServer, SERVER, SERVER, add_header) \
+  CORS_TEST_XHR_NO_PREFLIGHT(name##HttpSchemeToServer, HTTP_SCHEME, SERVER,    \
+                             add_header)                                       \
+  CORS_TEST_XHR_NO_PREFLIGHT(name##CustomStandardSchemeToServer,               \
+                             CUSTOM_STANDARD_SCHEME, SERVER, add_header)       \
+  CORS_TEST_XHR_NO_PREFLIGHT(name##CustomNonStandardSchemeToServer,            \
+                             CUSTOM_NONSTANDARD_SCHEME, SERVER, add_header)
+
+// XHR requests without the "Access-Control-Allow-Origin" header.
+CORS_TEST_XHR_NO_PREFLIGHT_SERVER(NoHeaderNoPreflight, false)
+
+// XHR requests with the "Access-Control-Allow-Origin" header.
+CORS_TEST_XHR_NO_PREFLIGHT_SERVER(WithHeaderNoPreflight, true)
+
 // Test fetch requests with different origin combinations.
 #define CORS_TEST_FETCH(test_name, handler_main, handler_sub, add_header)  \
   TEST(CorsTest, Fetch##test_name) {                                       \
     CookieTestSetup setup;                                                 \
     CookieResource resource_main;                                          \
     SubResource resource_sub;                                              \
+    PreflightResource resource_preflight;                                  \
     SetupExecRequest(ExecMode::FETCH, &setup, "CorsTest.Fetch" #test_name, \
                      HandlerType::handler_main, &resource_main,            \
-                     HandlerType::handler_sub, &resource_sub, add_header); \
+                     HandlerType::handler_sub, &resource_sub,              \
+                     &resource_preflight, add_header);                     \
     CefRefPtr<CorsTestHandler> handler = new CorsTestHandler(&setup);      \
     handler->ExecuteTest();                                                \
     ReleaseAndWaitForDestructor(handler);                                  \
@@ -1099,6 +1297,38 @@ CORS_TEST_FETCH_ALL(NoHeader, false)
 // Fetch requests with the "Access-Control-Allow-Origin" header.
 CORS_TEST_FETCH_ALL(WithHeader, true)
 
+// Like above, but without handling CORS preflight requests.
+#define CORS_TEST_FETCH_NO_PREFLIGHT(test_name, handler_main, handler_sub, \
+                                     add_header)                           \
+  TEST(CorsTest, Fetch##test_name) {                                       \
+    CookieTestSetup setup;                                                 \
+    CookieResource resource_main;                                          \
+    SubResource resource_sub;                                              \
+    SetupExecRequest(ExecMode::FETCH, &setup, "CorsTest.Fetch" #test_name, \
+                     HandlerType::handler_main, &resource_main,            \
+                     HandlerType::handler_sub, &resource_sub, nullptr,     \
+                     add_header);                                          \
+    CefRefPtr<CorsTestHandler> handler = new CorsTestHandler(&setup);      \
+    handler->ExecuteTest();                                                \
+    ReleaseAndWaitForDestructor(handler);                                  \
+  }
+
+#define CORS_TEST_FETCH_NO_PREFLIGHT_SERVER(name, add_header)                 \
+  CORS_TEST_FETCH_NO_PREFLIGHT(name##ServerToServer, SERVER, SERVER,          \
+                               add_header)                                    \
+  CORS_TEST_FETCH_NO_PREFLIGHT(name##HttpSchemeToServer, HTTP_SCHEME, SERVER, \
+                               add_header)                                    \
+  CORS_TEST_FETCH_NO_PREFLIGHT(name##CustomStandardSchemeToServer,            \
+                               CUSTOM_STANDARD_SCHEME, SERVER, add_header)    \
+  CORS_TEST_FETCH_NO_PREFLIGHT(name##CustomNonStandardSchemeToServer,         \
+                               CUSTOM_NONSTANDARD_SCHEME, SERVER, add_header)
+
+// Fetch requests without the "Access-Control-Allow-Origin" header.
+CORS_TEST_FETCH_NO_PREFLIGHT_SERVER(NoHeaderNoPreflight, false)
+
+// Fetch requests with the "Access-Control-Allow-Origin" header.
+CORS_TEST_FETCH_NO_PREFLIGHT_SERVER(WithHeaderNoPreflight, true)
+
 namespace {
 
 enum class RedirectMode {

tests/ceftests/test_server.cc

@@ -247,13 +247,20 @@ class ServerManager {
     // Use a copy in case |observer_list_| is modified during iteration.
     ObserverList list = observer_list_;
 
+    bool handled = false;
+
     ObserverList::const_iterator it = list.begin();
     for (; it != list.end(); ++it) {
       if ((*it)->OnHttpRequest(server, connection_id, client_address,
                                request)) {
+        handled = true;
         break;
       }
     }
+
+    if (!handled) {
+      server->SendHttp500Response(connection_id, "Unhandled request.");
+    }
   }
 
  private: