Commit 5d4672be authored by rajendrant's avatar rajendrant Committed by Commit Bot

Revert "Change 502 response with no bypass info to block-once"

This reverts commit 452c35d1.

Reason for revert: <INSERT REASONING HERE>

Original change's description:
> Change 502 response with no bypass info to block-once
> 
> CORB failures could block any response headers being sent to the rendere. These
> are treated as a random 1-5 minute bypass, since Chrome-Proxy is not in the
> response. This CL changes those bypasses to a block-once.
> 
> Bug: 947736
> Change-Id: I83c6a9874ccb26e4f17d8dccf7e9eeac8dd7f535
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1546678
> Commit-Queue: rajendrant <rajendrant@chromium.org>
> Reviewed-by: Tarun Bansal <tbansal@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#646136}

TBR=tbansal@chromium.org,rajendrant@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 947736
Change-Id: Ie2fafbf28cb2549d3e4b0d056b420af198dd5cb7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1546886Reviewed-by: default avatarrajendrant <rajendrant@chromium.org>
Reviewed-by: default avatarTarun Bansal <tbansal@chromium.org>
Commit-Queue: rajendrant <rajendrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#646430}
parent e921966a
......@@ -776,30 +776,6 @@ IN_PROC_BROWSER_TEST_F(DataReductionProxyFallbackBrowsertest,
BYPASS_EVENT_TYPE_MALFORMED_407, 1);
}
IN_PROC_BROWSER_TEST_F(DataReductionProxyFallbackBrowsertest,
ProxyBypassedForCurrentRequestOn502Error) {
base::HistogramTester histogram_tester;
net::EmbeddedTestServer test_server;
test_server.RegisterRequestHandler(
base::BindRepeating(&BasicResponse, kDummyBody));
ASSERT_TRUE(test_server.Start());
SetStatusCode(net::HTTP_BAD_GATEWAY);
ui_test_utils::NavigateToURL(browser(),
GetURLWithMockHost(test_server, "/echo"));
EXPECT_THAT(GetBody(), kDummyBody);
histogram_tester.ExpectUniqueSample(
"DataReductionProxy.BlockTypePrimary",
BYPASS_EVENT_TYPE_STATUS_502_HTTP_BAD_GATEWAY, 1);
// Proxy should no longer be blocked, and use first proxy.
SetStatusCode(net::HTTP_OK);
ui_test_utils::NavigateToURL(browser(),
GetURLWithMockHost(test_server, "/echo"));
EXPECT_EQ(GetBody(), kPrimaryResponse);
}
// Tests that if using data reduction proxy results in redirect loop, then
// the proxy is bypassed, and the request is fetched directly.
IN_PROC_BROWSER_TEST_F(DataReductionProxyFallbackBrowsertest, RedirectCycle) {
......
......@@ -473,22 +473,6 @@ TEST_F(DataReductionProxyBypassStatsEndToEndTest,
histogram_tester, "DataReductionProxy.BypassedBytes.ProxyOverridden");
}
TEST_F(DataReductionProxyBypassStatsEndToEndTest, NoChromeProxy502AreBypassed) {
InitializeContext();
base::HistogramTester histogram_tester;
CreateAndExecuteRequest(GURL("http://foo.com"), net::LOAD_NORMAL, net::OK,
"HTTP/1.1 502 Bad Gateway\r\n"
"Via: 1.1 Chrome-Compression-Proxy\r\n"
"Chrome-Proxy: block-once\r\n\r\n",
kErrorBody.c_str(), "HTTP/1.1 200 OK\r\n\r\n",
kBody.c_str());
histogram_tester.ExpectUniqueSample(
"DataReductionProxy.BypassedBytes.Current", kBody.size(), 1);
ExpectOtherBypassedBytesHistogramsEmpty(
histogram_tester, "DataReductionProxy.BypassedBytes.Current");
}
TEST_F(DataReductionProxyBypassStatsEndToEndTest, BypassedBytesCurrent) {
InitializeContext();
struct TestCase {
......@@ -626,6 +610,9 @@ TEST_F(DataReductionProxyBypassStatsEndToEndTest,
{ "DataReductionProxy.BypassedBytes.Status500HttpInternalServerError",
"HTTP/1.1 500 Internal Server Error\r\n\r\n",
},
{ "DataReductionProxy.BypassedBytes.Status502HttpBadGateway",
"HTTP/1.1 502 Bad Gateway\r\n\r\n",
},
{ "DataReductionProxy.BypassedBytes.Status503HttpServiceUnavailable",
"HTTP/1.1 503 Service Unavailable\r\n\r\n",
},
......
......@@ -46,10 +46,5 @@ const base::Feature kDataReductionProxyEnabledWithNetworkService{
"DataReductionProxyEnabledWithNetworkService",
base::FEATURE_DISABLED_BY_DEFAULT};
// Enables block-once action when 502 is received with no Chrome-Proxy header.
const base::Feature kDataReductionProxyBlockOnceOnBadGatewayResponse{
"DataReductionProxyBlockOnceOnBadGatewayResponse",
base::FEATURE_ENABLED_BY_DEFAULT};
} // namespace features
} // namespace data_reduction_proxy
......@@ -17,7 +17,6 @@ extern const base::Feature kDogfood;
extern const base::Feature kDataSaverSiteBreakdownUsingPageLoadMetrics;
extern const base::Feature kDataReductionProxyEnabledWithNetworkService;
extern const base::Feature kDataSaverUseOnDeviceSafeBrowsing;
extern const base::Feature kDataReductionProxyBlockOnceOnBadGatewayResponse;
} // namespace features
} // namespace data_reduction_proxy
......
......@@ -418,16 +418,8 @@ DataReductionProxyBypassType GetDataReductionProxyBypassType(
// Fall back if a 500, 502 or 503 is returned.
if (headers.response_code() == net::HTTP_INTERNAL_SERVER_ERROR)
return BYPASS_EVENT_TYPE_STATUS_500_HTTP_INTERNAL_SERVER_ERROR;
if (headers.response_code() == net::HTTP_BAD_GATEWAY) {
if (base::FeatureList::IsEnabled(
features::kDataReductionProxyBlockOnceOnBadGatewayResponse)) {
data_reduction_proxy_info->bypass_all = true;
data_reduction_proxy_info->mark_proxies_as_bad = false;
data_reduction_proxy_info->bypass_duration = base::TimeDelta();
data_reduction_proxy_info->bypass_action = BYPASS_ACTION_TYPE_BLOCK_ONCE;
}
if (headers.response_code() == net::HTTP_BAD_GATEWAY)
return BYPASS_EVENT_TYPE_STATUS_502_HTTP_BAD_GATEWAY;
}
if (headers.response_code() == net::HTTP_SERVICE_UNAVAILABLE)
return BYPASS_EVENT_TYPE_STATUS_503_HTTP_SERVICE_UNAVAILABLE;
// TODO(kundaji): Bypass if Proxy-Authenticate header value cannot be
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment