Skip to content

GitLab

C
chromium
Commit c5dcb848 authored by Tarun Bansal's avatar Tarun Bansal Committed by Commit Bot
Browse files
Options

Miscellaneous cleanups to the data reduction proxy 

* Remove some of the leftover tamper detection code
* Add an explicit callback when a new client config is fetched
* Limit the use of mock config for only the tests where it is actually required
* Remove the deprecated mock call to the network quality estimator

Bug: 760294, 675761
Change-Id: Iaf8077e83bacca574aa6dea0ed9906f0d25d0176
Reviewed-on: https://chromium-review.googlesource.com/766072Reviewed-by: default avatarDoug Arnett <dougarnett@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516088}
parent 7d9a2a8a
Hide whitespace changes
Inline Side-by-side
Showing with 33 additions and 347 deletions
components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
View file @ c5dcb848
......@@ -170,6 +170,11 @@ bool DataReductionProxyConfig::ShouldAddDefaultProxyBypassRules() const {
return true;
}
void DataReductionProxyConfig::OnNewClientConfigFetched() {
DCHECK(thread_checker_.CalledOnValidThread());
ReloadConfig();
}
void DataReductionProxyConfig::ReloadConfig() {
DCHECK(thread_checker_.CalledOnValidThread());
DCHECK(configurator_);
......
components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h
View file @ c5dcb848
......@@ -118,11 +118,6 @@ class DataReductionProxyConfig
// InitDataReductionProxySettings.
void SetProxyConfig(bool enabled, bool at_startup);
// Provides a mechanism for an external object to force |this| to refresh
// the Data Reduction Proxy configuration from |config_values_| and apply to
// |configurator_|. Used by the Data Reduction Proxy config service client.
void ReloadConfig();
// Returns true if a Data Reduction Proxy was used for the given |request|.
// If true, |proxy_info.proxy_servers.front()| will contain the name of the
// proxy that was used. Subsequent entries in |proxy_info.proxy_servers| will
......@@ -207,6 +202,9 @@ class DataReductionProxyConfig
std::vector<DataReductionProxyServer> GetProxiesForHttp() const;
// Called when a new client config has been fetched.
void OnNewClientConfigFetched();
protected:
// Should be called when there is a change in the status of the availability
// of the insecure data saver proxies triggered due to warmup URL.
......@@ -246,6 +244,11 @@ class DataReductionProxyConfig
NETWORK_QUALITY_AT_LAST_QUERY_NOT_SLOW
};
// Provides a mechanism for an external object to force |this| to refresh
// the Data Reduction Proxy configuration from |config_values_| and apply to
// |configurator_|. Used by the Data Reduction Proxy config service client.
void ReloadConfig();
// NetworkChangeNotifier::IPAddressObserver implementation:
void OnIPAddressChanged() override;
......
components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client.cc
View file @ c5dcb848
......@@ -415,7 +415,7 @@ void DataReductionProxyConfigServiceClient::InvalidateConfig() {
request_options_->Invalidate();
config_values_->Invalidate();
io_data_->SetPingbackReportingFraction(0.0f);
config_->ReloadConfig();
config_->OnNewClientConfigFetched();
}
std::unique_ptr<net::URLFetcher>
......@@ -543,7 +543,7 @@ bool DataReductionProxyConfigServiceClient::ParseAndApplyProxyConfig(
request_options_->SetSecureSession(config.session_key());
config_values_->UpdateValues(proxies);
config_->ReloadConfig();
config_->OnNewClientConfigFetched();
remote_config_applied_ = true;
return true;
}
......
components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.h
View file @ c5dcb848
......@@ -21,7 +21,6 @@ class TickClock;
}
namespace net {
class NetworkQualityEstimator;
class NetLog;
class ProxyServer;
}
......@@ -156,9 +155,6 @@ class MockDataReductionProxyConfig : public TestDataReductionProxyConfig {
base::TimeDelta* min_retry_delay));
MOCK_METHOD1(SecureProxyCheck,
void(SecureProxyCheckerCallback fetcher_callback));
MOCK_METHOD1(
IsNetworkQualityProhibitivelySlow,
bool(const net::NetworkQualityEstimator* network_quality_estimator));
using DataReductionProxyConfig::UpdateConfigForTesting;
......
components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
View file @ c5dcb848
......@@ -119,6 +119,16 @@ class DataReductionProxyConfigTest : public testing::Test {
base::RunLoop().RunUntilIdle();
network_change_notifier_.reset(net::NetworkChangeNotifier::CreateMock());
test_context_ = DataReductionProxyTestContext::Builder()
.WithMockDataReductionProxyService()
.Build();
ResetSettings();
expected_params_.reset(new TestDataReductionProxyParams());
}
void RecreateContextWithMockConfig() {
test_context_ = DataReductionProxyTestContext::Builder()
.WithMockConfig()
.WithMockDataReductionProxyService()
......@@ -240,7 +250,7 @@ TEST_F(DataReductionProxyConfigTest, TestReloadConfigHoldback) {
ResetSettings();
config()->UpdateConfigForTesting(true, false, true);
config()->ReloadConfig();
config()->OnNewClientConfigFetched();
EXPECT_EQ(std::vector<net::ProxyServer>(), GetConfiguredProxiesForHttp());
}
......@@ -259,21 +269,21 @@ TEST_F(DataReductionProxyConfigTest,
config()->UpdateConfigForTesting(true /* enabled */,
false /* secure_proxies_allowed */,
true /* insecure_proxies_allowed */);
config()->ReloadConfig();
config()->OnNewClientConfigFetched();
EXPECT_EQ(std::vector<net::ProxyServer>({kHttpProxy}),
GetConfiguredProxiesForHttp());
config()->UpdateConfigForTesting(true, true, false);
config()->ReloadConfig();
config()->OnNewClientConfigFetched();
EXPECT_EQ(std::vector<net::ProxyServer>({kHttpsProxy}),
GetConfiguredProxiesForHttp());
config()->UpdateConfigForTesting(true, false, false);
config()->ReloadConfig();
config()->OnNewClientConfigFetched();
EXPECT_EQ(std::vector<net::ProxyServer>(), GetConfiguredProxiesForHttp());
config()->UpdateConfigForTesting(true, true, true);
config()->ReloadConfig();
config()->OnNewClientConfigFetched();
EXPECT_EQ(std::vector<net::ProxyServer>({kHttpsProxy, kHttpProxy}),
GetConfiguredProxiesForHttp());
......@@ -294,6 +304,7 @@ TEST_F(DataReductionProxyConfigTest,
}
TEST_F(DataReductionProxyConfigTest, TestOnIPAddressChanged) {
RecreateContextWithMockConfig();
const net::URLRequestStatus kSuccess(net::URLRequestStatus::SUCCESS, net::OK);
const net::ProxyServer kHttpsProxy = net::ProxyServer::FromURI(
"https://secure_origin.net:443", net::ProxyServer::SCHEME_HTTP);
......@@ -305,7 +316,7 @@ TEST_F(DataReductionProxyConfigTest, TestOnIPAddressChanged) {
// The proxy is enabled initially.
config()->UpdateConfigForTesting(true, true, true);
config()->ReloadConfig();
config()->OnNewClientConfigFetched();
// IP address change triggers a secure proxy check that succeeds. Proxy
// remains unrestricted.
......@@ -917,9 +928,6 @@ TEST_F(DataReductionProxyConfigTest, ShouldEnableLoFi) {
scoped_feature_list.InitAndEnableFeature(
features::kDataReductionProxyDecidesTransform);
// Expect network quality check is never called.
EXPECT_CALL(*config(), IsNetworkQualityProhibitivelySlow(_)).Times(0);
net::TestURLRequestContext context;
net::TestDelegate delegate;
std::unique_ptr<net::URLRequest> request = context.CreateRequest(
......@@ -941,9 +949,6 @@ TEST_F(DataReductionProxyConfigTest, ShouldEnableLitePages) {
scoped_feature_list.InitAndEnableFeature(
features::kDataReductionProxyDecidesTransform);
// Expect network quality check is never called.
EXPECT_CALL(*config(), IsNetworkQualityProhibitivelySlow(_)).Times(0);
net::TestURLRequestContext context_;
net::TestDelegate delegate_;
std::unique_ptr<net::URLRequest> request = context_.CreateRequest(
......
components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc
View file @ c5dcb848
......@@ -459,6 +459,7 @@ DataReductionProxyTestContext::Builder::Build() {
std::move(params), task_runner, net_log.get(), configurator.get(),
event_creator.get()));
} else {
test_context_flags ^= USE_MOCK_CONFIG;
if (!proxy_servers_.empty()) {
params->SetProxiesForHttp(proxy_servers_);
}
......
components/data_reduction_proxy/core/common/data_reduction_proxy_headers.cc
View file @ c5dcb848
......@@ -54,12 +54,6 @@ const char kChromeProxyActionBlockOnce[] = "block-once";
const char kChromeProxyActionBlock[] = "block";
const char kChromeProxyActionBypass[] = "bypass";
// Actions for tamper detection fingerprints.
const char kChromeProxyActionFingerprintChromeProxy[] = "fcp";
const char kChromeProxyActionFingerprintVia[] = "fvia";
const char kChromeProxyActionFingerprintOtherHeaders[] = "foh";
const char kChromeProxyActionFingerprintContentLength[] = "fcl";
const int kShortBypassMaxSeconds = 59;
const int kMediumBypassMaxSeconds = 300;
......@@ -470,56 +464,6 @@ DataReductionProxyBypassType GetDataReductionProxyBypassType(
return BYPASS_EVENT_TYPE_MAX;
}
bool GetDataReductionProxyActionFingerprintChromeProxy(
const net::HttpResponseHeaders* headers,
std::string* chrome_proxy_fingerprint) {
return GetDataReductionProxyActionValue(
headers,
kChromeProxyActionFingerprintChromeProxy,
chrome_proxy_fingerprint);
}
bool GetDataReductionProxyActionFingerprintVia(
const net::HttpResponseHeaders* headers,
std::string* via_fingerprint) {
return GetDataReductionProxyActionValue(
headers,
kChromeProxyActionFingerprintVia,
via_fingerprint);
}
bool GetDataReductionProxyActionFingerprintOtherHeaders(
const net::HttpResponseHeaders* headers,
std::string* other_headers_fingerprint) {
return GetDataReductionProxyActionValue(
headers,
kChromeProxyActionFingerprintOtherHeaders,
other_headers_fingerprint);
}
bool GetDataReductionProxyActionFingerprintContentLength(
const net::HttpResponseHeaders* headers,
std::string* content_length_fingerprint) {
return GetDataReductionProxyActionValue(
headers,
kChromeProxyActionFingerprintContentLength,
content_length_fingerprint);
}
void GetDataReductionProxyHeaderWithFingerprintRemoved(
const net::HttpResponseHeaders* headers,
std::vector<std::string>* values) {
DCHECK(values);
std::string value;
size_t iter = 0;
while (headers->EnumerateHeader(&iter, kChromeProxyHeader, &value)) {
if (StartsWithActionPrefix(value, kChromeProxyActionFingerprintChromeProxy))
continue;
values->push_back(std::move(value));
}
}
int64_t GetDataReductionProxyOFCL(const net::HttpResponseHeaders* headers) {
std::string ofcl_str;
int64_t ofcl;
......
components/data_reduction_proxy/core/common/data_reduction_proxy_headers.h
View file @ c5dcb848
......@@ -184,31 +184,6 @@ bool ParseHeadersAndSetBypassDuration(const net::HttpResponseHeaders* headers,
base::StringPiece action_prefix,
base::TimeDelta* bypass_duration);
// Gets the fingerprint of the Chrome-Proxy header.
bool GetDataReductionProxyActionFingerprintChromeProxy(
const net::HttpResponseHeaders* headers,
std::string* chrome_proxy_fingerprint);
// Gets the fingerprint of the Via header.
bool GetDataReductionProxyActionFingerprintVia(
const net::HttpResponseHeaders* headers,
std::string* via_fingerprint);
// Gets the fingerprint of a list of headers.
bool GetDataReductionProxyActionFingerprintOtherHeaders(
const net::HttpResponseHeaders* headers,
std::string* other_headers_fingerprint);
// Gets the fingerprint of Content-Length header.
bool GetDataReductionProxyActionFingerprintContentLength(
const net::HttpResponseHeaders* headers,
std::string* content_length_fingerprint);
// Returns values of the Chrome-Proxy header, but with its fingerprint removed.
void GetDataReductionProxyHeaderWithFingerprintRemoved(
const net::HttpResponseHeaders* headers,
std::vector<std::string>* values);
// Returns the OFCL value in the Chrome-Proxy header. Returns -1 in case of
// of error or if OFCL does not exist. |headers| must be non-null.
int64_t GetDataReductionProxyOFCL(const net::HttpResponseHeaders* headers);
......
components/data_reduction_proxy/core/common/data_reduction_proxy_headers_unittest.cc
View file @ c5dcb848
......@@ -877,247 +877,4 @@ TEST_F(DataReductionProxyHeadersTest,
}
}
TEST_F(DataReductionProxyHeadersTest,
GetDataReductionProxyActionFingerprintChromeProxy) {
const struct {
std::string label;
const char* headers;
bool expected_fingerprint_exist;
std::string expected_fingerprint;
} tests[] = {
{ "fingerprint doesn't exist",
"HTTP/1.1 200 OK\n"
"Chrome-Proxy: bypass=0\n",
false,
"",
},
{ "fingerprint occurs once",
"HTTP/1.1 200 OK\n"
"Chrome-Proxy: bypass=1, fcp=fp\n",
true,
"fp",
},
{ "fingerprint occurs twice",
"HTTP/1.1 200 OK\n"
"Chrome-Proxy: bypass=2, fcp=fp1, fcp=fp2\n",
true,
"fp1",
},
};
for (size_t i = 0; i < arraysize(tests); ++i) {
std::string headers(tests[i].headers);
HeadersToRaw(&headers);
scoped_refptr<net::HttpResponseHeaders> parsed(
new net::HttpResponseHeaders(headers));
std::string fingerprint;
bool fingerprint_exist = GetDataReductionProxyActionFingerprintChromeProxy(
parsed.get(), &fingerprint);
EXPECT_EQ(tests[i].expected_fingerprint_exist, fingerprint_exist)
<< tests[i].label;
if (fingerprint_exist)
EXPECT_EQ(tests[i].expected_fingerprint, fingerprint) << tests[i].label;
}
}
TEST_F(DataReductionProxyHeadersTest,
GetDataReductionProxyActionFingerprintVia) {
const struct {
std::string label;
const char* headers;
bool expected_fingerprint_exist;
std::string expected_fingerprint;
} tests[] = {
{ "fingerprint doesn't exist",
"HTTP/1.1 200 OK\n"
"Chrome-Proxy: bypass=0\n",
false,
"",
},
{ "fingerprint occurs once",
"HTTP/1.1 200 OK\n"
"Chrome-Proxy: bypass=1, fvia=fvia\n",
true,
"fvia",
},
{ "fingerprint occurs twice",
"HTTP/1.1 200 OK\n"
"Chrome-Proxy: bypass=2, fvia=fvia1, fvia=fvia2\n",
true,
"fvia1",
},
};
for (size_t i = 0; i < arraysize(tests); ++i) {
std::string headers(tests[i].headers);
HeadersToRaw(&headers);
scoped_refptr<net::HttpResponseHeaders> parsed(
new net::HttpResponseHeaders(headers));
std::string fingerprint;
bool fingerprint_exist =
GetDataReductionProxyActionFingerprintVia(parsed.get(), &fingerprint);
EXPECT_EQ(tests[i].expected_fingerprint_exist, fingerprint_exist)
<< tests[i].label;
if (fingerprint_exist)
EXPECT_EQ(tests[i].expected_fingerprint, fingerprint) << tests[i].label;
}
}
TEST_F(DataReductionProxyHeadersTest,
GetDataReductionProxyActionFingerprintOtherHeaders) {
const struct {
std::string label;
const char* headers;
bool expected_fingerprint_exist;
std::string expected_fingerprint;
} tests[] = {
{ "fingerprint doesn't exist",
"HTTP/1.1 200 OK\n"
"Chrome-Proxy: bypass=0\n",
false,
"",
},
{ "fingerprint occurs once",
"HTTP/1.1 200 OK\n"
"Chrome-Proxy: bypass=1, foh=foh\n",
true,
"foh",
},
{ "fingerprint occurs twice",
"HTTP/1.1 200 OK\n"
"Chrome-Proxy: bypass=2, foh=foh1, foh=foh2\n",
true,
"foh1",
},
};
for (size_t i = 0; i < arraysize(tests); ++i) {
std::string headers(tests[i].headers);
HeadersToRaw(&headers);
scoped_refptr<net::HttpResponseHeaders> parsed(
new net::HttpResponseHeaders(headers));
std::string fingerprint;
bool fingerprint_exist = GetDataReductionProxyActionFingerprintOtherHeaders(
parsed.get(), &fingerprint);
EXPECT_EQ(tests[i].expected_fingerprint_exist, fingerprint_exist)
<< tests[i].label;
if (fingerprint_exist)
EXPECT_EQ(tests[i].expected_fingerprint, fingerprint) << tests[i].label;
}
}
TEST_F(DataReductionProxyHeadersTest,
GetDataReductionProxyActionFingerprintContentLength) {
const struct {
std::string label;
const char* headers;
bool expected_fingerprint_exist;
std::string expected_fingerprint;
} tests[] = {
{ "fingerprint doesn't exist",
"HTTP/1.1 200 OK\n"
"Chrome-Proxy: bypass=0\n",
false,
"",
},
{ "fingerprint occurs once",
"HTTP/1.1 200 OK\n"
"Chrome-Proxy: bypass=1, fcl=fcl\n",
true,
"fcl",
},
{ "fingerprint occurs twice",
"HTTP/1.1 200 OK\n"
"Chrome-Proxy: bypass=2, fcl=fcl1, fcl=fcl2\n",
true,
"fcl1",
},
};
for (size_t i = 0; i < arraysize(tests); ++i) {
std::string headers(tests[i].headers);
HeadersToRaw(&headers);
scoped_refptr<net::HttpResponseHeaders> parsed(
new net::HttpResponseHeaders(headers));
std::string fingerprint;
bool fingerprint_exist =
GetDataReductionProxyActionFingerprintContentLength(parsed.get(),
&fingerprint);
EXPECT_EQ(tests[i].expected_fingerprint_exist, fingerprint_exist)
<< tests[i].label;
if (fingerprint_exist)
EXPECT_EQ(tests[i].expected_fingerprint, fingerprint) << tests[i].label;
}
}
TEST_F(DataReductionProxyHeadersTest,
GetDataReductionProxyHeaderWithFingerprintRemoved) {
const struct {
std::string label;
const char* headers;
std::string expected_output_values_string;
} test[] = {
{
"Checks the case that there is no Chrome-Proxy header's fingerprint.",
"HTTP/1.1 200 OK\n"
"Chrome-Proxy: 1,2,3,5\n",
"1,2,3,5,",
},
{
"Checks the case that there is Chrome-Proxy header's fingerprint.",
"HTTP/1.1 200 OK\n"
"Chrome-Proxy: 1,2,3,fcp=4,5\n",
"1,2,3,5,",
},
{
"Checks the case that there is Chrome-Proxy header's fingerprint, and it"
"occurs at the end.",
"HTTP/1.1 200 OK\n"
"Chrome-Proxy: 1,2,3,fcp=4,",
"1,2,3,",
},
{
"Checks the case that there is Chrome-Proxy header's fingerprint, and it"
"occurs at the beginning.",
"HTTP/1.1 200 OK\n"
"Chrome-Proxy: fcp=1,2,3,",
"2,3,",
},
{
"Checks the case that value is longer than prefix.",
"HTTP/1.1 200 OK\n"
"Chrome-Proxy: fcp=1,fcp!=1,fcp!=2,fcpfcp=3",
"fcp!=1,fcp!=2,fcpfcp=3,",
},
{
"Checks the case that value is shorter than prefix but similar.",
"HTTP/1.1 200 OK\n"
"Chrome-Proxy: fcp=1,fcp,fcp=",
"fcp,fcp=,",
},
};
for (size_t i = 0; i < arraysize(test); ++i) {
std::string headers(test[i].headers);
HeadersToRaw(&headers);
scoped_refptr<net::HttpResponseHeaders> parsed(
new net::HttpResponseHeaders(headers));
std::vector<std::string> output_values;
GetDataReductionProxyHeaderWithFingerprintRemoved(parsed.get(),
&output_values);
std::string output_values_string;
for (size_t j = 0; j < output_values.size(); ++j)
output_values_string += output_values[j] + ",";
EXPECT_EQ(test[i].expected_output_values_string, output_values_string)
<< test[i].label;
}
}
} // namespace data_reduction_proxy
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