Use a different tpl= parameter when reporting an error from client-side phishing detection.

This extends SafeBrowsingService::UrlCheckResult to add a new type for client-side phishing urls.

BUG=83436
TEST=ClientSideDetectionHostTest

Review URL: http://codereview.chromium.org/7076009

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

8 files changed, 59 insertions, 27 deletions

diff --git a/chrome/browser/safe_browsing/client_side_detection_host.cc b/chrome/browser/safe_browsing/client_side_detection_host.cc
index ad66ac4..3979cf0 100644
--- a/chrome/browser/safe_browsing/client_side_detection_host.cc
+++ b/chrome/browser/safe_browsing/client_side_detection_host.cc
@@ -373,9 +373,7 @@ void ClientSideDetectionHost::MaybeShowPhishingWarning(GURL phishing_url,
                             redirect_urls,
                             // We only classify the main frame URL.
                             ResourceType::MAIN_FRAME,
-                            // TODO(noelutz): create a separate threat type
-                            // for client-side phishing detection.
-                            SafeBrowsingService::URL_PHISHING,
+                            SafeBrowsingService::CLIENT_SIDE_PHISHING_URL,
                             new CsdClient() /* will delete itself */,
                             tab_contents()->GetRenderProcessHost()->id(),
                             tab_contents()->render_view_host()->routing_id()));
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 1448694..2599b95 100644
--- a/chrome/browser/safe_browsing/client_side_detection_host_unittest.cc
+++ b/chrome/browser/safe_browsing/client_side_detection_host_unittest.cc
@@ -310,7 +310,7 @@ TEST_F(ClientSideDetectionHostTest, OnDetectedPhishingSiteShowInterstitial) {
                   phishing_url,
                   _,
                   ResourceType::MAIN_FRAME,
-                  SafeBrowsingService::URL_PHISHING,
+                  SafeBrowsingService::CLIENT_SIDE_PHISHING_URL,
                   _ /* a CsdClient object */,
                   contents()->GetRenderProcessHost()->id(),
                   contents()->render_view_host()->routing_id()))
@@ -388,7 +388,7 @@ TEST_F(ClientSideDetectionHostTest, OnDetectedPhishingSiteMultiplePings) {
                   other_phishing_url,
                   _,
                   ResourceType::MAIN_FRAME,
-                  SafeBrowsingService::URL_PHISHING,
+                  SafeBrowsingService::CLIENT_SIDE_PHISHING_URL,
                   _ /* a CsdClient object */,
                   contents()->GetRenderProcessHost()->id(),
                   contents()->render_view_host()->routing_id()))
diff --git a/chrome/browser/safe_browsing/protocol_manager.cc b/chrome/browser/safe_browsing/protocol_manager.cc
index 609e44f..dfc4b82 100644
--- a/chrome/browser/safe_browsing/protocol_manager.cc
+++ b/chrome/browser/safe_browsing/protocol_manager.cc
@@ -769,7 +769,8 @@ GURL SafeBrowsingProtocolManager::SafeBrowsingHitUrl(
   DCHECK(threat_type == SafeBrowsingService::URL_MALWARE ||
          threat_type == SafeBrowsingService::URL_PHISHING ||
          threat_type == SafeBrowsingService::BINARY_MALWARE_URL ||
-         threat_type == SafeBrowsingService::BINARY_MALWARE_HASH);
+         threat_type == SafeBrowsingService::BINARY_MALWARE_HASH ||
+         threat_type == SafeBrowsingService::CLIENT_SIDE_PHISHING_URL);
   // The malware and phishing hits go over HTTP.
   std::string url = ComposeUrl(http_url_prefix_, "report", client_name_,
                                version_, additional_query_);
@@ -787,6 +788,9 @@ GURL SafeBrowsingProtocolManager::SafeBrowsingHitUrl(
     case SafeBrowsingService::BINARY_MALWARE_HASH:
       threat_list = "binhashhit";
       break;
+    case SafeBrowsingService::CLIENT_SIDE_PHISHING_URL:
+      threat_list = "phishcsdhit";
+      break;
     default:
       NOTREACHED();
   }
diff --git a/chrome/browser/safe_browsing/protocol_manager_unittest.cc b/chrome/browser/safe_browsing/protocol_manager_unittest.cc
index 69bff62..c621879 100644
--- a/chrome/browser/safe_browsing/protocol_manager_unittest.cc
+++ b/chrome/browser/safe_browsing/protocol_manager_unittest.cc
@@ -236,6 +236,15 @@ TEST_F(SafeBrowsingProtocolManagerTest, TestSafeBrowsingHitUrl) {
             pm.SafeBrowsingHitUrl(
                 malicious_url, page_url, referrer_url,
                 false, SafeBrowsingService::BINARY_MALWARE_HASH).spec());
+
+  EXPECT_EQ("http://info.prefix.com/foo/report?client=unittest&appver=1.0&"
+            "pver=2.2&additional_query&evts=phishcsdhit&"
+            "evtd=http%3A%2F%2Fmalicious.url.com%2F&"
+            "evtr=http%3A%2F%2Fpage.url.com%2F&evhr=http%3A%2F%2Freferrer."
+            "url.com%2F&evtb=0",
+            pm.SafeBrowsingHitUrl(
+                malicious_url, page_url, referrer_url,
+                false, SafeBrowsingService::CLIENT_SIDE_PHISHING_URL).spec());
 }
 
 TEST_F(SafeBrowsingProtocolManagerTest, TestMalwareDetailsUrl) {
diff --git a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
index dec87be..1e99a0f 100644
--- a/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_blocking_page.cc
@@ -48,7 +48,7 @@ static const char* const kSbDiagnosticUrl =
     "http://safebrowsing.clients.google.com/safebrowsing/diagnostic?site=%s&client=chromium";
 #endif
 
-static const char* const kSbReportPhishingUrl =
+static const char* const kSbReportPhishingErrorUrl =
     "http://www.google.com/safebrowsing/report_error/";
 
 // URL for the "Learn more" link on the multi threat malware blocking page.
@@ -175,15 +175,20 @@ std::string SafeBrowsingBlockingPage::GetHTMLContents() {
     PopulateMultipleThreatStringDictionary(&strings);
     html = rb.GetRawDataResource(
         IDR_SAFE_BROWSING_MULTIPLE_THREAT_BLOCK).as_string();
-  } else if (unsafe_resources_[0].threat_type ==
-             SafeBrowsingService::URL_MALWARE) {
-    PopulateMalwareStringDictionary(&strings);
-    html = rb.GetRawDataResource(IDR_SAFE_BROWSING_MALWARE_BLOCK).as_string();
-  } else {  // Phishing.
-    DCHECK(unsafe_resources_[0].threat_type ==
-           SafeBrowsingService::URL_PHISHING);
-    PopulatePhishingStringDictionary(&strings);
-    html = rb.GetRawDataResource(IDR_SAFE_BROWSING_PHISHING_BLOCK).as_string();
+  } else {
+    SafeBrowsingService::UrlCheckResult threat_type =
+        unsafe_resources_[0].threat_type;
+    if (threat_type == SafeBrowsingService::URL_MALWARE) {
+      PopulateMalwareStringDictionary(&strings);
+      html = rb.GetRawDataResource(
+          IDR_SAFE_BROWSING_MALWARE_BLOCK).as_string();
+    } else {  // Phishing.
+      DCHECK(threat_type == SafeBrowsingService::URL_PHISHING ||
+             threat_type == SafeBrowsingService::CLIENT_SIDE_PHISHING_URL);
+      PopulatePhishingStringDictionary(&strings);
+      html = rb.GetRawDataResource(
+          IDR_SAFE_BROWSING_PHISHING_BLOCK).as_string();
+    }
   }
 
   return jstemplate_builder::GetTemplatesHtml(html, &strings, "template_root");
@@ -221,14 +226,16 @@ void SafeBrowsingBlockingPage::PopulateMultipleThreatStringDictionary(
   for (UnsafeResourceList::const_iterator iter = unsafe_resources_.begin();
        iter != unsafe_resources_.end(); ++iter) {
     const SafeBrowsingService::UnsafeResource& resource = *iter;
+    SafeBrowsingService::UrlCheckResult threat_type = resource.threat_type;
     DictionaryValue* current_error_strings = new DictionaryValue;
-    if (resource.threat_type == SafeBrowsingService::URL_MALWARE) {
+    if (threat_type == SafeBrowsingService::URL_MALWARE) {
       malware = true;
       current_error_strings->SetString("type", "malware");
       current_error_strings->SetString("typeLabel", malware_label);
       current_error_strings->SetString("errorLink", malware_link);
     } else {
-      DCHECK(resource.threat_type == SafeBrowsingService::URL_PHISHING);
+      DCHECK(threat_type == SafeBrowsingService::URL_PHISHING ||
+             threat_type == SafeBrowsingService::CLIENT_SIDE_PHISHING_URL);
       phishing = true;
       current_error_strings->SetString("type", "phishing");
       current_error_strings->SetString("typeLabel", phishing_label);
@@ -408,10 +415,12 @@ void SafeBrowsingBlockingPage::CommandReceived(const std::string& cmd) {
   if (command == kLearnMoreCommand) {
     // User pressed "Learn more".
     GURL url;
-    if (unsafe_resources_[0].threat_type == SafeBrowsingService::URL_MALWARE) {
+    SafeBrowsingService::UrlCheckResult threat_type =
+        unsafe_resources_[0].threat_type;
+    if (threat_type == SafeBrowsingService::URL_MALWARE) {
       url = google_util::AppendGoogleLocaleParam(GURL(kLearnMoreMalwareUrl));
-    } else if (unsafe_resources_[0].threat_type ==
-               SafeBrowsingService::URL_PHISHING) {
+    } else if (threat_type == SafeBrowsingService::URL_PHISHING ||
+               threat_type == SafeBrowsingService::CLIENT_SIDE_PHISHING_URL) {
       url = google_util::AppendGoogleLocaleParam(GURL(kLearnMorePhishingUrl));
     } else {
       NOTREACHED();
@@ -462,11 +471,15 @@ void SafeBrowsingBlockingPage::CommandReceived(const std::string& cmd) {
     // User pressed "Report error" for a phishing site.
     // Note that we cannot just put a link in the interstitial at this point.
     // It is not OK to navigate in the context of an interstitial page.
-    DCHECK(unsafe_resources_[element_index].threat_type ==
-           SafeBrowsingService::URL_PHISHING);
+    SafeBrowsingService::UrlCheckResult threat_type =
+        unsafe_resources_[element_index].threat_type;
+    DCHECK(threat_type == SafeBrowsingService::URL_PHISHING ||
+           threat_type == SafeBrowsingService::CLIENT_SIDE_PHISHING_URL);
     GURL report_url =
-        safe_browsing_util::GeneratePhishingReportUrl(kSbReportPhishingUrl,
-                                                      bad_url_spec);
+        safe_browsing_util::GeneratePhishingReportUrl(
+            kSbReportPhishingErrorUrl,
+            bad_url_spec,
+            threat_type == SafeBrowsingService::CLIENT_SIDE_PHISHING_URL);
     tab()->OpenURL(report_url, GURL(), CURRENT_TAB, PageTransition::LINK);
     return;
   }
diff --git a/chrome/browser/safe_browsing/safe_browsing_service.h b/chrome/browser/safe_browsing/safe_browsing_service.h
index 0803a578..84423b1 100644
--- a/chrome/browser/safe_browsing/safe_browsing_service.h
+++ b/chrome/browser/safe_browsing/safe_browsing_service.h
@@ -51,6 +51,10 @@ class SafeBrowsingService
     URL_MALWARE,
     BINARY_MALWARE_URL,  // Binary url leads to a malware.
     BINARY_MALWARE_HASH,  // Binary hash indicates this is a malware.
+
+    // Url detected by the client-side phishing model.  Note that unlike the
+    // above values, this does not correspond to a downloaded list.
+    CLIENT_SIDE_PHISHING_URL,
   };
 
   // Structure used to pass parameters between the IO and UI thread when
diff --git a/chrome/browser/safe_browsing/safe_browsing_util.cc b/chrome/browser/safe_browsing/safe_browsing_util.cc
index c7739f9..9fe10ed 100644
--- a/chrome/browser/safe_browsing/safe_browsing_util.cc
+++ b/chrome/browser/safe_browsing/safe_browsing_util.cc
@@ -499,7 +499,8 @@ bool VerifyMAC(const std::string& key, const std::string& mac,
 }
 
 GURL GeneratePhishingReportUrl(const std::string& report_page,
-                               const std::string& url_to_report) {
+                               const std::string& url_to_report,
+                               bool is_client_side_detection) {
   icu::Locale locale = icu::Locale::getDefault();
   const char* lang = locale.getLanguage();
   if (!lang)
@@ -514,6 +515,8 @@ GURL GeneratePhishingReportUrl(const std::string& report_page,
 #else
   std::string client_name("googlechrome");
 #endif
+  if (is_client_side_detection)
+    client_name.append("_csd");
 
   GURL report_url(report_page + base::StringPrintf(kReportParams,
                                                    client_name.c_str(),
diff --git a/chrome/browser/safe_browsing/safe_browsing_util.h b/chrome/browser/safe_browsing/safe_browsing_util.h
index d2e9204..bec4dc8 100644
--- a/chrome/browser/safe_browsing/safe_browsing_util.h
+++ b/chrome/browser/safe_browsing/safe_browsing_util.h
@@ -317,7 +317,8 @@ bool VerifyMAC(const std::string& key,
                int data_length);
 
 GURL GeneratePhishingReportUrl(const std::string& report_page,
-                               const std::string& url_to_report);
+                               const std::string& url_to_report,
+                               bool is_client_side_detection);
 
 void StringToSBFullHash(const std::string& hash_in, SBFullHash* hash_out);
 std::string SBFullHashToString(const SBFullHash& hash_out);