summaryrefslogtreecommitdiffstats
path: root/net
diff options
context:
space:
mode:
authorbengr@chromium.org <bengr@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-03-31 17:54:55 +0000
committerbengr@chromium.org <bengr@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-03-31 17:54:55 +0000
commit313feebdc8dc898e4cccae929096a0a40474da11 (patch)
treee1129a02f3412c007e215652199c03e67cadc576 /net
parent49128c94b9d163f5f4824a7d87e965bae4f821b7 (diff)
downloadchromium_src-313feebdc8dc898e4cccae929096a0a40474da11.zip
chromium_src-313feebdc8dc898e4cccae929096a0a40474da11.tar.gz
chromium_src-313feebdc8dc898e4cccae929096a0a40474da11.tar.bz2
Fix UMA reporting of data reduction proxy bypass reasons
5xx responses from the proxy that should induce proxy bypass were previously reported as bypassing due to a missing via header value, because such responses often also are missing that header value. This change fixes that reporting. BUG=356445 Review URL: https://codereview.chromium.org/212873004 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@260598 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'net')
-rw-r--r--net/http/http_network_transaction.cc27
-rw-r--r--net/http/http_response_headers.cc34
-rw-r--r--net/http/http_response_headers.h10
-rw-r--r--net/http/http_response_headers_unittest.cc77
4 files changed, 123 insertions, 25 deletions
diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index ba27cb7..3d58afd 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -995,7 +995,6 @@ int HttpNetworkTransaction::DoReadHeadersComplete(int result) {
if (response_.was_fetched_via_proxy) {
ProxyService::DataReductionProxyBypassEventType proxy_bypass_event =
ProxyService::BYPASS_EVENT_TYPE_MAX;
- net::HttpResponseHeaders::ChromeProxyInfo chrome_proxy_info;
bool chrome_proxy_used =
proxy_info_.proxy_server().isDataReductionProxy();
bool chrome_fallback_proxy_used = false;
@@ -1007,29 +1006,9 @@ int HttpNetworkTransaction::DoReadHeadersComplete(int result) {
#endif
if (chrome_proxy_used || chrome_fallback_proxy_used) {
- // A Via header might not be present in a 304. Since the goal of a 304
- // response is to minimize information transfer, a sender in general
- // should not generate representation metadata other than Cache-Control,
- // Content-Location, Date, ETag, Expires, and Vary.
- if (!response_.headers->IsChromeProxyResponse() &&
- (response_.headers->response_code() != HTTP_NOT_MODIFIED)) {
- proxy_bypass_event = ProxyService::MISSING_VIA_HEADER;
- } else if (response_.headers->GetChromeProxyInfo(&chrome_proxy_info)) {
- if (chrome_proxy_info.bypass_duration < TimeDelta::FromMinutes(30))
- proxy_bypass_event = ProxyService::SHORT_BYPASS;
- else
- proxy_bypass_event = ProxyService::LONG_BYPASS;
- } else {
- // Additionally, fallback if a 500, 502 or 503 is returned via the data
- // reduction proxy. This is conservative, as the 500, 502 or 503 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 ||
- response_.headers->response_code() == HTTP_SERVICE_UNAVAILABLE) {
- proxy_bypass_event = ProxyService::INTERNAL_SERVER_ERROR_BYPASS;
- }
- }
-
+ net::HttpResponseHeaders::ChromeProxyInfo chrome_proxy_info;
+ proxy_bypass_event = response_.headers->GetChromeProxyBypassEventType(
+ &chrome_proxy_info);
if (proxy_bypass_event < ProxyService::BYPASS_EVENT_TYPE_MAX) {
ProxyService* proxy_service = session_->proxy_service();
diff --git a/net/http/http_response_headers.cc b/net/http/http_response_headers.cc
index f6e3658..c067e8f 100644
--- a/net/http/http_response_headers.cc
+++ b/net/http/http_response_headers.cc
@@ -26,6 +26,11 @@
#include "net/http/http_log_util.h"
#include "net/http/http_util.h"
+#if defined(SPDY_PROXY_AUTH_ORIGIN)
+#include "net/http/http_status_code.h"
+#include "net/proxy/proxy_service.h"
+#endif
+
using base::StringPiece;
using base::Time;
using base::TimeDelta;
@@ -1417,7 +1422,6 @@ bool HttpResponseHeaders::GetChromeProxyInfo(
DCHECK(proxy_info);
proxy_info->bypass_all = false;
proxy_info->bypass_duration = base::TimeDelta();
-
// Support header of the form Chrome-Proxy: bypass|block=<duration>, where
// <duration> is the number of seconds to wait before retrying
// the proxy. If the duration is 0, then the default proxy retry delay
@@ -1464,6 +1468,34 @@ bool HttpResponseHeaders::IsChromeProxyResponse() const {
return false;
}
+
+ProxyService::DataReductionProxyBypassEventType
+HttpResponseHeaders::GetChromeProxyBypassEventType(
+ ChromeProxyInfo* chrome_proxy_info) const {
+ DCHECK(chrome_proxy_info);
+ if (GetChromeProxyInfo(chrome_proxy_info)) {
+ // A chrome-proxy response header is only present in a 502. For proper
+ // reporting, this check must come before the 5xx checks below.
+ if (chrome_proxy_info->bypass_duration < TimeDelta::FromMinutes(30))
+ return ProxyService::SHORT_BYPASS;
+ return ProxyService::LONG_BYPASS;
+ }
+ if (response_code() == HTTP_INTERNAL_SERVER_ERROR ||
+ response_code() == HTTP_BAD_GATEWAY ||
+ response_code() == HTTP_SERVICE_UNAVAILABLE) {
+ // Fall back if a 500, 502 or 503 is returned.
+ return ProxyService::INTERNAL_SERVER_ERROR_BYPASS;
+ }
+ if (!IsChromeProxyResponse() && (response_code() != HTTP_NOT_MODIFIED)) {
+ // A Via header might not be present in a 304. Since the goal of a 304
+ // response is to minimize information transfer, a sender in general
+ // should not generate representation metadata other than Cache-Control,
+ // Content-Location, Date, ETag, Expires, and Vary.
+ return ProxyService::MISSING_VIA_HEADER;
+ }
+ // There is no bypass event.
+ return ProxyService::BYPASS_EVENT_TYPE_MAX;
+}
#endif // defined(SPDY_PROXY_AUTH_ORIGIN)
} // namespace net
diff --git a/net/http/http_response_headers.h b/net/http/http_response_headers.h
index 2eb0c67..62207ed 100644
--- a/net/http/http_response_headers.h
+++ b/net/http/http_response_headers.h
@@ -16,6 +16,10 @@
#include "net/base/net_log.h"
#include "net/http/http_version.h"
+#if defined(SPDY_PROXY_AUTH_ORIGIN)
+#include "net/proxy/proxy_service.h"
+#endif
+
class Pickle;
class PickleIterator;
@@ -286,6 +290,12 @@ class NET_EXPORT HttpResponseHeaders
// Returns true if response headers contain the Chrome proxy Via header value.
bool IsChromeProxyResponse() const;
+
+ // Returns the reason why the Chrome proxy should be bypassed or not, and
+ // populates |proxy_info| with information on how long to bypass if
+ // applicable.
+ ProxyService::DataReductionProxyBypassEventType
+ GetChromeProxyBypassEventType(ChromeProxyInfo* proxy_info) const;
#endif
// Creates a Value for use with the NetLog containing the response headers.
diff --git a/net/http/http_response_headers_unittest.cc b/net/http/http_response_headers_unittest.cc
index 0b17b8f..f73e176 100644
--- a/net/http/http_response_headers_unittest.cc
+++ b/net/http/http_response_headers_unittest.cc
@@ -13,6 +13,10 @@
#include "net/http/http_response_headers.h"
#include "testing/gtest/include/gtest/gtest.h"
+#if defined(SPDY_PROXY_AUTH_ORIGIN)
+#include "net/proxy/proxy_service.h"
+#endif
+
namespace {
struct TestData {
@@ -2172,4 +2176,77 @@ TEST(HttpResponseHeadersTest, IsChromeProxyResponse) {
EXPECT_EQ(tests[i].expected_result, parsed->IsChromeProxyResponse());
}
}
+
+TEST(HttpResponseHeadersTest, GetChromeProxyBypassEventType) {
+ const struct {
+ const char* headers;
+ net::ProxyService::DataReductionProxyBypassEventType expected_result;
+ } tests[] = {
+ { "HTTP/1.1 200 OK\n"
+ "Chrome-Proxy: bypass=0\n"
+ "Via: 1.1 Chrome-Compression-Proxy\n",
+ net::ProxyService::SHORT_BYPASS,
+ },
+ { "HTTP/1.1 200 OK\n"
+ "Chrome-Proxy: bypass=1799\n"
+ "Via: 1.1 Chrome-Compression-Proxy\n",
+ net::ProxyService::SHORT_BYPASS,
+ },
+ { "HTTP/1.1 200 OK\n"
+ "Chrome-Proxy: bypass=1800\n"
+ "Via: 1.1 Chrome-Compression-Proxy\n",
+ net::ProxyService::LONG_BYPASS,
+ },
+ { "HTTP/1.1 500 Internal Server Error\n"
+ "Via: 1.1 Chrome-Compression-Proxy\n",
+ net::ProxyService::INTERNAL_SERVER_ERROR_BYPASS,
+ },
+ { "HTTP/1.1 501 Not Implemented\n"
+ "Via: 1.1 Chrome-Compression-Proxy\n",
+ net::ProxyService::BYPASS_EVENT_TYPE_MAX,
+ },
+ { "HTTP/1.1 502 Bad Gateway\n"
+ "Via: 1.1 Chrome-Compression-Proxy\n",
+ net::ProxyService::INTERNAL_SERVER_ERROR_BYPASS,
+ },
+ { "HTTP/1.1 503 Service Unavailable\n"
+ "Via: 1.1 Chrome-Compression-Proxy\n",
+ net::ProxyService::INTERNAL_SERVER_ERROR_BYPASS,
+ },
+ { "HTTP/1.1 504 Gateway Timeout\n"
+ "Via: 1.1 Chrome-Compression-Proxy\n",
+ net::ProxyService::BYPASS_EVENT_TYPE_MAX,
+ },
+ { "HTTP/1.1 505 HTTP Version Not Supported\n"
+ "Via: 1.1 Chrome-Compression-Proxy\n",
+ net::ProxyService::BYPASS_EVENT_TYPE_MAX,
+ },
+ { "HTTP/1.1 304 Not Modified\n",
+ net::ProxyService::BYPASS_EVENT_TYPE_MAX,
+ },
+ { "HTTP/1.1 200 OK\n",
+ net::ProxyService::MISSING_VIA_HEADER,
+ },
+ { "HTTP/1.1 200 OK\n"
+ "Chrome-Proxy: bypass=1799\n",
+ net::ProxyService::SHORT_BYPASS,
+ },
+ { "HTTP/1.1 502 Bad Gateway\n",
+ net::ProxyService::INTERNAL_SERVER_ERROR_BYPASS,
+ },
+ { "HTTP/1.1 502 Bad Gateway\n"
+ "Chrome-Proxy: bypass=1799\n",
+ net::ProxyService::SHORT_BYPASS,
+ },
+ };
+ for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) {
+ std::string headers(tests[i].headers);
+ HeadersToRaw(&headers);
+ scoped_refptr<net::HttpResponseHeaders> parsed(
+ new net::HttpResponseHeaders(headers));
+ net::HttpResponseHeaders::ChromeProxyInfo chrome_proxy_info;
+ EXPECT_EQ(tests[i].expected_result,
+ parsed->GetChromeProxyBypassEventType(&chrome_proxy_info));
+ }
+}
#endif // defined(SPDY_PROXY_AUTH_ORIGIN)