[Safe browsing] Clean up code to scan hash results for threats.

chrome/browser/safe_browsing/database_manager.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/safe_browsing/database_manager.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 50 matching lines @@
 }
 
 bool IsExpectedThreat(
     const SBThreatType threat_type,
     const std::vector<SBThreatType>& expected_threats) {
   return expected_threats.end() != std::find(expected_threats.begin(),
                                              expected_threats.end(),
                                              threat_type);
 }
 
-// |list_id| is from |safe_browsing_util::ListType|.
-SBThreatType GetThreatTypeFromListId(int list_id) {
-  if (list_id == safe_browsing_util::PHISH) {
-    return SB_THREAT_TYPE_URL_PHISHING;
+// Return the list id from the first result in |full_hashes| which matches
+// |hash|, or INVALID if none match.
+safe_browsing_util::ListType GetHashThreatListType(
+    const SBFullHash& hash,
+    const std::vector<SBFullHashResult>& full_hashes) {
+  for (size_t i = 0; i < full_hashes.size(); ++i) {
+    if (SBFullHashEqual(hash, full_hashes[i].hash))
+      return static_cast<safe_browsing_util::ListType>(full_hashes[i].list_id);
   }
+  return safe_browsing_util::INVALID;
+}
 
-  if (list_id == safe_browsing_util::MALWARE) {
-    return SB_THREAT_TYPE_URL_MALWARE;
+// Given a URL, compare all the possible host + path full hashes to the set of
+// provided full hashes.  Returns the list id of the a matching result from
+// |full_hashes|, or INVALID if none match.
+safe_browsing_util::ListType GetUrlThreatListType(
+    const GURL& url,
+    const std::vector<SBFullHashResult>& full_hashes) {
+  if (full_hashes.empty())
+    return safe_browsing_util::INVALID;
+
+  std::vector<std::string> patterns;
+  safe_browsing_util::GeneratePatternsToCheck(url, &patterns);
+
+  for (size_t i = 0; i < patterns.size(); ++i) {
+    safe_browsing_util::ListType threat =
+        GetHashThreatListType(SBFullHashForString(patterns[i]), full_hashes);
+    if (threat != safe_browsing_util::INVALID)
+      return threat;
   }
+  return safe_browsing_util::INVALID;
+}
 
-  if (list_id == safe_browsing_util::BINURL) {
-    return SB_THREAT_TYPE_BINARY_MALWARE_URL;
+SBThreatType GetThreatTypeFromListType(safe_browsing_util::ListType list_type) {
+  switch (list_type) {
+    case safe_browsing_util::PHISH:
+      return SB_THREAT_TYPE_URL_PHISHING;
+    case safe_browsing_util::MALWARE:
+      return SB_THREAT_TYPE_URL_MALWARE;
+    case safe_browsing_util::BINURL:
+      return SB_THREAT_TYPE_BINARY_MALWARE_URL;
+    case safe_browsing_util::EXTENSIONBLACKLIST:
+      return SB_THREAT_TYPE_EXTENSION;
+    default:
+      DVLOG(1) << "Unknown safe browsing list id " << list_type;
+      return SB_THREAT_TYPE_SAFE;
   }
-
-  if (list_id == safe_browsing_util::EXTENSIONBLACKLIST) {
-    return SB_THREAT_TYPE_EXTENSION;
-  }
-
-  DVLOG(1) << "Unknown safe browsing list id " << list_id;
-  return SB_THREAT_TYPE_SAFE;
 }
 
 }  // namespace
 
+// static
+SBThreatType SafeBrowsingDatabaseManager::GetHashThreatType(
+    const SBFullHash& hash,
+    const std::vector<SBFullHashResult>& full_hashes) {
+  return GetThreatTypeFromListType(GetHashThreatListType(hash, full_hashes));
+}
+
+// static
+SBThreatType SafeBrowsingDatabaseManager::GetUrlThreatType(
+    const GURL& url,
+    const std::vector<SBFullHashResult>& full_hashes) {
+  return GetThreatTypeFromListType(GetUrlThreatListType(url, full_hashes));
+}
+
 SafeBrowsingDatabaseManager::SafeBrowsingCheck::SafeBrowsingCheck(
     const std::vector<GURL>& urls,
     const std::vector<SBFullHash>& full_hashes,
     Client* client,
     safe_browsing_util::ListType check_type,
     const std::vector<SBThreatType>& expected_threats)
     : urls(urls),
       url_results(urls.size(), SB_THREAT_TYPE_SAFE),
       full_hashes(full_hashes),
       full_hash_results(full_hashes.size(), SB_THREAT_TYPE_SAFE),
@@ skipping 769 matching lines @@
 }
 
 bool SafeBrowsingDatabaseManager::HandleOneCheck(
     SafeBrowsingCheck* check,
     const std::vector<SBFullHashResult>& full_hashes) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   DCHECK(check);
 
   bool is_threat = false;
 
+  // TODO(shess): GetHashThreadListType() contains a loop,
+  // GetUrlThreatListType() a loop around that loop.  Having another loop out
+  // here concerns me.  It is likely that SAFE is an expected outcome, which
+  // means all of those loops run to completion.  Refactoring this to generate a
+  // set of sorted items to compare in sequence would probably improve things.
+  //
+  // Additionally, the set of patterns generated from the urls is very similar
+  // to the patterns generated in ContainsBrowseUrl() and other database checks,
+  // which are called from this code.  Refactoring that across the checks could
+  // interact well with batching the checks here.
+
   for (size_t i = 0; i < check->urls.size(); ++i) {
-    int index =
-        safe_browsing_util::GetUrlHashIndex(check->urls[i], full_hashes);
-    if (index == -1)
-      continue;
-    SBThreatType threat =
-        GetThreatTypeFromListId(full_hashes[index].list_id);
+    SBThreatType threat = GetUrlThreatType(check->urls[i], full_hashes);
     if (threat != SB_THREAT_TYPE_SAFE &&
         IsExpectedThreat(threat, check->expected_threats)) {
       check->url_results[i] = threat;
       is_threat = true;
     }
   }
 
   for (size_t i = 0; i < check->full_hashes.size(); ++i) {
-    int index =
-        safe_browsing_util::GetHashIndex(check->full_hashes[i], full_hashes);
-    if (index == -1)
-      continue;
-    SBThreatType threat =
-        GetThreatTypeFromListId(full_hashes[index].list_id);
+    SBThreatType threat = GetHashThreatType(check->full_hashes[i], full_hashes);
     if (threat != SB_THREAT_TYPE_SAFE &&
         IsExpectedThreat(threat, check->expected_threats)) {
       check->full_hash_results[i] = threat;
       is_threat = true;
     }
   }
 
   SafeBrowsingCheckDone(check);
   return is_threat;
 }
@@ skipping 96 matching lines @@
       new base::WeakPtrFactory<SafeBrowsingDatabaseManager>(this));
   checks_.insert(check);
 
   safe_browsing_thread_->message_loop()->PostTask(FROM_HERE, task);
 
   base::MessageLoop::current()->PostDelayedTask(FROM_HERE,
       base::Bind(&SafeBrowsingDatabaseManager::TimeoutCallback,
                  check->timeout_factory_->GetWeakPtr(), check),
       check_timeout_);
 }