Opened on 04/12/2019 at 05:14:28 PM

Closed on 04/15/2019 at 10:46:06 PM

Last modified on 04/16/2019 at 04:08:26 AM

#7462 closed defect (duplicate)

Different results based on filter order

Reported by: greiner Assignee:
Priority: Unknown Milestone:
Module: Core Keywords:
Cc: mjethani, agiammarchi, hfiguiere, sebastian, kzar Blocked By:
Blocking: #6538 Platform: Unknown / Cross platform
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

Description

Environment

Ubuntu 16.04
Chrome 73
Adblock Plus 3.5.1

How to reproduce

  1. Add Snippets (A) to lib/content/snippets.js
  2. Add Custom filters (B) and save
  3. Go to example.com
  4. Remove Custom filters (B) and save (Note: merely replacing them would keep them in the same order)
  5. Add Custom filters (C) and save
  6. Reload example.com

Snippets (A)

function depFirst()
{
  window.foo = true;
}
exports["dep-first"] = makeInjector(depFirst);

function depSecond()
{
  window.foo = window.foo || false;
  window.alert(`Filters are in correct order: ${foo}`);
}
exports["dep-second"] = makeInjector(depSecond);

Custom filters (B)

example.com#$#dep-first
example.com#$#dep-second

Custom filters (C)

example.com#$#dep-second
example.com#$#dep-first

Observed behavior

After 3) Message "Filters are in correct order: true" shown
After 6) Message "Filters are in correct order: false" shown

Expected behavior

After 3) Message "Filters are in correct order: true" shown
After 6) Message "Filters are in correct order: true" shown

Further information

Filters are expected to always cause the same behavior regardless of their position in a filter list or across filter lists.

Attachments (0)

Change History (8)

comment:1 Changed on 04/13/2019 at 02:00:23 PM by mjethani

  • Cc hfiguiere sebastian kzar added

comment:2 follow-up: Changed on 04/13/2019 at 02:03:10 PM by mjethani

This is by design.

Snippet filters, like all content filters (element hiding, etc.), are applied in an order that is some function of the order in which they occur in the subscriptions. In the past the order was random, but after #7178 it is deterministic (but still unspecified). Because of the optimizations on which we are continually working, we cannot guarantee the final order.

Where the order of the snippet invocations matters, they should be part of the same filter. e.g. example.com#$#dep-first; dep-second. The dep-first; dep-second part is the "script," and like any script it is a sequence of commands. Note that this works also for "Type 1" snippets (i.e. snippets that are not defined using the makeInjector() utility).

Filters are expected to always cause the same behavior regardless of their position in a filter list or across filter lists.

This has not been true for a long time if it ever was. e.g. consider two filters ||foo^$csp=script-src 'none' and ||foo^$csp=script-src https:'. Before #7179, only one of the filters would be applied to the document, and which one it would be was unspecified (random in practice); after #7179, both are applied, but the order is still unspecified (deterministic after #7178 but the order could keep changing from release to release).

For request blocking filters and element hiding filters, the order makes no difference except for testing purposes (see #7248). It does make a difference in the case of element hiding emulation filters (if display: none !important is part of the CSS selector). For snippet filters, it may or may not matter, depending on the case, but if the order is important then the best we can do right now is to string related snippet invocations together into a single filter.

If there is a specific case in which the unspecified order turns out to be an issue in practice, it would be nice to know so we can try to address it.

comment:3 Changed on 04/13/2019 at 05:24:34 PM by mjethani

  • Blocking 6538 added

comment:4 in reply to: ↑ 2 Changed on 04/13/2019 at 05:30:21 PM by greiner

Thanks for clarifying this decision.

I'm not trying to question the necessity of their order in applying certain filters but there is currently no expectation of the effect of filters being dependent on their order. Therefore if we indeed want to include the order as a factor in the evaluation of filters, we should clearly state so in our filter documentation as well as make the necessary changes to the UI that allows filter authors to control the filter order and gain insights into how filters will be applied to avoid any unnecessary confusion.

In that regard, we should also revisit the order of subscriptions because I assume that it then also makes a difference in what order subscriptions were added (see also #6856).

comment:5 Changed on 04/14/2019 at 05:36:17 AM by mjethani

As of now, the order matters in any possibly significant way for two kinds of filters: (1) snippet filters, and (2) request blocking filters using the $rewrite option.

In the latter case, we have documented this on /filters as follows:

If both, a filter with/without $rewrite option matches, the behavior is undefined, i.e. the request might either be blocked or redirected.

In the case of snippet filters, because this feature is considered to be "experimental," we have not documented anything anywhere. For filter list authors working on ABP filters, we have either directed them to the specification of the syntax in #6538 or asked them to seek any clarifications about the behavior directly from the developers (although there is a "Snippets FAQ" on an internal wiki).

Now the question is what we should do going forward.

We need to answer the following questions:

  1. For how long is the snippets feature going to be "experimental?"
  2. With or without the snippets feature, if the order of filters makes a difference to what happens in a document, should this order be left unspecified or should it be specified in some way to make it easier for filter list authors to predict how their filters would work?

I do not have the answer to the first question, but I can say that it depends on how successful this feature is in practice (which means it depends on feedback from ABP filters).

About the second question: If we specify the order, not only would we have to implement the specification exactly, it would also constrain us if we want to make any changes. For this we would have to weigh the pros against the cons. For request blocking filters, for example, there are multiple "buckets" if you will: "simple" filters, "type-specific" filters, domain-specific filters, etc., and we go through each of these buckets in a certain order; if we were to document this order and make it part of a specification, would it help filter list authors at all, given the complexity of this? I doubt it, frankly. In that case, we would have to simplify it so it's easier to understand, which in turn would mean giving up on the optimizations we have made.

For snippet filters, they are applied simply in the order in which they occur in the subscriptions, but this is only because there are not too many of these filters (and this is changing!); if we optimize this, then the order would be similar to element hiding filters, which is as follows: on domain foo.example.com, first the filters for foo.example.com in subscription order, then example.com in subscription order, then com in subscription order.

About "subscription order", I should also point out the following: Let's say you have two subscriptions containing two filters each: foo1 and bar1, and foo2 and bar2. In this case the order would be foo1, bar1, foo2, and bar2. If the first subscription is updated so that a new filter lambda1 is added, then the order would be foo1, bar1, foo2, bar2, and lambda1. i.e. even though the first subscription comes before the second, not all of its filters necessarily come before all of the second one's. It depends on the sequence of events. After restarting the extension, the order would be foo1, bar1, lambda1, foo2, and bar2. Therefore, even though the order of the subscriptions is significant, it is not a guarantee.

Therefore if we indeed want to include the order as a factor in the evaluation of filters, we should clearly state so in our filter documentation as well as make the necessary changes to the UI that allows filter authors to control the filter order and gain insights into how filters will be applied to avoid any unnecessary confusion.

I agree it would be nice for filter list authors to be able to know the exact order, but for this we would have to make changes in a number of places, and whether it's worth doing this depends on how much of a real pain point this is for filter list authors. Do you have any insight into this?

One last thing I should add is that we're working on incremental filter list updates (#6833). I am yet to look at the specification, but most likely it's a diff of the old and the new versions of the filter list. If order is important, then we would have to make sure that we take it into account so that the "patch" is applied correctly.

Last edited on 04/14/2019 at 05:38:40 AM by mjethani

comment:6 follow-up: Changed on 04/14/2019 at 05:44:31 AM by mjethani

Since this has come up, I should add that the idea of implementing an $important filter option has been suggested a few times.

For snippet filters, we could do it like so:

  1. If the script begins with a !, then those filters are prioritized (e.g. example.com#$#!snippet-2 always applies before example.com#$#snippet-1)
  2. For scripts at the same level of importance, the order is still unspecified

This is just an idea off the top of my head.

comment:7 in reply to: ↑ 6 Changed on 04/14/2019 at 06:01:49 AM by mjethani

Replying to mjethani:

For snippet filters, we could do it like so: [...]

I have logged the idea:

https://gitlab.com/eyeo/adblockplus/adblockpluscore/issues/3

comment:8 Changed on 04/15/2019 at 10:46:06 PM by sebastian

  • Resolution set to duplicate
  • Status changed from new to closed

For reference, the $rewrite filter option (except for abp-resource:) is to be unsupported with core#4. Anyway, if there are any concerns left that aren't covered in code#3 and core#4, please file a new issue on GitLab.

Last edited on 04/16/2019 at 04:08:26 AM by hfiguiere

Add Comment

Modify Ticket

Change Properties
Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from (none).
 
Note: See TracTickets for help on using tickets.