Don't do $removeparam in default blocking mode (uplift to 1.58.x) (#…

…20106)

Uplift of #19954 (squashed) to release

browser/brave_shields/ad_block_service_browsertest.cc

@@ -1781,6 +1781,32 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, RemoveparamTopLevelNavigation) {
   ASSERT_TRUE(dynamic_server_.ShutdownAndWaitUntilComplete());
 }
 
+// `$removeparam` should not be activated in default blocking mode
+IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, DefaultNoRemoveparam) {
+  ASSERT_TRUE(InstallDefaultAdBlockExtension());
+  DisableAggressiveMode();
+
+  EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 0ULL);
+
+  UpdateAdBlockInstanceWithRules("*$subdocument,removeparam=evil");
+
+  GURL tab_url =
+      embedded_test_server()->GetURL("b.com", "/cosmetic_filtering.html");
+  ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), tab_url));
+
+  content::WebContents* contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+
+  GURL frame_url = embedded_test_server()->GetURL(
+      "frame.com", "/cosmetic_frame.html?evil=true&test=true");
+  content::NavigateIframeToURL(contents, "iframe", frame_url);
+
+  ASSERT_NE(nullptr,
+            content::FrameMatchingPredicateOrNullptr(
+                contents->GetPrimaryPage(),
+                base::BindRepeating(content::FrameHasSourceUrl, frame_url)));
+}
+
 // Verify that scripts violating a Content Security Policy from a `$csp` rule
 // are not loaded.
 IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, CspRule) {

browser/net/brave_ad_block_tp_network_delegate_helper.cc

@@ -201,7 +201,8 @@ EngineFlags ShouldBlockRequestOnTaskRunner(
   if (adblock_result.rewritten_url.has_value &&
       GURL(std::string(adblock_result.rewritten_url.value)).is_valid() &&
       (ctx->method == "GET" || ctx->method == "HEAD" ||
-       ctx->method == "OPTIONS")) {
+       ctx->method == "OPTIONS") &&
+      ctx->aggressive_blocking) {
     ctx->new_url_spec = std::string(adblock_result.rewritten_url.value);
   }
 

components/brave_shields/browser/domain_block_navigation_throttle.cc

@@ -34,20 +34,24 @@ namespace {
 
 std::pair<bool, std::string> ShouldBlockDomainOnTaskRunner(
     brave_shields::AdBlockService* ad_block_service,
-    const GURL& url) {
+    const GURL& url,
+    bool aggressive_setting) {
   SCOPED_UMA_HISTOGRAM_TIMER("Brave.DomainBlock.ShouldBlock");
   // force aggressive blocking to `true` for domain blocking - these requests
   // are all "first-party", but the throttle is already only called when
   // necessary.
-  bool aggressive_blocking = true;
+  bool aggressive_for_engine = true;
   auto result = ad_block_service->ShouldStartRequest(
       url, blink::mojom::ResourceType::kMainFrame, url.host(),
-      aggressive_blocking, false, false, false);
+      aggressive_for_engine, false, false, false);
 
   const bool should_block =
       result.important || (result.matched && !result.has_exception);
 
-  return std::pair(should_block, std::string(result.rewritten_url.value));
+  std::string new_url = aggressive_setting && result.rewritten_url.has_value
+                            ? std::string(result.rewritten_url.value)
+                            : std::string();
+  return std::pair(should_block, new_url);
 }
 
 }  // namespace
@@ -122,12 +126,16 @@ DomainBlockNavigationThrottle::WillStartRequest() {
   if (tab_storage->IsProceeding())
     return content::NavigationThrottle::PROCEED;
 
+  bool aggressive_mode =
+      brave_shields::GetCosmeticFilteringControlType(
+          content_settings_, request_url) == brave_shields::ControlType::BLOCK;
+
   // Otherwise, call the ad block service on a task runner to determine whether
   // this domain should be blocked.
   ad_block_service_->GetTaskRunner()->PostTaskAndReplyWithResult(
       FROM_HERE,
       base::BindOnce(&ShouldBlockDomainOnTaskRunner, ad_block_service_,
-                     request_url),
+                     request_url, aggressive_mode),
       base::BindOnce(&DomainBlockNavigationThrottle::OnShouldBlockDomain,
                      weak_ptr_factory_.GetWeakPtr()));