Remove custom per-request user-agent to HTTP proxies for CONNECTs.

When establishing an HTTPS connection over an HTTP/HTTPS/H2/QUIC proxy,
used the global User-Agent headers with the CONNECT request, instead of
using the custom User-Agent string associated with the original request.

Because of late binding, preconnects, and socket reuse, we could never
guarantee we'd actually use a tunnel creating using a particular
request's User-Agent header, and it's also a bit weird to have a side
channel between websites and the proxy.

The intent to deprecate and remove thread has some more details:
https://groups.google.com/a/chromium.org/forum/?utm_medium=email&utm_source=footer#!msg/blink-dev/Bm40gPAv4RE/Yt6r27pOBQAJ

TBR=seantopping@chromium.org

Bug: 934325
Change-Id: I54b41a965b28df3a70bbe0c4c6ede3e709f7d548
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1487890
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Reviewed-by: Misha Efimov <mef@chromium.org>
Reviewed-by: Richard Coles <torne@chromium.org>
Reviewed-by: Eric Roman <eroman@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#638981}

android_webview/browser/net/aw_http_user_agent_settings.h

@@ -9,7 +9,7 @@
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
-#include "net/url_request/http_user_agent_settings.h"
+#include "net/base/http_user_agent_settings.h"
 
 namespace android_webview {
 

chromecast/browser/cast_http_user_agent_settings.h

@@ -9,7 +9,7 @@
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
-#include "net/url_request/http_user_agent_settings.h"
+#include "net/base/http_user_agent_settings.h"
 
 namespace chromecast {
 namespace shell {

components/cronet/android/cronet_bidirectional_stream_adapter.cc

@@ -17,6 +17,7 @@
 #include "components/cronet/android/url_request_error.h"
 #include "components/cronet/metrics_util.h"
 #include "jni/CronetBidirectionalStream_jni.h"
+#include "net/base/http_user_agent_settings.h"
 #include "net/base/net_errors.h"
 #include "net/base/request_priority.h"
 #include "net/http/bidirectional_stream_request_info.h"
@@ -28,7 +29,6 @@
 #include "net/ssl/ssl_info.h"
 #include "net/third_party/quic/core/quic_packets.h"
 #include "net/third_party/quiche/src/spdy/core/spdy_header_block.h"
-#include "net/url_request/http_user_agent_settings.h"
 #include "net/url_request/url_request_context.h"
 #include "url/gurl.h"
 

components/cronet/ios/cronet_environment.mm

@@ -30,6 +30,7 @@
 #include "ios/web/public/global_state/ios_global_state.h"
 #include "ios/web/public/global_state/ios_global_state_configuration.h"
 #include "ios/web/public/user_agent.h"
+#include "net/base/http_user_agent_settings.h"
 #include "net/base/network_change_notifier.h"
 #include "net/base/url_util.h"
 #include "net/cert/cert_verifier.h"
@@ -48,7 +49,6 @@
 #include "net/ssl/channel_id_service.h"
 #include "net/ssl/ssl_key_logger_impl.h"
 #include "net/third_party/quic/core/quic_versions.h"
-#include "net/url_request/http_user_agent_settings.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_builder.h"
 #include "net/url_request/url_request_context_storage.h"

components/cronet/url_request_context_config_unittest.cc

@@ -15,6 +15,7 @@
 #include "base/values.h"
 #include "build/build_config.h"
 #include "net/base/host_port_pair.h"
+#include "net/base/http_user_agent_settings.h"
 #include "net/base/net_errors.h"
 #include "net/cert/cert_verifier.h"
 #include "net/dns/host_resolver.h"
@@ -23,7 +24,6 @@
 #include "net/log/net_log_with_source.h"
 #include "net/proxy_resolution/proxy_config.h"
 #include "net/proxy_resolution/proxy_config_service_fixed.h"
-#include "net/url_request/http_user_agent_settings.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_builder.h"
 #include "testing/gtest/include/gtest/gtest.h"

components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.cc

@@ -31,8 +31,6 @@
 #include "components/data_reduction_proxy/core/common/data_reduction_proxy_throttle_manager.h"
 #include "net/base/load_flags.h"
 #include "net/http/http_request_headers.h"
-#include "net/url_request/http_user_agent_settings.h"
-#include "net/url_request/static_http_user_agent_settings.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_builder.h"
 #include "net/url_request/url_request_context_getter.h"

components/grpc_support/bidirectional_stream.cc

@@ -15,6 +15,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/string_number_conversions.h"
+#include "net/base/http_user_agent_settings.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
 #include "net/base/request_priority.h"
@@ -27,7 +28,6 @@
 #include "net/http/http_util.h"
 #include "net/ssl/ssl_info.h"
 #include "net/third_party/quiche/src/spdy/core/spdy_header_block.h"
-#include "net/url_request/http_user_agent_settings.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "url/gurl.h"

components/grpc_support/bidirectional_stream_c.cc

@@ -31,7 +31,6 @@
 #include "net/http/http_util.h"
 #include "net/ssl/ssl_info.h"
 #include "net/third_party/quiche/src/spdy/core/spdy_header_block.h"
-#include "net/url_request/http_user_agent_settings.h"
 #include "net/url_request/url_request_context.h"
 #include "url/gurl.h"
 

content/browser/net/reporting_service_proxy.cc

@@ -18,7 +18,6 @@
 #include "mojo/public/cpp/bindings/strong_binding.h"
 #include "net/reporting/reporting_report.h"
 #include "net/reporting/reporting_service.h"
-#include "net/url_request/http_user_agent_settings.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "services/network/public/mojom/network_context.mojom.h"

ios/chrome/browser/net/ios_chrome_http_user_agent_settings.h

@@ -10,7 +10,7 @@
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "components/prefs/pref_member.h"
-#include "net/url_request/http_user_agent_settings.h"
+#include "net/base/http_user_agent_settings.h"
 
 class PrefService;
 

net/BUILD.gn

@@ -454,6 +454,7 @@ component("net") {
       "base/hex_utils.h",
       "base/host_mapping_rules.cc",
       "base/host_mapping_rules.h",
+      "base/http_user_agent_settings.h",
       "base/ip_pattern.cc",
       "base/ip_pattern.h",
       "base/layered_network_delegate.cc",
@@ -1719,7 +1720,6 @@ component("net") {
       "third_party/quiche/src/spdy/platform/api/spdy_unsafe_arena.h",
       "url_request/data_protocol_handler.cc",
       "url_request/data_protocol_handler.h",
-      "url_request/http_user_agent_settings.h",
       "url_request/redirect_info.cc",
       "url_request/redirect_info.h",
       "url_request/redirect_util.cc",

net/url_request/http_user_agent_settings.h → net/base/http_user_agent_settings.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef NET_URL_REQUEST_HTTP_USER_AGENT_SETTINGS_H_
-#define NET_URL_REQUEST_HTTP_USER_AGENT_SETTINGS_H_
+#ifndef NET_BASE_HTTP_USER_AGENT_SETTINGS_H_
+#define NET_BASE_HTTP_USER_AGENT_SETTINGS_H_
 
 #include <string>
 
@@ -31,4 +31,4 @@ class NET_EXPORT HttpUserAgentSettings {
 
 }  // namespace net
 
-#endif  // NET_URL_REQUEST_HTTP_USER_AGENT_SETTINGS_H_
+#endif  // NET_BASE_HTTP_USER_AGENT_SETTINGS_H_

net/http/http_network_session.cc

@@ -59,7 +59,8 @@ std::unique_ptr<ClientSocketPoolManager> CreateSocketPoolManager(
       context.transport_security_state, context.cert_transparency_verifier,
       context.ct_policy_enforcer, ssl_client_session_cache,
       ssl_client_session_cache_privacy_mode, context.ssl_config_service,
-      websocket_endpoint_lock_manager, context.proxy_delegate, pool_type);
+      websocket_endpoint_lock_manager, context.proxy_delegate,
+      context.http_user_agent_settings, pool_type);
 }
 
 }  // unnamed namespace
@@ -165,6 +166,7 @@ HttpNetworkSession::Context::Context()
       ct_policy_enforcer(nullptr),
       proxy_resolution_service(nullptr),
       proxy_delegate(nullptr),
+      http_user_agent_settings(nullptr),
       ssl_config_service(nullptr),
       http_auth_handler_factory(nullptr),
       net_log(nullptr),

net/http/http_network_session.h

@@ -61,6 +61,7 @@ class HttpAuthHandlerFactory;
 class HttpNetworkSessionPeer;
 class HttpResponseBodyDrainer;
 class HttpServerProperties;
+class HttpUserAgentSettings;
 class NetLog;
 #if BUILDFLAG(ENABLE_REPORTING)
 class NetworkErrorLoggingService;
@@ -257,6 +258,7 @@ class NET_EXPORT HttpNetworkSession {
     CTPolicyEnforcer* ct_policy_enforcer;
     ProxyResolutionService* proxy_resolution_service;
     ProxyDelegate* proxy_delegate;
+    const HttpUserAgentSettings* http_user_agent_settings;
     SSLConfigService* ssl_config_service;
     HttpAuthHandlerFactory* http_auth_handler_factory;
     HttpServerProperties* http_server_properties;

net/http/http_network_transaction_unittest.cc

@@ -27,6 +27,7 @@
 #include "base/stl_util.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_task_environment.h"
@@ -103,6 +104,7 @@
 #include "net/test/test_with_scoped_task_environment.h"
 #include "net/third_party/quiche/src/spdy/core/spdy_framer.h"
 #include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
+#include "net/url_request/static_http_user_agent_settings.h"
 #include "net/websockets/websocket_handshake_stream_base.h"
 #include "net/websockets/websocket_test_util.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -708,6 +710,7 @@ CaptureGroupNameSocketPool<ParentPool>::CaptureGroupNameSocketPool(
                  NULL,
                  NULL,
                  NULL,
+                 NULL,
                  NULL) {}
 
 //-----------------------------------------------------------------------------
@@ -10482,43 +10485,73 @@ TEST_F(HttpNetworkTransactionTest, BuildRequest_UserAgent) {
 }
 
 TEST_F(HttpNetworkTransactionTest, BuildRequest_UserAgentOverTunnel) {
-  HttpRequestInfo request;
-  request.method = "GET";
-  request.url = GURL("https://www.example.org/");
-  request.extra_headers.SetHeader(HttpRequestHeaders::kUserAgent,
-                                  "Chromium Ultra Awesome X Edition");
-  request.traffic_annotation =
-      net::MutableNetworkTrafficAnnotationTag(TRAFFIC_ANNOTATION_FOR_TESTS);
+  // Test user agent values, used both for the request header of the original
+  // request, and the value returned by the HttpUserAgentSettings. nullptr means
+  // no request header / no HttpUserAgentSettings object.
+  const char* kTestUserAgents[] = {nullptr, "", "Foopy"};
+
+  for (const char* setting_user_agent : kTestUserAgents) {
+    if (!setting_user_agent) {
+      session_deps_.http_user_agent_settings.reset();
+    } else {
+      session_deps_.http_user_agent_settings =
+          std::make_unique<StaticHttpUserAgentSettings>(
+              std::string() /* accept-language */, setting_user_agent);
+    }
+    session_deps_.proxy_resolution_service =
+        ProxyResolutionService::CreateFixed("myproxy:70",
+                                            TRAFFIC_ANNOTATION_FOR_TESTS);
+    std::unique_ptr<HttpNetworkSession> session(CreateSession(&session_deps_));
+    for (const char* request_user_agent : kTestUserAgents) {
+      HttpRequestInfo request;
+      request.method = "GET";
+      request.url = GURL("https://www.example.org/");
+      if (request_user_agent) {
+        request.extra_headers.SetHeader(HttpRequestHeaders::kUserAgent,
+                                        request_user_agent);
+      }
+      request.traffic_annotation =
+          net::MutableNetworkTrafficAnnotationTag(TRAFFIC_ANNOTATION_FOR_TESTS);
 
-  session_deps_.proxy_resolution_service = ProxyResolutionService::CreateFixed(
-      "myproxy:70", TRAFFIC_ANNOTATION_FOR_TESTS);
-  std::unique_ptr<HttpNetworkSession> session(CreateSession(&session_deps_));
-  HttpNetworkTransaction trans(DEFAULT_PRIORITY, session.get());
+      HttpNetworkTransaction trans(DEFAULT_PRIORITY, session.get());
 
-  MockWrite data_writes[] = {
-      MockWrite("CONNECT www.example.org:443 HTTP/1.1\r\n"
-                "Host: www.example.org:443\r\n"
-                "Proxy-Connection: keep-alive\r\n"
-                "User-Agent: Chromium Ultra Awesome X Edition\r\n\r\n"),
-  };
-  MockRead data_reads[] = {
-    // Return an error, so the transaction stops here (this test isn't
-    // interested in the rest).
-    MockRead("HTTP/1.1 407 Proxy Authentication Required\r\n"),
-    MockRead("Proxy-Authenticate: Basic realm=\"MyRealm1\"\r\n"),
-    MockRead("Proxy-Connection: close\r\n\r\n"),
-  };
-
-  StaticSocketDataProvider data(data_reads, data_writes);
-  session_deps_.socket_factory->AddSocketDataProvider(&data);
+      std::string expected_request;
+      if (!setting_user_agent || strlen(setting_user_agent) == 0) {
+        expected_request =
+            "CONNECT www.example.org:443 HTTP/1.1\r\n"
+            "Host: www.example.org:443\r\n"
+            "Proxy-Connection: keep-alive\r\n\r\n";
+      } else {
+        expected_request = base::StringPrintf(
+            "CONNECT www.example.org:443 HTTP/1.1\r\n"
+            "Host: www.example.org:443\r\n"
+            "Proxy-Connection: keep-alive\r\n"
+            "User-Agent: %s\r\n\r\n",
+            setting_user_agent);
+      }
+      MockWrite data_writes[] = {
+          MockWrite(expected_request.c_str()),
+      };
+      MockRead data_reads[] = {
+          // Return an error, so the transaction stops here (this test isn't
+          // interested in the rest).
+          MockRead("HTTP/1.1 407 Proxy Authentication Required\r\n"),
+          MockRead("Proxy-Authenticate: Basic realm=\"MyRealm1\"\r\n"),
+          MockRead("Proxy-Connection: close\r\n\r\n"),
+      };
+
+      StaticSocketDataProvider data(data_reads, data_writes);
+      session_deps_.socket_factory->AddSocketDataProvider(&data);
 
-  TestCompletionCallback callback;
+      TestCompletionCallback callback;
 
-  int rv = trans.Start(&request, callback.callback(), NetLogWithSource());
-  EXPECT_THAT(rv, IsError(ERR_IO_PENDING));
+      int rv = trans.Start(&request, callback.callback(), NetLogWithSource());
+      EXPECT_THAT(rv, IsError(ERR_IO_PENDING));
 
-  rv = callback.WaitForResult();
-  EXPECT_THAT(rv, IsOk());
+      rv = callback.WaitForResult();
+      EXPECT_THAT(rv, IsOk());
+    }
+  }
 }
 
 TEST_F(HttpNetworkTransactionTest, BuildRequest_Referer) {
@@ -14374,8 +14407,8 @@ TEST_F(HttpNetworkTransactionTest, MultiRoundAuth) {
       1,                                 // Max sockets per group
       base::TimeDelta::FromSeconds(10),  // unused_idle_socket_timeout
       session_deps_.socket_factory.get(), session_deps_.host_resolver.get(),
-      nullptr /* proxy_delegate */, session_deps_.cert_verifier.get(),
-      session_deps_.channel_id_service.get(),
+      nullptr /* proxy_delegate */, nullptr /* http_user_agent_settings */,
+      session_deps_.cert_verifier.get(), session_deps_.channel_id_service.get(),
       session_deps_.transport_security_state.get(),
       session_deps_.cert_transparency_verifier.get(),
       session_deps_.ct_policy_enforcer.get(),

net/http/http_proxy_client_socket_pool_unittest.cc

@@ -79,6 +79,7 @@ class HttpProxyClientSocketPoolTest
             &socket_factory_,
             session_deps_.host_resolver.get(),
             nullptr /* proxy_delegate */,
+            nullptr /* http_user_agent_settings */,
             session_deps_.cert_verifier.get(),
             session_deps_.channel_id_service.get(),
             session_deps_.transport_security_state.get(),
@@ -100,6 +101,7 @@ class HttpProxyClientSocketPoolTest
     pool_ = std::make_unique<TransportClientSocketPool>(
         kMaxSockets, kMaxSocketsPerGroup, kUnusedIdleSocketTimeout,
         &socket_factory_, session_deps_.host_resolver.get(), proxy_delegate,
+        nullptr /* http_user_agent_settings */,
         session_deps_.cert_verifier.get(),
         session_deps_.channel_id_service.get(),
         session_deps_.transport_security_state.get(),
@@ -151,7 +153,7 @@ class HttpProxyClientSocketPoolTest
         CreateFromHttpProxySocketParams(
             base::MakeRefCounted<HttpProxySocketParams>(
                 CreateHttpProxyParams(), CreateHttpsProxyParams(),
-                quic::QUIC_VERSION_UNSUPPORTED, std::string(),
+                quic::QUIC_VERSION_UNSUPPORTED,
                 HostPortPair("www.google.com", tunnel ? 443 : 80),
                 session_->http_auth_cache(),
                 session_->http_auth_handler_factory(),