Bypass data reduction proxy on 502 responses

BUG=325401

Review URL: https://codereview.chromium.org/103003002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@239508 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 73 insertions, 59 deletions

diff --git a/net/http/http_network_layer_unittest.cc b/net/http/http_network_layer_unittest.cc
index b040595..e22b6d2 100644
--- a/net/http/http_network_layer_unittest.cc
+++ b/net/http/http_network_layer_unittest.cc
@@ -378,8 +378,8 @@ TEST_F(HttpNetworkLayerTest, ServerOneProxyNoDirectBypassFixed) {
 }
 
 TEST_F(HttpNetworkLayerTest, ServerFallbackOnInternalServerError) {
-  // Verify that "500 Internal Server Error" via the data reduction proxy
-  // induces proxy fallback to a second proxy, if configured.
+  // Verify that "500 Internal Server Error" and "502 Bad Gateway" via the data
+  // reduction proxy induce 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
@@ -388,68 +388,80 @@ TEST_F(HttpNetworkLayerTest, ServerFallbackOnInternalServerError) {
       HostPortPair::FromURL(GURL(SPDY_PROXY_AUTH_ORIGIN)).ToString());
   std::string pac_string = base::StringPrintf(
       "PROXY %s; PROXY good:8080", data_reduction_proxy.data());
-  ConfigureTestDependencies(ProxyService::CreateFixedFromPacResult(pac_string));
 
-  MockRead data_reads[] = {
-    MockRead("HTTP/1.1 500 Internal Server Error\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"),
+  std::string headers[] = {
+    "HTTP/1.1 500 Internal Server Error\r\n\r\n",
+    "HTTP/1.1 502 Bad Gateway\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);
+  for (size_t i = 0; i < arraysize(headers); ++i) {
+    ConfigureTestDependencies(
+        ProxyService::CreateFixedFromPacResult(pac_string));
 
-  TestCompletionCallback callback;
+    MockRead data_reads[] = {
+      MockRead(headers[i].c_str()),
+      MockRead("Bypass message"),
+      MockRead(SYNCHRONOUS, OK),
+    };
 
-  HttpRequestInfo request_info;
-  request_info.url = GURL("http://www.google.com/");
-  request_info.method = "GET";
-  request_info.load_flags = LOAD_NORMAL;
+    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"),
+    };
 
-  scoped_ptr<HttpTransaction> trans;
-  int rv = factory_->CreateTransaction(DEFAULT_PRIORITY, &trans, NULL);
-  EXPECT_EQ(OK, rv);
+    StaticSocketDataProvider data1(data_reads, arraysize(data_reads),
+                                   data_writes, arraysize(data_writes));
+    mock_socket_factory_.AddSocketDataProvider(&data1);
 
-  rv = trans->Start(&request_info, callback.callback(), BoundNetLog());
-  if (rv == ERR_IO_PENDING)
-    rv = callback.WaitForResult();
-  ASSERT_EQ(OK, rv);
+    // 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"),
+    };
 
-  std::string contents;
-  rv = ReadTransaction(trans.get(), &contents);
-  EXPECT_EQ(OK, rv);
+    StaticSocketDataProvider data2(data_reads2, arraysize(data_reads2),
+                                   data_writes2, arraysize(data_writes2));
+    mock_socket_factory_.AddSocketDataProvider(&data2);
+
+    TestCompletionCallback callback;
 
-  // 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 data reduction proxy in the retry list.
-  ASSERT_TRUE(1u == proxy_service_->proxy_retry_info().size());
-  EXPECT_EQ(data_reduction_proxy,
-            (*proxy_service_->proxy_retry_info().begin()).first);
+    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(DEFAULT_PRIORITY, &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 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);
+  }
 }
 #endif  // defined(SPDY_PROXY_AUTH_ORIGIN)
 
diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index fbc83a9..bb3f77f 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -969,11 +969,13 @@ int HttpNetworkTransaction::DoReadHeadersComplete(int result) {
                 ProxyService::SHORT_BYPASS :
                 ProxyService::LONG_BYPASS);
       } else {
-        // Additionally, fallback if a 500 is returned via the data reduction
-        // proxy. This is conservative, as the 500 might have been generated by
-        // the origin, and not the proxy.
-        if (response_.headers->response_code() == HTTP_INTERNAL_SERVER_ERROR)
+        // Additionally, fallback if a 500 or 502 is returned via the data
+        // reduction proxy. This is conservative, as the 500 or 502 might have
+        // been generated by the origin, and not the proxy.
+        if (response_.headers->response_code() == HTTP_INTERNAL_SERVER_ERROR ||
+            response_.headers->response_code() == HTTP_BAD_GATEWAY) {
           proxy_bypass_event = ProxyService::INTERNAL_SERVER_ERROR_BYPASS;
+        }
       }
 
       if (proxy_bypass_event < ProxyService::BYPASS_EVENT_TYPE_MAX) {