Commit ec0871e3 authored by Karandeep Bhatia's avatar Karandeep Bhatia Committed by Commit Bot

DNR: Allow tests to override rule limits.

Tests exercising the DNR rule limits can currently time out since they
may index up to 30K rules. Prevent this by allowing tests to override
the rule limits.

BUG=1092777, 1092776

Change-Id: I4126563a4f6c0e299470491232cd5c8798a6342e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2244000Reviewed-by: default avatarKelvin Jiang <kelvinjiang@chromium.org>
Commit-Queue: Kelvin Jiang <kelvinjiang@chromium.org>
Auto-Submit: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#780112}
parent 23801a6a
......@@ -38,6 +38,7 @@
#include "extensions/browser/api/declarative_net_request/ruleset_manager.h"
#include "extensions/browser/api/declarative_net_request/ruleset_matcher.h"
#include "extensions/browser/api/declarative_net_request/test_utils.h"
#include "extensions/browser/api/declarative_net_request/utils.h"
#include "extensions/browser/api_test_utils.h"
#include "extensions/browser/disable_reason.h"
#include "extensions/browser/test_extension_registry_observer.h"
......@@ -335,9 +336,9 @@ class SingleRulesetTest : public DeclarativeNetRequestUnittest {
else
rules_count = rules_list_.size();
// We only index up to dnr_api::MAX_NUMBER_OF_RULES rules per ruleset.
rules_count = std::min(rules_count,
static_cast<size_t>(dnr_api::MAX_NUMBER_OF_RULES));
// We only index up to GetStaticRuleLimit() rules per ruleset.
rules_count =
std::min(rules_count, static_cast<size_t>(GetStaticRuleLimit()));
DeclarativeNetRequestUnittest::LoadAndExpectSuccess(rules_count,
rules_count, true);
......@@ -652,10 +653,14 @@ TEST_P(SingleRulesetTest, InvalidJSONRules_Parsed) {
}
}
// Ensure that we can add up to MAX_NUMBER_OF_RULES.
// Ensure that we can add up to GetStaticRuleLimit() rules.
TEST_P(SingleRulesetTest, RuleCountLimitMatched) {
// Override the API rule limit to prevent a timeout on loading the extension.
base::AutoReset<int> rule_limit_override =
CreateScopedStaticRuleLimitOverrideForTesting(100);
TestRule rule = CreateGenericRule();
for (int i = 0; i < dnr_api::MAX_NUMBER_OF_RULES; ++i) {
for (int i = 0; i < GetStaticRuleLimit(); ++i) {
rule.id = kMinValidID + i;
rule.condition->url_filter = std::to_string(i);
AddRule(rule);
......@@ -665,8 +670,12 @@ TEST_P(SingleRulesetTest, RuleCountLimitMatched) {
// Ensure that we get an install warning on exceeding the rule count limit.
TEST_P(SingleRulesetTest, RuleCountLimitExceeded) {
// Override the API rule limit to prevent a timeout on loading the extension.
base::AutoReset<int> rule_limit_override =
CreateScopedStaticRuleLimitOverrideForTesting(100);
TestRule rule = CreateGenericRule();
for (int i = 1; i <= dnr_api::MAX_NUMBER_OF_RULES + 1; ++i) {
for (int i = 1; i <= GetStaticRuleLimit() + 1; ++i) {
rule.id = kMinValidID + i;
rule.condition->url_filter = std::to_string(i);
AddRule(rule);
......@@ -749,10 +758,14 @@ TEST_P(SingleRulesetTest, WarningAndError) {
// Ensure that we get an install warning on exceeding the regex rule count
// limit.
TEST_P(SingleRulesetTest, RegexRuleCountExceeded) {
// Override the API rule limit to prevent a timeout on loading the extension.
base::AutoReset<int> rule_limit_override =
CreateScopedRegexRuleLimitOverrideForTesting(100);
TestRule regex_rule = CreateGenericRule();
regex_rule.condition->url_filter.reset();
int rule_id = kMinValidID;
for (int i = 1; i <= dnr_api::MAX_NUMBER_OF_REGEX_RULES + 5; ++i, ++rule_id) {
for (int i = 1; i <= GetRegexRuleLimit() + 5; ++i, ++rule_id) {
regex_rule.id = rule_id;
regex_rule.condition->regex_filter = std::to_string(i);
AddRule(regex_rule);
......@@ -767,8 +780,7 @@ TEST_P(SingleRulesetTest, RegexRuleCountExceeded) {
}
extension_loader()->set_ignore_manifest_warnings(true);
LoadAndExpectSuccess(dnr_api::MAX_NUMBER_OF_REGEX_RULES +
kCountNonRegexRules);
LoadAndExpectSuccess(GetRegexRuleLimit() + kCountNonRegexRules);
// TODO(crbug.com/879355): CrxInstaller reloads the extension after moving it,
// which causes it to lose the install warning. This should be fixed.
if (GetParam() != ExtensionLoadType::PACKED) {
......@@ -952,10 +964,9 @@ class MultipleRulesetsTest : public DeclarativeNetRequestUnittest {
for (const TestRulesetInfo& info : rulesets_) {
size_t count = info.rules_value.GetList().size();
// We only index up to dnr_api::MAX_NUMBER_OF_RULES per ruleset, but may
// We only index up to GetStaticRuleLimit() rules per ruleset, but may
// index more rules than this limit across rulesets.
count =
std::min(count, static_cast<size_t>(dnr_api::MAX_NUMBER_OF_RULES));
count = std::min(count, static_cast<size_t>(GetStaticRuleLimit()));
rules_count += count;
if (info.enabled)
......@@ -1017,6 +1028,12 @@ TEST_P(MultipleRulesetsTest, ListNotPassed) {
// Tests an extension with multiple static rulesets with each ruleset generating
// some install warnings.
TEST_P(MultipleRulesetsTest, InstallWarnings) {
// Override the API rule limit to prevent a timeout on loading the extension.
base::AutoReset<int> rule_limit_override =
CreateScopedStaticRuleLimitOverrideForTesting(100);
base::AutoReset<int> regex_rule_limit_override =
CreateScopedRegexRuleLimitOverrideForTesting(60);
size_t expected_rule_count = 0;
size_t enabled_rule_count = 0;
std::vector<std::string> expected_warnings;
......@@ -1045,29 +1062,27 @@ TEST_P(MultipleRulesetsTest, InstallWarnings) {
{
// Persist a ruleset with an install warning for exceeding the rule count.
TestRulesetInfo info =
CreateRuleset(kId2, dnr_api::MAX_NUMBER_OF_RULES + 1, 0, false);
CreateRuleset(kId2, GetStaticRuleLimit() + 1, 0, false);
AddRuleset(info);
expected_warnings.push_back(
GetErrorWithFilename(kRuleCountExceeded, info.relative_file_path));
expected_rule_count += dnr_api::MAX_NUMBER_OF_RULES;
expected_rule_count += GetStaticRuleLimit();
}
{
// Persist a ruleset with an install warning for exceeding the regex rule
// count.
size_t kCountNonRegexRules = 5;
TestRulesetInfo info =
CreateRuleset(kId3, kCountNonRegexRules,
dnr_api::MAX_NUMBER_OF_REGEX_RULES + 1, false);
TestRulesetInfo info = CreateRuleset(kId3, kCountNonRegexRules,
GetRegexRuleLimit() + 1, false);
AddRuleset(info);
expected_warnings.push_back(
GetErrorWithFilename(kRegexRuleCountExceeded, info.relative_file_path));
expected_rule_count +=
kCountNonRegexRules + dnr_api::MAX_NUMBER_OF_REGEX_RULES;
expected_rule_count += kCountNonRegexRules + GetRegexRuleLimit();
}
extension_loader()->set_ignore_manifest_warnings(true);
......@@ -1111,12 +1126,16 @@ TEST_P(MultipleRulesetsTest, EnabledRulesCount) {
// Ensure that exceeding the rules count limit across rulesets raises an install
// warning.
TEST_P(MultipleRulesetsTest, StaticRuleCountExceeded) {
// Override the API rule limit to prevent a timeout on loading the extension.
base::AutoReset<int> rule_limit_override =
CreateScopedStaticRuleLimitOverrideForTesting(50);
// Enabled on load.
AddRuleset(CreateRuleset(kId1, 10, 0, true));
// Disabled by default.
AddRuleset(CreateRuleset(kId2, 20, 0, false));
// Not enabled on load since including it exceeds the static rules count.
AddRuleset(CreateRuleset(kId3, dnr_api::MAX_NUMBER_OF_RULES + 10, 0, true));
AddRuleset(CreateRuleset(kId3, GetStaticRuleLimit() + 10, 0, true));
// Enabled on load.
AddRuleset(CreateRuleset(kId4, 30, 0, true));
......@@ -1126,8 +1145,6 @@ TEST_P(MultipleRulesetsTest, StaticRuleCountExceeded) {
{
// To prevent timeouts in debug builds, increase the wait timeout to the
// test launcher's timeout. See crbug.com/1071403.
// TODO(karandeepb): Provide a way to fake dnr_api::MAX_NUMBER_OF_RULES in
// tests to decrease test runtime.
base::test::ScopedRunLoopTimeout specific_timeout(
FROM_HERE, TestTimeouts::test_launcher_timeout());
LoadAndExpectSuccess();
......@@ -1167,7 +1184,7 @@ TEST_P(MultipleRulesetsTest, RegexRuleCountExceeded) {
AddRuleset(CreateRuleset(kId1, 10000, 100, true));
// Won't be enabled on load since including it will exceed the regex rule
// count.
AddRuleset(CreateRuleset(kId2, 1, dnr_api::MAX_NUMBER_OF_REGEX_RULES, true));
AddRuleset(CreateRuleset(kId2, 1, GetRegexRuleLimit(), true));
// Won't be enabled on load since it is disabled by default.
AddRuleset(CreateRuleset(kId3, 10, 10, false));
// Enabled on load.
......@@ -1225,8 +1242,12 @@ TEST_P(MultipleRulesetsTest, UpdateEnabledRulesets_InvalidRulesetID) {
}
TEST_P(MultipleRulesetsTest, UpdateEnabledRulesets_RuleCountExceeded) {
// Override the API rule limit to prevent a timeout on loading the extension.
base::AutoReset<int> rule_limit_override =
CreateScopedStaticRuleLimitOverrideForTesting(100);
AddRuleset(CreateRuleset(kId1, 10, 10, true));
AddRuleset(CreateRuleset(kId2, dnr_api::MAX_NUMBER_OF_RULES, 0, false));
AddRuleset(CreateRuleset(kId2, GetStaticRuleLimit(), 0, false));
RulesetManagerObserver ruleset_waiter(manager());
LoadAndExpectSuccess();
......@@ -1245,7 +1266,7 @@ TEST_P(MultipleRulesetsTest, UpdateEnabledRulesets_RuleCountExceeded) {
TEST_P(MultipleRulesetsTest, UpdateEnabledRulesets_RegexRuleCountExceeded) {
AddRuleset(CreateRuleset(kId1, 0, 10, false));
AddRuleset(CreateRuleset(kId2, 0, dnr_api::MAX_NUMBER_OF_REGEX_RULES, true));
AddRuleset(CreateRuleset(kId2, 0, GetRegexRuleLimit(), true));
RulesetManagerObserver ruleset_waiter(manager());
LoadAndExpectSuccess();
......
......@@ -205,7 +205,7 @@ bool GetNewDynamicRules(const RulesetSource& source,
int regex_rule_count = std::count_if(
new_rules->begin(), new_rules->end(),
[](const dnr_api::Rule& rule) { return !!rule.condition.regex_filter; });
if (regex_rule_count > dnr_api::MAX_NUMBER_OF_REGEX_RULES) {
if (regex_rule_count > GetRegexRuleLimit()) {
*status = UpdateDynamicRulesStatus::kErrorRegexRuleCountExceeded;
*error = kDynamicRegexRuleCountExceeded;
return false;
......
......@@ -11,6 +11,7 @@
#include "base/metrics/histogram_macros.h"
#include "base/time/time.h"
#include "extensions/browser/api/declarative_net_request/constants.h"
#include "extensions/browser/api/declarative_net_request/utils.h"
#include "extensions/common/api/declarative_net_request.h"
#include "extensions/common/extension.h"
#include "extensions/common/manifest.h"
......@@ -45,7 +46,7 @@ IndexHelper::Result CombineResults(
// Per-ruleset limits should have been enforced during ruleset indexing.
DCHECK_LE(index_result.regex_rules_count,
static_cast<size_t>(dnr_api::MAX_NUMBER_OF_REGEX_RULES));
static_cast<size_t>(GetRegexRuleLimit()));
DCHECK_LE(index_result.rules_count, source->rule_count_limit());
if (!index_result.success) {
......@@ -72,12 +73,12 @@ IndexHelper::Result CombineResults(
// Raise an install warning if the enabled rule count exceeds the API limits.
// We don't raise a hard error to maintain forwards compatibility.
if (enabled_rules_count > static_cast<size_t>(dnr_api::MAX_NUMBER_OF_RULES)) {
if (enabled_rules_count > static_cast<size_t>(GetStaticRuleLimit())) {
total_result.warnings.emplace_back(
kEnabledRuleCountExceeded, manifest_keys::kDeclarativeNetRequestKey,
manifest_keys::kDeclarativeRuleResourcesKey);
} else if (enabled_regex_rules_count >
static_cast<size_t>(dnr_api::MAX_NUMBER_OF_REGEX_RULES)) {
static_cast<size_t>(GetRegexRuleLimit())) {
total_result.warnings.emplace_back(
kEnabledRegexRuleCountExceeded,
manifest_keys::kDeclarativeNetRequestKey,
......
......@@ -441,7 +441,7 @@ void RulesMonitorService::OnInitialRulesetsLoaded(LoadRequestData load_data) {
// Per-ruleset limits should have been enforced during
// indexing/installation.
DCHECK_LE(matcher->GetRegexRulesCount(),
static_cast<size_t>(dnr_api::MAX_NUMBER_OF_REGEX_RULES));
static_cast<size_t>(GetRegexRuleLimit()));
DCHECK_LE(matcher->GetRulesCount(), ruleset.source().rule_count_limit());
if (ruleset.source().is_dynamic_ruleset()) {
......@@ -450,13 +450,12 @@ void RulesMonitorService::OnInitialRulesetsLoaded(LoadRequestData load_data) {
}
size_t new_rules_count = static_rules_count + matcher->GetRulesCount();
if (new_rules_count > static_cast<size_t>(dnr_api::MAX_NUMBER_OF_RULES))
if (new_rules_count > static_cast<size_t>(GetStaticRuleLimit()))
continue;
size_t new_regex_rules_count =
static_regex_rules_count + matcher->GetRegexRulesCount();
if (new_regex_rules_count >
static_cast<size_t>(dnr_api::MAX_NUMBER_OF_REGEX_RULES)) {
if (new_regex_rules_count > static_cast<size_t>(GetRegexRuleLimit())) {
continue;
}
......@@ -532,7 +531,7 @@ void RulesMonitorService::OnNewStaticRulesetsLoaded(
// Per-ruleset limits should have been enforced during
// indexing/installation.
DCHECK_LE(matcher->GetRegexRulesCount(),
static_cast<size_t>(dnr_api::MAX_NUMBER_OF_REGEX_RULES));
static_cast<size_t>(GetRegexRuleLimit()));
DCHECK_LE(matcher->GetRulesCount(), ruleset.source().rule_count_limit());
static_rules_count += matcher->GetRulesCount();
......@@ -540,13 +539,12 @@ void RulesMonitorService::OnNewStaticRulesetsLoaded(
new_matchers.push_back(std::move(matcher));
}
if (static_rules_count > static_cast<size_t>(dnr_api::MAX_NUMBER_OF_RULES)) {
if (static_rules_count > static_cast<size_t>(GetStaticRuleLimit())) {
std::move(callback).Run(kEnabledRulesetsRuleCountExceeded);
return;
}
if (static_regex_rules_count >
static_cast<size_t>(dnr_api::MAX_NUMBER_OF_REGEX_RULES)) {
if (static_regex_rules_count > static_cast<size_t>(GetRegexRuleLimit())) {
std::move(callback).Run(kEnabledRulesetsRegexRuleCountExceeded);
return;
}
......
......@@ -142,8 +142,7 @@ ReadJSONRulesResult ParseRulesFromJSON(const base::FilePath& json_path,
}
const bool is_regex_rule = !!parsed_rule.condition.regex_filter;
if (is_regex_rule &&
++regex_rule_count > dnr_api::MAX_NUMBER_OF_REGEX_RULES) {
if (is_regex_rule && ++regex_rule_count > GetRegexRuleLimit()) {
// Only add the install warning once.
if (!regex_rule_count_exceeded) {
regex_rule_count_exceeded = true;
......@@ -317,7 +316,7 @@ RulesetSource RulesetSource::CreateStatic(
extension.path().Append(info.relative_path),
extension.path().Append(
file_util::GetIndexedRulesetRelativePath(info.id.value())),
info.id, dnr_api::MAX_NUMBER_OF_RULES, extension.id(), info.enabled);
info.id, GetStaticRuleLimit(), extension.id(), info.enabled);
}
// static
......@@ -330,7 +329,7 @@ RulesetSource RulesetSource::CreateDynamic(content::BrowserContext* context,
return RulesetSource(
dynamic_ruleset_directory.AppendASCII(kDynamicRulesJSONFilename),
dynamic_ruleset_directory.AppendASCII(kDynamicIndexedRulesFilename),
kDynamicRulesetID, dnr_api::MAX_NUMBER_OF_DYNAMIC_RULES, extension_id,
kDynamicRulesetID, GetDynamicRuleLimit(), extension_id,
true /* enabled_by_default */);
}
......
......@@ -49,13 +49,16 @@ static_assert(url_pattern_index::kUrlPatternIndexFormatVersion == 6,
"also updated kIndexedRulesetFormatVersion above.");
constexpr int kInvalidIndexedRulesetFormatVersion = -1;
int g_indexed_ruleset_format_version_for_testing =
kInvalidIndexedRulesetFormatVersion;
constexpr int kInvalidOverrideChecksumForTest = -1;
int g_override_checksum_for_test = kInvalidOverrideChecksumForTest;
constexpr int kInvalidRuleLimit = -1;
int g_static_rule_limit_for_testing = kInvalidRuleLimit;
int g_regex_rule_limit_for_testing = kInvalidRuleLimit;
int GetIndexedRulesetFormatVersion() {
return g_indexed_ruleset_format_version_for_testing ==
kInvalidIndexedRulesetFormatVersion
......@@ -287,5 +290,31 @@ std::vector<std::string> GetPublicRulesetIDs(const Extension& extension,
return ids;
}
int GetStaticRuleLimit() {
return g_static_rule_limit_for_testing == kInvalidRuleLimit
? dnr_api::MAX_NUMBER_OF_RULES
: g_static_rule_limit_for_testing;
}
int GetDynamicRuleLimit() {
return dnr_api::MAX_NUMBER_OF_DYNAMIC_RULES;
}
int GetRegexRuleLimit() {
return g_regex_rule_limit_for_testing == kInvalidRuleLimit
? dnr_api::MAX_NUMBER_OF_REGEX_RULES
: g_regex_rule_limit_for_testing;
}
ScopedRuleLimitOverride CreateScopedStaticRuleLimitOverrideForTesting(
int limit) {
return base::AutoReset<int>(&g_static_rule_limit_for_testing, limit);
}
ScopedRuleLimitOverride CreateScopedRegexRuleLimitOverrideForTesting(
int limit) {
return base::AutoReset<int>(&g_regex_rule_limit_for_testing, limit);
}
} // namespace declarative_net_request
} // namespace extensions
......@@ -99,6 +99,23 @@ std::string GetPublicRulesetID(const Extension& extension,
std::vector<std::string> GetPublicRulesetIDs(const Extension& extension,
const CompositeMatcher& matcher);
// Returns the per-extension static rule limit.
int GetStaticRuleLimit();
// Returns the per-extension dynamic rule limit.
int GetDynamicRuleLimit();
// Returns the per-extension regex rules limit. This is enforced separately for
// static and dynamic rulesets.
int GetRegexRuleLimit();
// Test helpers to override the various rule limits until the returned value is
// in scope.
using ScopedRuleLimitOverride = base::AutoReset<int>;
ScopedRuleLimitOverride CreateScopedStaticRuleLimitOverrideForTesting(
int limit);
ScopedRuleLimitOverride CreateScopedRegexRuleLimitOverrideForTesting(int limit);
// Helper to convert a flatbufffers::String to a string-like object with type T.
template <typename T>
T CreateString(const flatbuffers::String& str) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment