fix(blocking): bound EDE reason, round-trip regex rules, simplify trie

Follow-up fixes from code review of the matched-rule block reason:

- Cap EDE extra text (maxEDETextLength) so an unbounded matched rule (e.g.
  a long regex) can't bloat the OPT record and push the answer out of a
  size-limited UDP response during dns.Msg.Truncate; the full reason is
  still kept in the query log.
- Report regex rules wrapped in their '/.../' delimiters so the reported
  rule round-trips to the configured entry instead of looking like a
  plain-string rule (raised by Copilot review).
- Reuse the slices.Sorted(maps.Keys(...)) idiom in formatBlockReason and
  return a clean "BLOCKED[ TYPE]" when there are no matches.
- Add trie.JoinTLD as the inverse of SplitTLD so the wildcard cache no
  longer hard-codes the trie's label separator.
- Collapse parent.hasParentOf into polymorphic dispatch: removes the
  duplicated return and the unreachable (uncoverable) fallthrough; trie
  package back to 100% coverage.
- Document the chained-cache one-rule-per-group (last-write-wins) choice.

Author: 0xERR0R

cache/stringcache/chained_grouped_cache.go

@@ -23,6 +23,11 @@ func (c *ChainedGroupedCache) Contains(searchString string, groups []string) map
 	// result is allocated lazily so the common no-match case stays allocation-free.
 	// Ordering of matched groups is not defined here; callers that render the
 	// result sort it (see resolver.formatBlockReason).
+	//
+	// If a group matches in more than one chained cache (e.g. an exact entry and
+	// a wildcard), we keep a single rule per group: the last chained cache wins.
+	// One representative rule per group is enough for the block reason, and the
+	// chain order is fixed, so the choice is stable across requests.
 	var result map[string]string
 
 	for _, cache := range c.caches {

cache/stringcache/in_memory_grouped_cache_test.go

@@ -85,9 +85,9 @@ var _ = Describe("In-Memory grouped cache", func() {
 				Expect(cache.ElementCount("group1")).Should(BeNumerically("==", 1))
 				Expect(cache.Contains("string1", []string{"group1"})).Should(BeEmpty())
 				Expect(cache.Contains("string2", []string{"group1"})).
-					Should(Equal(map[string]string{"group1": "string2"}))
+					Should(Equal(map[string]string{"group1": "/string2/"}))
 				Expect(cache.Contains("shouldalsomatchstring2", []string{"group1"})).
-					Should(Equal(map[string]string{"group1": "string2"}))
+					Should(Equal(map[string]string{"group1": "/string2/"}))
 			})
 		})
 		When("Wildcard grouped cache is used", func() {

cache/stringcache/string_caches.go

@@ -133,7 +133,9 @@ func (cache regexCache) findMatch(searchString string) (string, bool) {
 		if regex.MatchString(searchString) {
 			log.PrefixedLog("regex_cache").Debugf("regex '%s' matched with '%s'", regex, searchString)
 
-			return regex.String(), true
+			// re-wrap in the '/.../' delimiters that addEntry strips on insertion
+			// so the reported rule matches the entry as configured by the user.
+			return "/" + regex.String() + "/", true
 		}
 	}
 
@@ -199,8 +201,10 @@ func (cache wildcardCache) findMatch(domain string) (string, bool) {
 
 	// labels reconstruct the stored wildcard base (normalized, with the "*."
 	// prefix stripped on insertion); re-prepend "*." so the reported rule
-	// matches the entry as configured by the user.
-	rule := "*." + strings.Join(labels, ".")
+	// matches the entry as configured by the user. trie.JoinTLD pairs with the
+	// trie.SplitTLD this cache is built with, so the separator stays the trie's
+	// concern rather than being hard-coded here.
+	rule := "*." + trie.JoinTLD(labels)
 
 	log.PrefixedLog("wildcard_cache").Debugf("wildcard block rule '%s' matched with '%s'", rule, domain)
 

cache/stringcache/string_caches_test.go

@@ -138,7 +138,7 @@ var _ = Describe("Caches", func() {
 			It("should match if one regex in StringCache matches string and return the pattern", func() {
 				rule, ok := cache.findMatch("google.com")
 				Expect(ok).Should(BeTrue())
-				Expect(rule).Should(Equal(".*google.com"))
+				Expect(rule).Should(Equal("/.*google.com/"))
 
 				_, ok = cache.findMatch("google.coma")
 				Expect(ok).Should(BeTrue())
@@ -149,7 +149,7 @@ var _ = Describe("Caches", func() {
 
 				rule, ok = cache.findMatch("apple.com")
 				Expect(ok).Should(BeTrue())
-				Expect(rule).Should(Equal("^apple\\.(de|com)$"))
+				Expect(rule).Should(Equal("/^apple\\.(de|com)$/"))
 
 				_, ok = cache.findMatch("apple.de")
 				Expect(ok).Should(BeTrue())
@@ -162,7 +162,7 @@ var _ = Describe("Caches", func() {
 
 				rule, ok = cache.findMatch("www.amazon.com")
 				Expect(ok).Should(BeTrue())
-				Expect(rule).Should(Equal("amazon"))
+				Expect(rule).Should(Equal("/amazon/"))
 
 				_, ok = cache.findMatch("amazon.com")
 				Expect(ok).Should(BeTrue())

resolver/blocking_resolver.go

@@ -571,21 +571,18 @@ func (r *BlockingResolver) matches(groupsToCheck []string, m lists.Matcher,
 // "BLOCKED[ TYPE] (group: rule[, group: rule...])". Matched groups are sorted
 // so the rendered reason is deterministic when more than one group matches.
 func formatBlockReason(matches map[string]string, typeName string) string {
-	groups := make([]string, 0, len(matches))
-	for group := range matches {
-		groups = append(groups, group)
+	reason := "BLOCKED"
+	if typeName != "" {
+		reason += " " + typeName
 	}
 
-	sort.Strings(groups)
-
-	entries := make([]string, 0, len(groups))
-	for _, group := range groups {
-		entries = append(entries, fmt.Sprintf("%s: %s", group, matches[group]))
+	if len(matches) == 0 {
+		return reason
 	}
 
-	reason := "BLOCKED"
-	if typeName != "" {
-		reason += " " + typeName
+	entries := make([]string, 0, len(matches))
+	for _, group := range slices.Sorted(maps.Keys(matches)) {
+		entries = append(entries, fmt.Sprintf("%s: %s", group, matches[group]))
 	}
 
 	return fmt.Sprintf("%s (%s)", reason, strings.Join(entries, ", "))

resolver/ede_resolver.go

@@ -2,13 +2,22 @@ package resolver
 
 import (
 	"context"
+	"unicode/utf8"
 
 	"github.com/0xERR0R/blocky/config"
 	"github.com/0xERR0R/blocky/model"
 	"github.com/0xERR0R/blocky/util"
 	"github.com/miekg/dns"
 )
 
+// maxEDETextLength bounds the EDE extra text. The reason now embeds the matched
+// denylist rule, which is unbounded for regex entries. dns.Msg.Truncate keeps
+// the OPT record and subtracts its full length from the size budget, so an
+// oversized extra text would shrink (or eliminate) the room left for the actual
+// answer on a size-limited (typically UDP) response. Bounding it here keeps the
+// OPT small; the full reason is still recorded in the query log.
+const maxEDETextLength = 200
+
 // A EDEResolver is responsible for adding the reason for the response as EDNS0 option
 type EDEResolver struct {
 	configurable[*config.EDE]
@@ -53,7 +62,22 @@ func (r *EDEResolver) addExtraReasoning(res *model.Response) {
 
 	edeOption := new(dns.EDNS0_EDE)
 	edeOption.InfoCode = infocode
-	edeOption.ExtraText = res.Reason
+	edeOption.ExtraText = truncateText(res.Reason, maxEDETextLength)
 
 	util.SetEdns0Option(res.Res, edeOption)
 }
+
+// truncateText limits s to at most maxLen bytes, cutting on a UTF-8 rune
+// boundary so it never emits an invalid (partial) rune.
+func truncateText(s string, maxLen int) string {
+	if len(s) <= maxLen {
+		return s
+	}
+
+	cut := maxLen
+	for cut > 0 && !utf8.RuneStart(s[cut]) {
+		cut--
+	}
+
+	return s[:cut]
+}

resolver/ede_resolver_test.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"errors"
 	"math"
+	"strings"
 
 	"github.com/0xERR0R/blocky/config"
 	. "github.com/0xERR0R/blocky/helpertest"
@@ -135,6 +136,30 @@ var _ = Describe("EdeResolver", func() {
 			})
 		})
 
+		When("resolver returns a blocked response with an oversized reason", func() {
+			longReason := "BLOCKED (ads: /" + strings.Repeat("a", 500) + "/)"
+
+			BeforeEach(func() {
+				m = &mockResolver{}
+				m.On("Resolve", mock.Anything).Return(&Response{
+					Res:    mockAnswer,
+					RType:  ResponseTypeBLOCKED,
+					Reason: longReason,
+				}, nil)
+			})
+
+			It("caps the EDE extra text so it can't bloat the OPT record", func() {
+				Expect(sut.Resolve(ctx, newRequest("example.com.", A))).
+					Should(
+						SatisfyAll(
+							HaveEdnsOption(dns.EDNS0EDE),
+							WithTransform(extractEdeOption,
+								WithTransform(func(o dns.EDNS0_EDE) int { return len(o.ExtraText) },
+									BeNumerically("<=", maxEDETextLength))),
+						))
+			})
+		})
+
 		When("resolver returns other", func() {
 			BeforeEach(func() {
 				m = &mockResolver{}

trie/split.go

@@ -18,3 +18,10 @@ func SplitTLD(domain string) (label, rest string) {
 
 	return label, rest
 }
+
+// JoinTLD is the inverse of SplitTLD: it reconstructs an entry from the labels
+// returned by HasParentOf on a trie built with SplitTLD.
+// ["example", "com"] -> "example.com"
+func JoinTLD(labels []string) string {
+	return strings.Join(labels, ".")
+}

trie/split_test.go

@@ -44,3 +44,22 @@ var _ = Describe("SpltTLD", func() {
 		Expect(rest).Should(Equal("www"))
 	})
 })
+
+var _ = Describe("JoinTLD", func() {
+	It("reconstructs an entry from labels in entry order", func() {
+		Expect(JoinTLD([]string{"example", "com"})).Should(Equal("example.com"))
+	})
+
+	It("handles a single label", func() {
+		Expect(JoinTLD([]string{"blocked"})).Should(Equal("blocked"))
+	})
+
+	It("reproduces the labels returned by HasParentOf", func() {
+		sut := NewTrie(SplitTLD)
+		sut.Insert("sub.example.com")
+
+		labels, ok := sut.HasParentOf("a.sub.example.com")
+		Expect(ok).Should(BeTrue())
+		Expect(JoinTLD(labels)).Should(Equal("sub.example.com"))
+	})
+})

trie/trie.go

@@ -122,34 +122,22 @@ func (n *parent) hasParentOf(key string, split SplitFunc) ([]string, bool) {
 		return nil, false
 	}
 
-	switch child := child.(type) {
-	case *parent:
-		if len(rest) == 0 {
-			// The trie only contains children/"suffixes" of the
-			// key we're searching for
-			return nil, false
-		}
-
-		labels, ok := child.hasParentOf(rest, split)
-		if !ok {
-			return nil, false
-		}
-
-		// On the matching path only: prepend-by-append this node's label.
-		// The deeper labels were collected first, so appending the current
-		// (more significant) label keeps the entry's natural order.
-		return append(labels, label), true
-
-	case terminal:
-		labels, ok := child.hasParentOf(rest, split)
-		if !ok {
-			return nil, false
-		}
+	// A *parent child means the trie only stores longer entries/"suffixes" below
+	// this node; if the search key has no more labels, none of them can be a
+	// parent of it.
+	if _, isParent := child.(*parent); isParent && len(rest) == 0 {
+		return nil, false
+	}
 
-		return append(labels, label), true
+	labels, ok := child.hasParentOf(rest, split)
+	if !ok {
+		return nil, false
 	}
 
-	return nil, false
+	// On the matching path only: prepend-by-append this node's label. The deeper
+	// labels were collected first, so appending the current (more significant)
+	// label keeps the entry's natural order.
+	return append(labels, label), true
 }
 
 type terminal string