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

Third-party rules do not block 4-th level subdomains #116

Closed
ameshkov opened this issue Dec 28, 2015 · 19 comments
Closed

Third-party rules do not block 4-th level subdomains #116

ameshkov opened this issue Dec 28, 2015 · 19 comments
Assignees
Milestone

Comments

@ameshkov
Copy link
Member

The source:
AdguardTeam/AdguardFilters#1192

The problem:

  1. We have a rule ||innity.net^$third-party
  2. For some reason this rule is not applied to https://ssl-cdn.media.innity.net/lib/global.js
@Mizzick
Copy link
Contributor

Mizzick commented Dec 28, 2015

Content blocker prevented frame displaying http://cn.cari.com.my/portal.php from loading a resource from https://ssl-cdn.media.innity.net/lib/global.js
Content blocker prevented frame displaying http://cn.cari.com.my/portal.php from loading a resource from http://partner.googleadservices.com/gpt/pubads_impl_78.js

As I can see in console, global.js is blocked and on the next line an other 4-th level domain is blocked. So everything is fine?

@Mizzick Mizzick closed this as completed Dec 28, 2015
@ameshkov
Copy link
Member Author

No, it's not fine.

partner.googleadservices.com - is a 3rd level subdomain.
And ssl-cdn.media.innity.net currently is blocked due to the ||media.innity.net^$third-party I've added in AdguardTeam/AdguardFilters#1192

Just disable "Safari filter" and it won't be blocked again.

@ameshkov ameshkov reopened this Dec 28, 2015
@ameshkov
Copy link
Member Author

The same issue with exception rules:

Content blocker prevented frame displaying http://www.pornhub.com/ from loading a resource from http://cdn1b.static.pornhub.phncdn.com/images/categories/118x88/7.jpg

While here is the rule that should unblock it:

@@||pornhub.phncdn.com^$image,object-subrequest,domain=pornhub.com|redtube.com|tube8.com|tube8.es|tube8.fr|youporn.com|youporngay.com

Mizzick added a commit that referenced this issue Dec 29, 2015
@Mizzick
Copy link
Contributor

Mizzick commented Dec 29, 2015

Fixed, the issue was in incorrect regexp:
"url-filter": "^https?://[^.].?test-elemhide.com"
changed to:
"url-filter": "^https?://[^.]
.*?test-elemhide.com"

@Mizzick Mizzick closed this as completed Dec 29, 2015
@ameshkov
Copy link
Member Author

Hm, seems that the incorrect regexp has became even more incorrect...
Just use regexr.com and check how it works.

Also keep in mind, that the whole purpose of new regexp is to avoid using regexp groups.
Use testRegexpPerformance to check the speed of new regexps.

@ameshkov ameshkov reopened this Dec 29, 2015
@Mizzick
Copy link
Contributor

Mizzick commented Dec 29, 2015

What?
The previous matches only one domain level up, cause it allows only one dot character before the domain name.

@ameshkov
Copy link
Member Author

Right. The same as your new version.

Mizzick added a commit that referenced this issue Dec 29, 2015
Mizzick added a commit that referenced this issue Dec 29, 2015
@Mizzick
Copy link
Contributor

Mizzick commented Dec 29, 2015

https?://[^.][^/]*.?domain.com

@Mizzick Mizzick closed this as completed Dec 29, 2015
@ameshkov
Copy link
Member Author

verygooddomain.com is matched by this regexp:
http://www.regexr.com/3cg43

@ameshkov ameshkov reopened this Dec 29, 2015
@ameshkov
Copy link
Member Author

Huh, I think we had the same issue with a previous regexp too

@Mizzick
Copy link
Contributor

Mizzick commented Dec 29, 2015

I don't think it's a bug.

@ameshkov
Copy link
Member Author

I am pretty sure this is wrong.
||domain.com should match domain.com and it's subdomains and SHOULD NOT match other domains.

@Mizzick
Copy link
Contributor

Mizzick commented Dec 29, 2015

This regexp preffix is used not only for || rules

@ameshkov
Copy link
Member Author

In fact it is used for || prefixes only.

url-filter-rule:

    if (StringUtils.startWith(regex, UrlFilterRule.MASK_START_URL)) {
        regex = UrlFilterRule.REGEXP_START_URL + regex.substring(UrlFilterRule.MASK_START_URL.length);

converter:

urlFilter = StringUtils.replaceAll(urlFilter, UrlFilterRule.REGEXP_START_URL, URL_FILTER_REGEXP_START_URL);

@ameshkov
Copy link
Member Author

We may ignore this for now and use even simpler regexp:

https?:\/\/[^/]*\.?domain.com

Or we can try a bit more complex version that works properly:

https?:\/\/([^/]*\.)?domain.com

The question is: what's the difference in terms of performance if we use the second regexp?

Mizzick added a commit that referenced this issue Dec 29, 2015
@Mizzick
Copy link
Contributor

Mizzick commented Dec 29, 2015

Okay changed to https?://([^/]*.)?domain.com

@Mizzick Mizzick closed this as completed Dec 29, 2015
@ameshkov
Copy link
Member Author

What with performance test? What's the difference?

@Mizzick
Copy link
Contributor

Mizzick commented Dec 29, 2015

var regExp3 = new RegExp('^https?://([^/]*.)?some-domain.com.com[/:&?]?', 'i');
base-test.js:52 2015-12-29T15:41:05.270Z DEBUG Elapsed: 127ms
base-test.js:52 2015-12-29T15:41:05.377Z DEBUG Elapsed: 107ms
base-test.js:52 2015-12-29T15:41:05.377Z DEBUG Performance gain: 16%

var regExp4 = new RegExp('^https?://[^/]*.?some-domain.com.com[/:&?]?', 'i');
base-test.js:52 2015-12-29T15:41:05.509Z DEBUG Elapsed: 131ms
base-test.js:52 2015-12-29T15:41:05.608Z DEBUG Elapsed: 98ms
base-test.js:52 2015-12-29T15:41:05.609Z DEBUG Performance gain: 26%

@ameshkov
Copy link
Member Author

Ok then. You'd better increase converter version

Mizzick added a commit that referenced this issue Dec 29, 2015
Mizzick pushed a commit that referenced this issue Apr 9, 2018
…issues/995-redirect to master

* commit 'b01f68dfe5e34987a913e9c490769faba1f6c401':
  Fixed duplicated safebrowsing tabs
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

2 participants