Skip to content

Commit

Permalink
Uplift of #14031 (squashed) to beta
Browse files Browse the repository at this point in the history
  • Loading branch information
brave-browser-releases committed Jul 7, 2022
1 parent 1328f79 commit bcb0525
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 4 deletions.
24 changes: 22 additions & 2 deletions components/brave_shields/browser/brave_shields_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@

#include <memory>

#include "base/debug/dump_without_crashing.h"
#include "base/feature_list.h"
#include "base/logging.h"
#include "base/notreached.h"
#include "base/strings/string_number_conversions.h"
#include "brave/components/brave_shields/browser/brave_shields_p3a.h"
Expand Down Expand Up @@ -181,12 +183,30 @@ void SetBraveShieldsEnabled(HostContentSettingsMap* map,
if (url.is_valid() && !url.SchemeIsHTTPOrHTTPS())
return;

DCHECK(!url.is_empty()) << "url for shields setting cannot be blank";
if (url.is_empty()) {
LOG(ERROR) << "url for shields setting cannot be blank";
return;
}

auto primary_pattern = GetPatternFromURL(url);

if (!primary_pattern.IsValid())
if (primary_pattern.MatchesAllHosts()) {
LOG(ERROR) << "Url for shields setting cannot be blank or result in a "
"wildcard content setting.";

#if DCHECK_IS_ON()
DCHECK(false);
#else
base::debug::DumpWithoutCrashing();
#endif
return;
}

if (!primary_pattern.IsValid()) {
DLOG(ERROR) << "Invalid primary pattern for Url: "
<< url.possibly_invalid_spec();
return;
}

map->SetContentSettingCustomScope(
primary_pattern, ContentSettingsPattern::Wildcard(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,10 @@ void BravePrefProvider::MigrateShieldsSettings(bool incognito) {
*shields_cookies);
}

// Fix any wildcard entries that could cause issues like
// https://github.com/brave/brave-browser/issues/23113
EnsureNoWildcardEntries();

// Prior to Chromium 88, we used the "plugins" ContentSettingsType along with
// ResourceIdentifiers to store our settings, which we need to migrate now
// first of all, before attempting any other migration.
Expand All @@ -168,6 +172,17 @@ void BravePrefProvider::MigrateShieldsSettings(bool incognito) {
MigrateShieldsSettingsV2ToV3();
}

void BravePrefProvider::EnsureNoWildcardEntries() {
// ContentSettingsType::BRAVE_SHIELDS should not have wildcard entries, i.e.
// there is no global disabled value.
// TODO(petemill): This should also be done for the other shields
// content settings types, and we can use default boolean prefs to represent
// defaults, e.g. `profile.default_content_setting_values.https_everywhere`.
SetWebsiteSetting(ContentSettingsPattern::Wildcard(),
ContentSettingsPattern::Wildcard(),
ContentSettingsType::BRAVE_SHIELDS, base::Value(), {});
}

void BravePrefProvider::MigrateShieldsSettingsFromResourceIds() {
BravePrefProvider::CopyPluginSettingsForMigration(prefs_);

Expand Down Expand Up @@ -552,8 +567,12 @@ void BravePrefProvider::UpdateCookieRules(ContentSettingsType content_type,

// Adding shields down rules (they always override cookie rules).
for (const auto& shield_rule : shield_rules) {
// There is no global shields rule
DCHECK(!shield_rule.primary_pattern.MatchesAllHosts());
// There is no global shields rule, so if we have one ignore it. It would
// get replaced with EnsureNoWildcardEntries().
if (shield_rule.primary_pattern.MatchesAllHosts()) {
LOG(ERROR) << "Found a wildcard shields rule which matches all hosts.";
continue;
}

// Shields down.
if (ValueToContentSetting(shield_rule.value) == CONTENT_SETTING_BLOCK) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@ class BravePrefProvider : public PrefProvider,
TestShieldsSettingsMigrationFromResourceIDs);
FRIEND_TEST_ALL_PREFIXES(BravePrefProviderTest,
TestShieldsSettingsMigrationFromUnknownSettings);
FRIEND_TEST_ALL_PREFIXES(BravePrefProviderTest, EnsureNoWildcardEntries);
void MigrateShieldsSettings(bool incognito);
void EnsureNoWildcardEntries();
void MigrateShieldsSettingsFromResourceIds();
void MigrateShieldsSettingsFromResourceIdsForOneType(
const std::string& preference_path,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "brave/components/content_settings/core/browser/brave_content_settings_pref_provider.h"
#include "brave/components/content_settings/core/browser/brave_content_settings_utils.h"
#include "chrome/test/base/testing_profile.h"
#include "components/content_settings/core/browser/content_settings_pref.h"
#include "components/content_settings/core/browser/content_settings_registry.h"
#include "components/content_settings/core/common/content_settings.h"
#include "components/content_settings/core/common/content_settings_pattern.h"
Expand Down Expand Up @@ -611,4 +612,25 @@ TEST_F(BravePrefProviderTest, TestShieldsSettingsMigrationV2toV3) {
provider.ShutdownOnUIThread();
}

TEST_F(BravePrefProviderTest, EnsureNoWildcardEntries) {
BravePrefProvider provider(
testing_profile()->GetPrefs(), false /* incognito */,
true /* store_last_modified */, false /* restore_session */);
ShieldsEnabledSetting shields_enabled_settings(&provider);
GURL example_url("https://example.com");
shields_enabled_settings.CheckSettingsAreDefault(example_url);
// Set wildcard entry
auto pattern = ContentSettingsPattern::Wildcard();
provider.SetWebsiteSetting(pattern, pattern,
ContentSettingsType::BRAVE_SHIELDS,
base::Value(CONTENT_SETTING_ALLOW), {});
// Verify global has changed
shields_enabled_settings.CheckSettingsWouldAllow(example_url);
// Remove wildcards
provider.EnsureNoWildcardEntries();
// Verify global has reset
shields_enabled_settings.CheckSettingsAreDefault(example_url);
provider.ShutdownOnUIThread();
}

} // namespace content_settings

0 comments on commit bcb0525

Please sign in to comment.