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

Unified Diff: components/safe_browsing/password_protection/password_protection_service.cc

Issue 2892093003: Don't trigger Phishguard pings if we cannot compute URL reputation. (Closed)
Patch Set: update comments Created 3 years, 7 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
Index: components/safe_browsing/password_protection/password_protection_service.cc
diff --git a/components/safe_browsing/password_protection/password_protection_service.cc b/components/safe_browsing/password_protection/password_protection_service.cc
index d2ef322d9231a1f358fba401eebf118cb806a355..c61267f65c058fcb994b169a307d67e7308c344d 100644
--- a/components/safe_browsing/password_protection/password_protection_service.cc
+++ b/components/safe_browsing/password_protection/password_protection_service.cc
@@ -18,10 +18,13 @@
#include "components/safe_browsing_db/database_manager.h"
#include "components/safe_browsing_db/v4_protocol_manager_util.h"
#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/web_contents.h"
#include "google_apis/google_api_keys.h"
#include "net/base/escape.h"
+#include "net/base/url_util.h"
using content::BrowserThread;
+using content::WebContents;
using history::HistoryService;
namespace safe_browsing {
@@ -106,6 +109,15 @@ void PasswordProtectionService::CheckCsdWhitelistOnIOThread(
url.is_valid() ? database_manager_->MatchCsdWhitelistUrl(url) : true;
}
+bool PasswordProtectionService::CanGetReputationOfURL(const GURL& url) {
+ if (!url.is_valid() || !url.SchemeIsHTTPOrHTTPS())
+ return false;
+
+ const std::string& hostname = url.HostNoBrackets();
+ return !net::IsLocalhost(hostname) && !net::IsHostnameNonUnique(hostname) &&
+ hostname.find('.') != std::string::npos;
+}
+
LoginReputationClientResponse::VerdictType
PasswordProtectionService::GetCachedVerdict(
const GURL& url,
@@ -251,6 +263,7 @@ void PasswordProtectionService::CleanUpExpiredVerdicts() {
}
void PasswordProtectionService::StartRequest(
+ WebContents* web_contents,
const GURL& main_frame_url,
const GURL& password_form_action,
const GURL& password_form_frame_url,
@@ -258,7 +271,8 @@ void PasswordProtectionService::StartRequest(
LoginReputationClientRequest::TriggerType type) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
scoped_refptr<PasswordProtectionRequest> request(
- new PasswordProtectionRequest(main_frame_url, password_form_action,
+ new PasswordProtectionRequest(web_contents, main_frame_url,
+ password_form_action,
password_form_frame_url, saved_domain, type,
this, GetRequestTimeoutInMS()));
DCHECK(request);
@@ -267,44 +281,39 @@ void PasswordProtectionService::StartRequest(
}
void PasswordProtectionService::MaybeStartPasswordFieldOnFocusRequest(
+ WebContents* web_contents,
const GURL& main_frame_url,
const GURL& password_form_action,
const GURL& password_form_frame_url) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
- RequestOutcome request_outcome;
- if (!IsPingingEnabled(kPasswordFieldOnFocusPinging, &request_outcome)) {
- RecordPingingDisabledReason(kPasswordFieldOnFocusPinging, request_outcome);
- return;
- }
-
- // Skip URLs that we can't get a reliable reputation for.
- if (!main_frame_url.is_valid() || !main_frame_url.SchemeIsHTTPOrHTTPS()) {
- return;
+ if (CanSendPing(kPasswordFieldOnFocusPinging, main_frame_url)) {
+ StartRequest(web_contents, main_frame_url, password_form_action,
+ password_form_frame_url,
+ std::string(), /* saved_domain: not used for this type */
+ LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE);
}
-
- StartRequest(main_frame_url, password_form_action, password_form_frame_url,
- std::string(), /* saved_domain: not used for this type */
- LoginReputationClientRequest::UNFAMILIAR_LOGIN_PAGE);
}
void PasswordProtectionService::MaybeStartProtectedPasswordEntryRequest(
+ WebContents* web_contents,
const GURL& main_frame_url,
const std::string& saved_domain) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
- RequestOutcome request_outcome;
- if (!IsPingingEnabled(kProtectedPasswordEntryPinging, &request_outcome)) {
- RecordPingingDisabledReason(kProtectedPasswordEntryPinging,
- request_outcome);
- return;
+ if (CanSendPing(kProtectedPasswordEntryPinging, main_frame_url)) {
+ StartRequest(web_contents, main_frame_url, GURL(), GURL(), saved_domain,
+ LoginReputationClientRequest::PASSWORD_REUSE_EVENT);
}
+}
- // Skip URLs that we can't get a reliable reputation for.
- if (!main_frame_url.is_valid() || !main_frame_url.SchemeIsHTTPOrHTTPS()) {
- return;
+bool PasswordProtectionService::CanSendPing(const base::Feature& feature,
+ const GURL& main_frame_url) {
+ RequestOutcome request_outcome = URL_NOT_VALID_FOR_REPUTATION_COMPUTING;
+ if (IsPingingEnabled(kPasswordFieldOnFocusPinging, &request_outcome) &&
+ CanGetReputationOfURL(main_frame_url)) {
+ return true;
}
-
- StartRequest(main_frame_url, GURL(), GURL(), saved_domain,
- LoginReputationClientRequest::PASSWORD_REUSE_EVENT);
+ RecordNoPingingReason(feature, request_outcome);
+ return false;
}
void PasswordProtectionService::RequestFinished(
@@ -313,9 +322,7 @@ void PasswordProtectionService::RequestFinished(
DCHECK_CURRENTLY_ON(BrowserThread::UI);
DCHECK(request);
- // TODO(jialiul): We don't cache verdict for incognito mode for now.
- // Later we may consider temporarily caching verdict.
- if (response && !IsIncognito())
+ if (response)
CacheVerdict(request->main_frame_url(), response.get(), base::Time::Now());
// Finished processing this request. Remove it from pending list.
@@ -554,7 +561,7 @@ PasswordProtectionService::CreateDictionaryFromVerdict(
return result;
}
-void PasswordProtectionService::RecordPingingDisabledReason(
+void PasswordProtectionService::RecordNoPingingReason(
const base::Feature& feature,
RequestOutcome reason) {
DCHECK(feature.name == kProtectedPasswordEntryPinging.name ||

Powered by Google App Engine
This is Rietveld 408576698