Skip to content

Commit

Permalink
Merge pull request #25977 from brave/issues/41627
Browse files Browse the repository at this point in the history
[ads] Deprecate `ReplaceBraveSchemeWithChromeScheme`
  • Loading branch information
tmancey authored Oct 15, 2024
2 parents a81b2b1 + db4bf0b commit 1e5d680
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 86 deletions.
6 changes: 3 additions & 3 deletions components/brave_ads/core/internal/common/url/url_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,13 @@ bool ShouldSupportUrl(const GURL& url) {
return false;
}

if (internal::DoesETLDPlusOneContainWildcards(url)) {
if (DoesETLDPlusOneContainWildcards(url)) {
return false;
}

return url.SchemeIs(url::kHttpsScheme)
? internal::HostHasRegistryControlledDomain(url.host_piece())
: internal::ShouldSupportInternalUrl(url);
? HostHasRegistryControlledDomain(url.host_piece())
: ShouldSupportInternalUrl(url);
}

bool MatchUrlPattern(const GURL& url, const std::string& pattern) {
Expand Down
34 changes: 7 additions & 27 deletions components/brave_ads/core/internal/common/url/url_util_internal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,10 @@
#include "net/base/url_util.h"
#include "url/gurl.h"

namespace brave_ads::internal {
namespace brave_ads {

namespace {

constexpr char kBraveScheme[] = "brave";
constexpr char kChromeScheme[] = "chrome";

constexpr char kRewardsHostName[] = "rewards";
Expand All @@ -31,20 +30,6 @@ constexpr char kSearchQuery[] = "search";

} // namespace

GURL ReplaceBraveSchemeWithChromeScheme(const GURL& url) {
if (!url.is_valid()) {
return url;
}

if (!url.SchemeIs(kBraveScheme)) {
return url;
}

GURL::Replacements replacements;
replacements.SetSchemeStr(kChromeScheme);
return url.ReplaceComponents(replacements);
}

bool HasSearchQuery(const GURL& url) {
CHECK(url.is_valid());
CHECK(url.has_query());
Expand All @@ -63,16 +48,12 @@ bool ShouldSupportInternalUrl(const GURL& url) {
return false;
}

// The internal brave:// scheme must be replaced with chrome:// because `GURL`
// does not parse the brave:// scheme.
const GURL modified_url = ReplaceBraveSchemeWithChromeScheme(url);

if (!modified_url.SchemeIs(kChromeScheme)) {
if (!url.SchemeIs(kChromeScheme)) {
// Do not support schemes other than chrome://.
return false;
}

const std::string host_name = modified_url.host();
const std::string host_name = url.host();

if (host_name == kRewardsHostName || host_name == kSyncHostName ||
host_name == kWalletHostName) {
Expand All @@ -85,15 +66,14 @@ bool ShouldSupportInternalUrl(const GURL& url) {
return false;
}

if (modified_url.path() == kSearchEnginesPath ||
modified_url.path() == kSearchPath) {
if (!modified_url.has_query()) {
if (url.path() == kSearchEnginesPath || url.path() == kSearchPath) {
if (!url.has_query()) {
// Support chrome://settings/searchEngines and chrome://settings/search
// paths without a query.
return true;
}

return HasSearchQuery(modified_url);
return HasSearchQuery(url);
}

// We can reject everything else!
Expand Down Expand Up @@ -122,4 +102,4 @@ bool DoesETLDPlusOneContainWildcards(const GURL& url) {
kUrlEncodedAsteriskWildcard);
}

} // namespace brave_ads::internal
} // namespace brave_ads
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@

class GURL;

namespace brave_ads::internal {

GURL ReplaceBraveSchemeWithChromeScheme(const GURL& url);
namespace brave_ads {

bool HasSearchQuery(const GURL& url);
bool ShouldSupportInternalUrl(const GURL& url);
Expand All @@ -21,6 +19,6 @@ bool HostHasRegistryControlledDomain(std::string_view host);

bool DoesETLDPlusOneContainWildcards(const GURL& url);

} // namespace brave_ads::internal
} // namespace brave_ads

#endif // BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_COMMON_URL_URL_UTIL_INTERNAL_H_
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,6 @@

namespace brave_ads::internal {

TEST(BraveAdsUrlUtilInternalTest, ReplaceBraveSchemeWithChromeScheme) {
// Act & Assert
EXPECT_EQ(GURL("chrome://xyzzy"),
ReplaceBraveSchemeWithChromeScheme(GURL("brave://xyzzy")));
}

TEST(BraveAdsUrlUtilInternalTest, DoNotReplaceNonBraveSchemeWithChromeScheme) {
// Act & Assert
EXPECT_EQ(GURL("file://xyzzy"),
ReplaceBraveSchemeWithChromeScheme(GURL("file://xyzzy")));
}

TEST(BraveAdsUrlUtilInternalTest, HasUrlSearchQueryNameAndValue) {
// Act & Assert
EXPECT_TRUE(HasSearchQuery(GURL("https://xyzzy.com/?search=thud")));
Expand Down Expand Up @@ -75,137 +63,138 @@ TEST(BraveAdsUrlUtilInternalTest, HostDoesNotHaveRegistryControlledDomain) {
TEST(BraveAdsUrlUtilInternalTest,
ShouldNotSupportInternalUrlWithBraveSchemeAndFooBarHostName) {
// Act & Assert
EXPECT_FALSE(ShouldSupportInternalUrl(GURL("brave://foobar")));
EXPECT_FALSE(ShouldSupportInternalUrl(GURL("chrome://foobar")));
}

TEST(BraveAdsUrlUtilInternalTest,
ShouldSupportInternalUrlWithBraveSchemeAndWalletHostName) {
// Act & Assert
EXPECT_TRUE(ShouldSupportInternalUrl(GURL("brave://wallet")));
EXPECT_TRUE(ShouldSupportInternalUrl(GURL("chrome://wallet")));
}

TEST(BraveAdsUrlUtilInternalTest,
ShouldSupportInternalUrlWithBraveSchemeAndWalletHostNameAndPath) {
// Act & Assert
EXPECT_TRUE(ShouldSupportInternalUrl(GURL("brave://wallet/foo")));
EXPECT_TRUE(ShouldSupportInternalUrl(GURL("chrome://wallet/foo")));
}

TEST(BraveAdsUrlUtilInternalTest,
ShouldSupportInternalUrlWithBraveSchemeAndSyncHostName) {
// Act & Assert
EXPECT_TRUE(ShouldSupportInternalUrl(GURL("brave://sync")));
EXPECT_TRUE(ShouldSupportInternalUrl(GURL("chrome://sync")));
}

TEST(BraveAdsUrlUtilInternalTest,
ShouldSupportInternalUrlWithBraveSchemeAndSyncHostNameAndPath) {
// Act & Assert
EXPECT_TRUE(ShouldSupportInternalUrl(GURL("brave://sync/foo")));
EXPECT_TRUE(ShouldSupportInternalUrl(GURL("chrome://sync/foo")));
}

TEST(BraveAdsUrlUtilInternalTest,
ShouldSupportInternalUrlWithBraveSchemeAndRewardsHostName) {
// Act & Assert
EXPECT_TRUE(ShouldSupportInternalUrl(GURL("brave://rewards")));
EXPECT_TRUE(ShouldSupportInternalUrl(GURL("chrome://rewards")));
}

TEST(BraveAdsUrlUtilInternalTest,
ShouldSupportInternalUrlWithBraveSchemeAndRewardsHostNameAndPath) {
// Act & Assert
EXPECT_TRUE(ShouldSupportInternalUrl(GURL("brave://rewards/foo")));
EXPECT_TRUE(ShouldSupportInternalUrl(GURL("chrome://rewards/foo")));
}

TEST(BraveAdsUrlUtilInternalTest,
ShouldNotSupportInternalUrlWithBraveSchemeAndSettingsHostName) {
// Act & Assert
EXPECT_FALSE(ShouldSupportInternalUrl(GURL("brave://settings")));
EXPECT_FALSE(ShouldSupportInternalUrl(GURL("chrome://settings")));
}

TEST(
BraveAdsUrlUtilInternalTest,
ShouldNotSupportInternalUrlWithBraveSchemeAndSettingsHostNameAndFooBarPath) {
// Act & Assert
EXPECT_FALSE(ShouldSupportInternalUrl(GURL("brave://settings/foobar")));
EXPECT_FALSE(ShouldSupportInternalUrl(GURL("chrome://settings/foobar")));
}

TEST(
BraveAdsUrlUtilInternalTest,
ShouldSupportInternalUrlWithBraveSchemeAndSettingsHostNameAndSearchEnginesPath) {
// Act & Assert
EXPECT_TRUE(ShouldSupportInternalUrl(GURL("brave://settings/searchEngines")));
EXPECT_TRUE(
ShouldSupportInternalUrl(GURL("chrome://settings/searchEngines")));
}

TEST(
BraveAdsUrlUtilInternalTest,
ShouldSupportInternalUrlWithBraveSchemeAndSettingsHostNameSearchEnginesPathAndSearchQuery) {
// Act & Assert
EXPECT_TRUE(ShouldSupportInternalUrl(
GURL("brave://settings/searchEngines?search=foobar")));
GURL("chrome://settings/searchEngines?search=foobar")));
}

TEST(
BraveAdsUrlUtilInternalTest,
ShouldNotSupportInternalUrlWithBraveSchemeAndSettingsHostNameSearchEnginesPathAndMultipleSearchQueries) {
// Act & Assert
EXPECT_FALSE(ShouldSupportInternalUrl(
GURL("brave://settings/searchEngines?search=foo&bar=baz")));
GURL("chrome://settings/searchEngines?search=foo&bar=baz")));
}

TEST(
BraveAdsUrlUtilInternalTest,
ShouldNotSupportInternalUrlWithBraveSchemeAndSettingsHostNameSearchEnginesPathAndInvalidQuery) {
// Act & Assert
EXPECT_FALSE(
ShouldSupportInternalUrl(GURL("brave://settings/searchEngines?search")));
ShouldSupportInternalUrl(GURL("chrome://settings/searchEngines?search")));
}

TEST(BraveAdsUrlUtilInternalTest,
ShouldSupportInternalUrlWithBraveSchemeAndSettingsHostNameAndSearchPath) {
// Act & Assert
EXPECT_TRUE(ShouldSupportInternalUrl(GURL("brave://settings/search")));
EXPECT_TRUE(ShouldSupportInternalUrl(GURL("chrome://settings/search")));
}

TEST(
BraveAdsUrlUtilInternalTest,
ShouldSupportInternalUrlWithBraveSchemeAndSettingsHostNameSearchPathAndSearchQuery) {
// Act & Assert
EXPECT_TRUE(
ShouldSupportInternalUrl(GURL("brave://settings/search?search=foobar")));
ShouldSupportInternalUrl(GURL("chrome://settings/search?search=foobar")));
}

TEST(
BraveAdsUrlUtilInternalTest,
ShouldNotSupportInternalUrlWithBraveSchemeAndSettingsHostNameSearchPathAndMultipleSearchQueries) {
// Act & Assert
EXPECT_FALSE(ShouldSupportInternalUrl(
GURL("brave://settings/search?search=foo&bar=baz")));
GURL("chrome://settings/search?search=foo&bar=baz")));
}

TEST(
BraveAdsUrlUtilInternalTest,
ShouldNotSupportInternalUrlWithBraveSchemeAndSettingsHostNameSearchPathAndInvalidQuery) {
// Act & Assert
EXPECT_FALSE(
ShouldSupportInternalUrl(GURL("brave://settings/search?search")));
ShouldSupportInternalUrl(GURL("chrome://settings/search?search")));
}

TEST(BraveAdsUrlUtilInternalTest,
ShouldNotSupportInternalUrlWithBraveSchemeAndSettingsHostNameAndQuery) {
// Act & Assert
EXPECT_FALSE(
ShouldSupportInternalUrl(GURL("brave://settings/?search=foobar")));
ShouldSupportInternalUrl(GURL("chrome://settings/?search=foobar")));
}

TEST(
BraveAdsUrlUtilInternalTest,
ShouldNotSupportInternalUrlWithBraveSchemeAndSettingsHostNameAndInvalidQuery) {
// Act & Assert
EXPECT_FALSE(ShouldSupportInternalUrl(GURL("brave://settings/?search")));
EXPECT_FALSE(ShouldSupportInternalUrl(GURL("chrome://settings/?search")));
}

TEST(BraveAdsUrlUtilInternalTest, ShouldNotSupportMalformedUrl) {
// Act & Assert
EXPECT_FALSE(
ShouldSupportInternalUrl(GURL("http://foobar.com/brave://wallet")));
ShouldSupportInternalUrl(GURL("http://foobar.com/chrome://wallet")));
}

} // namespace brave_ads::internal
Loading

0 comments on commit 1e5d680

Please sign in to comment.