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

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 8b1a1904e138d6b16a2a8abd23afc368b74605af
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 2f9d480b4aca719e43d55427361b5a77a98d13cf
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Mon Jul 28 21:10:01 2025 +0300

    filtering: imp gocognit

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

    filtering: imp docs

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

    filtering: imp docs

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

    bamboo-specs: fix ci

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

    all: filtering imp gocognit

internal/filtering/filter.go

@@ -144,21 +144,22 @@ func (d *DNSFilter) filterSetProperties(
 		shouldRestart = true
 	}
 
-	if flt.Enabled {
+	if !flt.Enabled {
-		if shouldRestart {
-			// Download the filter contents.
-			shouldRestart, err = d.update(flt)
-		}
-	} else {
 		// 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
+	failNum, updateFlags := d.updateFilterList(ctx, updateFilters)
-	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
-		}
-	}
-
 	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 {
+	if updNum == 0 {
-		d.EnableFilters(false)
+		return 0, false
+	}
 
-		for i := range lists {
+	d.EnableFilters(false)
-			uf := &lists[i]
-			updated := toUpd[i]
-			if !updated {
-				continue
-			}
 
-			p := uf.Path(d.conf.DataDir)
+	for i := range lists {
-			err := os.Remove(p + ".old")
+		uf := &lists[i]
-			if err != nil {
+		updated := toUpd[i]
-				d.logger.ErrorContext(ctx, "removing old filter", "path", p, slogutil.KeyError, err)
+		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)
 		}
 	}
 

internal/filtering/filtering.go

@@ -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); {
+		var rl filterlist.RuleList
-		case len(f.Data) != 0:
+		var skip bool
-			lists = append(lists, &filterlist.StringRuleList{
+		rl, skip, err = ruleListFromFilter(f)
-				ID:             id,
+		if skip {
-				RulesText:      string(f.Data),
-				IgnoreCosmetic: true,
-			})
-		case f.FilePath == "":
 			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 {
+	if result, ok := resultFromHostRules(qtype, dnsres); ok {
-	case dns.TypeA:
+		return result
-		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.
 	}
 
 	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 {
+	err = d.validateSafeFSPatterns(c.SafeFSPatterns)
-		// Use Match to validate the patterns here.
+	if err != nil {
-		_, err = filepath.Match(p, "test")
+		// Don't wrap the error, because it's informative enough as is.
-		if err != nil {
+		return nil, err
-			return nil, fmt.Errorf("safe_fs_patterns: at index %d: %w", i, err)
-		}
-
-		d.safeFSPatterns = append(d.safeFSPatterns, p)
 	}
 
 	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)

internal/filtering/rewrite/storage.go

@@ -97,16 +97,38 @@ func (s *DefaultStorage) MatchRequest(dReq *urlfilter.DNSRequest) (rws []*rules.
 
 	ctx := context.TODO()
 
-	rrules := s.rewriteRulesForReq(dReq)
+	rewriteRules := s.rewriteRulesForReq(dReq)
-	if len(rrules) == 0 {
+	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 != "" {
+	for len(rewriteRules) > 0 &&
-		rule := rrules[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) {
+		if isSelfMatchingWildcard(host, rwAns, rule.RuleText) {
-			// An "*.example.com → sub.example.com" rewrite matching in a loop.
+			return nil, rule.DNSRewrite
-			//
-			// See https://github.com/AdguardTeam/AdGuardHome/issues/4016.
-
-			return []*rules.DNSRewrite{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 {
+		if rewriteRulesForReq != nil {
-			rrules = drules
+			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.

scripts/make/go-lint.sh

@@ -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/ \
-	./internal/filtering/rulelist/ \
-	./internal/filtering/safesearch/ \
 	./internal/ipset \
 	./internal/next/ \
 	./internal/rdns/ \