Update ProxyDelegate to handle proxy chains

ProxyDelegate handles both proxy failure, and headers sent to specific
proxies. Instead of getting a ProxyServer, it now gets a ProxyChain with
an index indicating the specific server involved. This allows
implementers to recognize either the full chain (e.g., for fallback from
chain to chain) or a specific server (e.g., to send authorization
headers to that server).

The NetworkServiceProxyDelegate::Observer, which has many of the same
methods, remains in terms of ProxyServer; this will be addressed in a
further CL.

Bug: 1491092
Change-Id: I4d468c82d41fec240a050aff97db519c3dff967a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4936618
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Andrew Williams <awillia@chromium.org>
Commit-Queue: Andrew Williams <awillia@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1211626}

net/base/proxy_delegate.h

@@ -9,6 +9,7 @@
 
 #include "net/base/net_errors.h"
 #include "net/base/net_export.h"
+#include "net/base/proxy_chain.h"
 #include "net/proxy_resolution/proxy_retry_info.h"
 
 class GURL;
@@ -18,7 +19,6 @@ namespace net {
 class HttpRequestHeaders;
 class HttpResponseHeaders;
 class ProxyInfo;
-class ProxyServer;
 
 // Delegate for setting up a connection.
 class NET_EXPORT ProxyDelegate {
@@ -39,25 +39,50 @@ class NET_EXPORT ProxyDelegate {
                               const ProxyRetryInfoMap& proxy_retry_info,
                               ProxyInfo* result) = 0;
 
-  // Called when use of |bad_proxy| fails due to |net_error|. |net_error| is
-  // the network error encountered, if any, and OK if the fallback was
-  // for a reason other than a network error (e.g. the proxy service was
-  // explicitly directed to skip a proxy).
-  virtual void OnFallback(const ProxyServer& bad_proxy, int net_error) = 0;
+  // Called when use of a proxy chain failed due to `net_error`, but another
+  // proxy chain in the list succeeded. The failed proxy is within `bad_chain`,
+  // but it is undefined at which proxy in that chain. `net_error` is the
+  // network error encountered, if any, and OK if the fallback was for a reason
+  // other than a network error (e.g. the proxy service was explicitly directed
+  // to skip a proxy).
+  virtual void OnFallback(const ProxyChain& bad_chain, int net_error) = 0;
 
   // Called immediately before a proxy tunnel request is sent. Provides the
   // embedder an opportunity to add extra request headers.
-  virtual void OnBeforeTunnelRequest(const ProxyServer& proxy_server,
+  virtual void OnBeforeTunnelRequest(const ProxyChain& proxy_chain,
+                                     size_t chain_index,
                                      HttpRequestHeaders* extra_headers) = 0;
 
   // Called when the response headers for the proxy tunnel request have been
   // received. Allows the delegate to override the net error code of the tunnel
   // request. Returning OK causes the standard tunnel response handling to be
-  // performed. Implementations should make sure they can trust |proxy_server|
-  // before making decisions based on |response_headers|.
+  // performed. Implementations should make sure they can trust the proxy server
+  // at position `chain_index` in `proxy_chain` before making decisions based on
+  // `response_headers`.
   virtual Error OnTunnelHeadersReceived(
-      const ProxyServer& proxy_server,
+      const ProxyChain& proxy_chain,
+      size_t chain_index,
       const HttpResponseHeaders& response_headers) = 0;
+
+  // Compatibility methods for the transition to using ProxyChain instead of
+  // ProxyServer. TODO(crbug.com/1491092): Remove these methods.
+  void OnFallbackServerOnly(const ProxyServer& bad_server, int net_error) {
+    OnFallback(ProxyChain(bad_server), net_error);
+  }
+
+  void OnBeforeTunnelRequestServerOnly(const ProxyServer& proxy_server,
+                                       HttpRequestHeaders* extra_headers) {
+    DCHECK(!proxy_server.is_direct());
+    OnBeforeTunnelRequest(ProxyChain(proxy_server), /*chain_index=*/0,
+                          extra_headers);
+  }
+
+  Error OnTunnelHeadersReceivedServerOnly(
+      const ProxyServer& proxy_server,
+      const HttpResponseHeaders& response_headers) {
+    return OnTunnelHeadersReceived(ProxyChain(proxy_server),
+                                   /*chain_index=*/0, response_headers);
+  }
 };
 
 }  // namespace net

net/base/test_proxy_delegate.cc

@@ -20,10 +20,12 @@ TestProxyDelegate::TestProxyDelegate() = default;
 TestProxyDelegate::~TestProxyDelegate() = default;
 
 void TestProxyDelegate::VerifyOnTunnelHeadersReceived(
-    const ProxyServer& proxy_server,
+    const ProxyChain& proxy_chain,
+    size_t chain_index,
     const std::string& response_header_name,
     const std::string& response_header_value) const {
-  EXPECT_EQ(proxy_server, on_tunnel_headers_received_proxy_server_);
+  EXPECT_EQ(proxy_chain, on_tunnel_headers_received_proxy_chain_);
+  EXPECT_EQ(chain_index, on_tunnel_headers_received_chain_index_);
   ASSERT_NE(on_tunnel_headers_received_headers_.get(), nullptr);
   EXPECT_TRUE(on_tunnel_headers_received_headers_->HasHeaderValue(
       response_header_name, response_header_value));
@@ -36,25 +38,31 @@ void TestProxyDelegate::OnResolveProxy(
     const ProxyRetryInfoMap& proxy_retry_info,
     ProxyInfo* result) {}
 
-void TestProxyDelegate::OnFallback(const ProxyServer& bad_proxy,
-                                   int net_error) {}
+void TestProxyDelegate::OnFallback(const ProxyChain& bad_chain, int net_error) {
+}
 
 void TestProxyDelegate::OnBeforeTunnelRequest(
-    const ProxyServer& proxy_server,
+    const ProxyChain& proxy_chain,
+    size_t chain_index,
     HttpRequestHeaders* extra_headers) {
   on_before_tunnel_request_called_ = true;
-  if (extra_headers)
-    extra_headers->SetHeader("Foo", ProxyServerToProxyUri(proxy_server));
+  if (extra_headers) {
+    // TODO(crbug.com/1491092): Include the entire chain in the header.
+    extra_headers->SetHeader("Foo",
+                             ProxyServerToProxyUri(proxy_chain.proxy_server()));
+  }
 }
 
 Error TestProxyDelegate::OnTunnelHeadersReceived(
-    const ProxyServer& proxy_server,
+    const ProxyChain& proxy_chain,
+    size_t chain_index,
     const HttpResponseHeaders& response_headers) {
   EXPECT_EQ(on_tunnel_headers_received_headers_.get(), nullptr);
   on_tunnel_headers_received_headers_ =
       base::MakeRefCounted<HttpResponseHeaders>(response_headers.raw_headers());
 
-  on_tunnel_headers_received_proxy_server_ = proxy_server;
+  on_tunnel_headers_received_proxy_chain_ = proxy_chain;
+  on_tunnel_headers_received_chain_index_ = chain_index;
   return OK;
 }
 

net/base/test_proxy_delegate.h

@@ -8,8 +8,8 @@
 #include <string>
 
 #include "base/memory/scoped_refptr.h"
+#include "net/base/proxy_chain.h"
 #include "net/base/proxy_delegate.h"
-#include "net/base/proxy_server.h"
 
 class GURL;
 
@@ -27,7 +27,8 @@ class TestProxyDelegate : public ProxyDelegate {
   }
 
   void VerifyOnTunnelHeadersReceived(
-      const ProxyServer& proxy_server,
+      const ProxyChain& proxy_chain,
+      size_t chain_index,
       const std::string& response_header_name,
       const std::string& response_header_value) const;
 
@@ -37,16 +38,19 @@ class TestProxyDelegate : public ProxyDelegate {
                       const std::string& method,
                       const ProxyRetryInfoMap& proxy_retry_info,
                       ProxyInfo* result) override;
-  void OnFallback(const ProxyServer& bad_proxy, int net_error) override;
-  void OnBeforeTunnelRequest(const ProxyServer& proxy_server,
+  void OnFallback(const ProxyChain& bad_chain, int net_error) override;
+  void OnBeforeTunnelRequest(const ProxyChain& proxy_chain,
+                             size_t chain_index,
                              HttpRequestHeaders* extra_headers) override;
   Error OnTunnelHeadersReceived(
-      const ProxyServer& proxy_server,
+      const ProxyChain& proxy_chain,
+      size_t chain_index,
       const HttpResponseHeaders& response_headers) override;
 
  private:
   bool on_before_tunnel_request_called_ = false;
-  ProxyServer on_tunnel_headers_received_proxy_server_;
+  ProxyChain on_tunnel_headers_received_proxy_chain_;
+  size_t on_tunnel_headers_received_chain_index_;
   scoped_refptr<HttpResponseHeaders> on_tunnel_headers_received_headers_;
 };
 

net/http/http_network_transaction_unittest.cc

@@ -381,11 +381,13 @@ class SingleProxyDelegate : public ProxyDelegate {
     if (proxy_server_.is_valid())
       result->UseProxyServer(proxy_server_);
   }
-  void OnFallback(const ProxyServer& bad_proxy, int net_error) override {}
-  void OnBeforeTunnelRequest(const ProxyServer& proxy_server,
+  void OnFallback(const ProxyChain& bad_chain, int net_error) override {}
+  void OnBeforeTunnelRequest(const ProxyChain& proxy_chain,
+                             size_t chain_index,
                              HttpRequestHeaders* extra_headers) override {}
   Error OnTunnelHeadersReceived(
-      const ProxyServer& proxy_server,
+      const ProxyChain& proxy_chain,
+      size_t chain_index,
       const HttpResponseHeaders& response_headers) override {
     return OK;
   }

net/http/http_proxy_client_socket.cc

@@ -14,6 +14,7 @@
 #include "net/base/auth.h"
 #include "net/base/host_port_pair.h"
 #include "net/base/io_buffer.h"
+#include "net/base/proxy_chain.h"
 #include "net/base/proxy_delegate.h"
 #include "net/base/proxy_server.h"
 #include "net/http/http_basic_stream.h"
@@ -352,8 +353,9 @@ int HttpProxyClientSocket::DoSendRequest() {
 
     if (proxy_delegate_) {
       HttpRequestHeaders proxy_delegate_headers;
-      proxy_delegate_->OnBeforeTunnelRequest(proxy_server_,
-                                             &proxy_delegate_headers);
+      // TODO(crbug.com/1491092): Provide the full chain with appropriate index.
+      proxy_delegate_->OnBeforeTunnelRequestServerOnly(proxy_server_,
+                                                       &proxy_delegate_headers);
       extra_headers.MergeFrom(proxy_delegate_headers);
     }
 
@@ -404,8 +406,9 @@ int HttpProxyClientSocket::DoReadHeadersComplete(int result) {
       response_.headers.get());
 
   if (proxy_delegate_) {
-    int rv = proxy_delegate_->OnTunnelHeadersReceived(proxy_server_,
-                                                      *response_.headers);
+    // TODO(crbug.com/1491092): Provide the full chain with appropriate index.
+    int rv = proxy_delegate_->OnTunnelHeadersReceivedServerOnly(
+        proxy_server_, *response_.headers);
     if (rv != OK) {
       DCHECK_NE(ERR_IO_PENDING, rv);
       return rv;

net/http/http_proxy_connect_job_unittest.cc

@@ -532,7 +532,7 @@ TEST_P(HttpProxyConnectJobTest, ProxyDelegateExtraHeaders) {
   test_delegate.StartJobExpectingResult(connect_job.get(), OK,
                                         false /* expect_sync_result */);
   proxy_delegate_->VerifyOnTunnelHeadersReceived(
-      proxy_server, kResponseHeaderName, kResponseHeaderValue);
+      ProxyChain(proxy_server), 0, kResponseHeaderName, kResponseHeaderValue);
 }
 
 // Test the case where auth credentials are not cached.

net/proxy_resolution/configured_proxy_resolution_service.cc

@@ -1137,10 +1137,13 @@ void ConfiguredProxyResolutionService::ReportSuccess(const ProxyInfo& result) {
     if (existing == proxy_retry_info_.end()) {
       proxy_retry_info_[iter.first] = iter.second;
       if (proxy_delegate_) {
+        // TODO(crbug.com/1491092): Provide the full chain when that is
+        // available from retry info, and never provide a direct chain.
         const ProxyServer& bad_proxy =
             ProxyUriToProxyServer(iter.first, ProxyServer::SCHEME_HTTP);
         const ProxyRetryInfo& proxy_retry_info = iter.second;
-        proxy_delegate_->OnFallback(bad_proxy, proxy_retry_info.net_error);
+        proxy_delegate_->OnFallbackServerOnly(bad_proxy,
+                                              proxy_retry_info.net_error);
       }
     } else if (existing->second.bad_until < iter.second.bad_until) {
       existing->second.bad_until = iter.second.bad_until;

net/proxy_resolution/configured_proxy_resolution_service_unittest.cc

@@ -233,13 +233,15 @@ class TestResolveProxyDelegate : public ProxyDelegate {
     return proxy_retry_info_;
   }
 
-  void OnFallback(const ProxyServer& bad_proxy, int net_error) override {}
+  void OnFallback(const ProxyChain& bad_chain, int net_error) override {}
 
-  void OnBeforeTunnelRequest(const ProxyServer& proxy_server,
+  void OnBeforeTunnelRequest(const ProxyChain& proxy_chain,
+                             size_t chain_index,
                              HttpRequestHeaders* extra_headers) override {}
 
   Error OnTunnelHeadersReceived(
-      const ProxyServer& proxy_server,
+      const ProxyChain& proxy_chain,
+      size_t chain_index,
       const HttpResponseHeaders& response_headers) override {
     return OK;
   }
@@ -262,32 +264,34 @@ class TestProxyFallbackProxyDelegate : public ProxyDelegate {
                       const ProxyRetryInfoMap& proxy_retry_info,
                       ProxyInfo* result) override {}
 
-  void OnFallback(const ProxyServer& bad_proxy, int net_error) override {
-    proxy_server_ = bad_proxy;
+  void OnFallback(const ProxyChain& bad_chain, int net_error) override {
+    proxy_chain_ = bad_chain;
     last_proxy_fallback_net_error_ = net_error;
     num_proxy_fallback_called_++;
   }
 
-  void OnBeforeTunnelRequest(const ProxyServer& proxy_server,
+  void OnBeforeTunnelRequest(const ProxyChain& proxy_chain,
+                             size_t chain_index,
                              HttpRequestHeaders* extra_headers) override {}
 
   Error OnTunnelHeadersReceived(
-      const ProxyServer& proxy_server,
+      const ProxyChain& proxy_chain,
+      size_t chain_index,
       const HttpResponseHeaders& response_headers) override {
     return OK;
   }
 
   bool num_proxy_fallback_called() const { return num_proxy_fallback_called_; }
 
-  const ProxyServer& proxy_server() const { return proxy_server_; }
+  const ProxyChain& proxy_chain() const { return proxy_chain_; }
 
   int last_proxy_fallback_net_error() const {
     return last_proxy_fallback_net_error_;
   }
 
  private:
   int num_proxy_fallback_called_ = 0;
-  ProxyServer proxy_server_;
+  ProxyChain proxy_chain_;
   int last_proxy_fallback_net_error_ = OK;
 };
 
@@ -1621,7 +1625,7 @@ TEST_F(ConfiguredProxyResolutionServiceTest, ProxyFallback) {
   TestProxyFallbackProxyDelegate test_delegate;
   service.SetProxyDelegate(&test_delegate);
   service.ReportSuccess(info);
-  EXPECT_EQ("foopy1:8080", ProxyServerToProxyUri(test_delegate.proxy_server()));
+  EXPECT_EQ("foopy1:8080", test_delegate.proxy_chain().ToDebugString());
   EXPECT_EQ(ERR_PROXY_CONNECTION_FAILED,
             test_delegate.last_proxy_fallback_net_error());
   service.SetProxyDelegate(nullptr);

net/quic/quic_proxy_client_socket.cc

@@ -10,6 +10,7 @@
 #include "base/functional/bind.h"
 #include "base/functional/callback_helpers.h"
 #include "base/values.h"
+#include "net/base/proxy_chain.h"
 #include "net/base/proxy_delegate.h"
 #include "net/http/http_auth_controller.h"
 #include "net/http/http_log_util.h"
@@ -344,8 +345,9 @@ int QuicProxyClientSocket::DoSendRequest() {
 
   if (proxy_delegate_) {
     HttpRequestHeaders proxy_delegate_headers;
-    proxy_delegate_->OnBeforeTunnelRequest(proxy_server_,
-                                           &proxy_delegate_headers);
+    // TODO(crbug.com/1491092): Provide the full chain with appropriate index.
+    proxy_delegate_->OnBeforeTunnelRequestServerOnly(proxy_server_,
+                                                     &proxy_delegate_headers);
     request_.extra_headers.MergeFrom(proxy_delegate_headers);
   }
 
@@ -407,8 +409,9 @@ int QuicProxyClientSocket::DoReadReplyComplete(int result) {
       response_.headers.get());
 
   if (proxy_delegate_) {
-    int rv = proxy_delegate_->OnTunnelHeadersReceived(proxy_server_,
-                                                      *response_.headers);
+    // TODO(crbug.com/1491092): Provide the full chain with appropriate index.
+    int rv = proxy_delegate_->OnTunnelHeadersReceivedServerOnly(
+        proxy_server_, *response_.headers);
     if (rv != OK) {
       DCHECK_NE(ERR_IO_PENDING, rv);
       return rv;

net/quic/quic_proxy_client_socket_unittest.cc

@@ -687,7 +687,8 @@ TEST_P(QuicProxyClientSocketTest, ProxyDelegateExtraHeaders) {
   ASSERT_TRUE(response != nullptr);
   ASSERT_EQ(200, response->headers->response_code());
   proxy_delegate_->VerifyOnTunnelHeadersReceived(
-      proxy_server, kResponseHeaderName, kResponseHeaderValue);
+      ProxyChain(proxy_server), /*chain_index=*/0, kResponseHeaderName,
+      kResponseHeaderValue);
 }
 
 TEST_P(QuicProxyClientSocketTest, ConnectWithAuthRequested) {

net/spdy/spdy_proxy_client_socket.cc

@@ -17,6 +17,7 @@
 #include "base/values.h"
 #include "net/base/auth.h"
 #include "net/base/io_buffer.h"
+#include "net/base/proxy_chain.h"
 #include "net/base/proxy_delegate.h"
 #include "net/http/http_auth_cache.h"
 #include "net/http/http_auth_handler_factory.h"
@@ -376,8 +377,9 @@ int SpdyProxyClientSocket::DoSendRequest() {
 
   if (proxy_delegate_) {
     HttpRequestHeaders proxy_delegate_headers;
-    proxy_delegate_->OnBeforeTunnelRequest(proxy_server_,
-                                           &proxy_delegate_headers);
+    // TODO(crbug.com/1491092): Provide the full chain with appropriate index.
+    proxy_delegate_->OnBeforeTunnelRequestServerOnly(proxy_server_,
+                                                     &proxy_delegate_headers);
     request_.extra_headers.MergeFrom(proxy_delegate_headers);
   }
 
@@ -422,8 +424,9 @@ int SpdyProxyClientSocket::DoReadReplyComplete(int result) {
       response_.headers.get());
 
   if (proxy_delegate_) {
-    int rv = proxy_delegate_->OnTunnelHeadersReceived(proxy_server_,
-                                                      *response_.headers);
+    // TODO(crbug.com/1491092): Provide the full chain with appropriate index.
+    int rv = proxy_delegate_->OnTunnelHeadersReceivedServerOnly(
+        proxy_server_, *response_.headers);
     if (rv != OK) {
       DCHECK_NE(ERR_IO_PENDING, rv);
       return rv;