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

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: chrome/browser/safe_browsing/client_side_detection_host_unittest.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: Address more comments Created 7 years, 2 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
« chrome/browser/safe_browsing/browser_feature_extractor_unittest.cc ('K') | « chrome/browser/safe_browsing/client_side_detection_host.cc ('k') | chrome/browser/safe_browsing/client_side_detection_service.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/safe_browsing/client_side_detection_host_unittest.cc
diff --git a/chrome/browser/safe_browsing/client_side_detection_host_unittest.cc b/chrome/browser/safe_browsing/client_side_detection_host_unittest.cc
index 629cbf09ea407bb002948d1d0ea933e34f45389c..36775b1ae714c8a7d6af61aac78bfe6f9111bf07 100644
--- a/chrome/browser/safe_browsing/client_side_detection_host_unittest.cc
+++ b/chrome/browser/safe_browsing/client_side_detection_host_unittest.cc
@@ -86,6 +86,12 @@ ACTION_TEMPLATE(InvokeCallbackArgument,
::std::tr1::get<k>(args).Run(p0, p1);
}
+ACTION_P(InvokeMalwareCallback, verdict) {
+ scoped_ptr<ClientMalwareRequest> request(::std::tr1::get<1>(args));
+ request->CopyFrom(*verdict);
+ ::std::tr1::get<2>(args).Run(true, request.Pass());
+}
+
void EmptyUrlCheckCallback(bool processed) {
}
@@ -139,6 +145,7 @@ class MockSafeBrowsingDatabaseManager : public SafeBrowsingDatabaseManager {
: SafeBrowsingDatabaseManager(service) { }
MOCK_METHOD1(MatchCsdWhitelistUrl, bool(const GURL&));
+ MOCK_METHOD1(MatchMalwareIP, bool(const std::string& ip_address));
protected:
virtual ~MockSafeBrowsingDatabaseManager() {}
@@ -159,18 +166,19 @@ class MockBrowserFeatureExtractor : public BrowserFeatureExtractor {
public:
explicit MockBrowserFeatureExtractor(
WebContents* tab,
- ClientSideDetectionService* service)
- : BrowserFeatureExtractor(tab, service) {}
+ ClientSideDetectionHost* host)
+ : BrowserFeatureExtractor(tab, host) {}
virtual ~MockBrowserFeatureExtractor() {}
MOCK_METHOD3(ExtractFeatures,
- void(const BrowseInfo* info,
+ void(const BrowseInfo*,
ClientPhishingRequest*,
const BrowserFeatureExtractor::DoneCallback&));
- MOCK_METHOD2(ExtractMalwareFeatures,
- void(const BrowseInfo* info,
- ClientMalwareRequest*));
+ MOCK_METHOD3(ExtractMalwareFeatures,
+ void(BrowseInfo*,
+ ClientMalwareRequest*,
+ const BrowserFeatureExtractor::MalwareDoneCallback&));
};
} // namespace
@@ -332,7 +340,7 @@ TEST_F(ClientSideDetectionHostTest, OnPhishingDetectionDoneInvalidVerdict) {
// parse.
MockBrowserFeatureExtractor* mock_extractor = new MockBrowserFeatureExtractor(
web_contents(),
- csd_service_.get());
+ csd_host_.get());
SetFeatureExtractor(mock_extractor); // The host class takes ownership.
EXPECT_CALL(*mock_extractor, ExtractFeatures(_, _, _)).Times(0);
OnPhishingDetectionDone("Invalid Protocol Buffer");
@@ -350,7 +358,7 @@ TEST_F(ClientSideDetectionHostTest, OnPhishingDetectionDoneNotPhishing) {
// No interstitial is shown.
MockBrowserFeatureExtractor* mock_extractor = new MockBrowserFeatureExtractor(
web_contents(),
- csd_service_.get());
+ csd_host_.get());
SetFeatureExtractor(mock_extractor); // The host class takes ownership.
ClientSideDetectionService::ClientReportPhishingRequestCallback cb;
@@ -367,7 +375,7 @@ TEST_F(ClientSideDetectionHostTest, OnPhishingDetectionDoneNotPhishing) {
Pointee(PartiallyEqualVerdict(verdict)), _))
.WillOnce(SaveArg<1>(&cb));
OnPhishingDetectionDone(verdict.SerializeAsString());
- EXPECT_TRUE(Mock::VerifyAndClear(csd_service_.get()));
+ EXPECT_TRUE(Mock::VerifyAndClear(csd_host_.get()));
ASSERT_FALSE(cb.is_null());
// Make sure DoDisplayBlockingPage is not going to be called.
@@ -387,7 +395,7 @@ TEST_F(ClientSideDetectionHostTest, OnPhishingDetectionDoneDisabled) {
// showing the interstitial is disabled => no interstitial is shown.
MockBrowserFeatureExtractor* mock_extractor = new MockBrowserFeatureExtractor(
web_contents(),
- csd_service_.get());
+ csd_host_.get());
SetFeatureExtractor(mock_extractor); // The host class takes ownership.
ClientSideDetectionService::ClientReportPhishingRequestCallback cb;
@@ -404,7 +412,7 @@ TEST_F(ClientSideDetectionHostTest, OnPhishingDetectionDoneDisabled) {
Pointee(PartiallyEqualVerdict(verdict)), _))
.WillOnce(SaveArg<1>(&cb));
OnPhishingDetectionDone(verdict.SerializeAsString());
- EXPECT_TRUE(Mock::VerifyAndClear(csd_service_.get()));
+ EXPECT_TRUE(Mock::VerifyAndClear(csd_host_.get()));
ASSERT_FALSE(cb.is_null());
// Make sure DoDisplayBlockingPage is not going to be called.
@@ -425,7 +433,7 @@ TEST_F(ClientSideDetectionHostTest, OnPhishingDetectionDoneShowInterstitial) {
// We show an interstitial.
MockBrowserFeatureExtractor* mock_extractor = new MockBrowserFeatureExtractor(
web_contents(),
- csd_service_.get());
+ csd_host_.get());
SetFeatureExtractor(mock_extractor); // The host class takes ownership.
ClientSideDetectionService::ClientReportPhishingRequestCallback cb;
@@ -443,6 +451,7 @@ TEST_F(ClientSideDetectionHostTest, OnPhishingDetectionDoneShowInterstitial) {
Pointee(PartiallyEqualVerdict(verdict)), _))
.WillOnce(SaveArg<1>(&cb));
OnPhishingDetectionDone(verdict.SerializeAsString());
+ EXPECT_TRUE(Mock::VerifyAndClear(csd_host_.get()));
EXPECT_TRUE(Mock::VerifyAndClear(csd_service_.get()));
ASSERT_FALSE(cb.is_null());
@@ -484,7 +493,7 @@ TEST_F(ClientSideDetectionHostTest, OnPhishingDetectionDoneMultiplePings) {
// a single interstitial is shown for the second URL.
MockBrowserFeatureExtractor* mock_extractor = new MockBrowserFeatureExtractor(
web_contents(),
- csd_service_.get());
+ csd_host_.get());
SetFeatureExtractor(mock_extractor); // The host class takes ownership.
ClientSideDetectionService::ClientReportPhishingRequestCallback cb;
@@ -502,6 +511,7 @@ TEST_F(ClientSideDetectionHostTest, OnPhishingDetectionDoneMultiplePings) {
Pointee(PartiallyEqualVerdict(verdict)), _))
.WillOnce(SaveArg<1>(&cb));
OnPhishingDetectionDone(verdict.SerializeAsString());
+ EXPECT_TRUE(Mock::VerifyAndClear(csd_host_.get()));
EXPECT_TRUE(Mock::VerifyAndClear(csd_service_.get()));
ASSERT_FALSE(cb.is_null());
@@ -509,7 +519,7 @@ TEST_F(ClientSideDetectionHostTest, OnPhishingDetectionDoneMultiplePings) {
// NavigateAndCommit() and it's easier to use the real thing than setting up
// mock expectations.
SetFeatureExtractor(new BrowserFeatureExtractor(web_contents(),
- csd_service_.get()));
+ csd_host_.get()));
GURL other_phishing_url("http://other_phishing_url.com/bla");
ExpectPreClassificationChecks(other_phishing_url, &kFalse, &kFalse, &kFalse,
&kFalse, &kFalse, &kFalse);
@@ -532,6 +542,7 @@ TEST_F(ClientSideDetectionHostTest, OnPhishingDetectionDoneMultiplePings) {
SetRedirectChain(redirect_chain);
OnPhishingDetectionDone(verdict.SerializeAsString());
base::MessageLoop::current()->Run();
+ EXPECT_TRUE(Mock::VerifyAndClear(csd_host_.get()));
EXPECT_TRUE(Mock::VerifyAndClear(csd_service_.get()));
ASSERT_FALSE(cb_other.is_null());
@@ -574,7 +585,7 @@ TEST_F(ClientSideDetectionHostTest,
// Case 6: renderer sends a verdict string that isn't phishing.
MockBrowserFeatureExtractor* mock_extractor = new MockBrowserFeatureExtractor(
web_contents(),
- csd_service_.get());
+ csd_host_.get());
SetFeatureExtractor(mock_extractor); // The host class takes ownership.
ClientPhishingRequest verdict;
@@ -619,7 +630,7 @@ TEST_F(ClientSideDetectionHostTest,
SetRedirectChain(redirect_chain);
OnPhishingDetectionDone(verdict.SerializeAsString());
base::MessageLoop::current()->Run();
- EXPECT_TRUE(Mock::VerifyAndClear(csd_service_.get()));
+ EXPECT_TRUE(Mock::VerifyAndClear(csd_host_.get()));
}
#if defined(OS_WIN)
@@ -692,7 +703,7 @@ TEST_F(ClientSideDetectionHostTest,
// malware bad IP list
MockBrowserFeatureExtractor* mock_extractor = new MockBrowserFeatureExtractor(
web_contents(),
- csd_service_.get());
+ csd_host_.get());
SetFeatureExtractor(mock_extractor); // The host class takes ownership.
ClientPhishingRequest verdict;
@@ -703,8 +714,10 @@ TEST_F(ClientSideDetectionHostTest,
ClientMalwareRequest malware_verdict;
malware_verdict.set_url("http://not-phishing.com/");
- EXPECT_CALL(*mock_extractor, ExtractMalwareFeatures(_, _))
- .WillOnce(SetArgumentPointee<1>(malware_verdict));
+ // That is a special case. If there were no IP matches or if feature
+ // extraction failed the callback will delete the malware_verdict.
+ EXPECT_CALL(*mock_extractor, ExtractMalwareFeatures(_, _, _))
+ .WillOnce(InvokeMalwareCallback(&malware_verdict));
EXPECT_CALL(*csd_service_,
SendClientReportMalwareRequest(_, _)).Times(0);
EXPECT_CALL(*mock_extractor, ExtractFeatures(_, _, _)).Times(0);
@@ -725,7 +738,7 @@ TEST_F(ClientSideDetectionHostTest,
// malware bad IP list
MockBrowserFeatureExtractor* mock_extractor = new MockBrowserFeatureExtractor(
web_contents(),
- csd_service_.get());
+ csd_host_.get());
SetFeatureExtractor(mock_extractor); // The host class takes ownership.
ClientPhishingRequest verdict;
@@ -740,8 +753,8 @@ TEST_F(ClientSideDetectionHostTest,
feature->set_value(1.0);
feature->add_metainfo("badip.com");
- EXPECT_CALL(*mock_extractor, ExtractMalwareFeatures(_, _))
- .WillOnce(SetArgumentPointee<1>(malware_verdict));
+ EXPECT_CALL(*mock_extractor, ExtractMalwareFeatures(_, _, _))
+ .WillOnce(InvokeMalwareCallback(&malware_verdict));
EXPECT_CALL(*csd_service_,
SendClientReportMalwareRequest(
Pointee(PartiallyEqualMalwareVerdict(malware_verdict)), _))
@@ -764,7 +777,7 @@ TEST_F(ClientSideDetectionHostTest,
// malware bad IP list
MockBrowserFeatureExtractor* mock_extractor = new MockBrowserFeatureExtractor(
web_contents(),
- csd_service_.get());
+ csd_host_.get());
SetFeatureExtractor(mock_extractor); // The host class takes ownership.
ClientSideDetectionService::ClientReportPhishingRequestCallback cb;
@@ -780,8 +793,8 @@ TEST_F(ClientSideDetectionHostTest,
feature->set_value(1.0);
feature->add_metainfo("badip.com");
- EXPECT_CALL(*mock_extractor, ExtractMalwareFeatures(_, _))
- .WillOnce(SetArgumentPointee<1>(malware_verdict));
+ EXPECT_CALL(*mock_extractor, ExtractMalwareFeatures(_, _, _))
+ .WillOnce(InvokeMalwareCallback(&malware_verdict));
EXPECT_CALL(*csd_service_,
SendClientReportMalwareRequest(
Pointee(PartiallyEqualMalwareVerdict(malware_verdict)), _))
@@ -797,6 +810,7 @@ TEST_F(ClientSideDetectionHostTest,
.WillOnce(SaveArg<1>(&cb));
OnPhishingDetectionDone(verdict.SerializeAsString());
EXPECT_TRUE(Mock::VerifyAndClear(mock_extractor));
+ EXPECT_TRUE(Mock::VerifyAndClear(csd_host_.get()));
EXPECT_TRUE(Mock::VerifyAndClear(csd_service_.get()));
ASSERT_FALSE(cb.is_null());
}
@@ -813,7 +827,7 @@ TEST_F(ClientSideDetectionHostTest,
// We show an sub-resource malware interstitial.
MockBrowserFeatureExtractor* mock_extractor = new MockBrowserFeatureExtractor(
web_contents(),
- csd_service_.get());
+ csd_host_.get());
SetFeatureExtractor(mock_extractor); // The host class takes ownership.
ClientPhishingRequest verdict;
@@ -831,14 +845,14 @@ TEST_F(ClientSideDetectionHostTest,
feature->set_value(1.0);
feature->add_metainfo("http://badip.com");
- EXPECT_CALL(*mock_extractor, ExtractMalwareFeatures(_, _))
- .WillOnce(SetArgumentPointee<1>(malware_verdict));
+ EXPECT_CALL(*mock_extractor, ExtractMalwareFeatures(_, _, _))
+ .WillOnce(InvokeMalwareCallback(&malware_verdict));
EXPECT_CALL(*csd_service_,
SendClientReportMalwareRequest(
Pointee(PartiallyEqualMalwareVerdict(malware_verdict)), _))
- .WillOnce(DoAll(DeleteArg<0>(),
- SaveArg<1>(&cb)));
+ .WillOnce(DoAll(DeleteArg<0>(), SaveArg<1>(&cb)));
OnPhishingDetectionDone(verdict.SerializeAsString());
+ EXPECT_TRUE(Mock::VerifyAndClear(csd_host_.get()));
EXPECT_TRUE(Mock::VerifyAndClear(csd_service_.get()));
ASSERT_FALSE(cb.is_null());
@@ -1045,7 +1059,7 @@ TEST_F(ClientSideDetectionHostTest, ShouldClassifyUrl) {
base::RunLoop().RunUntilIdle();
// Now we check that all expected functions were indeed called on the two
// service objects.
- EXPECT_TRUE(Mock::VerifyAndClear(csd_service_.get()));
+ EXPECT_TRUE(Mock::VerifyAndClear(csd_host_.get()));
EXPECT_TRUE(Mock::VerifyAndClear(ui_manager_.get()));
EXPECT_EQ(url, resource.url);
EXPECT_EQ(url, resource.original_url);
@@ -1054,5 +1068,4 @@ TEST_F(ClientSideDetectionHostTest, ShouldClassifyUrl) {
SafeBrowsingMsg_StartPhishingDetection::ID);
ASSERT_FALSE(msg);
}
-
} // namespace safe_browsing
« chrome/browser/safe_browsing/browser_feature_extractor_unittest.cc ('K') | « chrome/browser/safe_browsing/client_side_detection_host.cc ('k') | chrome/browser/safe_browsing/client_side_detection_service.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698