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

Rules with modifier like for example $media shouldn't be applied to apps with disabled HTTPS filtering #1066

Closed
AdamWr opened this issue Jul 31, 2019 · 8 comments

Comments

@AdamWr
Copy link
Member

AdamWr commented Jul 31, 2019

It seems that now in apps with disabled HTTPS filtering adding rule like this:
*.*$media,important
causes that requests are not blocked.

Steps to reproduce

  1. Add this rule:
    *.*$media,important
    or
    *.*$subdocument,important
  2. Open any app with disabled HTTPS filtering

Expected behavior

These kind of rules shouldn't be applied to apps with disabled HTTPS filtering, and ad/tracker etc. requests should be blocked.

Actual behavior

All request are passed and not blocked.

Your environment

  • AdGuard for Android nightly v3.2.118(CL 1.4.120)
@ameshkov ameshkov added this to the v1.5 milestone Jul 31, 2019
@AdamWr
Copy link
Member Author

AdamWr commented Jul 31, 2019

By the way, it seems that $empty modifier doesn't work in apps with disabled HTTPS filtering too.

Steps to reproduce

  1. Install this app - https://play.google.com/store/apps/details?id=com.mobilefootie.wc2010
  2. Add this rule - ||images.fotmob.com^$empty

Expected behavior

Images in app shouldn't load.

Actual behavior

Images load.

@ameshkov
Copy link
Member

In fact, that's intended. $emtpy implies that the requests aren't blocked, they are modified. We can't modify responses without https filtering so that's why it shouldn't do anything.

@sxgunchenko what bothers me is why $media -- I don't understand how this rule matched the request in the first place. Internally, HTTPS tunnel request is processed as if it has "DOCUMENT" content type so I can understand how $subdocument matched it, but not $media.

@sxgunchenko
Copy link

sxgunchenko commented Aug 1, 2019

what bothers me is why $media -- I don't understand how this rule matched the request in the first place.

It matches because we pass any-content-type in matching procedure. So we should pass some set of content types which are applicable for https tunnel requests.

@ameshkov
Copy link
Member

ameshkov commented Aug 1, 2019

It matches because we pass any-content-type in matching procedure. So we should pass some set of content types which are applicable for https tunnel requests.

Yes, we should pass DOCUMENT and then filter our $subdocument and cosmetic modifiers like $elemhide, $content, etc, etc

@sxgunchenko
Copy link

core/pull-requests/1435

@AdamWr
Copy link
Member Author

AdamWr commented Apr 12, 2020

In fact, that's intended. $emtpy implies that the requests aren't blocked, they are modified. We can't modify responses without https filtering so that's why it shouldn't do anything.

One question about that. For example, we have this rule with $empty modifier in Base filter:
||adnxs.com^$empty
If I understand correctly, it means that in app which https filtering is disabled this requests will be not blocked even if we have a blocking rule too ||adnxs.com^ (in Mobile ads filter).
Shouldn't it be somehow checked that if $empty/redirect rule can't be applied in situation like this then apply blocking rule?

Steps to reproduce:

  1. Add these rules:
|http$redirect=nooptext,app=APP_NAME
|http$app=APP_NAME

or just

|http$redirect=nooptext
|http
  1. Open "APP_NAME"

It will causes that all requests will be not blocked.

@ameshkov
Copy link
Member

If I understand correctly, it means that in app which https filtering is disabled this requests will be not blocked even if we have a blocking rule too ||adnxs.com^ (in Mobile ads filter).

No, it should be blocked actually

@ameshkov
Copy link
Member

@sxgunchenko could you please check this case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants