OOR-CORS: Make VariationsHttpHeadersBrowserTest pass with kOutOfBlinkCORS

Chrome internally uses X-Client-Data header, and this should not
triggers CORS preflight request.

TBR=jochen@chromium.org

Bug: 870173, 907018
Change-Id: I67f1711b9065223f9e174e207980940e175031e9
Reviewed-on: https://chromium-review.googlesource.com/c/1335076
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610313}

chrome/common/google_url_loader_throttle.cc

@@ -28,8 +28,7 @@ void GoogleURLLoaderThrottle::WillStartRequest(
   if (!is_off_the_record_ &&
       variations::ShouldAppendVariationHeaders(request->url) &&
       !dynamic_params_.variation_ids_header.empty()) {
-    request->headers.SetHeaderIfMissing(variations::kClientDataHeader,
-                                        dynamic_params_.variation_ids_header);
+    request->client_data_header = dynamic_params_.variation_ids_header;
   }
 
   if (dynamic_params_.force_safe_search) {

content/browser/loader/resource_dispatcher_host_impl.cc

@@ -871,10 +871,16 @@ void ResourceDispatcherHostImpl::ContinuePendingBeginRequest(
   new_request->set_referrer_policy(request_data.referrer_policy);
 
   new_request->SetExtraRequestHeaders(headers);
-  if (!request_data.requested_with.empty()) {
-    // X-Requested-With header must be set here to avoid breaking CORS checks.
-    new_request->SetExtraRequestHeaderByName("X-Requested-With",
-                                             request_data.requested_with, true);
+  // X-Requested-With and X-Client-Data header must be set here to avoid
+  // breaking CORS checks. They are non-empty when the values are given by the
+  // UA code, therefore they should be ignored by CORS checks.
+  if (!request_data.requested_with_header.empty()) {
+    new_request->SetExtraRequestHeaderByName(
+        "X-Requested-With", request_data.requested_with_header, true);
+  }
+  if (!request_data.client_data_header.empty()) {
+    new_request->SetExtraRequestHeaderByName(
+        "X-Client-Data", request_data.client_data_header, true);
   }
 
   std::unique_ptr<network::ScopedThrottlingToken> throttling_token =

content/renderer/loader/web_url_loader_impl.cc

@@ -681,8 +681,10 @@ void WebURLLoaderImpl::Context::Start(const WebURLRequest& request,
     resource_request->headers.SetHeaderIfMissing(network::kAcceptHeader,
                                                  network::kDefaultAcceptHeader);
   }
-  resource_request->requested_with =
-      WebString(request.GetRequestedWith()).Utf8();
+  resource_request->requested_with_header =
+      WebString(request.GetRequestedWithHeader()).Utf8();
+  resource_request->client_data_header =
+      WebString(request.GetClientDataHeader()).Utf8();
 
   if (resource_request->resource_type == RESOURCE_TYPE_PREFETCH ||
       resource_request->resource_type == RESOURCE_TYPE_FAVICON) {

content/renderer/pepper/url_request_info_util.cc

@@ -239,7 +239,7 @@ bool CreateWebURLRequest(PP_Instance instance,
   }
 
   if (!name_version.empty())
-    dest->SetRequestedWith(WebString::FromUTF8(name_version));
+    dest->SetRequestedWithHeader(WebString::FromUTF8(name_version));
 
   if (data->has_custom_user_agent) {
     auto extra_data = std::make_unique<RequestExtraData>();

services/network/public/cpp/network_ipc_param_traits.h

@@ -146,7 +146,8 @@ IPC_STRUCT_TRAITS_BEGIN(network::ResourceRequest)
   IPC_STRUCT_TRAITS_MEMBER(referrer_policy)
   IPC_STRUCT_TRAITS_MEMBER(is_prerendering)
   IPC_STRUCT_TRAITS_MEMBER(headers)
-  IPC_STRUCT_TRAITS_MEMBER(requested_with)
+  IPC_STRUCT_TRAITS_MEMBER(requested_with_header)
+  IPC_STRUCT_TRAITS_MEMBER(client_data_header)
   IPC_STRUCT_TRAITS_MEMBER(load_flags)
   IPC_STRUCT_TRAITS_MEMBER(allow_credentials)
   IPC_STRUCT_TRAITS_MEMBER(plugin_child_id)

services/network/public/cpp/resource_request.h

@@ -75,7 +75,14 @@ struct COMPONENT_EXPORT(NETWORK_CPP_BASE) ResourceRequest {
   // Network Service), so the value is stored here (rather than in |headers|)
   // and later populated in the headers after CORS check.
   // TODO(toyoshim): Remove it once PPAPI is deprecated.
-  std::string requested_with;
+  std::string requested_with_header;
+
+  // 'X-Client-Data' header value. See comments for |requested_with_header|
+  // above, too.
+  // TODO(toyoshim): Consider to rename this to have a chrome specific prefix
+  // such as 'Chrome-' instead of 'X-', and to add 'Chrome-' prefixed header
+  // names into the forbidden header name list.
+  std::string client_data_header;
 
   // net::URLRequest load flags (0 by default).
   int load_flags = 0;

services/network/url_loader.cc

@@ -371,10 +371,16 @@ URLLoader::URLLoader(
   url_request_->SetReferrer(ComputeReferrer(request.referrer));
   url_request_->set_referrer_policy(request.referrer_policy);
   url_request_->SetExtraRequestHeaders(request.headers);
-  if (!request.requested_with.empty()) {
-    // X-Requested-With header must be set here to avoid breaking CORS checks.
-    url_request_->SetExtraRequestHeaderByName("X-Requested-With",
-                                              request.requested_with, true);
+  // X-Requested-With and X-Client-Data header must be set here to avoid
+  // breaking CORS checks. They are non-empty when the values are given by the
+  // UA code, therefore they should be ignored by CORS checks.
+  if (!request.requested_with_header.empty()) {
+    url_request_->SetExtraRequestHeaderByName(
+        "X-Requested-With", request.requested_with_header, true);
+  }
+  if (!request.client_data_header.empty()) {
+    url_request_->SetExtraRequestHeaderByName("X-Client-Data",
+                                              request.client_data_header, true);
   }
   url_request_->set_upgrade_if_insecure(request.upgrade_if_insecure);
 

third_party/blink/public/platform/web_url_request.h

@@ -326,8 +326,14 @@ class WebURLRequest {
   // Remembers 'X-Requested-With' header value. Blink should not set this header
   // value until CORS checks are done to avoid running checks even against
   // headers that are internally set.
-  BLINK_PLATFORM_EXPORT const WebString GetRequestedWith() const;
-  BLINK_PLATFORM_EXPORT void SetRequestedWith(const WebString&);
+  BLINK_PLATFORM_EXPORT const WebString GetRequestedWithHeader() const;
+  BLINK_PLATFORM_EXPORT void SetRequestedWithHeader(const WebString&);
+
+  // Remembers 'X-Client-Data' header value. Blink should not set this header
+  // value until CORS checks are done to avoid running checks even against
+  // headers that are internally set.
+  BLINK_PLATFORM_EXPORT const WebString GetClientDataHeader() const;
+  BLINK_PLATFORM_EXPORT void SetClientDataHeader(const WebString&);
 
   // https://fetch.spec.whatwg.org/#concept-request-window
   // See network::ResourceRequest::fetch_window_id for details.

third_party/blink/renderer/platform/exported/web_url_request.cc

@@ -434,12 +434,20 @@ void WebURLRequest::SetOriginPolicy(const WebString& policy) {
   resource_request_->SetOriginPolicy(policy);
 }
 
-const WebString WebURLRequest::GetRequestedWith() const {
-  return resource_request_->GetRequestedWith();
+const WebString WebURLRequest::GetRequestedWithHeader() const {
+  return resource_request_->GetRequestedWithHeader();
 }
 
-void WebURLRequest::SetRequestedWith(const WebString& value) {
-  resource_request_->SetRequestedWith(value);
+void WebURLRequest::SetRequestedWithHeader(const WebString& value) {
+  resource_request_->SetRequestedWithHeader(value);
+}
+
+const WebString WebURLRequest::GetClientDataHeader() const {
+  return resource_request_->GetClientDataHeader();
+}
+
+void WebURLRequest::SetClientDataHeader(const WebString& value) {
+  resource_request_->SetClientDataHeader(value);
 }
 
 const base::UnguessableToken& WebURLRequest::GetFetchWindowId() const {

third_party/blink/renderer/platform/loader/fetch/resource_request.cc

@@ -130,7 +130,8 @@ std::unique_ptr<ResourceRequest> ResourceRequest::CreateRedirectRequest(
   request->SetInitiatorCSP(GetInitiatorCSP());
   request->SetUpgradeIfInsecure(UpgradeIfInsecure());
   request->SetIsAutomaticUpgrade(IsAutomaticUpgrade());
-  request->SetRequestedWith(GetRequestedWith());
+  request->SetRequestedWithHeader(GetRequestedWithHeader());
+  request->SetClientDataHeader(GetClientDataHeader());
   request->SetUkmSourceId(GetUkmSourceId());
 
   return request;

third_party/blink/renderer/platform/loader/fetch/resource_request.h

@@ -402,8 +402,15 @@ class PLATFORM_EXPORT ResourceRequest final {
   void SetOriginPolicy(const String& policy) { origin_policy_ = policy; }
   const String& GetOriginPolicy() const { return origin_policy_; }
 
-  void SetRequestedWith(const String& value) { requested_with_ = value; }
-  const String& GetRequestedWith() const { return requested_with_; }
+  void SetRequestedWithHeader(const String& value) {
+    requested_with_header_ = value;
+  }
+  const String& GetRequestedWithHeader() const {
+    return requested_with_header_;
+  }
+
+  void SetClientDataHeader(const String& value) { client_data_header_ = value; }
+  const String& GetClientDataHeader() const { return client_data_header_; }
 
   void SetUkmSourceId(int64_t ukm_source_id) { ukm_source_id_ = ukm_source_id; }
   int64_t GetUkmSourceId() const { return ukm_source_id_; }
@@ -490,7 +497,8 @@ class PLATFORM_EXPORT ResourceRequest final {
 
   base::Optional<base::UnguessableToken> devtools_token_;
   String origin_policy_;
-  String requested_with_;
+  String requested_with_header_;
+  String client_data_header_;
 
   int64_t ukm_source_id_;