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

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

Issue 1579083002: Add UMA metric for GetHash parsing errors. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@osb-pm-2
Patch Set: Review Comments 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 f56103ff9ae8c328727aac6d31181c8d97fcadaf..3a7b8d47703b427a413696e10dcc3f3371e9fec7 100644
--- a/chrome/browser/safe_browsing/protocol_manager.cc
+++ b/chrome/browser/safe_browsing/protocol_manager.cc
@@ -75,6 +75,36 @@ base::TimeDelta GetNextUpdateIntervalFromFinch() {
return base::TimeDelta::FromMinutes(finch_next_update_interval_minutes);
}
+// Enumerate V4 parsing failures for histogramming purposes. DO NOT CHANGE
+// THE ORDERING OF THESE VALUES.
+enum ParseResultType {
+ // Error parsing the protocol buffer from a string.
+ PARSE_FROM_STRING_ERROR = 0,
+
+ // A match in the response had an unexpected THREAT_ENTRY_TYPE.
+ UNEXPECTED_THREAT_ENTRY_TYPE_ERROR = 1,
+
+ // A match in the response had an unexpected THREAT_TYPE.
+ UNEXPECTED_THREAT_TYPE_ERROR = 2,
+
+ // A match in the response had an unexpected PLATFORM_TYPE.
+ UNEXPECTED_PLATFORM_TYPE_ERROR = 3,
+
+ // A match in teh response contained no metadata where metadata was
+ // expected.
+ NO_METADATA_ERROR = 4,
+
+ // Memory space for histograms is determined by the max. ALWAYS
+ // ADD NEW VALUES BEFORE THIS ONE.
+ PARSE_GET_HASH_RESULT_MAX = 5
+};
+
+// Record parsing errors of a GetHash result.
+void RecordParseGetHashResult(ParseResultType result_type) {
+ UMA_HISTOGRAM_ENUMERATION("SafeBrowsing.ParseV4HashResult", result_type,
+ PARSE_GET_HASH_RESULT_MAX);
+}
+
} // namespace
namespace safe_browsing {
@@ -314,7 +344,7 @@ bool SafeBrowsingProtocolManager::ParseV4HashResponse(
FindFullHashesResponse response;
if (!response.ParseFromString(data)) {
- // TODO(kcarattini): Add UMA.
+ RecordParseGetHashResult(PARSE_FROM_STRING_ERROR);
return false;
}
@@ -336,7 +366,8 @@ bool SafeBrowsingProtocolManager::ParseV4HashResponse(
if (!(match.has_threat_entry_type() &&
match.threat_entry_type() == URL_EXPRESSION &&
match.has_threat())) {
- continue;
+ RecordParseGetHashResult(UNEXPECTED_THREAT_ENTRY_TYPE_ERROR);
+ return false;
}
// Fill in the full hash.
@@ -350,19 +381,27 @@ bool SafeBrowsingProtocolManager::ParseV4HashResponse(
}
// Different threat types will handle the metadata differently.
- if (match.has_threat_type() && match.threat_type() == API_ABUSE &&
- match.has_platform_type() &&
- match.platform_type() == CHROME_PLATFORM &&
- match.has_threat_entry_metadata()) {
- // For API Abuse, store a csv of the returned permissions.
- for (const ThreatEntryMetadata::MetadataEntry& m :
- match.threat_entry_metadata().entries()) {
- if (m.key() == "permission") {
- result.metadata += m.value() + ",";
+ if (match.has_threat_type() && match.threat_type() == API_ABUSE) {
+ if (match.has_platform_type() &&
+ match.platform_type() == CHROME_PLATFORM) {
+ if (match.has_threat_entry_metadata()) {
+ // For API Abuse, store a csv of the returned permissions.
+ for (const ThreatEntryMetadata::MetadataEntry& m :
+ match.threat_entry_metadata().entries()) {
+ if (m.key() == "permission") {
+ result.metadata += m.value() + ",";
+ }
+ }
+ } else {
+ RecordParseGetHashResult(NO_METADATA_ERROR);
+ return false;
}
+ } else {
+ RecordParseGetHashResult(UNEXPECTED_PLATFORM_TYPE_ERROR);
+ return false;
}
} else {
- // TODO(kcarattini): Add UMA for unexpected threat type match.
+ RecordParseGetHashResult(UNEXPECTED_THREAT_TYPE_ERROR);
return false;
}
« no previous file with comments | « no previous file | chrome/browser/safe_browsing/protocol_manager_unittest.cc » ('j') | tools/metrics/histograms/histograms.xml » ('J')

Powered by Google App Engine
This is Rietveld 408576698