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

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

Issue 42553002: Mostly integrate new malware IP blacklist with the csd client. When (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Fix leaks in the unit-tests Created 7 years, 1 month 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/browser_feature_extractor.cc
diff --git a/chrome/browser/safe_browsing/browser_feature_extractor.cc b/chrome/browser/safe_browsing/browser_feature_extractor.cc
index 29e70336f998fb1e83251d79f797594e49c23d47..b35fee338724ff020caafdde8f16994299b250d0 100644
--- a/chrome/browser/safe_browsing/browser_feature_extractor.cc
+++ b/chrome/browser/safe_browsing/browser_feature_extractor.cc
@@ -19,7 +19,8 @@
#include "chrome/browser/history/history_types.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/safe_browsing/browser_features.h"
-#include "chrome/browser/safe_browsing/client_side_detection_service.h"
+#include "chrome/browser/safe_browsing/client_side_detection_host.h"
+#include "chrome/browser/safe_browsing/database_manager.h"
#include "chrome/common/safe_browsing/csd.pb.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/navigation_controller.h"
@@ -35,7 +36,25 @@ using content::WebContents;
namespace safe_browsing {
-const int BrowserFeatureExtractor::kMaxMalwareIPPerRequest = 5;
+namespace {
+
+const int kMaxMalwareIPPerRequest = 5;
+
+void FilterBenignIpsOnIOThread(
+ scoped_refptr<SafeBrowsingDatabaseManager> database_manager,
+ IPUrlMap* ips) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+ for (IPUrlMap::iterator it = ips->begin(); it != ips->end();) {
+ if (!database_manager.get() ||
+ !database_manager->MatchMalwareIP(it->first)) {
+ // it++ here returns a copy of the old iterator and passes it to erase.
+ ips->erase(it++);
+ } else {
+ ++it;
+ }
+ }
+}
+} // namespace
BrowseInfo::BrowseInfo() : http_status_code(0) {}
@@ -132,9 +151,9 @@ static void AddNavigationFeatures(
BrowserFeatureExtractor::BrowserFeatureExtractor(
WebContents* tab,
- ClientSideDetectionService* service)
+ ClientSideDetectionHost* host)
: tab_(tab),
- service_(service),
+ host_(host),
weak_factory_(this) {
DCHECK(tab);
}
@@ -169,11 +188,6 @@ void BrowserFeatureExtractor::ExtractFeatures(const BrowseInfo* info,
DCHECK(info);
DCHECK_EQ(0U, request->url().find("http:"));
DCHECK(!callback.is_null());
- if (callback.is_null()) {
- DLOG(ERROR) << "ExtractFeatures called without a callback object";
- return;
- }
-
// Extract features pertaining to this navigation.
const NavigationController& controller = tab_->GetController();
int url_index = -1;
@@ -230,42 +244,39 @@ void BrowserFeatureExtractor::ExtractFeatures(const BrowseInfo* info,
}
void BrowserFeatureExtractor::ExtractMalwareFeatures(
- const BrowseInfo* info,
- ClientMalwareRequest* request) {
+ BrowseInfo* info,
+ ClientMalwareRequest* request,
+ const MalwareDoneCallback& callback) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
- DCHECK(request);
- DCHECK(info);
DCHECK_EQ(0U, request->url().find("http:"));
- // get the IPs and urls that match the malware blacklisted IP list.
- if (service_) {
- int matched_bad_ips = 0;
- for (IPUrlMap::const_iterator it = info->ips.begin();
- it != info->ips.end(); ++it) {
- if (service_->IsBadIpAddress(it->first)) {
- AddMalwareFeature(features::kBadIpFetch + it->first,
- it->second, 1.0, request);
- ++matched_bad_ips;
- // Limit the number of matched bad IPs in one request to control
- // the request's size
- if (matched_bad_ips >= kMaxMalwareIPPerRequest) {
- return;
- }
- }
- }
- }
+ DCHECK(!callback.is_null());
+
+ // Grab the IPs because they might go away before we're done
+ // checking them against the IP blacklist on the IO thread.
+ scoped_ptr<IPUrlMap> ips(new IPUrlMap);
+ ips->swap(info->ips);
+
+ IPUrlMap* ips_ptr = ips.get();
+
+ // The API doesn't take a scoped_ptr because the API gets mocked and we
+ // cannot mock an API that takes scoped_ptr as arguments.
+ scoped_ptr<ClientMalwareRequest> req(request);
+
+ // IP blacklist lookups have to happen on the IO thread.
+ BrowserThread::PostTaskAndReply(
+ BrowserThread::IO,
+ FROM_HERE,
+ base::Bind(&FilterBenignIpsOnIOThread,
+ host_->database_manager(),
+ ips_ptr),
+ base::Bind(&BrowserFeatureExtractor::FinishExtractMalwareFeatures,
+ weak_factory_.GetWeakPtr(),
+ base::Passed(&ips), callback, base::Passed(&req)));
}
void BrowserFeatureExtractor::ExtractBrowseInfoFeatures(
const BrowseInfo& info,
ClientPhishingRequest* request) {
- if (service_) {
- for (IPUrlMap::const_iterator it = info.ips.begin();
- it != info.ips.end(); ++it) {
- if (service_->IsBadIpAddress(it->first)) {
- AddFeature(features::kBadIpFetch + it->first, 1.0, request);
- }
- }
- }
if (info.unsafe_resource.get()) {
// A SafeBrowsing interstitial was shown for the current URL.
AddFeature(features::kSafeBrowsingMaliciousUrl +
@@ -495,4 +506,24 @@ bool BrowserFeatureExtractor::GetHistoryService(HistoryService** history) {
return false;
}
+void BrowserFeatureExtractor::FinishExtractMalwareFeatures(
+ scoped_ptr<IPUrlMap> bad_ips,
+ MalwareDoneCallback callback,
+ scoped_ptr<ClientMalwareRequest> request) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+ int matched_bad_ips = 0;
+ for (IPUrlMap::const_iterator it = bad_ips->begin();
+ it != bad_ips->end(); ++it) {
+ AddMalwareFeature(features::kBadIpFetch + it->first,
+ it->second, 1.0, request.get());
+ ++matched_bad_ips;
+ // Limit the number of matched bad IPs in one request to control
+ // the request's size
+ if (matched_bad_ips >= kMaxMalwareIPPerRequest) {
+ break;
+ }
+ }
+ callback.Run(true, request.Pass());
+}
+
} // namespace safe_browsing

Powered by Google App Engine
This is Rietveld 408576698