AdguardTeam / AdGuardSDNSFilter

AdGuard DNS filter
https://adguard-dns.io/
GNU General Public License v3.0
697 stars 125 forks source link

fix #463 in HostlistsRegistry: remove internal IPs #1646

Closed Sergey-Lyapin closed 2 months ago

Sergey-Lyapin commented 2 months ago

https://github.com/AdguardTeam/HostlistsRegistry/issues/463

Alex-302 commented 2 months ago

It is related to https://github.com/AdguardTeam/AdGuardSDNSFilter/issues/1224 and should be handled by HostlistCompiler

Sergey-Lyapin commented 2 months ago

@Alex-302 But can we exclude these rules temporarily until that task is completed?

Alex-302 commented 2 months ago

Better use regexps. I created few regexps for private IP ranges: ~https://regex101.com/r/eW4OBB/1~ https://regex101.com/r/kGYkDI/1

! 10.0.0.0 – 10.255.255.255
(?:^\|\|?|^)(10\.[\d\.*]+)\|{0,1}
! 127.0.0.0 – 127.255.255.255
(?:^\|\|?|^)(127\.[\d\.*]+)\|{0,1}
! 172.16.0.0 – 172.31.255.255
(?:^\|\|?|^)(172\.(?:1[6-9]|2[0-9]|3[0-1])\.[\d\.*]+)\|{0,1}
! 192.168.0.0 – 192.168.255.255
(?:^\|\|?|^)(192\.168\.[\d\.*]+)\|{0,1}

Note - in exclusions.txt expressions must be wrapped - /regexp/

You can add test rules and run local build to check result.

@AdamWr @piquark6046 Please also take a look, maybe regexps can be improved.

piquark6046 commented 2 months ago

https://github.com/AdguardTeam/AdGuardSDNSFilter/pull/1646#issuecomment-1988307749

https://regex101.com/r/qXOcCP/1 Is the above case included?

Alex-302 commented 2 months ago

Is the above case included?

What is it?) Just invalid IP?

AdamWr commented 2 months ago

If I'm not wrong, rule like this:

||10.32.17.182espn.hb.tt.omtrdc.net^

will be also excluded by this regex, or not?

If so, maybe something like this:

(?:^\|\|?|^)(10\.[\d\.*\^\|]+)(?:\|\^){0,1}$

can be used?

piquark6046 commented 2 months ago

https://github.com/AdguardTeam/AdGuardSDNSFilter/pull/1646#issuecomment-1988442989

{0,1} can be replaced with ?.

Alex-302 commented 2 months ago

@piquark6046 replaced. In general ok?

piquark6046 commented 2 months ago

https://github.com/AdguardTeam/AdGuardSDNSFilter/pull/1646#issuecomment-1988460044 I think so.

Alex-302 commented 2 months ago

@Sergey-Lyapin Please update PR according to https://github.com/AdguardTeam/AdGuardSDNSFilter/pull/1646#issuecomment-1988307749

Sergey-Lyapin commented 2 months ago

@Alex-302 updated