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

uBO should ignore redirect= directives with unrecognized redirect token #1366

Closed
bogachenko opened this issue Nov 28, 2020 · 35 comments
Closed
Labels
bug Something isn't working fixed issue has been addressed

Comments

@bogachenko
Copy link

bogachenko commented Nov 28, 2020

Actually I described part of this bug here but today for some reason it has changed, and what I described in the link above will no longer help

SO. add this adguard rule to uBO my filters

||pagead2.googlesyndication.com^$important,script,redirect=googlesyndication-adsbygoogle,domain=animedigitalnetwork.fr|baddiehub.com|clockks.com|coron.tech|earn4files.com|extremereportbot.com|gatcha.org|huljjang.com|jemerik.com|moviessquad.com|scat.gold|sitarchive.com|tellynewsarticles.com|webkod.pl|youneed.win|darmowa-tv.ws|mr9soft.com

and go to the site, for example here https://baddiehub.com/ {NSFW}, please note that this filter is a redirect= and now what do you get https://user-images.githubusercontent.com/8535285/100518332-f6ed8d80-31e4-11eb-9a61-9ef8cfe94b92.png

tested on a stable release version and on the latest version for developers, of course with default settings

PS: You have a rule in your main list, add badfilter flag for a second. so that he does not bother you to check
||googlesyndication.com^$script,redirect-rule=googlesyndication_adsbygoogle.js,domain=baddiehub.com,badfilter

@uBlock-user
Copy link
Contributor

redirect=googlesyndication-adsbygoogle

Invalid, use redirect=googlesyndication_adsbygoogle.js

@bogachenko
Copy link
Author

add this adguard rule to uBO my filters

adguard rule

@bogachenko
Copy link
Author

there are 2 filters in my sheet, both for adguard and for ubo. read the link I posted above

@gorhill
Copy link
Member

gorhill commented Nov 28, 2020

There is no resource token matching googlesyndication-adsbygoogle, the proper token is googlesyndication_adsbygoogle.js.

@uBlock-user
Copy link
Contributor

That AdGuard rule will not work in uBO, I can reproduce what you're saying but you need to use the one I posted.

@gorhill
Copy link
Member

gorhill commented Nov 28, 2020

Which AdGuard list(s) make use of googlesyndication-adsbygoogle?

@bogachenko
Copy link
Author

@uBlock-user Fuck, I know the adguard rule won't work in uBO. I'm talking about something else.

  1. it is detected incorrectly.
  2. Why is it determined at all?

@bogachenko
Copy link
Author

@gorhill english

@gorhill
Copy link
Member

gorhill commented Nov 28, 2020

What do you mean "detected incorrectly"?

The URL I get is:

https://pagead2.googlesyndication.com/pagead/js/adsbygoogle.js

So the filter you use does match that URL. uBO tries to redirect to googlesyndication-adsbygoogle, but since the resource token is not found, ultimately it does not redirect, hence no yellow line in the logger for the actual redirection.

@uBlock-user
Copy link
Contributor

@bogachenko Okay, I think I understand what you're saying now, that rule is invalid and hnce shouldn't be compiled, but somehow it got compiled and applied as per https://user-images.githubusercontent.com/8535285/100518332-f6ed8d80-31e4-11eb-9a61-9ef8cfe94b92.png ?

@uBlock-user
Copy link
Contributor

@gorhill Seems the redirection does occur as per https://user-images.githubusercontent.com/8535285/100518332-f6ed8d80-31e4-11eb-9a61-9ef8cfe94b92.png even though the resource token name doesn't exist ?

@gorhill
Copy link
Member

gorhill commented Nov 28, 2020

Well I made the choice in the static filtering parser to not discard redirect filters for which no token exists, the idea is that maybe someone will modify userResourcesLocation such that the resource token will become valid.

The redirect should not be fired, the yellow line is ultimately confirmation the redirect occurs, the other lines are just confirmation a filter is matching. No yellow << line = no redirection.

@uBlock-user
Copy link
Contributor

uBlock-user commented Nov 28, 2020

The line is light blue there as per the above picture, what does that signify ?

Modifier filters are coloured in blue.

@gorhill gorhill reopened this Nov 28, 2020
@gorhill
Copy link
Member

gorhill commented Nov 28, 2020

I prefer to keep open. There is this edge case where uBO should fall back to a working redirect filter if the picked one is not ultimately taken. I can reproduce this with the provided test case if I remove the badfilter and add :1 to the added redirect= filter.

@uBlock-user uBlock-user added the bug Something isn't working label Nov 28, 2020
@gorhill
Copy link
Member

gorhill commented Nov 28, 2020

Also, unrelated but I will try to fix at the same time, adding a priority to the redirect token causes uBO to think the token is invalid in the editor, though it ultimately work fine at runtime.

@bogachenko
Copy link
Author

bogachenko commented Nov 28, 2020

@gorhill explain why the speed race does not work(although recently worked), I have 2 filters , see

1 for ubo
||pagead2.googlesyndication.com^$important,script,redirect=googlesyndication_adsbygoogle.js,domain=animedigitalnetwork.fr|baddiehub.com|clockks.com|coron.tech|earn4files.com|extremereportbot.com|gatcha.org|huljjang.com|jemerik.com|moviessquad.com|scat.gold|sitarchive.com|tellynewsarticles.com|webkod.pl|youneed.win|darmowa-tv.ws|mr9soft.com

2 for adguard
||pagead2.googlesyndication.com^$important,script,redirect=googlesyndication-adsbygoogle,domain=animedigitalnetwork.fr|baddiehub.com|clockks.com|coron.tech|earn4files.com|extremereportbot.com|gatcha.org|huljjang.com|jemerik.com|moviessquad.com|scat.gold|sitarchive.com|tellynewsarticles.com|webkod.pl|youneed.win|darmowa-tv.ws|mr9soft.com

if you add them together in strict order (rule for ubo on 1 line, for adguard on 2), as I wrote, that's the rule for adguard. your extension will try to execute, but the rule for ubo will not. although it is the first

@gorhill
Copy link
Member

gorhill commented Nov 28, 2020

That it worked before is just luck, there has never been documentation stating the order of the redirect= filters is meaningful. It looks like before the redirect= directive for uBO was found first, and now since the redirect= code has been refactored, the AdGuard one is looked up first.

In any case, the fix proposed here is that uBO will disregard redirect= directives which do not match an actual resource, so this will fix your issue.

@bogachenko
Copy link
Author

that would be wonderful

gorhill added a commit to gorhill/uBlock that referenced this issue Nov 28, 2020
`match-case`
------------

Related issue:
- uBlockOrigin/uAssets#8280 (comment)

The new filter option `match-case` can be used only for
regex-based filters. Using `match-case` with any other
sort of filters will cause uBO to discard the filter.

`redirect=`
-----------

Related issue:
- uBlockOrigin/uBlock-issues#1366

`redirect=` filters with unresolvable resource token at
runtime will be discarded.

Additionally, the implicit priority is now set to 1
(was 0). The idea is to allow custom `redirect=` filters
to be used strictly as fallback `redirect=` filters in case
another `redirect=` filter is not picked up.

For example, one might create a `redirect=click2load.html:0`
filter, to be taken if and only if the blocked resource is
not already being redirected by another "official" filter
in one of the enabled filter lists.
@gorhill gorhill changed the title Redirect bug uBO should ignore redirect= directives with unrecognized redirect token Nov 28, 2020
@uBlock-user uBlock-user added the fixed issue has been addressed label Nov 28, 2020
@bogachenko
Copy link
Author

@gorhill The problem is not fixed, not completely. I added a commit to my list, pay attention to ,redirect=noopjs, and what did I get.

test link https://swingerpornfun.com/2020/11/28/kinky-husband-lends-his-cute-submissive-wife-to-a-dominant-couple/ NSFW

latest version 1.31.1b8 opera

@uBlock-user
Copy link
Contributor

@bogachenko Any change if you add ||vkcdnservice.com^$third-party,badfilter to my Filters ?

@bogachenko
Copy link
Author

nope

@uBlock-user
Copy link
Contributor

Nevermind, internally the redirect filter is split into a blocking filter and a redirecting filter which is redirect-rule, not related to this issue, you can verify this by filtering vkcdn into the filter widget into the logger.

@bogachenko
Copy link
Author

the redirect filter has been transformed into a redirect-rule filter, and you want to say that this is normal?

@gorhill
Copy link
Member

gorhill commented Nov 29, 2020

the redirect filter has been transformed into a redirect-rule filter, and you want to say that this is normal?

This has always been the case, redirect= means "block and redirect", while redirect-rule= means "redirect if blocked".

@uBlock-user
Copy link
Contributor

Yes, the only difference now is you will be seeing the internal representation in the logger too

@gorhill
Copy link
Member

gorhill commented Nov 29, 2020

and what did I get.

Looks fine to me. You created a redirect= rule and it got applied as expected.

@Yuki2718
Copy link

Yuki2718 commented Dec 9, 2020

Isn't it better if uBO translates redirect resources of AG into uBO counterparts? If AG adds a redirect filter there must be a reason and uBO uses some AG lists including French and Japanese. In case of googlesyndication-adsbygoogle it's no matter as uBlock Privacy redirects it by default, but what about other resources uBO doesn't redirect by default - well, fortunately the most common one, noopjs, is compatible to uBO but wiki suggests the current formal form for uBO is noop.js and noopjs works only as alias.

Compatibility table of redirect resources is here https://github.com/AdguardTeam/Scriptlets/blob/master/wiki/compatibility-table.md#redirects and apparently most of incompatible ones are what are redirected by default, just not sure if they're all. In rare occasion AG's redirect conflicts with uAssets' solution e.g. https://forums.lanik.us/viewtopic.php?f=62&t=45330 but we can address such cases by priority now.

@gorhill
Copy link
Member

gorhill commented Dec 9, 2020

The only one not available in uBO which I see used in AdGuard Base is noopvast-2.0, I would be willing to add support for this. Do you see anything else which would be needed beside this one?

@Yuki2718
Copy link

Yuki2718 commented Dec 9, 2020

What about prevent-fab-3.2.0? Old uBO syntax was fuckadblock.js-3.2.0. Well, but it's not used as redirect resource in AG lists which I've checked.

@gwarser
Copy link

gwarser commented Dec 16, 2020

I think gorhill/uBlock@904aa87 broke this again. OP filter has important option and because of this ||googlesyndication.com^$script,redirect-rule=googlesyndication_adsbygoogle.js,domain=baddiehub.com from uBlock filters is not applied. Or is this expected for important?

@gorhill
Copy link
Member

gorhill commented Dec 16, 2020

Yes, important applies to redirect-rule too.

@gorhill
Copy link
Member

gorhill commented Dec 16, 2020

Ok I see your point now that I can test (was on my phone when I answered yesterday), so yes, I need to fix this.

@gwarser gwarser reopened this Dec 16, 2020
gorhill added a commit to gorhill/uBlock that referenced this issue Dec 16, 2020
@gwarser
Copy link

gwarser commented Dec 16, 2020

Correct redirection is now applied when both filters are used, but filter with invalid token is still logged.

When only one filter, the one with invalid token is used, no redirection is logged.

@gorhill
Copy link
Member

gorhill commented Dec 16, 2020

You think the redirect directives with invalid token should not be logged?

@gwarser
Copy link

gwarser commented Dec 16, 2020

I'm asking for opinions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed issue has been addressed
Projects
None yet
Development

No branches or pull requests

5 participants