Record errors that trigger a data reduction proxy bypass

net/http/http_network_layer_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/http/http_network_layer.h"
 
 #include "base/basictypes.h"
 #include "base/strings/stringprintf.h"
 #include "net/base/net_log.h"
 #include "net/cert/mock_cert_verifier.h"
@@ skipping 458 matching lines @@
       "PROXY " + bad_proxy));
   TestProxyFallbackFail(1u, bad_proxy, "");
 }
 
 TEST_F(HttpNetworkLayerTest, ServerOneProxyNoDirectBypassFixed) {
   std::string bad_proxy = GetDataReductionProxy();
   ConfigureTestDependencies(ProxyService::CreateFixed(bad_proxy));
   TestProxyFallbackFail(1u, bad_proxy, "");
 }
 
-TEST_F(HttpNetworkLayerTest, ServerFallbackOn5xxError) {
+TEST_F(HttpNetworkLayerTest, ServerFallbackOn4xxAnd5xxErrors) {
   // Verify that "500 Internal Server Error", "502 Bad Gateway", and
   // "503 Service Unavailable" via the data reduction proxy induce proxy
   // fallback to a second proxy, if configured.
 
+  // Verify that 4xx responses bypass both data reduction proxies, when
+  // the proxy originates them.
+
   // 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.
+  // a pair or triplet of proxies. We'll induce fallback via the first and
+  // return the expected data via the second, or third if the first two are
+  // both bypassed.
   std::string data_reduction_proxy(
       HostPortPair::FromURL(GURL(SPDY_PROXY_AUTH_ORIGIN)).ToString());
-  std::string pac_string = base::StringPrintf(
-      "PROXY %s; PROXY good:8080", data_reduction_proxy.data());
+  std::string data_reduction_proxy_fallback;
+  size_t data_reduction_proxy_count = 1u;
+#if defined(DATA_REDUCTION_FALLBACK_HOST)
+  data_reduction_proxy_fallback =
+      HostPortPair::FromURL(GURL(DATA_REDUCTION_FALLBACK_HOST)).ToString();
+  data_reduction_proxy_count = 2u;
+#endif
 
-  std::string headers[] = {
-    "HTTP/1.1 500 Internal Server Error\r\n\r\n",
-    "HTTP/1.1 502 Bad Gateway\r\n\r\n",
-    "HTTP/1.1 503 Service Unavailable\r\n\r\n"
+
+  const struct {
+    const char* headers;
+    size_t expected_proxies_on_retry_list;
+  } tests[] = {
+      { "HTTP/1.1 500 Internal Server Error\r\n\r\n", 1u },
+      { "HTTP/1.1 502 Bad Gateway\r\n\r\n", 1u },
+      { "HTTP/1.1 503 Service Unavailable\r\n\r\n", 1u },
+      { "HTTP/1.1 414 Request-URI Too Long\r\nServer: GFE/2.0\r\n\r\n",
+        data_reduction_proxy_count },
+      { "HTTP/1.1 414 Request-URI Too Long\r\n\r\n", 1u}
   };
 
-  for (size_t i = 0; i < arraysize(headers); ++i) {
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) {
+    std::string pac_string;
+    if (data_reduction_proxy_count == 1u) {
+      pac_string = base::StringPrintf(
+          "PROXY %s; PROXY good:8080", data_reduction_proxy.data());
+    } else {
+      pac_string = base::StringPrintf(
+          "PROXY %s; PROXY %s; PROXY good:8080", data_reduction_proxy.data(),
+          data_reduction_proxy_fallback.data());
+    }
     ConfigureTestDependencies(
         ProxyService::CreateFixedFromPacResult(pac_string));
 
     MockRead data_reads[] = {
-      MockRead(headers[i].c_str()),
+      MockRead(tests[i].headers),
       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"),
+               "Server: good-proxy\r\n"

[mef, 2014/05/29 16:41:38]

Is this the data that is expected from server? Why did it change to
'good-proxy'?

[bengr, 2014/05/29 18:46:19]

"Not proxy" was incorrect. The test falls from a bad proxy to a good one.

+               "Via: 1.1 Chrome-Compression-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),
@@ skipping 18 matching lines @@
 
     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"));
+        "server", "good-proxy"));
     // We should also observe the data reduction proxy in the retry list.
-    ASSERT_EQ(1u, proxy_service_->proxy_retry_info().size());
-    EXPECT_EQ(data_reduction_proxy,
-              (*proxy_service_->proxy_retry_info().begin()).first);
+    ASSERT_EQ(tests[i].expected_proxies_on_retry_list,
+              proxy_service_->proxy_retry_info().size());
+    if (tests[i].expected_proxies_on_retry_list == 1u) {
+      EXPECT_EQ(data_reduction_proxy,
+                (*proxy_service_->proxy_retry_info().begin()).first);
+    } else {
+      ProxyRetryInfoMap::const_iterator i =

[mef, 2014/05/29 16:41:38]

Given that this is a map I think it is too brittle to expect particular order of
proxies. 

I'd suggest EXPECT_EQ(1u,
proxy_service_->proxy_retry_info().count(data_reduction_proxy_fallback));

[bengr, 2014/05/29 18:46:19]

Good suggestion. Thanks. Done.

+          (proxy_service_->proxy_retry_info()).begin();
+      EXPECT_EQ(data_reduction_proxy_fallback, (*i++).first);
+      EXPECT_EQ(data_reduction_proxy, (*i).first);
+    }
   }
 }
 #endif  // defined(SPDY_PROXY_AUTH_ORIGIN)
 
 TEST_F(HttpNetworkLayerTest, ProxyBypassIgnoredOnDirectConnectionPac) {
   // Verify that a Chrome-Proxy header is ignored when returned from a directly
   // connected origin server.
   ConfigureTestDependencies(ProxyService::CreateFixedFromPacResult("DIRECT"));
 
   MockRead data_reads[] = {
@@ skipping 89 matching lines @@
     MockRead("HTTP/1.1 200 OK\r\n"
              "Connection: keep-alive\r\n\r\n"),
     MockRead("Bypass message"),
     MockRead(SYNCHRONOUS, OK),
   };
 
   TestProxyFallbackWithMockReads(chrome_proxy, std::string(), data_reads,
                                  arraysize(data_reads), 1u);
 }
 
+TEST_F(HttpNetworkLayerTest, NoServerFallbackWith4xxFromOrigin) {
+  // Verify that Chrome will not be induced to bypass the data reduction proxy
+  // when the data reduction proxy via header is absent on a 4xx.

[mef, 2014/05/29 16:41:38]

Is this a valid comment? I see 'Via' header in mock reads...

[bengr, 2014/05/29 18:46:19]

Done.

+  std::string chrome_proxy = GetDataReductionProxy();
+  ConfigureTestDependencies(ProxyService::CreateFixedFromPacResult(
+      "PROXY " + chrome_proxy + "; PROXY good:8080"));
+
+  MockRead data_reads[] = {
+    MockRead("HTTP/1.1 414 Request-URI Too Long\r\n"
+             "Connection: keep-alive\r\n"
+             "Via: 1.1 Chrome-Compression-Proxy\r\n\r\n"),
+    MockRead(SYNCHRONOUS, OK),
+  };
+
+  TestProxyFallbackByMethodWithMockReads(chrome_proxy, std::string(),
+                                         data_reads, arraysize(data_reads),
+                                         "GET", std::string(), false, 0);
+}
+
 TEST_F(HttpNetworkLayerTest, NoServerFallbackWith304Response) {
   // Verify that Chrome will not be induced to bypass the data reduction proxy
   // when the data reduction proxy via header is absent on a 304.
   std::string chrome_proxy = GetDataReductionProxy();
   ConfigureTestDependencies(ProxyService::CreateFixedFromPacResult(
       "PROXY " + chrome_proxy + "; PROXY good:8080"));
 
   MockRead data_reads[] = {
     MockRead("HTTP/1.1 304 Not Modified\r\n"
              "Connection: keep-alive\r\n\r\n"),
@@ skipping 141 matching lines @@
   ASSERT_EQ(ERR_CONNECTION_RESET, callback.GetResult(rv));
 
   // If the response info is null, that means that any consumer won't
   // see the network accessed bit set.
   EXPECT_EQ(NULL, trans->GetResponseInfo());
 }
 
 }  // namespace
 
 }  // namespace net