1649872 - filter_expression for Plugin / Gfx / Extension v2 blocklists lacks environment with appinfo

Status: RESOLVED FIXED

(Firefox :: Remote Settings Client, defect, P3)

Description

[Rob Wu [:robwu]]

The environment for blocklist collections was already lacking most of the environment, but with the change from https://hg.mozilla.org/mozilla-central/rev/728cb5c13241586325e977315535cc6bec64a6b3 , the full environment is missing (which is the opposite of what the associated bug wanted to achieve).

While it's easy to fix the bug by adding the environment parameter to the _filterItem functions in Blocklist.jsm, I think that it is preferable to fully handle the filter_expression implementation in the RemoteSettings client itself. It is not obvious that the use of a custom filterFunc results in the loss of filter_expression functionality.

The documentation does state that the default implementation filters based on the JEXL expression, but doesn't clearly state that the override ought to handle the replacement, nor how the default filter_expression functionality has to be restored: https://searchfox.org/mozilla-central/rev/31d8600b73dc85b4cdbabf45ac3f1a9c11700d8e/services/common/docs/RemoteSettings.rst#321-330

The Blocklist v3 (Extension blocklist) doesn't override the filterFunc method, so only the others blocklist implementations in Blocklist.jsm that do override it are affected (i.e. gfx, plugins, blocklist v2). If we ever want to remove versionRange from the blocklist impl (bug 1463377), then this bug should be fixed (and be uplifted to ESR78).

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:leplatrem, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 2

[Mathieu Leplatre [:leplatrem]]

Attachment: Bug 1649872 - Fix environment in blocklists filter expressions r?robwu

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fa0ad42a026f
Fix environment in blocklists filter expressions r=robwu

Comment 4

[Mathieu Leplatre [:leplatrem]]

Comment on attachment 9164077 [details]
Bug 1649872 - Fix environment in blocklists filter expressions r?robwu

Beta/Release Uplift Approval Request

• User impact if declined: The targeting features for blocklist entries will be limited to version ranges instead of the full environment info. That means blocklist authors won't be able to leverage JEXL expressions to target nightly/beta/release/esr homogeneously.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The nature and scope of the fix is restricted to the blocklist target function
• String changes made/needed:

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:leplatrem, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 6

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/fa0ad42a026f

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Do we need this on ESR78 also?

Comment 8

[Rob Wu [:robwu]]

Comment on attachment 9164077 [details]
Bug 1649872 - Fix environment in blocklists filter expressions r?robwu

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This patch fixes a bug that prevents us from replacing the old versionRange targeting method with Remote Setting's JEXL filter expressions.
• User impact if declined: Blocklist (plugins, gfx, blocklist v2) don't have the full environment information available, so the RemoteSettings server will be unable to target specific classes of clients. Without the fix, we cannot migrate away from blocklist-specific versionRange to the more general JEXL filter expressions (bug 1463377).
• Fix Landed on Version: 80 with pending uplift to 79
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The nature and scope of the fix is restricted to the blocklist target function.
• String or UUID changes made by this patch:

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9164077 [details]
Bug 1649872 - Fix environment in blocklists filter expressions r?robwu

Approved for 79.0rc1 and 78.1esr.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/99b9af3ea508

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/a732eb418781