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

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: 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/client_side_detection_host.cc ('K') | « chrome/browser/safe_browsing/client_side_detection_host.cc ('k') | chrome/browser/safe_browsing/client_side_detection_service.h » ('j') | chrome/browser/safe_browsing/client_side_detection_service_unittest.cc » ('J')
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 6f92402912cd9f2dab8a167dd5ab3d8eac455f18..9e93367ab45a3364fa942ac32d9699ce72aa10e0 100644
--- a/chrome/browser/safe_browsing/client_side_detection_host_unittest.cc
+++ b/chrome/browser/safe_browsing/client_side_detection_host_unittest.cc
@@ -139,6 +139,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 +160,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(const BrowseInfo&,
+ ClientMalwareRequest*,
+ const BrowserFeatureExtractor::MalwareDoneCallback&));
};
} // namespace
@@ -332,7 +334,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");
@@ -344,7 +346,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;
@@ -361,7 +363,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.
@@ -376,7 +378,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;
@@ -393,7 +395,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.
@@ -408,7 +410,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;
@@ -426,6 +428,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());
@@ -461,7 +464,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;
@@ -479,6 +482,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());
@@ -486,7 +490,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);
@@ -509,6 +513,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());
@@ -545,7 +550,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;
@@ -584,7 +589,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()));
}
TEST_F(ClientSideDetectionHostTest, UpdateIPUrlMap) {
@@ -646,7 +651,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;
@@ -657,8 +662,14 @@ 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(DoAll(
+ DeleteArg<1>(),
+ InvokeCallbackArgument<2>(
+ true,
+ new ClientMalwareRequest(malware_verdict))));
EXPECT_CALL(*csd_service_,
SendClientReportMalwareRequest(_, _)).Times(0);
EXPECT_CALL(*mock_extractor, ExtractFeatures(_, _, _)).Times(0);
@@ -673,7 +684,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;
@@ -688,12 +699,13 @@ 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(DoAll(DeleteArg<1>(),
+ InvokeCallbackArgument<2>(true, &malware_verdict)));
EXPECT_CALL(*csd_service_,
SendClientReportMalwareRequest(
Pointee(PartiallyEqualMalwareVerdict(malware_verdict)), _))
- .WillOnce(DeleteArg<0>());
+ .Times(1);
EXPECT_CALL(*mock_extractor, ExtractFeatures(_, _, _)).Times(0);
OnPhishingDetectionDone(verdict.SerializeAsString());
@@ -706,7 +718,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;
@@ -722,12 +734,13 @@ 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(DoAll(DeleteArg<1>(),
+ InvokeCallbackArgument<2>(true, &malware_verdict)));
EXPECT_CALL(*csd_service_,
SendClientReportMalwareRequest(
Pointee(PartiallyEqualMalwareVerdict(malware_verdict)), _))
- .WillOnce(DeleteArg<0>());
+ .Times(1);
EXPECT_CALL(*mock_extractor, ExtractFeatures(_, _, _))
.WillOnce(DoAll(DeleteArg<1>(),
@@ -739,6 +752,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());
}
@@ -749,7 +763,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;
@@ -767,14 +781,15 @@ 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(DoAll(DeleteArg<1>(),
+ InvokeCallbackArgument<2>(true, &malware_verdict)));
EXPECT_CALL(*csd_service_,
SendClientReportMalwareRequest(
Pointee(PartiallyEqualMalwareVerdict(malware_verdict)), _))
- .WillOnce(DoAll(DeleteArg<0>(),
- SaveArg<1>(&cb)));
+ .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());
@@ -970,7 +985,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);
@@ -980,4 +995,13 @@ TEST_F(ClientSideDetectionHostTest, ShouldClassifyUrl) {
ASSERT_FALSE(msg);
}
+TEST_F(ClientSideDetectionHostTest, IsBadIpAddress) {
+ EXPECT_CALL(*database_manager_.get(), MatchMalwareIP("1.2.3.4"))
+ .WillOnce(Return(false));
+ EXPECT_FALSE(csd_host_->IsBadIpAddress("1.2.3.4"));
+ EXPECT_CALL(*database_manager_.get(), MatchMalwareIP("2.3.4.5"))
+ .WillOnce(Return(true));
+ EXPECT_TRUE(csd_host_->IsBadIpAddress("2.3.4.5"));
+}
+
} // namespace safe_browsing
« chrome/browser/safe_browsing/client_side_detection_host.cc ('K') | « chrome/browser/safe_browsing/client_side_detection_host.cc ('k') | chrome/browser/safe_browsing/client_side_detection_service.h » ('j') | chrome/browser/safe_browsing/client_side_detection_service_unittest.cc » ('J')

Powered by Google App Engine
This is Rietveld 408576698