Closed Bug 1821051 Opened 1 year ago Closed 1 year ago

[DNR] Enforce limits for session rules and regexFilter rules

Categories

(WebExtensions :: Request Handling, task, P2)

task

Tracking

(firefox113 fixed)

RESOLVED FIXED
113 Branch
Tracking Status
firefox113 --- fixed

People

(Reporter: robwu, Assigned: robwu)

References

(Blocks 1 open bug)

Details

(Whiteboard: [addons-jira])

Attachments

(4 files)

The initial implementation of updateSessionRules and regexFilter rules does not have any limits yet.

In bug 1745764, rule limits were added for dynamic rules, and in bug 1745763 for static rules.

But we still need to enforce limits for session rules (and regexp rules).

Notes:

  • When a ruleset were to exceed the regexFilter limit, Chrome refuses to load the proposed ruleset altogether. An alternative is to ignore only the rule that is invalid. Safari does not have any limit on the number of regexFilter rules.
  • While the current implementation of static rules is geared towards loading static rules from disk, we should also enforce the limits also when individual rules are disabled/enabled, at bug 1810762. That requires some refactoring, which may or may not be part of this bug.

For comparison, here is Chrome's logic.

Static rulesets loaded from disk:

  • Within a ruleset, regexFilter rules exceeding MAX_NUMBER_OF_REGEX_RULES are ignored, with a non-fatal warning (source).
  • While iterating over static rulesets to enable, if the inclusion of that ruleset would exceed the quota of static rules or regexFilter, then that ruleset is ignored entirely. (source)

Dynamic rules loaded from disk:

  • Dynamic rules are loaded after static rules (source) with rule limit set to MAX_NUMBER_OF_DYNAMIC_AND_SESSION_RULES (source).
  • Rules over the limit are ignored with a non-fatal warning. Rules up to the limit are included (source).
    • While dynamic rules and static rules are loaded through the same logic, dynamic rules do not count towards the quota of static rules (source).
    • The limit of dynamic rules is fixed, so this situation should not occur in practice. It could occur if the limit changes in a browser update (e.g. bug 1803370).
  • Similarly to static rules, regexFilter rules exceeding MAX_NUMBER_OF_REGEX_RULES are ignored, with a non-fatal warning (source).

updateDynamicRules and updateSessionRules:

  • API call rejected when about to exceed the quota of regexFilter (MAX_NUMBER_OF_REGEX_RULES) or number of rules (MAX_NUMBER_OF_DYNAMIC_AND_SESSION_RULES) (source for dynamic rules, source for session rules).

As a preparation for quota enforcement for session rules and dynamic
rules, this refactors the quota enforcement, by introducing a single
helper (RuleQuotaCounter) that keeps track of the rule counts and
is responsible for enforcing quotas.

This patch is purely a refactor, and the only behavioral changes are the
strings in error messages.

The error messages of static rules were previously not covered by unit
tests; this patch also improves test coverage in that area.

Lastly, removed TODO from the static rule loading. When a static ruleset
were to exceed the quota, the desired behavior is to ignore it and try
the next ruleset (this is consistent with Chromium).

Assignee: nobody → rob
Status: NEW → ASSIGNED

Rulesets are atomic units. It does not select an arbitrary subset as an
"error recovery mechanism". This commits switches to the common quota
verification logic from the previous commit (RuleQuotaCounter).

Severity: -- → N/A
Priority: -- → P2
Pushed by rob@robwu.nl:
https://hg.mozilla.org/integration/autoland/rev/9c145f2d20ea
Refactor rule limit counting r=rpl
https://hg.mozilla.org/integration/autoland/rev/64da164bc06e
Ignore all dynamic rules on quota violation at load r=rpl
https://hg.mozilla.org/integration/autoland/rev/64dca76f9691
Enforce rule limit for session rules r=rpl
https://hg.mozilla.org/integration/autoland/rev/6f43f2ca2f3b
Enforce quota for regexFilter rules r=rpl
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: