Skip to content

Loading…

Multiple domain negations are not respected #191

Closed
jblz opened this Issue · 8 comments

2 participants

@jblz

Rules like these:

||facebook.com^$domain=~facebook.com|~facebook.net|~fbcdn.com|~fbcdn.net
||facebook.net^$domain=~facebook.com|~facebook.net|~fbcdn.com|~fbcdn.net
||fbcdn.com^$domain=~facebook.com|~facebook.net|~fbcdn.com|~fbcdn.net
||fbcdn.net^$domain=~facebook.com|~facebook.net|~fbcdn.com|~fbcdn.net

(see this lifehacker article)

Block things like AJAX requests on the facebook.com domain.

Single domain negations like these:

||facebook.com^$domain=~facebook.com
||facebook.net^$domain=~facebook.com
||fbcdn.com^$domain=~facebook.com
||fbcdn.net^$domain=~facebook.com

...work as expected on the facebook.com domain.

Expected: the top set of rules should be applying all the negations separated by pipes (|) such that the rules don't block those requests when the $domain is one of:

  • facebook.com
  • facebook.net
  • fbcdn.com
  • fbcdn.net
@gorhill

Good one. There is obviously a flaw in my implementation of negated hostnames.

So looking at the most natural way to fit the proper semantic in the current code, isn't true that:

||facebook.com^$domain=~facebook.com|~facebook.net|~fbcdn.com|~fbcdn.net

Is equivalent to

||facebook.com^
@@||facebook.com^$domain=facebook.com
@@||facebook.com^$domain=facebook.net
@@||facebook.com^$domain=fbcdn.com
@@||facebook.com^$domain=fbcdn.net

Or am I missing something?

So I would simply have to convert the negated hostname to a non-negated one, while converting the block rule into an allow rule, all other directives if any being the same.

And as a bonus, less overhead as this would get rid of the flawed code path.

@gorhill

For the record, occurrences of this bug, for the pre-selected filters lists:

  • EasyList: 10 occurrences
    • /advertiser/*$domain=~mobileapptracking.com|~trialpay.com
    • /mydirtyhobby.$domain=~mydirtyhobby.com|~mydirtyhobby.de
    • ||adm.fwmrm.net/p/msnbc_live/$object-subrequest,third-party,domain=~msnbc.msn.com|~www.nbcnews.com
    • ||dt00.net^$third-party,domain=~marketgid.com|~marketgid.ru|~marketgid.ua|~mgid.com|~thechive.com
    • ||dt07.net^$third-party,domain=~marketgid.com|~marketgid.ru|~marketgid.ua|~mgid.com|~thechive.com
    • ||mgid.com^$third-party,domain=~marketgid.com|~marketgid.com.ua
    • ||adf.ly/js/$third-party,domain=~j.gs|~q.gs
    • ||mydirtyhobby.com^$third-party,domain=~my-dirty-hobby.com|~mydirtyhobby.de
    • ||graphics.pop6.com/javascript/$script,third-party,domain=~adultfriendfinder.co.uk|~adultfriendfinder.com
    • @@||tidaltv.com/crossdomain.xml$object-subrequest,domain=~channel4.com|~itv.com
  • EasyPrivacy: 0

Other pre-selected lists are list of plain hostnames, thus unaffected.

@gorhill

After the fix, as usual running the reference benchmark for CPU overhead of net requests since code was changed which affects that part:

µBlock > onBeforeRequest: 0.133 ms (19594 samples)
µBlock > onBeforeRequest: 0.133 ms (19870 samples)
µBlock > onBeforeRequest: 0.133 ms (20125 samples)
µBlock > onBeforeRequest: 0.133 ms (20446 samples)
µBlock > onBeforeRequest: 0.133 ms (20642 samples)
µBlock > onBeforeRequest: 0.132 ms (20992 samples)
µBlock > onBeforeRequest: 0.132 ms (21171 samples)
µBlock > onBeforeRequest: 0.132 ms (21342 samples)
µBlock > onBeforeRequest: 0.132 ms (21506 samples)
µBlock > onBeforeRequest: 0.132 ms (21725 samples)
@gorhill gorhill added a commit that closed this issue
@gorhill gorhill this fixes #191 527817d
@gorhill gorhill closed this in 527817d
@gorhill gorhill reopened this
@gorhill

The fix works well for block filters with negated hostnames.

It doesn't work for allow filters with negated hostnames, because of the order of evaluation. For example:

@@||g.doubleclick.net/crossdomain.xml$object-subrequest,domain=~newgrounds.com

Will result in:

  1. block filter found for http://g.doubleclick.net/crossdomain.xml when source is newgrounds.com
  2. allow filter found for http://g.doubleclick.net/crossdomain.xml when source is any

While it needs to be:

  1. block filter found for http://g.doubleclick.net/crossdomain.xml when source is newgrounds.com
  2. don't evaluate allow filters

For allow filters to work with negated hostnames, I need something more, something like a strong block, which will cause evaluation of allow filters to be bypassed. This could also be useful to address #139.

@jblz

Nice! Thanks for digging into this. And thank you for such a great piece of open-source software :)
I think the equivalence you mentioned here is spot on.

@gorhill

Fix for the second part done. Regarding allow filters with negated hostnames, six occurrences in EasyList:

@@||209.222.8.217/crossdomain.xml$object-subrequest,domain=~p2p.adserver.ip
@@||adtech.de/crossdomain.xml$object-subrequest,domain=~zattoo.com
@@||g.doubleclick.net/crossdomain.xml$object-subrequest,domain=~newgrounds.com
@@||innovid.com/crossdomain.xml$object-subrequest,domain=~channel4.com
@@||oas.bigflix.com/realmedia/ads/$object-subrequest,domain=~tamilflix.net
@@||tidaltv.com/crossdomain.xml$object-subrequest,domain=~channel4.com|~itv.com

Had to re-work enough code to warrant another CPU benchmark for net request handling overhead. Here:

µBlock > onBeforeRequest: 0.132 ms (13582 samples)
µBlock > onBeforeRequest: 0.132 ms (13713 samples)
µBlock > onBeforeRequest: 0.132 ms (13803 samples)
µBlock > onBeforeRequest: 0.132 ms (13871 samples)
µBlock > onBeforeRequest: 0.132 ms (13934 samples)
µBlock > onBeforeRequest: 0.132 ms (14021 samples)
µBlock > onBeforeRequest: 0.132 ms (14103 samples)
µBlock > onBeforeRequest: 0.132 ms (14161 samples)
µBlock > onBeforeRequest: 0.132 ms (14262 samples)
µBlock > onBeforeRequest: 0.131 ms (14440 samples)
@gorhill gorhill added a commit that closed this issue
@gorhill gorhill this fixes #191 completely fe2d761
@gorhill gorhill closed this in fe2d761
@gorhill

I would just like to know if the fix worked for your case (I don't have a Facebook account).

@jblz

Confirmed working with version 0.5.3.0. Thanks for the fix!

@gorhill gorhill referenced this issue in gorhill/uBlock
Closed

Problem on Yahoo Search Pages #621

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.