diff options
author | bengr@chromium.org <bengr@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-03-31 17:54:55 +0000 |
---|---|---|
committer | bengr@chromium.org <bengr@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-03-31 17:54:55 +0000 |
commit | 313feebdc8dc898e4cccae929096a0a40474da11 (patch) | |
tree | e1129a02f3412c007e215652199c03e67cadc576 /net | |
parent | 49128c94b9d163f5f4824a7d87e965bae4f821b7 (diff) | |
download | chromium_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.cc | 27 | ||||
-rw-r--r-- | net/http/http_response_headers.cc | 34 | ||||
-rw-r--r-- | net/http/http_response_headers.h | 10 | ||||
-rw-r--r-- | net/http/http_response_headers_unittest.cc | 77 |
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) |