mirrors/AdGuardHome
2
0
Fork 0
mirror of https://github.com/AdguardTeam/AdGuardHome.git synced 2025-10-26 11:27:18 +00:00
Code Issues Packages Projects Releases Wiki Activity

Pull request 2441: AGDNS-3060-filtering-imp-gocognit

Browse Source 
Merge in DNS/adguard-home from AGDNS-3060-filtering-imp-gocognit to master

Squashed commit of the following:

commit 7f0b812af4938b46263e085168842b4596a29729
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Mon Aug 4 20:05:53 2025 +0300

    scripts: fix lint

commit 9fd34a895776b48084a45edc8e0d5a95321bf75c
Merge: 8b1a1904e b8043e4f0
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Mon Aug 4 20:01:38 2025 +0300

    Merge branch 'master' into AGDNS-3060-filtering-imp-gocognit

commit 8b1a1904e1
Merge: 2f9d480b4 df258512d
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Tue Jul 29 20:36:25 2025 +0300

    Merge branch 'master' into AGDNS-3060-filtering-imp-gocognit

commit 2f9d480b4a
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Mon Jul 28 21:10:01 2025 +0300

    filtering: imp gocognit

commit ab00ca31dd
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Fri Jul 25 19:20:35 2025 +0300

    filtering: imp docs

commit cb80a76c45
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Fri Jul 25 19:15:29 2025 +0300

    filtering: imp docs

commit 9f61af2700
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Thu Jul 24 19:44:56 2025 +0300

    bamboo-specs: fix ci

commit e0bbc2e2b4
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Thu Jul 24 19:30:17 2025 +0300

    all: filtering imp gocognit
This commit is contained in:
Stanislav Chzhen 2025-08-04 20:18:03 +03:00
parent b8043e4f05
commit ced9a1694a
4 changed files with 207 additions and 125 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

93
internal/filtering/filter.go
View File

@ -144,21 +144,22 @@ func (d *DNSFilter) filterSetProperties(
shouldRestart = true
}
if flt.Enabled {
if shouldRestart {
// Download the filter contents.
shouldRestart, err = d.update(flt)
}
} else {
if !flt.Enabled {
// TODO(e.burkov): The validation of the contents of the new URL is
// currently skipped if the rule list is disabled. This makes it
// possible to set a bad rules source, but the validation should still
// kick in when the filter is enabled. Consider changing this behavior
// to be stricter.
flt.unload()
return shouldRestart, err
}
return shouldRestart, err
if !shouldRestart {
return false, nil
}
return d.update(flt)
}
// filterExists returns true if a filter with the same url exists in d. It's
@ -315,19 +316,7 @@ func (d *DNSFilter) refreshFiltersArray(
return 0, nil, nil, false
}
failNum := 0
for i := range updateFilters {
uf := &updateFilters[i]
updated, err := d.update(uf)
updateFlags = append(updateFlags, updated)
if err != nil {
failNum++
d.logger.ErrorContext(ctx, "updating filter", "url", uf.URL, slogutil.KeyError, err)
continue
}
}
failNum, updateFlags := d.updateFilterList(ctx, updateFilters)
if failNum == len(updateFilters) {
return 0, nil, nil, true
}
@ -335,6 +324,40 @@ func (d *DNSFilter) refreshFiltersArray(
d.conf.filtersMu.Lock()
defer d.conf.filtersMu.Unlock()
updateCount = d.syncUpdatedFilters(ctx, filters, updateFilters, updateFlags)
return updateCount, updateFilters, updateFlags, false
}
// updateFilterList updates each filter in updateFilters and returns the number
// of failures and the updateFlags slice aligned with updateFilters indicating
// whether each filter's data changed.
func (d *DNSFilter) updateFilterList(
ctx context.Context,
updateFilters []FilterYAML,
) (failNum int, updateFlags []bool) {
for i := range updateFilters {
uf := &updateFilters[i]
updated, err := d.update(uf)
updateFlags = append(updateFlags, updated)
if err != nil {
failNum++
d.logger.ErrorContext(ctx, "updating filter", "url", uf.URL, slogutil.KeyError, err)
}
}
return failNum, updateFlags
}
// syncUpdatedFilters syncs updated filters back to the original filters slice
// and returns the updateCount. filters must not be nil. updateFlags must
// align with updateFilters. d.conf.filtersMu must be locked.
func (d *DNSFilter) syncUpdatedFilters(
ctx context.Context,
filters *[]FilterYAML,
updateFilters []FilterYAML,
updateFlags []bool,
) (updateCount int) {
for i := range updateFilters {
uf := &updateFilters[i]
updated := updateFlags[i]
@ -365,7 +388,7 @@ func (d *DNSFilter) refreshFiltersArray(
}
}
return updateCount, updateFilters, updateFlags, false
return updateCount
}
// refreshFiltersIntl checks filters and updates them if necessary. If force is
@ -418,21 +441,23 @@ func (d *DNSFilter) refreshFiltersIntl(block, allow, force bool) (int, bool) {
return 0, true
}
if updNum != 0 {
d.EnableFilters(false)
if updNum == 0 {
return 0, false
}
for i := range lists {
uf := &lists[i]
updated := toUpd[i]
if !updated {
continue
}
d.EnableFilters(false)
p := uf.Path(d.conf.DataDir)
err := os.Remove(p + ".old")
if err != nil {
d.logger.ErrorContext(ctx, "removing old filter", "path", p, slogutil.KeyError, err)
}
for i := range lists {
uf := &lists[i]
updated := toUpd[i]
if !updated {
continue
}
p := uf.Path(d.conf.DataDir)
err := os.Remove(p + ".old")
if err != nil {
d.logger.ErrorContext(ctx, "removing old filter", "path", p, slogutil.KeyError, err)
}
}

165
internal/filtering/filtering.go
View File

@ -766,42 +766,19 @@ func (d *DNSFilter) matchBlockedServicesRules(
func newRuleStorage(filters []Filter) (rs *filterlist.RuleStorage, err error) {
lists := make([]filterlist.RuleList, 0, len(filters))
for _, f := range filters {
switch id := int(f.ID); {
case len(f.Data) != 0:
lists = append(lists, &filterlist.StringRuleList{
ID: id,
RulesText: string(f.Data),
IgnoreCosmetic: true,
})
case f.FilePath == "":
var rl filterlist.RuleList
var skip bool
rl, skip, err = ruleListFromFilter(f)
if skip {
continue
case runtime.GOOS == "windows":
// On Windows we don't pass a file to urlfilter because it's
// difficult to update this file while it's being used.
var data []byte
data, err = os.ReadFile(f.FilePath)
if errors.Is(err, fs.ErrNotExist) {
continue
} else if err != nil {
return nil, fmt.Errorf("reading filter content: %w", err)
}
lists = append(lists, &filterlist.StringRuleList{
ID: id,
RulesText: string(data),
IgnoreCosmetic: true,
})
default:
var list *filterlist.FileRuleList
list, err = filterlist.NewFileRuleList(id, f.FilePath, true)
if errors.Is(err, fs.ErrNotExist) {
continue
} else if err != nil {
return nil, fmt.Errorf("creating file rule list with %q: %w", f.FilePath, err)
}
lists = append(lists, list)
}
if err != nil {
// Don't wrap the error, because it's informative enough as is.
return nil, err
}
lists = append(lists, rl)
}
rs, err = filterlist.NewRuleStorage(lists)
@ -812,6 +789,51 @@ func newRuleStorage(filters []Filter) (rs *filterlist.RuleStorage, err error) {
return rs, nil
}
// ruleListFromFilter returns a rule list from a Filter.
func ruleListFromFilter(f Filter) (rl filterlist.RuleList, skip bool, err error) {
id := int(f.ID)
if len(f.Data) != 0 {
return &filterlist.StringRuleList{
ID: id,
RulesText: string(f.Data),
IgnoreCosmetic: true,
}, false, nil
}
if f.FilePath == "" {
return nil, true, nil
}
if runtime.GOOS == "windows" {
// On Windows we don't pass a file to urlfilter because it's
// difficult to update this file while it's being used.
var data []byte
data, err = os.ReadFile(f.FilePath)
if errors.Is(err, fs.ErrNotExist) {
return nil, true, nil
} else if err != nil {
return nil, false, fmt.Errorf("reading filter content: %w", err)
}
return &filterlist.StringRuleList{
ID: id,
RulesText: string(data),
IgnoreCosmetic: true,
}, false, nil
}
var list *filterlist.FileRuleList
list, err = filterlist.NewFileRuleList(id, f.FilePath, true)
if errors.Is(err, fs.ErrNotExist) {
return nil, true, nil
} else if err != nil {
return nil, false, fmt.Errorf("creating file rule list with %q: %w", f.FilePath, err)
}
return list, false, nil
}
// Initialize urlfilter objects.
func (d *DNSFilter) initFiltering(ctx context.Context, allowFilters, blockFilters []Filter) (err error) {
rulesStorage, err := newRuleStorage(blockFilters)
@ -904,32 +926,37 @@ func (d *DNSFilter) matchHostProcessDNSResult(
return makeResult([]rules.Rule{dnsres.NetworkRule}, reason)
}
switch qtype {
case dns.TypeA:
if dnsres.HostRulesV4 != nil {
res = makeResult(hostRulesToRules(dnsres.HostRulesV4), FilteredBlockList)
for i, hr := range dnsres.HostRulesV4 {
res.Rules[i].IP = hr.IP
}
return res
}
case dns.TypeAAAA:
if dnsres.HostRulesV6 != nil {
res = makeResult(hostRulesToRules(dnsres.HostRulesV6), FilteredBlockList)
for i, hr := range dnsres.HostRulesV6 {
res.Rules[i].IP = hr.IP
}
return res
}
default:
// Go on.
if result, ok := resultFromHostRules(qtype, dnsres); ok {
return result
}
return hostResultForOtherQType(dnsres)
}
// resultFromHostRules handles the HostRulesV4/HostRulesV6 case for
// [matchHostProcessDNSResult]. dnsres must not be nil.
func resultFromHostRules(qtype uint16, dnsres *urlfilter.DNSResult) (res Result, ok bool) {
if qtype == dns.TypeA && dnsres.HostRulesV4 != nil {
res = makeResult(hostRulesToRules(dnsres.HostRulesV4), FilteredBlockList)
for i, hr := range dnsres.HostRulesV4 {
res.Rules[i].IP = hr.IP
}
return res, true
}
if qtype == dns.TypeAAAA && dnsres.HostRulesV6 != nil {
res = makeResult(hostRulesToRules(dnsres.HostRulesV6), FilteredBlockList)
for i, hr := range dnsres.HostRulesV6 {
res.Rules[i].IP = hr.IP
}
return res, true
}
return Result{}, false
}
// hostResultForOtherQType returns a result based on the host rules in dnsres,
// if any. dnsres.HostRulesV4 take precedence over dnsres.HostRulesV6.
func hostResultForOtherQType(dnsres *urlfilter.DNSResult) (res Result) {
@ -1051,14 +1078,10 @@ func New(c *Config, blockFilters []Filter) (d *DNSFilter, err error) {
confMu: &sync.RWMutex{},
}
for i, p := range c.SafeFSPatterns {
// Use Match to validate the patterns here.
_, err = filepath.Match(p, "test")
if err != nil {
return nil, fmt.Errorf("safe_fs_patterns: at index %d: %w", i, err)
}
d.safeFSPatterns = append(d.safeFSPatterns, p)
err = d.validateSafeFSPatterns(c.SafeFSPatterns)
if err != nil {
// Don't wrap the error, because it's informative enough as is.
return nil, err
}
d.hostCheckers = []hostChecker{{
@ -1126,6 +1149,22 @@ func New(c *Config, blockFilters []Filter) (d *DNSFilter, err error) {
return d, nil
}
// validateSafeFSPatterns validates and stores patterns for local filtering‑rule
// files.
func (d *DNSFilter) validateSafeFSPatterns(patterns []string) (err error) {
for i, p := range patterns {
// Use Match to validate the patterns here.
_, err = filepath.Match(p, "test")
if err != nil {
return fmt.Errorf("safe_fs_patterns: at index %d: %w", i, err)
}
d.safeFSPatterns = append(d.safeFSPatterns, p)
}
return nil
}
// Start registers web handlers and starts filters updates loop.
func (d *DNSFilter) Start() {
d.filtersInitializerChan = make(chan filtersInitializerParams, 1)

62
internal/filtering/rewrite/storage.go
View File

@ -97,16 +97,38 @@ func (s *DefaultStorage) MatchRequest(dReq *urlfilter.DNSRequest) (rws []*rules.
ctx := context.TODO()
rrules := s.rewriteRulesForReq(dReq)
if len(rrules) == 0 {
rewriteRules := s.rewriteRulesForReq(dReq)
if len(rewriteRules) == 0 {
return nil
}
resolvedRules, wildcardRewrite := s.resolveCNAMEChain(ctx, dReq, rewriteRules)
if wildcardRewrite != nil {
return []*rules.DNSRewrite{wildcardRewrite}
}
if resolvedRules == nil {
return nil
}
return s.collectDNSRewrites(resolvedRules, dReq.DNSType)
}
// resolveCNAMEChain follows the CNAME chain for a DNS request, handling loops
// and special cases. dReq must not be nil, and rewriteRules must not contain
// nil elements.
func (s *DefaultStorage) resolveCNAMEChain(
ctx context.Context,
dReq *urlfilter.DNSRequest,
rewriteRules []*rules.NetworkRule,
) (resolvedRules []*rules.NetworkRule, wildcardRewrite *rules.DNSRewrite) {
// TODO(a.garipov): Check cnames for cycles on initialization.
cnames := container.NewMapSet[string]()
host := dReq.Hostname
for len(rrules) > 0 && rrules[0].DNSRewrite != nil && rrules[0].DNSRewrite.NewCNAME != "" {
rule := rrules[0]
for len(rewriteRules) > 0 &&
rewriteRules[0].DNSRewrite != nil &&
rewriteRules[0].DNSRewrite.NewCNAME != "" {
rule := rewriteRules[0]
rwAns := rule.DNSRewrite.NewCNAME
s.logger.DebugContext(ctx, "cname found", "host", host, "cname", rwAns)
@ -115,37 +137,43 @@ func (s *DefaultStorage) MatchRequest(dReq *urlfilter.DNSRequest) (rws []*rules.
// A request for the hostname itself is an exception rule.
// TODO(d.kolyshev): Check rewrite of a pattern onto itself.
return nil
return nil, nil
}
if host == rwAns && isWildcard(rule.RuleText) {
// An "*.example.com → sub.example.com" rewrite matching in a loop.
//
// See https://github.com/AdguardTeam/AdGuardHome/issues/4016.
return []*rules.DNSRewrite{rule.DNSRewrite}
if isSelfMatchingWildcard(host, rwAns, rule.RuleText) {
return nil, rule.DNSRewrite
}
if cnames.Has(rwAns) {
s.logger.InfoContext(ctx, "rewrite cname loop", "host", dReq.Hostname, "rewrite", rwAns)
s.logger.WarnContext(ctx, "rewrite cname loop", "host", dReq.Hostname, "rewrite", rwAns)
return nil
return nil, nil
}
cnames.Add(rwAns)
drules := s.rewriteRulesForReq(&urlfilter.DNSRequest{
rewriteRulesForReq := s.rewriteRulesForReq(&urlfilter.DNSRequest{
Hostname: rwAns,
DNSType: dReq.DNSType,
})
if drules != nil {
rrules = drules
if rewriteRulesForReq != nil {
rewriteRules = rewriteRulesForReq
}
host = rwAns
}
return s.collectDNSRewrites(rrules, dReq.DNSType)
return rewriteRules, nil
}
// isSelfMatchingWildcard returns true when a wildcard rewrite matches its own
// result.
//
// For example, an "*.example.com → sub.example.com" rewrite matching in a loop.
//
// See https://github.com/AdguardTeam/AdGuardHome/issues/4016.
func isSelfMatchingWildcard(host, rwAns, ruleText string) (ok bool) {
return host == rwAns && isWildcard(ruleText)
}
// collectDNSRewrites filters DNSRewrite by question type.

12
scripts/make/go-lint.sh
View File

@ -169,10 +169,6 @@ run_linter gocognit --over='19' \
./internal/home/ \
;
run_linter gocognit --over='15' \
./internal/filtering/ \
;
run_linter gocognit --over='14' \
./internal/dhcpd \
;
@ -181,10 +177,6 @@ run_linter gocognit --over='13' \
./internal/aghnet/ \
;
run_linter gocognit --over='12' \
./internal/filtering/rewrite/ \
;
run_linter gocognit --over='10' \
./internal/aghalg/ \
./internal/aghhttp/ \
@ -198,9 +190,7 @@ run_linter gocognit --over='10' \
./internal/configmigrate/ \
./internal/dhcpsvc \
./internal/dnsforward/ \
./internal/filtering/hashprefix/ \
./internal/filtering/rulelist/ \
./internal/filtering/safesearch/ \
./internal/filtering/ \
./internal/ipset \
./internal/next/ \
./internal/rdns/ \