Skip to content

Commit

Permalink
Merge pull request #9673 from brave/1p-blocking-flag
Browse files Browse the repository at this point in the history
add flag for first-party blocking in standard blocking mode
  • Loading branch information
antonok-edm authored Aug 16, 2021
2 parents 6fc408c + d66d36a commit a9f8e12
Show file tree
Hide file tree
Showing 18 changed files with 149 additions and 18 deletions.
85 changes: 85 additions & 0 deletions browser/brave_shields/ad_block_service_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ const char kRegionalAdBlockComponentTest64PublicKey[] =
using brave_shields::features::kBraveAdblockCnameUncloaking;
using brave_shields::features::kBraveAdblockCollapseBlockedElements;
using brave_shields::features::kBraveAdblockCosmeticFiltering;
using brave_shields::features::kBraveAdblockDefault1pBlocking;
using content::BrowserThread;

void AdBlockServiceTest::SetUpOnMainThread() {
Expand Down Expand Up @@ -1138,6 +1139,90 @@ IN_PROC_BROWSER_TEST_F(CollapseBlockedElementsFlagDisabledTest,
"i[0].clientHeight !== 0"));
}

class Default1pBlockingFlagDisabledTest : public AdBlockServiceTest {
public:
Default1pBlockingFlagDisabledTest() {
feature_list_.InitAndDisableFeature(kBraveAdblockDefault1pBlocking);
}

private:
base::test::ScopedFeatureList feature_list_;
};

// Load a page with an image from a first party and a third party, which both
// match the same filter in the default engine. Ensure the third-party one is
// blocked while the first-party one is allowed.
IN_PROC_BROWSER_TEST_F(Default1pBlockingFlagDisabledTest, Default1pBlocking) {
EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 0ULL);
UpdateAdBlockInstanceWithRules("^ad_banner.png");

GURL url = embedded_test_server()->GetURL(kAdBlockTestPage);
ui_test_utils::NavigateToURL(browser(), url);
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();

ASSERT_EQ(true, EvalJs(contents,
"setExpectations(1, 0, 0, 0);"
"addImage('ad_banner.png')"));
EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 0ULL);

ASSERT_EQ(true, EvalJs(contents,
"setExpectations(1, 1, 0, 0);"
"addImage('https://thirdparty.com/ad_banner.png')"));
EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 1ULL);
}

// Load a page with an image from a first party and a third party, which both
// match the same filter in the default engine. Enable aggressive mode, and
// ensure that both are blocked.
IN_PROC_BROWSER_TEST_F(Default1pBlockingFlagDisabledTest,
Aggressive1pBlocking) {
EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 0ULL);
brave_shields::SetCosmeticFilteringControlType(
content_settings(), brave_shields::ControlType::BLOCK, GURL());
UpdateAdBlockInstanceWithRules("^ad_banner.png");

GURL url = embedded_test_server()->GetURL(kAdBlockTestPage);
ui_test_utils::NavigateToURL(browser(), url);
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();

ASSERT_EQ(true, EvalJs(contents,
"setExpectations(0, 1, 0, 0);"
"addImage('ad_banner.png')"));
EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 1ULL);

ASSERT_EQ(true, EvalJs(contents,
"setExpectations(0, 2, 0, 0);"
"addImage('https://thirdparty.com/ad_banner.png')"));
EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 2ULL);
}

// Load a page with an image from a first party and a third party, which both
// match the same filter in the custom filters engine. Ensure that both are
// blocked.
IN_PROC_BROWSER_TEST_F(Default1pBlockingFlagDisabledTest, Custom1pBlocking) {
EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 0ULL);
ASSERT_TRUE(g_brave_browser_process->ad_block_custom_filters_service()
->UpdateCustomFilters("^ad_banner.png"));
WaitForAdBlockServiceThreads();

GURL url = embedded_test_server()->GetURL(kAdBlockTestPage);
ui_test_utils::NavigateToURL(browser(), url);
content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();

ASSERT_EQ(true, EvalJs(contents,
"setExpectations(0, 1, 0, 0);"
"addImage('ad_banner.png')"));
EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 1ULL);

ASSERT_EQ(true, EvalJs(contents,
"setExpectations(0, 2, 0, 0);"
"addImage('https://thirdparty.com/ad_banner.png')"));
EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 2ULL);
}

// Load a page with a script which uses a redirect data URL.
IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, RedirectRulesAreRespected) {
UpdateAdBlockInstanceWithRules("js_mock_me.js$redirect=noopjs",
Expand Down
2 changes: 1 addition & 1 deletion browser/net/brave_ad_block_tp_network_delegate_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ EngineFlags ShouldBlockRequestOnTaskRunner(

SCOPED_UMA_HISTOGRAM_TIMER("Brave.Adblock.ShouldBlockRequest");
g_brave_browser_process->ad_block_service()->ShouldStartRequest(
url_to_check, ctx->resource_type, source_host,
url_to_check, ctx->resource_type, source_host, ctx->aggressive_blocking,
&previous_result.did_match_rule, &previous_result.did_match_exception,
&previous_result.did_match_important, &ctx->mock_data_url);

Expand Down
5 changes: 5 additions & 0 deletions browser/net/url_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,11 @@ std::shared_ptr<brave::BraveRequestInfo> BraveRequestInfo::MakeCTX(
brave_shields::GetBraveShieldsEnabled(map, ctx->tab_origin);
ctx->allow_ads = brave_shields::GetAdControlType(map, ctx->tab_origin) ==
brave_shields::ControlType::ALLOW;
// Currently, "aggressive" mode is registered as a cosmetic filtering control
// type, even though it can also affect network blocking.
ctx->aggressive_blocking =
brave_shields::GetCosmeticFilteringControlType(map, ctx->tab_origin) ==
brave_shields::ControlType::BLOCK;
ctx->allow_http_upgradable_resource =
!brave_shields::GetHTTPSEverywhereEnabled(map, ctx->tab_origin);

Expand Down
3 changes: 3 additions & 0 deletions browser/net/url_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ struct BraveRequestInfo {
// TODO(iefremov): rename to shields_up.
bool allow_brave_shields = true;
bool allow_ads = false;
// Whether or not Shields "aggressive" mode was enabled where the request was
// initiated.
bool aggressive_blocking = false;
bool allow_http_upgradable_resource = false;
bool allow_referrers = false;
bool is_webtorrent_disabled = false;
Expand Down
5 changes: 5 additions & 0 deletions chromium_src/chrome/browser/about_flags.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ using brave_shields::features::kBraveAdblockCollapseBlockedElements;
using brave_shields::features::kBraveAdblockCosmeticFiltering;
using brave_shields::features::kBraveAdblockCosmeticFilteringNative;
using brave_shields::features::kBraveAdblockCspRules;
using brave_shields::features::kBraveAdblockDefault1pBlocking;
using brave_shields::features::kBraveDarkModeBlock;
using brave_shields::features::kBraveDomainBlock;
using brave_shields::features::kBraveExtensionNetworkBlocking;
Expand Down Expand Up @@ -195,6 +196,10 @@ using ntp_background_images::features::kBraveNTPSuperReferralWallpaper;
flag_descriptions::kBraveAdblockCspRulesName, \
flag_descriptions::kBraveAdblockCspRulesDescription, kOsAll, \
FEATURE_VALUE_TYPE(kBraveAdblockCspRules)}, \
{"brave-adblock-default-1p-blocking", \
flag_descriptions::kBraveAdblockDefault1pBlockingName, \
flag_descriptions::kBraveAdblockDefault1pBlockingDescription, kOsAll, \
FEATURE_VALUE_TYPE(kBraveAdblockDefault1pBlocking)}, \
{"brave-domain-block", \
flag_descriptions::kBraveDomainBlockName, \
flag_descriptions::kBraveDomainBlockDescription, kOsAll, \
Expand Down
5 changes: 5 additions & 0 deletions chromium_src/chrome/browser/flag_descriptions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ const char kBraveNTPBrandedWallpaperDemoDescription[] =
"Force dummy data for the Branded Wallpaper New Tab Page Experience. "
"View rate and user opt-in conditionals will still be followed to decide "
"when to display the Branded Wallpaper.";
const char kBraveAdblockDefault1pBlockingName[] =
"Shields first-party network blocking";
const char kBraveAdblockDefault1pBlockingDescription[] =
"Allow Brave Shields to block first-party network requests in Standard "
"blocking mode";
const char kBraveAdblockCnameUncloakingName[] = "Enable CNAME uncloaking";
const char kBraveAdblockCnameUncloakingDescription[] =
"Take DNS CNAME records into account when making network request blocking "
Expand Down
2 changes: 2 additions & 0 deletions chromium_src/chrome/browser/flag_descriptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ extern const char kBraveNTPBrandedWallpaperName[];
extern const char kBraveNTPBrandedWallpaperDescription[];
extern const char kBraveNTPBrandedWallpaperDemoName[];
extern const char kBraveNTPBrandedWallpaperDemoDescription[];
extern const char kBraveAdblockDefault1pBlockingName[];
extern const char kBraveAdblockDefault1pBlockingDescription[];
extern const char kBraveAdblockCnameUncloakingName[];
extern const char kBraveAdblockCnameUncloakingDescription[];
extern const char kBraveAdblockCollapseBlockedElementsName[];
Expand Down
1 change: 1 addition & 0 deletions components/brave_shields/browser/ad_block_base_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ void AdBlockBaseService::ShouldStartRequest(
const GURL& url,
blink::mojom::ResourceType resource_type,
const std::string& tab_host,
bool aggressive_blocking,
bool* did_match_rule,
bool* did_match_exception,
bool* did_match_important,
Expand Down
1 change: 1 addition & 0 deletions components/brave_shields/browser/ad_block_base_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ class AdBlockBaseService : public BaseBraveShieldsService {
void ShouldStartRequest(const GURL& url,
blink::mojom::ResourceType resource_type,
const std::string& tab_host,
bool aggressive_blocking,
bool* did_match_rule,
bool* did_match_exception,
bool* did_match_important,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ void AdBlockRegionalServiceManager::ShouldStartRequest(
const GURL& url,
blink::mojom::ResourceType resource_type,
const std::string& tab_host,
bool aggressive_blocking,
bool* did_match_rule,
bool* did_match_exception,
bool* did_match_important,
Expand All @@ -127,8 +128,8 @@ void AdBlockRegionalServiceManager::ShouldStartRequest(

for (const auto& regional_service : regional_services_) {
regional_service.second->ShouldStartRequest(
url, resource_type, tab_host, did_match_rule, did_match_exception,
did_match_important, mock_data_url);
url, resource_type, tab_host, aggressive_blocking, did_match_rule,
did_match_exception, did_match_important, mock_data_url);
if (did_match_important && *did_match_important) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ class AdBlockRegionalServiceManager {
void ShouldStartRequest(const GURL& url,
blink::mojom::ResourceType resource_type,
const std::string& tab_host,
bool aggressive_blocking,
bool* did_match_rule,
bool* did_match_exception,
bool* did_match_important,
Expand Down
30 changes: 19 additions & 11 deletions components/brave_shields/browser/ad_block_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "components/prefs/pref_registry_simple.h"
#include "components/prefs/pref_service.h"
#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
#include "url/origin.h"

#define DAT_FILE "rs-ABPFilterParserData.dat"
#define REGIONAL_CATALOG "regional_catalog.json"
Expand All @@ -44,8 +45,7 @@ void AdBlockServiceDomainResolver(const char* host,
uint32_t* end) {
const auto host_str = std::string(host);
const auto domain = net::registry_controlled_domains::GetDomainAndRegistry(
host_str,
net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES);
host_str, net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES);
const size_t match = host_str.rfind(domain);
if (match != std::string::npos) {
*start = match;
Expand All @@ -66,27 +66,35 @@ void AdBlockService::ShouldStartRequest(
const GURL& url,
blink::mojom::ResourceType resource_type,
const std::string& tab_host,
bool aggressive_blocking,
bool* did_match_rule,
bool* did_match_exception,
bool* did_match_important,
std::string* mock_data_url) {
AdBlockBaseService::ShouldStartRequest(
url, resource_type, tab_host, did_match_rule, did_match_exception,
did_match_important, mock_data_url);
if (did_match_important && *did_match_important) {
return;
if (aggressive_blocking ||
base::FeatureList::IsEnabled(
brave_shields::features::kBraveAdblockDefault1pBlocking) ||
!SameDomainOrHost(
url, url::Origin::CreateFromNormalizedTuple("https", tab_host, 80),
net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES)) {
AdBlockBaseService::ShouldStartRequest(
url, resource_type, tab_host, aggressive_blocking, did_match_rule,
did_match_exception, did_match_important, mock_data_url);
if (did_match_important && *did_match_important) {
return;
}
}

regional_service_manager()->ShouldStartRequest(
url, resource_type, tab_host, did_match_rule, did_match_exception,
did_match_important, mock_data_url);
url, resource_type, tab_host, aggressive_blocking, did_match_rule,
did_match_exception, did_match_important, mock_data_url);
if (did_match_important && *did_match_important) {
return;
}

custom_filters_service()->ShouldStartRequest(
url, resource_type, tab_host, did_match_rule, did_match_exception,
did_match_important, mock_data_url);
url, resource_type, tab_host, aggressive_blocking, did_match_rule,
did_match_exception, did_match_important, mock_data_url);
}

absl::optional<std::string> AdBlockService::GetCspDirectives(
Expand Down
1 change: 1 addition & 0 deletions components/brave_shields/browser/ad_block_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ class AdBlockService : public AdBlockBaseService {
void ShouldStartRequest(const GURL& url,
blink::mojom::ResourceType resource_type,
const std::string& tab_host,
bool aggressive_blocking,
bool* did_match_rule,
bool* did_match_exception,
bool* did_match_important,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ void BaseBraveShieldsService::ShouldStartRequest(
const GURL& url,
blink::mojom::ResourceType resource_type,
const std::string& tab_host,
bool aggressive_blocking,
bool* did_match_rule,
bool* did_match_exception,
bool* did_match_important,
std::string* mock_data_url) {
}
std::string* mock_data_url) {}

} // namespace brave_shields
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class BaseBraveShieldsService : public BraveComponent {
virtual void ShouldStartRequest(const GURL& url,
blink::mojom::ResourceType resource_type,
const std::string& tab_host,
bool aggressive_blocking,
bool* did_match_rule,
bool* did_match_exception,
bool* did_match_important,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,14 @@ bool ShouldBlockDomainOnTaskRunner(
bool did_match_rule = false;
bool did_match_important = false;
std::string mock_data_url;
// 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;
ad_block_service->ShouldStartRequest(
url, blink::mojom::ResourceType::kMainFrame, url.host(), &did_match_rule,
&did_match_exception, &did_match_important, &mock_data_url);
url, blink::mojom::ResourceType::kMainFrame, url.host(),
aggressive_blocking, &did_match_rule, &did_match_exception,
&did_match_important, &mock_data_url);
return (did_match_important || (did_match_rule && !did_match_exception));
}

Expand Down
6 changes: 6 additions & 0 deletions components/brave_shields/common/features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@
namespace brave_shields {
namespace features {

// When enabled, Brave will block first-party requests that appear in a filter
// list when Shields is in "standard" blocking mode. When disabled, Brave will
// allow first-party requests in "standard" blocking mode regardless of whether
// or not they appear in a filter list.
const base::Feature kBraveAdblockDefault1pBlocking{
"BraveAdblockDefault1pBlocking", base::FEATURE_ENABLED_BY_DEFAULT};
// When enabled, Brave will issue DNS queries for requests that the adblock
// engine has not blocked, then check them again with the original hostname
// substituted for any canonical name found.
Expand Down
1 change: 1 addition & 0 deletions components/brave_shields/common/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ struct Feature;

namespace brave_shields {
namespace features {
extern const base::Feature kBraveAdblockDefault1pBlocking;
extern const base::Feature kBraveAdblockCnameUncloaking;
extern const base::Feature kBraveAdblockCollapseBlockedElements;
extern const base::Feature kBraveAdblockCosmeticFiltering;
Expand Down

0 comments on commit a9f8e12

Please sign in to comment.