Revert "Revert "CORS check for prefetched subresource signed exchanges""

This reverts commit 6d9fb7a5.

Reason for revert: Fixed test

Initial CL: https://crrev.com/c/1616937
Revert CL: https://crrev.com/c/1624588

The original CL was reverted due to the data race bug in test.

Change-Id: I7280e271f4739da54b9fcf5e022b3a57256b05ee
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1625026
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Reviewed-by: Lowell Manners <lowell@google.com>
Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#662520}

content/browser/loader/prefetched_signed_exchange_cache.cc

@@ -16,6 +16,7 @@
 #include "net/http/http_util.h"
 #include "net/url_request/redirect_util.h"
 #include "services/network/public/cpp/constants.h"
+#include "services/network/public/cpp/cors/cors.h"
 #include "services/network/public/cpp/features.h"
 #include "services/network/public/cpp/resource_request.h"
 #include "services/network/public/cpp/resource_response.h"
@@ -116,6 +117,7 @@ class RedirectResponseURLLoader : public network::mojom::URLLoader {
 class InnerResponseURLLoader : public network::mojom::URLLoader {
  public:
   InnerResponseURLLoader(
+      const network::ResourceRequest& request,
       const network::ResourceResponseHead& inner_response,
       std::unique_ptr<const storage::BlobDataHandle> blob_data_handle,
       const network::URLLoaderCompletionStatus& completion_status,
@@ -125,6 +127,25 @@ class InnerResponseURLLoader : public network::mojom::URLLoader {
         completion_status_(completion_status),
         client_(std::move(client)),
         weak_factory_(this) {
+    DCHECK(inner_response.headers);
+    DCHECK(request.request_initiator);
+    if (network::cors::ShouldCheckCors(request.url, request.request_initiator,
+                                       request.fetch_request_mode)) {
+      const auto error_status = network::cors::CheckAccess(
+          request.url, inner_response.headers->response_code(),
+          GetHeaderString(
+              inner_response,
+              network::cors::header_names::kAccessControlAllowOrigin),
+          GetHeaderString(
+              inner_response,
+              network::cors::header_names::kAccessControlAllowCredentials),
+          request.fetch_credentials_mode, *request.request_initiator);
+      if (error_status) {
+        client_->OnComplete(network::URLLoaderCompletionStatus(*error_status));
+        return;
+      }
+    }
+
     network::ResourceResponseHead response = inner_response;
     UpdateRequestResponseStartTime(&response);
     response.encoded_data_length = 0;
@@ -141,6 +162,16 @@ class InnerResponseURLLoader : public network::mojom::URLLoader {
   ~InnerResponseURLLoader() override {}
 
  private:
+  static base::Optional<std::string> GetHeaderString(
+      const network::ResourceResponseHead& response,
+      const std::string& header_name) {
+    DCHECK(response.headers);
+    std::string header_value;
+    if (!response.headers->GetNormalizedHeader(header_name, &header_value))
+      return base::nullopt;
+    return header_value;
+  }
+
   // network::mojom::URLLoader overrides:
   void FollowRedirect(const std::vector<std::string>& removed_headers,
                       const net::HttpRequestHeaders& modified_headers,
@@ -238,10 +269,9 @@ class SubresourceSignedExchangeURLLoaderFactory
                             network::mojom::URLLoaderClientPtr client,
                             const net::MutableNetworkTrafficAnnotationTag&
                                 traffic_annotation) override {
-    // TODO(crbug.com/935267): Implement CORS check.
     DCHECK_EQ(request.url, entry_->inner_url());
     mojo::MakeStrongBinding(std::make_unique<InnerResponseURLLoader>(
-                                *entry_->inner_response(),
+                                request, *entry_->inner_response(),
                                 std::make_unique<const storage::BlobDataHandle>(
                                     *entry_->blob_data_handle()),
                                 *entry_->completion_status(), std::move(client),
@@ -340,7 +370,7 @@ class PrefetchedNavigationLoaderInterceptor
                           network::mojom::URLLoaderClientPtr client) {
     mojo::MakeStrongBinding(
         std::make_unique<InnerResponseURLLoader>(
-            *exchange_->inner_response(),
+            resource_request, *exchange_->inner_response(),
             std::make_unique<const storage::BlobDataHandle>(
                 *exchange_->blob_data_handle()),
             *exchange_->completion_status(), std::move(client),

content/browser/loader/prefetched_signed_exchange_cache.h

@@ -38,7 +38,7 @@ class NavigationLoaderInterceptor;
 class CONTENT_EXPORT PrefetchedSignedExchangeCache
     : public base::RefCountedThreadSafe<PrefetchedSignedExchangeCache> {
  public:
-  class Entry {
+  class CONTENT_EXPORT Entry {
    public:
     Entry();
     ~Entry();

services/network/cors/cors_url_loader.cc

@@ -497,18 +497,11 @@ void CorsURLLoader::SetCorsFlagIfNeeded() {
   if (fetch_cors_flag_)
     return;
 
-  if (request_.fetch_request_mode == mojom::FetchRequestMode::kNavigate ||
-      request_.fetch_request_mode == mojom::FetchRequestMode::kNoCors) {
+  if (!network::cors::ShouldCheckCors(request_.url, request_.request_initiator,
+                                      request_.fetch_request_mode)) {
     return;
   }
 
-  if (request_.url.SchemeIs(url::kDataScheme))
-    return;
-
-  // CORS needs a proper origin (including a unique opaque origin). If the
-  // request doesn't have one, CORS should not work.
-  DCHECK(request_.request_initiator);
-
   // The source origin and destination URL pair may be in the allow list.
   switch (origin_access_list_->CheckAccessState(*request_.request_initiator,
                                                 request_.url)) {
@@ -541,11 +534,6 @@ void CorsURLLoader::SetCorsFlagIfNeeded() {
     return;
   }
 
-  if (request_.request_initiator->IsSameOriginWith(
-          url::Origin::Create(request_.url))) {
-    return;
-  }
-
   fetch_cors_flag_ = true;
 }
 

services/network/public/cpp/cors/cors.cc

@@ -224,6 +224,28 @@ base::Optional<CorsErrorStatus> CheckAccess(
   return base::nullopt;
 }
 
+bool ShouldCheckCors(const GURL& request_url,
+                     const base::Optional<url::Origin>& request_initiator,
+                     mojom::FetchRequestMode fetch_request_mode) {
+  if (fetch_request_mode == network::mojom::FetchRequestMode::kNavigate ||
+      fetch_request_mode == network::mojom::FetchRequestMode::kNoCors) {
+    return false;
+  }
+
+  // CORS needs a proper origin (including a unique opaque origin). If the
+  // request doesn't have one, CORS should not work.
+  DCHECK(request_initiator);
+
+  // TODO(crbug.com/870173): Remove following scheme check once the network
+  // service is fully enabled.
+  if (request_url.SchemeIs(url::kDataScheme))
+    return false;
+
+  if (request_initiator->IsSameOriginWith(url::Origin::Create(request_url)))
+    return false;
+  return true;
+}
+
 base::Optional<CorsErrorStatus> CheckPreflightAccess(
     const GURL& response_url,
     const int response_status_code,

services/network/public/cpp/cors/cors.h

@@ -58,6 +58,14 @@ base::Optional<CorsErrorStatus> CheckAccess(
     mojom::FetchCredentialsMode credentials_mode,
     const url::Origin& origin);
 
+// Returns true if |fetch_request_mode| is not kNavigate nor kNoCors, and the
+// origin of |request_url| is not a data URL, and |request_initiator| is not
+// same as the origin of |request_url|,
+COMPONENT_EXPORT(NETWORK_CPP)
+bool ShouldCheckCors(const GURL& request_url,
+                     const base::Optional<url::Origin>& request_initiator,
+                     mojom::FetchRequestMode fetch_request_mode);
+
 // Performs a CORS access check on the CORS-preflight response parameters.
 // According to the note at https://fetch.spec.whatwg.org/#cors-preflight-fetch
 // step 6, even for a preflight check, |credentials_mode| should be checked on