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

Unified Diff: chrome/browser/safe_browsing/protocol_manager.cc

Issue 1555953002: Adds backoff and Min Wait Duration compliance to Protocolmanager pver4 handlers. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@osb-pm-2
Patch Set: Rebase Created 4 years, 11 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: chrome/browser/safe_browsing/protocol_manager.cc
diff --git a/chrome/browser/safe_browsing/protocol_manager.cc b/chrome/browser/safe_browsing/protocol_manager.cc
index 02d1c536d148e0c21faa083e2267def710510f45..51346e037eecad9022627137d3bcda776bdc0866 100644
--- a/chrome/browser/safe_browsing/protocol_manager.cc
+++ b/chrome/browser/safe_browsing/protocol_manager.cc
@@ -141,12 +141,15 @@ SafeBrowsingProtocolManager::SafeBrowsingProtocolManager(
request_type_(NO_REQUEST),
update_error_count_(0),
gethash_error_count_(0),
+ gethash_v4_error_count_(0),
update_back_off_mult_(1),
gethash_back_off_mult_(1),
+ gethash_v4_back_off_mult_(1),
next_update_interval_(base::TimeDelta::FromSeconds(
base::RandInt(kSbTimerStartIntervalSecMin,
kSbTimerStartIntervalSecMax))),
chunk_pending_to_write_(false),
+ next_gethash_v4_time_(Time::Now()),
version_(config.version),
update_size_(0),
client_name_(config.client_name),
@@ -280,6 +283,12 @@ bool SafeBrowsingProtocolManager::ParseV4HashResponse(
response.negative_cache_duration().seconds());
}
+ if (response.has_minimum_wait_duration()) {
+ // Seconds resolution is good enough so we ignore the nanos field.
+ next_gethash_v4_time_ = Time::Now() + base::TimeDelta::FromSeconds(
+ response.minimum_wait_duration().seconds());
+ }
+
// Loop over the threat matches and fill in full_hashes.
for (const ThreatMatch& match : response.matches()) {
// Make sure the platform and threat entry type match.
@@ -324,7 +333,16 @@ void SafeBrowsingProtocolManager::GetV4FullHashes(
ThreatType threat_type,
FullHashCallback callback) {
DCHECK(CalledOnValidThread());
- // TODO(kcarattini): Implement backoff behavior.
+ // We need to wait the minimum waiting duration, and if we are in backoff,
+ // we need to check if we're past the next allowed time. If we are, we can
+ // proceed with the request. If not, we are required to return empty results
+ // (i.e. treat the page as safe).
+ if (Time::Now() <= next_gethash_v4_time_) {
+ // TODO(kcarattini): Add UMA recording.
+ std::vector<SBFullHashResult> full_hashes;
+ callback.Run(full_hashes, base::TimeDelta());
+ return;
+ }
std::string req_base64 = GetV4HashRequest(prefixes, platforms, threat_type);
GURL gethash_url = GetV4HashUrl(req_base64);
@@ -431,8 +449,8 @@ void SafeBrowsingProtocolManager::OnURLFetchComplete(
base::TimeDelta negative_cache_duration;
if (status.is_success() && response_code == net::HTTP_OK) {
// TODO(kcarattini): Add UMA reporting.
- // TODO(kcarattini): Implement backoff and minimum waiting duration
- // compliance.
+ gethash_v4_error_count_ = 0;
+ gethash_v4_back_off_mult_ = 1;
std::string data;
source->GetResponseAsString(&data);
if (!ParseV4HashResponse(data, &full_hashes, &negative_cache_duration)) {
@@ -440,7 +458,7 @@ void SafeBrowsingProtocolManager::OnURLFetchComplete(
// TODO(kcarattini): Add UMA reporting.
}
} else {
- // TODO(kcarattini): Handle error by setting backoff interval.
+ HandleGetHashV4Error(Time::Now());
// TODO(kcarattini): Add UMA reporting.
if (status.status() == net::URLRequestStatus::FAILED) {
DVLOG(1) << "SafeBrowsing GetEncodedFullHashes request for: " <<
@@ -710,6 +728,28 @@ base::TimeDelta SafeBrowsingProtocolManager::GetNextBackOffInterval(
return base::TimeDelta::FromMinutes(1);
}
+// Backoff interval is MIN(((2^(n-1))*15 minutes) * (RAND + 1), 24 hours) where
+// n is the number of consecutive errors.
+base::TimeDelta SafeBrowsingProtocolManager::GetNextV4BackOffInterval(
Nathan Parker 2016/01/07 06:06:58 This could be static, if you remove the CalledOnVa
kcarattini 2016/01/07 06:48:53 Done.
+ size_t* error_count,
+ size_t* multiplier) const {
+ DCHECK(CalledOnValidThread());
+ DCHECK(multiplier && error_count);
+ (*error_count)++;
+ if (*error_count > 1) {
+ *multiplier *= 2;
Nathan Parker 2016/01/07 06:06:58 What happens on integer overflow? Probably want t
kcarattini 2016/01/07 06:48:53 Done.
+ }
+ base::TimeDelta next = base::TimeDelta::FromMinutes(
+ *multiplier * (1 + base::RandDouble()) * 15);
+
+ base::TimeDelta day = base::TimeDelta::FromHours(24);
+
+ if (next < day)
+ return next;
+ else
+ return day;
+}
+
// This request requires getting a list of all the chunks for each list from the
// database asynchronously. The request will be issued when we're called back in
// OnGetChunksComplete.
@@ -864,6 +904,13 @@ void SafeBrowsingProtocolManager::HandleGetHashError(const Time& now) {
next_gethash_time_ = now + next;
}
+void SafeBrowsingProtocolManager::HandleGetHashV4Error(const Time& now) {
+ DCHECK(CalledOnValidThread());
+ base::TimeDelta next = GetNextV4BackOffInterval(
+ &gethash_v4_error_count_, &gethash_v4_back_off_mult_);
+ next_gethash_v4_time_ = now + next;
+}
+
void SafeBrowsingProtocolManager::UpdateFinished(bool success) {
UpdateFinished(success, !success);
}

Powered by Google App Engine
This is Rietveld 408576698