diff options
| -rw-r--r-- | net/http/http_network_layer_unittest.cc | 185 | ||||
| -rw-r--r-- | net/http/http_network_transaction.cc | 17 | ||||
| -rw-r--r-- | net/proxy/proxy_list.cc | 43 | ||||
| -rw-r--r-- | net/proxy/proxy_list.h | 10 | ||||
| -rw-r--r-- | net/proxy/proxy_list_unittest.cc | 36 | ||||
| -rw-r--r-- | net/proxy/proxy_service.cc | 6 | ||||
| -rw-r--r-- | net/proxy/proxy_service.h | 6 |
7 files changed, 291 insertions, 12 deletions
diff --git a/net/http/http_network_layer_unittest.cc b/net/http/http_network_layer_unittest.cc index a82dd00..0891724 100644 --- a/net/http/http_network_layer_unittest.cc +++ b/net/http/http_network_layer_unittest.cc @@ -23,10 +23,15 @@ namespace { class HttpNetworkLayerTest : public PlatformTest { protected: - HttpNetworkLayerTest() - : cert_verifier_(new MockCertVerifier), - proxy_service_(ProxyService::CreateDirect()), - ssl_config_service_(new SSLConfigServiceDefaults) { + HttpNetworkLayerTest() : ssl_config_service_(new SSLConfigServiceDefaults) {} + + virtual void SetUp() { + ConfigureTestDependencies(ProxyService::CreateDirect()); + } + + void ConfigureTestDependencies(ProxyService* proxy_service) { + cert_verifier_.reset(new MockCertVerifier); + proxy_service_.reset(proxy_service); HttpNetworkSession::Params session_params; session_params.client_socket_factory = &mock_socket_factory_; session_params.host_resolver = &host_resolver_; @@ -41,7 +46,7 @@ class HttpNetworkLayerTest : public PlatformTest { MockClientSocketFactory mock_socket_factory_; MockHostResolver host_resolver_; scoped_ptr<CertVerifier> cert_verifier_; - const scoped_ptr<ProxyService> proxy_service_; + scoped_ptr<ProxyService> proxy_service_; const scoped_refptr<SSLConfigService> ssl_config_service_; scoped_refptr<HttpNetworkSession> network_session_; scoped_ptr<HttpNetworkLayer> factory_; @@ -115,6 +120,176 @@ TEST_F(HttpNetworkLayerTest, GET) { EXPECT_EQ("hello world", contents); } +TEST_F(HttpNetworkLayerTest, ServerFallback) { + // Verify that a Connection: Proxy-Bypass header induces proxy fallback to + // a second proxy, if configured. + + // To configure this test, we need to wire up a custom proxy service to use + // a pair of proxies. We'll induce fallback via the first and return + // the expected data via the second. + ConfigureTestDependencies(ProxyService::CreateFixedFromPacResult( + "PROXY bad:8080; PROXY good:8080")); + + MockRead data_reads[] = { + MockRead("HTTP/1.1 200 OK\r\n" + "Connection: proxy-bypass\r\n\r\n"), + MockRead("Bypass message"), + MockRead(SYNCHRONOUS, OK), + }; + MockWrite data_writes[] = { + MockWrite("GET http://www.google.com/ HTTP/1.1\r\n" + "Host: www.google.com\r\n" + "Proxy-Connection: keep-alive\r\n\r\n"), + }; + StaticSocketDataProvider data1(data_reads, arraysize(data_reads), + data_writes, arraysize(data_writes)); + mock_socket_factory_.AddSocketDataProvider(&data1); + + // Second data provider returns the expected content. + MockRead data_reads2[] = { + MockRead("HTTP/1.0 200 OK\r\n" + "Server: not-proxy\r\n\r\n"), + MockRead("content"), + MockRead(SYNCHRONOUS, OK), + }; + MockWrite data_writes2[] = { + MockWrite("GET http://www.google.com/ HTTP/1.1\r\n" + "Host: www.google.com\r\n" + "Proxy-Connection: keep-alive\r\n\r\n"), + }; + StaticSocketDataProvider data2(data_reads2, arraysize(data_reads2), + data_writes2, arraysize(data_writes2)); + mock_socket_factory_.AddSocketDataProvider(&data2); + + TestCompletionCallback callback; + + HttpRequestInfo request_info; + request_info.url = GURL("http://www.google.com/"); + request_info.method = "GET"; + request_info.load_flags = LOAD_NORMAL; + + scoped_ptr<HttpTransaction> trans; + int rv = factory_->CreateTransaction(&trans, NULL); + EXPECT_EQ(OK, rv); + + rv = trans->Start(&request_info, callback.callback(), BoundNetLog()); + if (rv == ERR_IO_PENDING) + rv = callback.WaitForResult(); + ASSERT_EQ(OK, rv); + + std::string contents; + rv = ReadTransaction(trans.get(), &contents); + EXPECT_EQ(OK, rv); + + // We should obtain content from the second socket provider write + // corresponding to the fallback proxy. + EXPECT_EQ("content", contents); + // We also have a server header here that isn't set by the proxy. + EXPECT_TRUE(trans->GetResponseInfo()->headers->HasHeaderValue( + "server", "not-proxy")); + // We should also observe the bad proxy in the retry list. + ASSERT_TRUE(1u == proxy_service_->proxy_retry_info().size()); + EXPECT_EQ("bad:8080", (*proxy_service_->proxy_retry_info().begin()).first); +} + +TEST_F(HttpNetworkLayerTest, ServerFallbackDoesntLoop) { + // Verify that a Connection: Proxy-Bypass header will display the original + // proxy's error page content if a fallback option is not configured. + ConfigureTestDependencies(ProxyService::CreateFixedFromPacResult( + "PROXY bad:8080; PROXY alsobad:8080")); + + MockRead data_reads[] = { + MockRead("HTTP/1.1 200 OK\r\n" + "Connection: proxy-bypass\r\n\r\n"), + MockRead("Bypass message"), + MockRead(SYNCHRONOUS, OK), + }; + MockWrite data_writes[] = { + MockWrite("GET http://www.google.com/ HTTP/1.1\r\n" + "Host: www.google.com\r\n" + "Proxy-Connection: keep-alive\r\n\r\n"), + }; + StaticSocketDataProvider data1(data_reads, arraysize(data_reads), + data_writes, arraysize(data_writes)); + StaticSocketDataProvider data2(data_reads, arraysize(data_reads), + data_writes, arraysize(data_writes)); + mock_socket_factory_.AddSocketDataProvider(&data1); + mock_socket_factory_.AddSocketDataProvider(&data2); + + TestCompletionCallback callback; + + HttpRequestInfo request_info; + request_info.url = GURL("http://www.google.com/"); + request_info.method = "GET"; + request_info.load_flags = LOAD_NORMAL; + + scoped_ptr<HttpTransaction> trans; + int rv = factory_->CreateTransaction(&trans, NULL); + EXPECT_EQ(OK, rv); + + rv = trans->Start(&request_info, callback.callback(), BoundNetLog()); + if (rv == ERR_IO_PENDING) + rv = callback.WaitForResult(); + ASSERT_EQ(OK, rv); + + std::string contents; + rv = ReadTransaction(trans.get(), &contents); + EXPECT_EQ(OK, rv); + EXPECT_EQ("Bypass message", contents); + + // Despite not falling back to anything, we should still observe the proxies + // in the bad proxies list. + const ProxyRetryInfoMap& retry_info = proxy_service_->proxy_retry_info(); + ASSERT_EQ(2u, retry_info.size()); + ASSERT_TRUE(retry_info.find("bad:8080") != retry_info.end()); + ASSERT_TRUE(retry_info.find("alsobad:8080") != retry_info.end()); +} + +TEST_F(HttpNetworkLayerTest, ProxyBypassIgnoredOnDirectConnection) { + // Verify that a Connection: proxy-bypass header is ignored when returned + // from a directly connected origin server. + ConfigureTestDependencies(ProxyService::CreateFixedFromPacResult("DIRECT")); + + MockRead data_reads[] = { + MockRead("HTTP/1.1 200 OK\r\n" + "Connection: proxy-bypass\r\n\r\n"), + MockRead("Bypass message"), + MockRead(SYNCHRONOUS, OK), + }; + MockWrite data_writes[] = { + MockWrite("GET / HTTP/1.1\r\n" + "Host: www.google.com\r\n" + "Connection: keep-alive\r\n\r\n"), + }; + StaticSocketDataProvider data1(data_reads, arraysize(data_reads), + data_writes, arraysize(data_writes)); + mock_socket_factory_.AddSocketDataProvider(&data1); + TestCompletionCallback callback; + + HttpRequestInfo request_info; + request_info.url = GURL("http://www.google.com/"); + request_info.method = "GET"; + request_info.load_flags = LOAD_NORMAL; + + scoped_ptr<HttpTransaction> trans; + int rv = factory_->CreateTransaction(&trans, NULL); + EXPECT_EQ(OK, rv); + + rv = trans->Start(&request_info, callback.callback(), BoundNetLog()); + if (rv == ERR_IO_PENDING) + rv = callback.WaitForResult(); + ASSERT_EQ(OK, rv); + + // We should have read the original page data. + std::string contents; + rv = ReadTransaction(trans.get(), &contents); + EXPECT_EQ(OK, rv); + EXPECT_EQ("Bypass message", contents); + + // We should have no entries in our bad proxy list. + ASSERT_EQ(0u, proxy_service_->proxy_retry_info().size()); +} + } // namespace } // namespace net diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc index dd538e2..df1d8fc 100644 --- a/net/http/http_network_transaction.cc +++ b/net/http/http_network_transaction.cc @@ -862,6 +862,23 @@ int HttpNetworkTransaction::DoReadHeadersComplete(int result) { } DCHECK(response_.headers); + // Server-induced fallback is supported only if this is a PAC configured + // proxy. See: http://crbug.com/143712 + if (response_.was_fetched_via_proxy && proxy_info_.did_use_pac_script()) { + if (response_.headers != NULL && + response_.headers->HasHeaderValue("connection", "proxy-bypass")) { + ProxyService* proxy_service = session_->proxy_service(); + if (proxy_service->MarkProxyAsBad(proxy_info_, net_log_)) { + // Only retry in the case of GETs. We don't want to resubmit a POST + // if the proxy took some action. + if (request_->method == "GET") { + ResetConnectionAndRequestForResend(); + return OK; + } + } + } + } + // Like Net.HttpResponseCode, but only for MAIN_FRAME loads. if (request_->load_flags & LOAD_MAIN_FRAME) { const int response_code = response_.headers->response_code(); diff --git a/net/proxy/proxy_list.cc b/net/proxy/proxy_list.cc index 3a63c6c..2057d50 100644 --- a/net/proxy/proxy_list.cc +++ b/net/proxy/proxy_list.cc @@ -67,6 +67,26 @@ void ProxyList::DeprioritizeBadProxies( proxies_.insert(proxies_.end(), bad_proxies.begin(), bad_proxies.end()); } +bool ProxyList::HasUntriedProxies( + const ProxyRetryInfoMap& proxy_retry_info) const { + std::vector<ProxyServer>::const_iterator iter = proxies_.begin(); + for (; iter != proxies_.end(); ++iter) { + ProxyRetryInfoMap::const_iterator bad_proxy = + proxy_retry_info.find(iter->ToURI()); + if (bad_proxy != proxy_retry_info.end()) { + // This proxy is bad. Check if it's time to retry. + if (bad_proxy->second.bad_until >= TimeTicks::Now()) { + continue; + } + } + // Either we've found the entry in the retry map and it's expired or we + // didn't find a corresponding entry in the retry map. In either case, we + // have a proxy to try. + return true; + } + return false; +} + void ProxyList::RemoveProxiesWithoutScheme(int scheme_bit_field) { for (std::vector<ProxyServer>::iterator it = proxies_.begin(); it != proxies_.end(); ) { @@ -126,8 +146,6 @@ std::string ProxyList::ToPacString() const { bool ProxyList::Fallback(ProxyRetryInfoMap* proxy_retry_info, const BoundNetLog& net_log) { - // Number of minutes to wait before retrying a bad proxy server. - const TimeDelta kProxyRetryDelay = TimeDelta::FromMinutes(5); // TODO(eroman): It would be good if instead of removing failed proxies // from the list, we simply annotated them with the error code they failed @@ -147,6 +165,22 @@ bool ProxyList::Fallback(ProxyRetryInfoMap* proxy_retry_info, NOTREACHED(); return false; } + UpdateRetryInfoOnFallback(proxy_retry_info, net_log); + + // Remove this proxy from our list. + proxies_.erase(proxies_.begin()); + return !proxies_.empty(); +} + +void ProxyList::UpdateRetryInfoOnFallback( + ProxyRetryInfoMap* proxy_retry_info, const BoundNetLog& net_log) const { + // Number of minutes to wait before retrying a bad proxy server. + const TimeDelta kProxyRetryDelay = TimeDelta::FromMinutes(5); + + if (proxies_.empty()) { + NOTREACHED(); + return; + } if (!proxies_[0].is_direct()) { std::string key = proxies_[0].ToURI(); @@ -165,11 +199,6 @@ bool ProxyList::Fallback(ProxyRetryInfoMap* proxy_retry_info, net_log.AddEvent(NetLog::TYPE_PROXY_LIST_FALLBACK, NetLog::StringCallback("bad_proxy", &key)); } - - // Remove this proxy from our list. - proxies_.erase(proxies_.begin()); - - return !proxies_.empty(); } } // namespace net diff --git a/net/proxy/proxy_list.h b/net/proxy/proxy_list.h index 045b7b4..209dd67 100644 --- a/net/proxy/proxy_list.h +++ b/net/proxy/proxy_list.h @@ -35,6 +35,10 @@ class NET_EXPORT_PRIVATE ProxyList { // them to the end of the fallback list. void DeprioritizeBadProxies(const ProxyRetryInfoMap& proxy_retry_info); + // Returns true if this proxy list contains at least one proxy that is + // not currently present in |proxy_retry_info|. + bool HasUntriedProxies(const ProxyRetryInfoMap& proxy_retry_info) const; + // Delete any entry which doesn't have one of the specified proxy schemes. // |scheme_bit_field| is a bunch of ProxyServer::Scheme bitwise ORed together. void RemoveProxiesWithoutScheme(int scheme_bit_field); @@ -70,6 +74,12 @@ class NET_EXPORT_PRIVATE ProxyList { bool Fallback(ProxyRetryInfoMap* proxy_retry_info, const BoundNetLog& net_log); + // Updates |proxy_retry_info| to indicate that the first proxy in the list + // is bad. This is distinct from Fallback(), above, to allow updating proxy + // retry information without modifying a given transction's proxy list. + void UpdateRetryInfoOnFallback(ProxyRetryInfoMap* proxy_retry_info, + const BoundNetLog& net_log) const; + private: // List of proxies. std::vector<ProxyServer> proxies_; diff --git a/net/proxy/proxy_list_unittest.cc b/net/proxy/proxy_list_unittest.cc index 04d9268..470a900 100644 --- a/net/proxy/proxy_list_unittest.cc +++ b/net/proxy/proxy_list_unittest.cc @@ -89,6 +89,42 @@ TEST(ProxyListTest, RemoveProxiesWithoutScheme) { } } +TEST(ProxyListTest, HasUntriedProxies) { + // As in DeprioritizeBadProxies, we use a lengthy timeout to avoid depending + // on the current time. + ProxyRetryInfo proxy_retry_info; + proxy_retry_info.bad_until = + base::TimeTicks::Now() + base::TimeDelta::FromDays(1); + + // An empty list has nothing to try. + { + ProxyList list; + ProxyRetryInfoMap proxy_retry_info; + EXPECT_FALSE(list.HasUntriedProxies(proxy_retry_info)); + } + + // A list with one bad proxy has something to try. With two bad proxies, + // there's nothing to try. + { + ProxyList list; + list.SetFromPacString("PROXY bad1:80; PROXY bad2:80"); + ProxyRetryInfoMap retry_info_map; + retry_info_map["bad1:80"] = proxy_retry_info; + EXPECT_TRUE(list.HasUntriedProxies(retry_info_map)); + retry_info_map["bad2:80"] = proxy_retry_info; + EXPECT_FALSE(list.HasUntriedProxies(retry_info_map)); + } + + // A list with one bad proxy and a DIRECT entry has something to try. + { + ProxyList list; + list.SetFromPacString("PROXY bad1:80; DIRECT"); + ProxyRetryInfoMap retry_info_map; + retry_info_map["bad1:80"] = proxy_retry_info; + EXPECT_TRUE(list.HasUntriedProxies(retry_info_map)); + } +} + TEST(ProxyListTest, DeprioritizeBadProxies) { // Retry info that marks a proxy as being bad for a *very* long time (to avoid // the test depending on the current time.) diff --git a/net/proxy/proxy_service.cc b/net/proxy/proxy_service.cc index 9989b82..615434b 100644 --- a/net/proxy/proxy_service.cc +++ b/net/proxy/proxy_service.cc @@ -1174,6 +1174,12 @@ int ProxyService::ReconsiderProxyAfterError(const GURL& url, return did_fallback ? OK : ERR_FAILED; } +bool ProxyService::MarkProxyAsBad(const ProxyInfo& result, + const BoundNetLog& net_log) { + result.proxy_list_.UpdateRetryInfoOnFallback(&proxy_retry_info_, net_log); + return result.proxy_list_.HasUntriedProxies(proxy_retry_info_); +} + void ProxyService::ReportSuccess(const ProxyInfo& result) { DCHECK(CalledOnValidThread()); diff --git a/net/proxy/proxy_service.h b/net/proxy/proxy_service.h index 3d89420..12a4784 100644 --- a/net/proxy/proxy_service.h +++ b/net/proxy/proxy_service.h @@ -146,6 +146,12 @@ class NET_EXPORT ProxyService : public NetworkChangeNotifier::IPAddressObserver, PacRequest** pac_request, const BoundNetLog& net_log); + // Explicitly trigger proxy fallback for the given |results| by updating our + // list of bad proxies to include the first entry of |results|. Returns true + // if there will be at least one proxy remaining in the list after fallback + // and false otherwise. + bool MarkProxyAsBad(const ProxyInfo& results, const BoundNetLog& net_log); + // Called to report that the last proxy connection succeeded. If |proxy_info| // has a non empty proxy_retry_info map, the proxies that have been tried (and // failed) for this request will be marked as bad. |