Merge proxy request headers correctly if header already exists

This CL fixes the video.testCheckVideoHasViaHeader webdriver test. Chrome-Proxy
header gets set to 'frfr' for first video requests (in resource_multibuffer_data_provider.cc).
So the proxy delegate should merge the headers considering existing values. If a header
already exists they should be combined with using ', '.

Bug: 893728
Change-Id: Icb1eb84ef9af149b6e34e5792ca4a7d079efebed
Reviewed-on: https://chromium-review.googlesource.com/c/1316096Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Eric Roman <eroman@chromium.org>
Commit-Queue: rajendrant <rajendrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605640}

components/data_reduction_proxy/content/common/data_reduction_proxy_url_loader_throttle.cc

@@ -31,7 +31,8 @@ std::vector<DataReductionProxyServer> GetConfiguredDataReductionProxies(
     const network::ResourceResponseHead& response_head) {
   std::vector<DataReductionProxyServer> result;
   if (!proxy_server.is_direct() &&
-      (response_head.headers->HasHeader("Chrome-Proxy") ||
+      ((response_head.headers &&
+        response_head.headers->HasHeader("Chrome-Proxy")) ||
        url.spec().find("Chrome-Proxy") != std::string::npos)) {
     result.push_back(
         DataReductionProxyServer(proxy_server, ProxyServer_ProxyType_CORE));

services/network/network_context_unittest.cc

@@ -4364,8 +4364,7 @@ TEST_F(NetworkContextMockHostTest, CustomProxyAddsHeaders) {
   EXPECT_EQ(client->response_head().proxy_server, proxy_server);
 }
 
-TEST_F(NetworkContextMockHostTest,
-       CustomProxyRequestHeadersOverrideConfigHeaders) {
+TEST_F(NetworkContextMockHostTest, CustomProxyHeadersAreMerged) {
   net::EmbeddedTestServer test_server;
   ASSERT_TRUE(test_server.Start());
 
@@ -4383,14 +4382,16 @@ TEST_F(NetworkContextMockHostTest,
   auto config = mojom::CustomProxyConfig::New();
   net::ProxyServer proxy_server = ConvertToProxyServer(proxy_test_server);
   config->rules.ParseFromString("http=" + proxy_server.ToURI());
-  config->pre_cache_headers.SetHeader("foo", "bad");
-  config->post_cache_headers.SetHeader("bar", "bad");
+  config->pre_cache_headers.SetHeader("foo", "first_foo_key=value1");
+  config->post_cache_headers.SetHeader("bar", "first_bar_key=value2");
   proxy_config_client->OnCustomProxyConfigUpdated(std::move(config));
   scoped_task_environment_.RunUntilIdle();
 
   ResourceRequest request;
-  request.custom_proxy_pre_cache_headers.SetHeader("foo", "foo_value");
-  request.custom_proxy_post_cache_headers.SetHeader("bar", "bar_value");
+  request.custom_proxy_pre_cache_headers.SetHeader("foo",
+                                                   "foo_next_key=value3");
+  request.custom_proxy_post_cache_headers.SetHeader("bar",
+                                                    "bar_next_key=value4");
   request.url = GetURLWithMockHost(test_server, "/echoheader?foo&bar");
   std::unique_ptr<TestURLLoaderClient> client =
       FetchRequest(request, network_context.get());
@@ -4398,7 +4399,10 @@ TEST_F(NetworkContextMockHostTest,
   EXPECT_TRUE(
       mojo::BlockingCopyToString(client->response_body_release(), &response));
 
-  EXPECT_EQ(response, base::JoinString({"bar_value", "foo_value"}, "\n"));
+  EXPECT_EQ(response,
+            base::JoinString({"first_bar_key=value2, bar_next_key=value4",
+                              "first_foo_key=value1, foo_next_key=value3"},
+                             "\n"));
   EXPECT_EQ(client->response_head().proxy_server, proxy_server);
 }
 

services/network/network_service_proxy_delegate.cc

@@ -81,6 +81,20 @@ void AddProxies(const net::ProxyList& proxies,
   }
 }
 
+// Merges headers from |in| to |out|. If the header already exists in |out| they
+// are combined.
+void MergeRequestHeaders(net::HttpRequestHeaders* out,
+                         const net::HttpRequestHeaders& in) {
+  for (net::HttpRequestHeaders::Iterator it(in); it.GetNext();) {
+    std::string old_value;
+    if (out->GetHeader(it.name(), &old_value)) {
+      out->SetHeader(it.name(), old_value + ", " + it.value());
+    } else {
+      out->SetHeader(it.name(), it.value());
+    }
+  }
+}
+
 }  // namespace
 
 NetworkServiceProxyDelegate::NetworkServiceProxyDelegate(
@@ -96,14 +110,14 @@ void NetworkServiceProxyDelegate::OnBeforeStartTransaction(
   if (!MayProxyURL(request->url()))
     return;
 
-  headers->MergeFrom(proxy_config_->pre_cache_headers);
+  MergeRequestHeaders(headers, proxy_config_->pre_cache_headers);
 
   auto* url_loader = URLLoader::ForRequest(*request);
   if (url_loader) {
     if (url_loader->custom_proxy_use_alternate_proxy_list()) {
       should_use_alternate_proxy_list_cache_.Put(request->url().spec(), true);
     }
-    headers->MergeFrom(url_loader->custom_proxy_pre_cache_headers());
+    MergeRequestHeaders(headers, url_loader->custom_proxy_pre_cache_headers());
   }
 }
 
@@ -113,10 +127,11 @@ void NetworkServiceProxyDelegate::OnBeforeSendHeaders(
     net::HttpRequestHeaders* headers) {
   auto* url_loader = URLLoader::ForRequest(*request);
   if (IsInProxyConfig(proxy_info.proxy_server())) {
-    headers->MergeFrom(proxy_config_->post_cache_headers);
+    MergeRequestHeaders(headers, proxy_config_->post_cache_headers);
 
     if (url_loader) {
-      headers->MergeFrom(url_loader->custom_proxy_post_cache_headers());
+      MergeRequestHeaders(headers,
+                          url_loader->custom_proxy_post_cache_headers());
     }
   } else if (MayHaveProxiedURL(request->url())) {
     for (const auto& kv : proxy_config_->pre_cache_headers.GetHeaderVector()) {