Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(150)

Unified Diff: net/http/http_server_properties_manager.cc

Issue 2932953002: Persist broken and recently-broken alt-svcs to prefs in HttpServerPropertiesManager (Closed)
Patch Set: Fixed rch's comments from PS14 Created 3 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: net/http/http_server_properties_manager.cc
diff --git a/net/http/http_server_properties_manager.cc b/net/http/http_server_properties_manager.cc
index d348d199a43fc746ec5a1f5f2c745078f03ce6a5..aed5d66d2c95aa84d1718862fe2531f8af5469dc 100644
--- a/net/http/http_server_properties_manager.cc
+++ b/net/http/http_server_properties_manager.cc
@@ -49,6 +49,15 @@ const int kMaxSupportsSpdyServerHostsToPersist = 300;
// Persist 200 ServerNetworkStats.
const int kMaxServerNetworkStatsHostsToPersist = 200;
+// TODO (wangyix): update values for how many broken alt svcs to persist after
+// looking at histograms showing how many are persisted in the real world.
+
+const int kMaxBrokenAlternativeServicesToPersist =
+ std::numeric_limits<int>::max();
+
+const int kMaxRecentlyBrokenAlternativeServicesToPersist =
+ std::numeric_limits<int>::max();
+
const char kVersionKey[] = "version";
const char kServersKey[] = "servers";
const char kSupportsSpdyKey[] = "supports_spdy";
@@ -64,6 +73,20 @@ const char kPortKey[] = "port";
const char kExpirationKey[] = "expiration";
const char kNetworkStatsKey[] = "network_stats";
const char kSrttKey[] = "srtt";
+const char kBrokenAlternativeServicesKey[] = "broken_alternative_services";
+const char kBrokenUntilKey[] = "broken_until";
+const char kBrokenCountKey[] = "broken_count";
+
+void AddAlternativeServiceFieldsToDictionaryValue(
+ const AlternativeService& alternative_service,
+ base::DictionaryValue* dict) {
+ dict->SetInteger(kPortKey, alternative_service.port);
+ if (!alternative_service.host.empty()) {
+ dict->SetString(kHostKey, alternative_service.host);
+ }
+ dict->SetString(kProtocolKey,
+ NextProtoToString(alternative_service.protocol));
+}
std::unique_ptr<base::Value> NetLogCallback(
const base::DictionaryValue& http_server_properties_dict,
@@ -84,9 +107,22 @@ HttpServerPropertiesManager::HttpServerPropertiesManager(
scoped_refptr<base::SingleThreadTaskRunner> pref_task_runner,
scoped_refptr<base::SingleThreadTaskRunner> network_task_runner,
NetLog* net_log)
+ : HttpServerPropertiesManager(pref_delegate,
+ pref_task_runner,
+ network_task_runner,
+ net_log,
+ nullptr) {}
+
+HttpServerPropertiesManager::HttpServerPropertiesManager(
+ PrefDelegate* pref_delegate,
+ scoped_refptr<base::SingleThreadTaskRunner> pref_task_runner,
+ scoped_refptr<base::SingleThreadTaskRunner> network_task_runner,
+ NetLog* net_log,
+ base::TickClock* clock)
: pref_task_runner_(std::move(pref_task_runner)),
pref_delegate_(pref_delegate),
setting_prefs_(false),
+ clock_(clock ? clock : &default_clock_),
is_initialized_(false),
network_task_runner_(std::move(network_task_runner)),
net_log_(
@@ -94,6 +130,7 @@ HttpServerPropertiesManager::HttpServerPropertiesManager(
NetLogSourceType::HTTP_SERVER_PROPERTIES)) {
DCHECK(pref_task_runner_->RunsTasksInCurrentSequence());
DCHECK(pref_delegate_);
+ DCHECK(clock_);
pref_weak_ptr_factory_.reset(
new base::WeakPtrFactory<HttpServerPropertiesManager>(this));
pref_weak_ptr_ = pref_weak_ptr_factory_->GetWeakPtr();
@@ -115,7 +152,7 @@ void HttpServerPropertiesManager::InitializeOnNetworkSequence() {
network_weak_ptr_factory_.reset(
new base::WeakPtrFactory<HttpServerPropertiesManager>(this));
- http_server_properties_impl_.reset(new HttpServerPropertiesImpl());
+ http_server_properties_impl_.reset(new HttpServerPropertiesImpl(clock_));
network_prefs_update_timer_.reset(new base::OneShotTimer);
network_prefs_update_timer_->SetTaskRunner(network_task_runner_);
@@ -533,6 +570,38 @@ void HttpServerPropertiesManager::UpdateCacheFromPrefsOnPrefSequence() {
detected_corrupted_prefs = true;
}
+ // Read broken alternnative services list if it exists.
+ std::unique_ptr<BrokenAlternativeServiceList> broken_alternative_service_list;
+ std::unique_ptr<RecentlyBrokenAlternativeServices>
+ recently_broken_alternative_services;
+ const base::ListValue* broken_alt_svc_list;
+ if (http_server_properties_dict.GetListWithoutPathExpansion(
+ kBrokenAlternativeServicesKey, &broken_alt_svc_list)) {
+ broken_alternative_service_list =
+ base::MakeUnique<BrokenAlternativeServiceList>();
+ recently_broken_alternative_services =
+ base::MakeUnique<RecentlyBrokenAlternativeServices>(
+ RecentlyBrokenAlternativeServices::NO_AUTO_EVICT);
+
+ // Iterate list in reverse-MRU order
+ for (base::ListValue::const_iterator it = broken_alt_svc_list->end();
+ it != broken_alt_svc_list->begin();) {
+ --it;
+ const base::DictionaryValue* entry_dict;
+ if (!it->GetAsDictionary(&entry_dict)) {
+ DVLOG(1) << "Malformed broken alterantive service entry.";
+ detected_corrupted_prefs = true;
+ continue;
+ }
+ if (!AddToBrokenAlternativeServices(
+ *entry_dict, broken_alternative_service_list.get(),
+ recently_broken_alternative_services.get())) {
+ detected_corrupted_prefs = true;
+ continue;
+ }
+ }
+ }
+
network_task_runner_->PostTask(
FROM_HERE,
base::Bind(
@@ -540,7 +609,61 @@ void HttpServerPropertiesManager::UpdateCacheFromPrefsOnPrefSequence() {
base::Unretained(this), base::Passed(&spdy_servers_map),
base::Passed(&alternative_service_map), base::Passed(&addr),
base::Passed(&server_network_stats_map),
- base::Passed(&quic_server_info_map), detected_corrupted_prefs));
+ base::Passed(&quic_server_info_map),
+ base::Passed(&broken_alternative_service_list),
+ base::Passed(&recently_broken_alternative_services),
+ detected_corrupted_prefs));
+}
+
+bool HttpServerPropertiesManager::AddToBrokenAlternativeServices(
+ const base::DictionaryValue& broken_alt_svc_entry_dict,
+ BrokenAlternativeServiceList* broken_alternative_service_list,
+ RecentlyBrokenAlternativeServices* recently_broken_alternative_services) {
+ AlternativeService alt_service;
+ if (!ParseAlternativeServiceDict(broken_alt_svc_entry_dict, true,
+ "broken alternative services",
+ &alt_service)) {
+ return false;
+ }
+
+ // Read broken-count and add an entry for |alt_service| into
+ // |recently_broken_alternative_services|.
+ int broken_count;
+ if (!broken_alt_svc_entry_dict.GetIntegerWithoutPathExpansion(
+ kBrokenCountKey, &broken_count)) {
+ DVLOG(1) << "Recently broken alternative service is missing "
+ << "broken-count.";
+ return false;
+ }
+ if (broken_count < 0) {
+ DVLOG(1) << "Broken alternative service has negative broken-count.";
+ return false;
+ }
+ recently_broken_alternative_services->Put(alt_service, broken_count);
+
+ // Read broken-until (optional) and if it exists, add an entry for
+ // |alt_service| in |broken_alternative_service_list|.
+ if (broken_alt_svc_entry_dict.HasKey(kBrokenUntilKey)) {
+ std::string expiration_string;
+ int64_t expiration_int64;
+ if (broken_alt_svc_entry_dict.GetStringWithoutPathExpansion(
+ kBrokenUntilKey, &expiration_string) &&
+ base::StringToInt64(expiration_string, &expiration_int64)) {
+ time_t expiration_time_t = static_cast<time_t>(expiration_int64);
+ // Convert expiration from time_t to Time to TimeTicks
+ base::TimeTicks expiration_time_ticks =
+ clock_->NowTicks() +
+ (base::Time::FromTimeT(expiration_time_t) - base::Time::Now());
+ broken_alternative_service_list->push_back(
+ std::make_pair(alt_service, expiration_time_ticks));
+ } else {
+ DVLOG(1) << "Broken alternative service has malformed broken-until "
+ << "string.";
+ return false;
+ }
+ }
+
+ return true;
}
bool HttpServerPropertiesManager::AddServersData(
@@ -588,56 +711,65 @@ bool HttpServerPropertiesManager::AddServersData(
}
bool HttpServerPropertiesManager::ParseAlternativeServiceDict(
- const base::DictionaryValue& alternative_service_dict,
- const std::string& server_str,
- AlternativeServiceInfo* alternative_service_info) {
+ const base::DictionaryValue& dict,
+ bool host_required,
+ const std::string& parsing_under,
+ AlternativeService* alternative_service) {
// Protocol is mandatory.
std::string protocol_str;
- if (!alternative_service_dict.GetStringWithoutPathExpansion(kProtocolKey,
- &protocol_str)) {
- DVLOG(1) << "Malformed alternative service protocol string for server: "
- << server_str;
+ if (!dict.GetStringWithoutPathExpansion(kProtocolKey, &protocol_str)) {
+ DVLOG(1) << "Malformed alternative service protocol string under: "
+ << parsing_under;
return false;
}
NextProto protocol = NextProtoFromString(protocol_str);
if (!IsAlternateProtocolValid(protocol)) {
- DVLOG(1) << "Invalid alternative service protocol string for server: "
- << server_str;
+ DVLOG(1) << "Invalid alternative service protocol string \"" << protocol_str
+ << "\" under: " << parsing_under;
return false;
}
- alternative_service_info->set_protocol(protocol);
+ alternative_service->protocol = protocol;
- // Host is optional, defaults to "".
+ // If host is optional, it defaults to "".
std::string host = "";
- if (alternative_service_dict.HasKey(kHostKey) &&
- !alternative_service_dict.GetStringWithoutPathExpansion(kHostKey,
- &host)) {
- DVLOG(1) << "Malformed alternative service host string for server: "
- << server_str;
+ if (dict.HasKey(kHostKey)) {
+ if (!dict.GetStringWithoutPathExpansion(kHostKey, &host)) {
+ DVLOG(1) << "Malformed alternative service host string under: "
+ << parsing_under;
+ return false;
+ }
+ } else if (host_required) {
+ DVLOG(1) << "alternative service missing host string under: "
+ << parsing_under;
return false;
}
- alternative_service_info->set_host(host);
+ alternative_service->host = host;
// Port is mandatory.
int port = 0;
- if (!alternative_service_dict.GetInteger(kPortKey, &port) ||
- !IsPortValid(port)) {
- DVLOG(1) << "Malformed alternative service port for server: " << server_str;
+ if (!dict.GetInteger(kPortKey, &port) || !IsPortValid(port)) {
+ DVLOG(1) << "Malformed alternative service port under: " << parsing_under;
return false;
}
- alternative_service_info->set_port(static_cast<uint32_t>(port));
+ alternative_service->port = static_cast<uint32_t>(port);
+
+ return true;
+}
+bool HttpServerPropertiesManager::ParseAlternativeServiceInfoExpiration(
+ const base::DictionaryValue& dict,
+ const std::string& server_str,
+ AlternativeServiceInfo* alternative_service_info) {
// Expiration is optional, defaults to one day.
base::Time expiration;
- if (!alternative_service_dict.HasKey(kExpirationKey)) {
+ if (!dict.HasKey(kExpirationKey)) {
alternative_service_info->set_expiration(base::Time::Now() +
base::TimeDelta::FromDays(1));
return true;
}
std::string expiration_string;
- if (alternative_service_dict.GetStringWithoutPathExpansion(
- kExpirationKey, &expiration_string)) {
+ if (dict.GetStringWithoutPathExpansion(kExpirationKey, &expiration_string)) {
int64_t expiration_int64 = 0;
if (!base::StringToInt64(expiration_string, &expiration_int64)) {
DVLOG(1) << "Malformed alternative service expiration for server: "
@@ -676,11 +808,17 @@ bool HttpServerPropertiesManager::AddToAlternativeServiceMap(
&alternative_service_dict))
return false;
AlternativeServiceInfo alternative_service_info;
- if (!ParseAlternativeServiceDict(*alternative_service_dict,
+ AlternativeService alternative_service;
+ if (!ParseAlternativeServiceDict(*alternative_service_dict, false,
server.Serialize(),
- &alternative_service_info)) {
+ &alternative_service) ||
+ !ParseAlternativeServiceInfoExpiration(*alternative_service_dict,
+ server.Serialize(),
+ &alternative_service_info)) {
return false;
}
+ alternative_service_info.set_alternative_service(alternative_service);
+
if (base::Time::Now() < alternative_service_info.expiration()) {
alternative_service_info_vector.push_back(alternative_service_info);
}
@@ -798,6 +936,10 @@ void HttpServerPropertiesManager::UpdateCacheFromPrefsOnNetworkSequence(
std::unique_ptr<IPAddress> last_quic_address,
std::unique_ptr<ServerNetworkStatsMap> server_network_stats_map,
std::unique_ptr<QuicServerInfoMap> quic_server_info_map,
+ std::unique_ptr<BrokenAlternativeServiceList>
+ broken_alternative_service_list,
+ std::unique_ptr<RecentlyBrokenAlternativeServices>
+ recently_broken_alternative_services,
bool detected_corrupted_prefs) {
// Preferences have the master data because admins might have pushed new
// preferences. Update the cached data with new data from preferences.
@@ -824,6 +966,19 @@ void HttpServerPropertiesManager::UpdateCacheFromPrefsOnNetworkSequence(
http_server_properties_impl_->SetQuicServerInfoMap(
std::move(quic_server_info_map));
+ if (recently_broken_alternative_services) {
+ DCHECK(broken_alternative_service_list);
+
+ UMA_HISTOGRAM_COUNTS_1000("Net.CountOfBrokenAlternativeServices",
+ broken_alternative_service_list->size());
+ UMA_HISTOGRAM_COUNTS_1000("Net.CountOfRecentlyBrokenAlternativeServices",
+ recently_broken_alternative_services->size());
+
+ http_server_properties_impl_->SetBrokenAndRecentlyBrokenAlternativeServices(
+ std::move(broken_alternative_service_list),
+ std::move(recently_broken_alternative_services));
+ }
+
// Update the prefs with what we have read (delete all corrupted prefs).
if (detected_corrupted_prefs)
ScheduleUpdatePrefsOnNetworkSequence(DETECTED_CORRUPTED_PREFS);
@@ -891,9 +1046,6 @@ void HttpServerPropertiesManager::UpdatePrefsFromCacheOnNetworkSequence(
if (alternative_service.host.empty()) {
alternative_service.host = server.host();
}
- if (IsAlternativeServiceBroken(alternative_service)) {
- continue;
- }
notbroken_alternative_service_info_vector.push_back(
alternative_service_info);
}
@@ -939,6 +1091,43 @@ void HttpServerPropertiesManager::UpdatePrefsFromCacheOnNetworkSequence(
}
}
+ // Make copy of list of broken alternative services stored in
+ // |http_server_properties_impl_|
+ std::unique_ptr<BrokenAlternativeServiceList> broken_alt_svc_list_copy;
+ const BrokenAlternativeServiceList& broken_alternative_service_list =
+ http_server_properties_impl_->broken_alternative_service_list();
+ if (broken_alternative_service_list.size() > 0) {
+ broken_alt_svc_list_copy = base::MakeUnique<BrokenAlternativeServiceList>();
+ size_t count = 0;
+ for (auto it = broken_alternative_service_list.begin();
+ it != broken_alternative_service_list.end() &&
+ count < kMaxBrokenAlternativeServicesToPersist;
+ ++it) {
+ broken_alt_svc_list_copy->push_back(*it);
+ ++count;
+ }
+ }
+
+ // Make copy of RecentlyBrokenAternativeServices stored in
+ // |http_server_properties_impl_|.
+ std::unique_ptr<RecentlyBrokenAlternativeServices>
+ recently_broken_alt_svc_copy;
+ const RecentlyBrokenAlternativeServices& recently_broken_alt_services =
+ http_server_properties_impl_->recently_broken_alternative_services();
+ if (recently_broken_alt_services.size() > 0) {
+ recently_broken_alt_svc_copy =
+ base::MakeUnique<RecentlyBrokenAlternativeServices>(
+ kMaxRecentlyBrokenAlternativeServicesToPersist);
+ size_t count = 0;
+ for (auto it = recently_broken_alt_services.rbegin();
+ it != recently_broken_alt_services.rend() &&
+ count < kMaxRecentlyBrokenAlternativeServicesToPersist;
+ ++it) {
+ recently_broken_alt_svc_copy->Put(it->first, it->second);
+ ++count;
+ }
+ }
+
std::unique_ptr<IPAddress> last_quic_addr = base::MakeUnique<IPAddress>();
http_server_properties_impl_->GetSupportsQuic(last_quic_addr.get());
// Update the preferences on the pref thread.
@@ -949,7 +1138,9 @@ void HttpServerPropertiesManager::UpdatePrefsFromCacheOnNetworkSequence(
base::Passed(&alternative_service_map),
base::Passed(&last_quic_addr),
base::Passed(&server_network_stats_map),
- base::Passed(&quic_server_info_map), completion));
+ base::Passed(&quic_server_info_map),
+ base::Passed(&broken_alt_svc_list_copy),
+ base::Passed(&recently_broken_alt_svc_copy), completion));
}
// A local or temporary data structure to hold |supports_spdy|, SpdySettings,
@@ -987,6 +1178,10 @@ void HttpServerPropertiesManager::UpdatePrefsOnPrefThread(
std::unique_ptr<IPAddress> last_quic_address,
std::unique_ptr<ServerNetworkStatsMap> server_network_stats_map,
std::unique_ptr<QuicServerInfoMap> quic_server_info_map,
+ std::unique_ptr<BrokenAlternativeServiceList>
+ broken_alternative_service_list,
+ std::unique_ptr<RecentlyBrokenAlternativeServices>
+ recently_broken_alternative_services,
const base::Closure& completion) {
typedef base::MRUCache<url::SchemeHostPort, ServerPref> ServerPrefMap;
ServerPrefMap server_pref_map(ServerPrefMap::NO_AUTO_EVICT);
@@ -1081,6 +1276,10 @@ void HttpServerPropertiesManager::UpdatePrefsOnPrefThread(
&http_server_properties_dict);
}
+ SaveBrokenAlternativeServicesToPrefs(
+ broken_alternative_service_list.get(),
+ recently_broken_alternative_services.get(), &http_server_properties_dict);
+
setting_prefs_ = true;
pref_delegate_->SetServerProperties(http_server_properties_dict);
setting_prefs_ = false;
@@ -1105,17 +1304,13 @@ void HttpServerPropertiesManager::SaveAlternativeServiceToServerPrefs(
new base::ListValue);
for (const AlternativeServiceInfo& alternative_service_info :
alternative_service_info_vector) {
- const AlternativeService alternative_service =
+ const AlternativeService& alternative_service =
alternative_service_info.alternative_service();
DCHECK(IsAlternateProtocolValid(alternative_service.protocol));
std::unique_ptr<base::DictionaryValue> alternative_service_dict(
new base::DictionaryValue);
- alternative_service_dict->SetInteger(kPortKey, alternative_service.port);
- if (!alternative_service.host.empty()) {
- alternative_service_dict->SetString(kHostKey, alternative_service.host);
- }
- alternative_service_dict->SetString(
- kProtocolKey, NextProtoToString(alternative_service.protocol));
+ AddAlternativeServiceFieldsToDictionaryValue(
+ alternative_service, alternative_service_dict.get());
// JSON cannot store int64_t, so expiration is converted to a string.
alternative_service_dict->SetString(
kExpirationKey,
@@ -1173,6 +1368,59 @@ void HttpServerPropertiesManager::SaveQuicServerInfoMapToServerPrefs(
kQuicServers, std::move(quic_servers_dict));
}
+void HttpServerPropertiesManager::SaveBrokenAlternativeServicesToPrefs(
+ const BrokenAlternativeServiceList* broken_alternative_service_list,
+ const RecentlyBrokenAlternativeServices*
+ recently_broken_alternative_services,
+ base::DictionaryValue* http_server_properties_dict) {
+ if (!recently_broken_alternative_services) {
+ DCHECK(!broken_alternative_service_list);
+ return;
+ }
+
+ // JSON list will be in MRU order according to
+ // |recently_broken_alternative_services|.
+ base::ListValue* json_list = new base::ListValue;
+ // Maps an alternative service to the index where it's stored in |json_list|.
+ std::unordered_map<AlternativeService, size_t, AlternativeServiceHash>
+ json_list_index_map;
+ for (auto it = recently_broken_alternative_services->rbegin();
+ it != recently_broken_alternative_services->rend(); ++it) {
+ const AlternativeService& alt_service = it->first;
+ int broken_count = it->second;
+ base::DictionaryValue entry_dict;
+ AddAlternativeServiceFieldsToDictionaryValue(alt_service, &entry_dict);
+ entry_dict.SetIntegerWithoutPathExpansion(kBrokenCountKey, broken_count);
+ json_list_index_map[alt_service] = json_list->GetList().size();
+ json_list->GetList().push_back(std::move(entry_dict));
+ }
+
+ if (!broken_alternative_service_list)
+ return;
+
+ // Add expiration time info from |broken_alternative_service_list| to
+ // the JSON list.
+ for (auto entry : *broken_alternative_service_list) {
+ const AlternativeService& alt_service = entry.first;
+ base::TimeTicks expiration_time_ticks = entry.second;
+ // Convert expiration from TimeTicks to Time to time_t
+ time_t expiration_time_t =
+ (base::Time::Now() + (expiration_time_ticks - clock_->NowTicks()))
+ .ToTimeT();
+ int64_t expiration_int64 = static_cast<int64_t>(expiration_time_t);
+
+ size_t json_list_index = json_list_index_map[alt_service];
+ base::DictionaryValue* entry_dict;
+ DCHECK(json_list->GetDictionary(json_list_index, &entry_dict));
+ entry_dict->SetStringWithoutPathExpansion(
+ kBrokenUntilKey, base::Int64ToString(expiration_int64));
+ }
+
+ http_server_properties_dict->SetWithoutPathExpansion(
+ kBrokenAlternativeServicesKey,
+ base::WrapUnique<base::ListValue>(json_list));
+}
+
void HttpServerPropertiesManager::OnHttpServerPropertiesChanged() {
DCHECK(pref_task_runner_->RunsTasksInCurrentSequence());
if (!setting_prefs_)

Powered by Google App Engine
This is Rietveld 408576698