Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

$important option doesn't work if domains are not prefixed by || #954

Closed
8 tasks done
Yuki2718 opened this issue Mar 21, 2020 · 8 comments
Closed
8 tasks done

$important option doesn't work if domains are not prefixed by || #954

Yuki2718 opened this issue Mar 21, 2020 · 8 comments
Labels
duplicate This issue or pull request already exists

Comments

@Yuki2718
Copy link

Yuki2718 commented Mar 21, 2020

Prerequisites

  • I verified that this is not a filter issue
  • This is not a support issue or a question
  • I performed a cursory search of the issue tracker to avoid opening a duplicate issue
    • Your issue may already be reported.
  • I tried to reproduce the issue when...
    • uBlock Origin is the only extension
    • uBlock Origin with default lists/settings
    • using a new, unmodified browser profile
  • I am running the latest version of uBlock Origin
  • I checked the documentation to understand that the issue I report is not a normal behavior

Description

$important option does not override exception filters unless its prefixed by ||example.com, in other words a rule like
/path/ads$important,domain=example.com|example.org
doesn't override the exception and this behavior is not explained in wiki.

A specific URL where the issue occurs

https://9to5mac.com/

Steps to Reproduce

  1. Go to the site with EasyList enabled and
    /wp-content/*/prebid$script,important,domain=9to5google.com|9to5mac.com|9to5toys.com|electrek.co|dronedj.com
    added to My Filters and also logger opened.
  2. See the request to the resource was allowed.
  3. If you have a rule like
    ||9to5mac.com/wp-content/*/prebid$script,important
    it will be blocked.

Expected behavior:

$important should override any exception.

Actual behavior:

Stated

Your environment

  • uBlock Origin version: 1.25.2
  • Browser Name and version: Firefox 74.0 / Firefox Android 68.6
  • Operating System and version: Windows 10 / Android 10
@Yuki2718
Copy link
Author

The title might be bad, as a rule like
/wp-content/*/prebid$script,important
also doesn't work.

@Yuki2718 Yuki2718 changed the title $important option doesn't work if domains are specified by $domain= and not || $important option doesn't work if domains are not prefixed by || Mar 21, 2020
@uBlock-user
Copy link
Contributor

/prebid$script,important is not working either.

@uBlock-user uBlock-user added bug Something isn't working something to address something to address labels Mar 21, 2020
@gorhill
Copy link
Member

gorhill commented Mar 21, 2020

The URL of the request which you expect uBO will block is:

https://9to5mac.com/wp-content/themes/9to5-2015/assets/js/vendor/prebid1.20.0.js

In uBO, /prebid won't match /prebid1, you need to make the rule /prebid*, so this works:

/wp-content/*/prebid*$script,important

This has been discussed in the past, see gorhill/uBlock#1065 (comment).

Now the fact that it works with || is completely accidental and due to implementation details (the use of the new biditrie code). To make that working case behave like the non-working case would be to add a pointless overhead. The thing to keep in mind is still true: if the token to match is not a bounded word as the case above, add an asterisk to make it explicit.

@uBlock-user uBlock-user removed the bug Something isn't working label Mar 21, 2020
@gorhill
Copy link
Member

gorhill commented Mar 21, 2020

due to implementation details (the use of the new biditrie code)

I was guessing as I wrote in there. The real reason is that wp and content -- the other tokens beside prebid -- are categorized as "bad tokens" by uBO, see https://github.com/gorhill/uBlock/blob/381498daa2a9ce089a69d044760190b1dd14b5ac/src/js/static-net-filtering.js#L2062.

So the best token uBO saw in the filter /wp-content/*/prebid was prebid, which it turns out was not to be used as a token (a trailing asterisk would have told uBO to not use prebid as token).

The filter ||9to5mac.com/wp-content/*/prebid worked because uBO used 9to5mac as token, which is not categorized as a "bad token".

@gorhill
Copy link
Member

gorhill commented Mar 21, 2020

So to conclude, the reported issue here is essentially a duplicate of gorhill/uBlock#1065.

@uBlock-user
Copy link
Contributor

are categorized as "bad tokens" by uBO

Is that list present since the beginning and why is this needed ?

@uBlock-user uBlock-user added duplicate This issue or pull request already exists and removed something to address something to address labels Mar 21, 2020
@gorhill
Copy link
Member

gorhill commented Mar 21, 2020

Is that list present since the beginning and why is this needed ?

A list of bad tokens has been present since the first commit, which was simply guessed and manually entered.

In this commit, it was expanded from a programmatically generated histogram (search for tokenHistogram in commit message).

ABP adopted the idea in https://gitlab.com/eyeo/adblockplus/adblockpluscore/-/issues/30.

"Bad tokens" are simply all the most frequent tokens from non-blocked URLs, so tokens which are not useful to use to store blocking filters.

@Yuki2718
Copy link
Author

Ah, I wish I knew it earlier. So the rule would have worked IF the request was like /prebid. or /prebid-. As some users suggested in the past issues it may be worth being added to the syntax page of wiki. Sorry for the duplicate, I thought it to be a matter of $important option.

Yuki2718 referenced this issue in acnapyx/paywall-remover Nov 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

3 participants