Exempt non-GET requests from the query filter (fixes brave/brave-brow…

…ser#26377)
brave · Oct 29, 2022 · f7e0a03 · f7e0a03
1 parent be63639
commit f7e0a03
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 0 deletions.
diff --git a/browser/net/brave_site_hacks_network_delegate_helper.cc b/browser/net/brave_site_hacks_network_delegate_helper.cc
@@ -31,6 +31,10 @@ void ApplyPotentialQueryStringFilter(std::shared_ptr<BraveRequestInfo> ctx) {
     return;
   }
 
+  if (ctx->method != "GET") {
+    return;
+  }
+
   if (ctx->redirect_source.is_valid()) {
     if (ctx->internal_redirect) {
       // Ignore internal redirects since we trigger them.

diff --git a/browser/net/brave_site_hacks_network_delegate_helper_browsertest.cc b/browser/net/brave_site_hacks_network_delegate_helper_browsertest.cc
@@ -52,6 +52,7 @@ class BraveSiteHacksNetworkDelegateBrowserTest : public InProcessBrowserTest {
         https_server_.GetURL("redir.a.com", "/cross-site/a.com/simple.html");
 
     cross_site_url_ = https_server_.GetURL("b.com", "/navigate-to-site.html");
+    cross_site_post_url_ = https_server_.GetURL("b.com", "/post-to-site.html");
     same_site_url_ =
         https_server_.GetURL("sub.a.com", "/navigate-to-site.html");
 
@@ -137,6 +138,7 @@ class BraveSiteHacksNetworkDelegateBrowserTest : public InProcessBrowserTest {
   const GURL& simple_landing_url() { return simple_landing_url_; }
 
   const GURL& cross_site_url() { return cross_site_url_; }
+  const GURL& cross_site_post_url() { return cross_site_post_url_; }
   const GURL& redirect_to_cross_site_url() {
     return redirect_to_cross_site_url_;
   }
@@ -199,6 +201,7 @@ class BraveSiteHacksNetworkDelegateBrowserTest : public InProcessBrowserTest {
 
  private:
   GURL cross_site_url_;
+  GURL cross_site_post_url_;
   GURL redirect_to_cross_site_landing_url_;
   GURL redirect_to_cross_site_url_;
   GURL redirect_to_same_site_landing_url_;
@@ -255,6 +258,13 @@ IN_PROC_BROWSER_TEST_F(BraveSiteHacksNetworkDelegateBrowserTest,
   }
 }
 
+IN_PROC_BROWSER_TEST_F(BraveSiteHacksNetworkDelegateBrowserTest,
+                       QueryStringCrossSitePost) {
+  NavigateToURLAndWaitForRedirects(
+      url(landing_url("fbclid=1", simple_landing_url()), cross_site_post_url()),
+      landing_url("fbclid=1", simple_landing_url()));
+}
+
 IN_PROC_BROWSER_TEST_F(BraveSiteHacksNetworkDelegateBrowserTest,
                        QueryStringFilterShieldsDown) {
   const std::string inputs[] = {

diff --git a/browser/net/brave_site_hacks_network_delegate_helper_unittest.cc b/browser/net/brave_site_hacks_network_delegate_helper_unittest.cc
@@ -146,6 +146,7 @@ TEST(BraveSiteHacksNetworkDelegateHelperTest, QueryStringUntouched) {
         std::make_shared<brave::BraveRequestInfo>(GURL(url));
     brave_request_info->initiator_url =
         GURL("https://example.net");  // cross-site
+    brave_request_info->method = "GET";
     int rc = brave::OnBeforeURLRequest_SiteHacksWork(ResponseCallback(),
                                                      brave_request_info);
     EXPECT_EQ(rc, net::OK);
@@ -166,6 +167,7 @@ TEST(BraveSiteHacksNetworkDelegateHelperTest, QueryStringExempted) {
     auto brave_request_info =
         std::make_shared<brave::BraveRequestInfo>(tracking_url);
     brave_request_info->initiator_url = GURL(initiator);
+    brave_request_info->method = "GET";
     int rc = brave::OnBeforeURLRequest_SiteHacksWork(ResponseCallback(),
                                                      brave_request_info);
     EXPECT_EQ(rc, net::OK);
@@ -179,6 +181,7 @@ TEST(BraveSiteHacksNetworkDelegateHelperTest, QueryStringExempted) {
         std::make_shared<brave::BraveRequestInfo>(tracking_url);
     brave_request_info->initiator_url =
         GURL("https://example.net");  // cross-site
+    brave_request_info->method = "GET";
     brave_request_info->internal_redirect = true;
     brave_request_info->redirect_source =
         GURL("https://example.org");  // cross-site
@@ -189,12 +192,29 @@ TEST(BraveSiteHacksNetworkDelegateHelperTest, QueryStringExempted) {
     EXPECT_TRUE(brave_request_info->new_url_spec.empty());
   }
 
+  // POST requests
+  {
+    auto brave_request_info =
+        std::make_shared<brave::BraveRequestInfo>(tracking_url);
+    brave_request_info->initiator_url =
+        GURL("https://example.net");  // cross-site
+    brave_request_info->method = "POST";
+    brave_request_info->redirect_source =
+        GURL("https://example.org");  // 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());
+  }
+
   // Same-site redirect
   {
     auto brave_request_info =
         std::make_shared<brave::BraveRequestInfo>(tracking_url);
     brave_request_info->initiator_url =
         GURL("https://example.net");  // cross-site
+    brave_request_info->method = "GET";
     brave_request_info->redirect_source =
         GURL("https://sub.example.com");  // same-site
     int rc = brave::OnBeforeURLRequest_SiteHacksWork(ResponseCallback(),
@@ -238,6 +258,7 @@ TEST(BraveSiteHacksNetworkDelegateHelperTest, QueryStringFiltered) {
         std::make_shared<brave::BraveRequestInfo>(GURL(pair.first));
     brave_request_info->initiator_url =
         GURL("https://example.net");  // cross-site
+    brave_request_info->method = "GET";
     int rc = brave::OnBeforeURLRequest_SiteHacksWork(ResponseCallback(),
                                                      brave_request_info);
     EXPECT_EQ(rc, net::OK);
@@ -250,6 +271,7 @@ TEST(BraveSiteHacksNetworkDelegateHelperTest, QueryStringFiltered) {
         GURL("https://example.com/?fbclid=1"));
     brave_request_info->initiator_url =
         GURL("https://example.com");  // same-origin
+    brave_request_info->method = "GET";
     brave_request_info->redirect_source =
         GURL("https://example.net");  // cross-site
     int rc = brave::OnBeforeURLRequest_SiteHacksWork(ResponseCallback(),
@@ -263,6 +285,7 @@ TEST(BraveSiteHacksNetworkDelegateHelperTest, QueryStringFiltered) {
     auto brave_request_info = std::make_shared<brave::BraveRequestInfo>(
         GURL("https://example.com/?fbclid=2"));
     brave_request_info->initiator_url = GURL();
+    brave_request_info->method = "GET";
     int rc = brave::OnBeforeURLRequest_SiteHacksWork(ResponseCallback(),
                                                      brave_request_info);
     EXPECT_EQ(rc, net::OK);