Limit the number of requests sent by phishing client side detection per day.

Currently we don't serialize this information so the counter will reset on
browser restart.

Patch from Garrett Casto <gcasto@google.com>

BUG=None
TEST=ran client_side_detection_service_unittest.cc

Review URL: http://codereview.chromium.org/6240011
Patch from Garrett Casto <gcasto@google.com>.

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@72848 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 83 insertions, 0 deletions

diff --git a/chrome/browser/safe_browsing/client_side_detection_service.cc b/chrome/browser/safe_browsing/client_side_detection_service.cc
index f655e2a..008a2d2 100644
--- a/chrome/browser/safe_browsing/client_side_detection_service.cc
+++ b/chrome/browser/safe_browsing/client_side_detection_service.cc
@@ -9,10 +9,12 @@
 #include "base/file_util_proxy.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
+#include "base/metrics/histogram.h"
 #include "base/platform_file.h"
 #include "base/scoped_ptr.h"
 #include "base/stl_util-inl.h"
 #include "base/task.h"
+#include "base/time.h"
 #include "chrome/browser/browser_thread.h"
 #include "chrome/browser/safe_browsing/csd.pb.h"
 #include "chrome/common/net/http_return.h"
@@ -24,6 +26,8 @@
 
 namespace safe_browsing {
 
+const int ClientSideDetectionService::kMaxReportsPerDay = 3;
+
 const char ClientSideDetectionService::kClientReportPhishingUrl[] =
     "https://sb-ssl.google.com/safebrowsing/clientreport/phishing";
 const char ClientSideDetectionService::kClientModelUrl[] =
@@ -222,6 +226,14 @@ void ClientSideDetectionService::StartClientReportPhishingRequest(
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   scoped_ptr<ClientReportPhishingRequestCallback> cb(callback);
 
+  if (GetNumReportsPerDay() > kMaxReportsPerDay) {
+    LOG(WARNING) << "Too many report phishing requests sent in the last day, "
+                 << "not checking " << phishing_url;
+    UMA_HISTOGRAM_COUNTS("SBClientPhishing.RequestNotSent", 1);
+    cb->Run(phishing_url, false);
+    return;
+  }
+
   ClientPhishingRequest request;
   request.set_url(phishing_url.spec());
   request.set_client_score(static_cast<float>(score));
@@ -249,6 +261,9 @@ void ClientSideDetectionService::StartClientReportPhishingRequest(
   fetcher->set_request_context(request_context_getter_.get());
   fetcher->set_upload_data("application/octet-stream", request_data);
   fetcher->Start();
+
+  // Record that we made a request
+  phishing_report_times_.push(base::Time::Now());
 }
 
 void ClientSideDetectionService::HandleModelResponse(
@@ -303,4 +318,18 @@ void ClientSideDetectionService::HandlePhishingVerdict(
   delete source;
 }
 
+int ClientSideDetectionService::GetNumReportsPerDay() {
+  base::Time cutoff = base::Time::Now() - base::TimeDelta::FromDays(1);
+
+  // Erase elements older than a day because we will never care about them
+  // again.
+  while (!phishing_report_times_.empty() &&
+         phishing_report_times_.front() < cutoff) {
+    phishing_report_times_.pop();
+  }
+
+  // Return the number of elements that are above the cutoff.
+  return phishing_report_times_.size();
+}
+
 }  // namespace safe_browsing
diff --git a/chrome/browser/safe_browsing/client_side_detection_service.h b/chrome/browser/safe_browsing/client_side_detection_service.h
index a2b63f7..a9bc7f6 100644
--- a/chrome/browser/safe_browsing/client_side_detection_service.h
+++ b/chrome/browser/safe_browsing/client_side_detection_service.h
@@ -17,6 +17,7 @@
 #pragma once
 
 #include <map>
+#include <queue>
 #include <string>
 #include <vector>
 
@@ -29,6 +30,7 @@
 #include "base/scoped_callback_factory.h"
 #include "base/scoped_ptr.h"
 #include "base/task.h"
+#include "base/time.h"
 #include "chrome/browser/safe_browsing/csd.pb.h"
 #include "chrome/common/net/url_fetcher.h"
 #include "googleurl/src/gurl.h"
@@ -99,6 +101,7 @@ class ClientSideDetectionService : public URLFetcher::Delegate {
 
   static const char kClientReportPhishingUrl[];
   static const char kClientModelUrl[];
+  static const int kMaxReportsPerDay;
 
   // Use Create() method to create an instance of this object.
   ClientSideDetectionService(const FilePath& model_path,
@@ -162,6 +165,10 @@ class ClientSideDetectionService : public URLFetcher::Delegate {
                              const ResponseCookies& cookies,
                              const std::string& data);
 
+  // Get the number of phishing reports that we have sent over the last 24
+  // hours.
+  int GetNumReportsPerDay();
+
   FilePath model_path_;
   ModelStatus model_status_;
   base::PlatformFile model_file_;
@@ -174,6 +181,11 @@ class ClientSideDetectionService : public URLFetcher::Delegate {
   struct ClientReportInfo;
   std::map<const URLFetcher*, ClientReportInfo*> client_phishing_reports_;
 
+  // Timestamp of when we sent a phishing request. Used to limit the number
+  // of phishing requests that we send in a day.
+  // TODO(gcasto): Serialize this so that it doesn't reset on browser restart.
+  std::queue<base::Time> phishing_report_times_;
+
   // Used to asynchronously call the callbacks for GetModelFile and
   // SendClientReportPhishingRequest.
   ScopedRunnableMethodFactory<ClientSideDetectionService> method_factory_;
diff --git a/chrome/browser/safe_browsing/client_side_detection_service_unittest.cc b/chrome/browser/safe_browsing/client_side_detection_service_unittest.cc
index d39c99f..b418d14 100644
--- a/chrome/browser/safe_browsing/client_side_detection_service_unittest.cc
+++ b/chrome/browser/safe_browsing/client_side_detection_service_unittest.cc
@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include <map>
+#include <queue>
 #include <string>
 
 #include "base/callback.h"
@@ -15,6 +16,7 @@
 #include "base/scoped_ptr.h"
 #include "base/scoped_temp_dir.h"
 #include "base/task.h"
+#include "base/time.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "chrome/browser/browser_thread.h"
 #include "chrome/browser/safe_browsing/client_side_detection_service.h"
@@ -84,6 +86,14 @@ class ClientSideDetectionServiceTest : public testing::Test {
         response_data, success);
   }
 
+  int GetNumReportsPerDay() {
+    return csd_service_->GetNumReportsPerDay();
+  }
+
+  std::queue<base::Time>& GetPhishingReportTimes() {
+    return csd_service_->phishing_report_times_;
+  }
+
  protected:
   scoped_ptr<ClientSideDetectionService> csd_service_;
   scoped_ptr<FakeURLFetcherFactory> factory_;
@@ -166,6 +176,8 @@ TEST_F(ClientSideDetectionServiceTest, SendClientReportPhishingRequest) {
   GURL url("http://a.com/");
   double score = 0.4;  // Some random client score.
 
+  base::Time before = base::Time::Now();
+
   // Invalid response body from the server.
   SetClientReportPhishingResponse("invalid proto response", true /* success */);
   EXPECT_FALSE(SendClientReportPhishingRequest(url, score));
@@ -180,6 +192,36 @@ TEST_F(ClientSideDetectionServiceTest, SendClientReportPhishingRequest) {
   SetClientReportPhishingResponse(response.SerializeAsString(),
                                   true /* success */);
   EXPECT_FALSE(SendClientReportPhishingRequest(url, score));
+
+  base::Time after = base::Time::Now();
+
+  // Check that we have recorded 3 requests, all within the correct time range.
+  std::queue<base::Time>& report_times = GetPhishingReportTimes();
+  EXPECT_EQ(3U, report_times.size());
+  while (!report_times.empty()) {
+    base::Time time = report_times.back();
+    report_times.pop();
+    EXPECT_LE(before, time);
+    EXPECT_GE(after, time);
+  }
+}
+
+TEST_F(ClientSideDetectionServiceTest, GetNumReportTest) {
+  SetModelFetchResponse("bogus model", true /* success */);
+  ScopedTempDir tmp_dir;
+  ASSERT_TRUE(tmp_dir.CreateUniqueTempDir());
+  csd_service_.reset(ClientSideDetectionService::Create(
+      tmp_dir.path().AppendASCII("model"), NULL));
+
+  std::queue<base::Time>& report_times = GetPhishingReportTimes();
+  base::Time now = base::Time::Now();
+  base::TimeDelta twenty_five_hours = base::TimeDelta::FromHours(25);
+  report_times.push(now - twenty_five_hours);
+  report_times.push(now - twenty_five_hours);
+  report_times.push(now);
+  report_times.push(now);
+
+  EXPECT_EQ(2, GetNumReportsPerDay());
 }
 
 }  // namespace safe_browsing