Exempt same-site requests from query string filter (fixes brave/brave…

…-browser#9020)
brave · Sep 18, 2020 · d6a5151 · d6a5151
1 parent b1186e8
commit d6a5151
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 3 deletions.
diff --git a/browser/net/brave_site_hacks_network_delegate_helper.cc b/browser/net/brave_site_hacks_network_delegate_helper.cc
@@ -19,6 +19,7 @@
 #include "brave/components/brave_shields/browser/brave_shields_util.h"
 #include "content/public/common/referrer.h"
 #include "extensions/common/url_pattern.h"
+#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
 #include "net/url_request/url_request.h"
 #include "third_party/blink/public/common/loader/network_utils.h"
 #include "third_party/blink/public/common/loader/referrer_utils.h"
@@ -73,10 +74,19 @@ DECLARE_LAZY_MATCHER(tracker_appended_matcher,
 
 #undef DECLARE_LAZY_MATCHER
 
-void ApplyPotentialQueryStringFilter(const GURL& request_url,
+void ApplyPotentialQueryStringFilter(const GURL& initiator_url,
+                                     const GURL& request_url,
                                      std::string* new_url_spec) {
   DCHECK(new_url_spec);
   SCOPED_UMA_HISTOGRAM_TIMER("Brave.SiteHacks.QueryFilter");
+
+  // Same-site requests are exempted from the filter.
+  if (net::registry_controlled_domains::SameDomainOrHost(
+          initiator_url, request_url,
+          net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES)) {
+    return;
+  }
+
   std::string new_query = request_url.query();
   // Note: the ordering of these replacements is important.
   const int replacement_count =
@@ -124,8 +134,9 @@ bool ApplyPotentialReferrerBlock(std::shared_ptr<BraveRequestInfo> ctx) {
 int OnBeforeURLRequest_SiteHacksWork(const ResponseCallback& next_callback,
                                      std::shared_ptr<BraveRequestInfo> ctx) {
   ApplyPotentialReferrerBlock(ctx);
-  if (ctx->request_url.has_query()) {
-    ApplyPotentialQueryStringFilter(ctx->request_url, &ctx->new_url_spec);
+  if (ctx->request_url.has_query() && ctx->initiator_url.is_valid()) {
+    ApplyPotentialQueryStringFilter(ctx->initiator_url, ctx->request_url,
+                                    &ctx->new_url_spec);
   }
   return net::OK;
 }

diff --git a/browser/net/brave_site_hacks_network_delegate_helper_unittest.cc b/browser/net/brave_site_hacks_network_delegate_helper_unittest.cc
@@ -174,6 +174,31 @@ TEST(BraveSiteHacksNetworkDelegateHelperTest, QueryStringUntouched) {
   for (const auto& url : urls) {
     auto brave_request_info =
         std::make_shared<brave::BraveRequestInfo>(GURL(url));
+    brave_request_info->initiator_url =
+        GURL("https://example.net");  // cross-site
+    int rc = brave::OnBeforeURLRequest_SiteHacksWork(ResponseCallback(),
+                                                     brave_request_info);
+    EXPECT_EQ(rc, net::OK);
+    // new_url should not be set
+    EXPECT_TRUE(brave_request_info->new_url_spec.empty());
+  }
+}
+
+TEST(BraveSiteHacksNetworkDelegateHelperTest, QueryStringExempted) {
+  const GURL tracking_url("https://example.com/?fbclid=1");
+
+  const std::string initiators[] = {
+      "",                              // Direct navigation
+      "https://example.com/path",      // Same-origin
+      "https://sub.example.com/path",  // Same-site
+  };
+
+  constexpr size_t initiator_count = sizeof(initiators) / sizeof(std::string);
+
+  for (size_t i = 0; i < initiator_count; i++) {
+    auto brave_request_info =
+        std::make_shared<brave::BraveRequestInfo>(tracking_url);
+    brave_request_info->initiator_url = GURL(initiators[i]);
     int rc = brave::OnBeforeURLRequest_SiteHacksWork(ResponseCallback(),
                                                      brave_request_info);
     EXPECT_EQ(rc, net::OK);
@@ -211,6 +236,8 @@ TEST(BraveSiteHacksNetworkDelegateHelperTest, QueryStringFiltered) {
   for (const auto& pair : urls) {
     auto brave_request_info =
         std::make_shared<brave::BraveRequestInfo>(GURL(pair.first));
+    brave_request_info->initiator_url =
+        GURL("https://example.net");  // cross-site
     int rc = brave::OnBeforeURLRequest_SiteHacksWork(ResponseCallback(),
                                                      brave_request_info);
     EXPECT_EQ(rc, net::OK);