Commit 695bc40c authored by Joe DeBlasio's avatar Joe DeBlasio Committed by Commit Bot

[Simplified domains] Respect lookalike allowlist when checking policy.

This CL ensures that a given URL is not on the lookalikes / safety tips
allowlist before eliding to eTLD+1.  At present, the allowlist is very
coarse-grained and doesn't disambiguate between what the allowlist entry
is there to allowlist against (i.e. which UI treatment shouldn't be
applied). A future change will add a bit more nuance.

Bug: 1106962
Change-Id: I31eeb921afd5af89c8a1a2b1efc2b91c521eedc6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2346841Reviewed-by: default avatarJustin Donnelly <jdonnelly@chromium.org>
Reviewed-by: default avatarMustafa Emre Acer <meacer@chromium.org>
Commit-Queue: Joe DeBlasio <jdeblasio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796820}
parent dfb96619
......@@ -74,3 +74,7 @@ void SetSafetyTipAllowlistPatterns(std::vector<std::string> patterns,
}
SetSafetyTipsRemoteConfigProto(std::move(config_proto));
}
void InitializeBlankLookalikeAllowlistForTesting() {
SetSafetyTipAllowlistPatterns({}, {});
}
......@@ -32,4 +32,10 @@ void SetSafetyTipBadRepPatterns(std::vector<std::string> pattern);
void SetSafetyTipAllowlistPatterns(std::vector<std::string> patterns,
std::vector<std::string> target_patterns);
// Ensure that the allowlist has been initialized. This is important as some
// code (e.g. the elision policy) is fail-open (i.e. it won't elide without an
// initialized allowlist). This is convenience wrapper around
// SetSafetyTipAllowlistPatterns().
void InitializeBlankLookalikeAllowlistForTesting();
#endif // CHROME_BROWSER_REPUTATION_SAFETY_TIP_TEST_UTILS_H_
......@@ -6,6 +6,8 @@
#include "base/feature_list.h"
#include "chrome/browser/reputation/local_heuristics.h"
#include "chrome/browser/reputation/safety_tip_test_utils.h"
#include "chrome/browser/reputation/safety_tips_config.h"
#include "components/lookalikes/core/lookalike_url_util.h"
#include "components/omnibox/common/omnibox_features.h"
#include "components/url_formatter/spoof_checks/top_domains/top500_domains.h"
......@@ -25,9 +27,14 @@ bool ShouldElideToRegistrableDomain(const GURL& url) {
return false;
}
auto host = url.host();
// TODO(jdeblasio): Check allowlist
auto* proto = GetSafetyTipsRemoteConfigProto();
if (!proto || IsUrlAllowlistedBySafetyTipsComponent(proto, url)) {
// Not having a proto happens when the component hasn't downloaded yet. This
// should only happen for a short window following initial Chrome install.
return false;
}
auto host = url.host();
if (static_cast<int>(host.length()) > kMaximumUnelidedHostnameLength.Get()) {
return true;
}
......
......@@ -5,6 +5,7 @@
#include "chrome/browser/reputation/url_elision_policy.h"
#include "base/test/scoped_feature_list.h"
#include "chrome/browser/reputation/safety_tip_test_utils.h"
#include "components/omnibox/common/omnibox_features.h"
#include "components/url_formatter/spoof_checks/common_words/common_words_util.h"
#include "testing/gtest/include/gtest/gtest.h"
......@@ -25,6 +26,9 @@ class UrlElisionPolicyTest : public testing::Test {
{});
url_formatter::common_words::SetCommonWordDAFSAForTesting(
test::kDafsa, sizeof(test::kDafsa));
// Ensure that the allowlist exists (as otherwise we'll never elide).
InitializeBlankLookalikeAllowlistForTesting();
}
~UrlElisionPolicyTest() override = default;
......@@ -51,6 +55,18 @@ TEST_F(UrlElisionPolicyTest, DoesntElideShortDomains) {
EXPECT_FALSE(ShouldElideToRegistrableDomain(kUrl));
}
// Ensure that domains are allowlisted are not elided.
TEST_F(UrlElisionPolicyTest, DoesntElideAllowlistedDomains) {
GURL kUrl = GURL("http://alongbutstillallowlisteddomain.com/xyz");
// This domain should be elided normally...
EXPECT_TRUE(ShouldElideToRegistrableDomain(kUrl));
// ...but not when allowlisted.
SetSafetyTipAllowlistPatterns({"alongbutstillallowlisteddomain.com/"}, {});
EXPECT_FALSE(ShouldElideToRegistrableDomain(kUrl));
}
// Ensure that sensitive keywords cause elision when expected and not otherwise.
// Note further tests in SafetyTipHeuristicsTest.SensitiveKeywordsTest.
TEST_F(UrlElisionPolicyTest, ElidesKeywordedDomainsAppropriately) {
......
......@@ -21,6 +21,7 @@
#include "chrome/browser/autocomplete/chrome_autocomplete_scheme_classifier.h"
#include "chrome/browser/command_updater.h"
#include "chrome/browser/command_updater_impl.h"
#include "chrome/browser/reputation/safety_tip_test_utils.h"
#include "chrome/browser/search_engines/template_url_service_factory_test_util.h"
#include "chrome/browser/ui/omnibox/chrome_omnibox_client.h"
#include "chrome/browser/ui/omnibox/chrome_omnibox_edit_controller.h"
......@@ -1624,7 +1625,10 @@ class OmniboxViewViewsRevealOnHoverTest
{{omnibox::kRevealSteadyStateUrlPathQueryAndRefOnHover,
{}}}),
{},
GetParam().second) {}
GetParam().second) {
// The lookalike allowlist is used by the registrable-domain-elision code.
InitializeBlankLookalikeAllowlistForTesting();
}
OmniboxViewViewsRevealOnHoverTest(const OmniboxViewViewsRevealOnHoverTest&) =
delete;
......@@ -1734,7 +1738,10 @@ class OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest
kHideSteadyStateUrlPathQueryAndRefOnInteraction,
{}}}),
{},
GetParam().second) {}
GetParam().second) {
// The lookalike allowlist is used by the registrable-domain-elision code.
InitializeBlankLookalikeAllowlistForTesting();
}
OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest(
const OmniboxViewViewsHideOnInteractionAndRevealOnHoverTest&) = delete;
......@@ -2098,7 +2105,11 @@ class OmniboxViewViewsHideOnInteractionTest
kHideSteadyStateUrlPathQueryAndRefOnInteraction,
{}}}),
{},
GetParam().second) {}
GetParam().second) {
// The lookalike allowlist is used by the registrable-domain-elision code.
InitializeBlankLookalikeAllowlistForTesting();
}
OmniboxViewViewsHideOnInteractionTest(
const OmniboxViewViewsHideOnInteractionTest&) = delete;
OmniboxViewViewsHideOnInteractionTest& operator=(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment