Address upstream PR feedback: use cfg.Callback, extract function, remove version annotation

Co-authored-by: thisisjaymehta <31812582+thisisjaymehta@users.noreply.github.com>

Author: Copilot

docs/reference/checks/dnsbl.md

@@ -203,8 +203,6 @@ rules is used instead of this flat score value.
 
 ### response _ip..._
 
-**New in 0.8**
-
 Defines per-response-code rules for scoring and custom messages. This is useful
 for combined DNSBLs like Spamhaus ZEN that return different codes for different
 listing types.

internal/check/dnsbl/common.go

@@ -94,6 +94,34 @@ func checkDomain(ctx context.Context, resolver dns.Resolver, cfg List, domain st
 	}
 }
 
+func matchResponseRules(addrs []net.IPAddr, rules []ResponseRule) (score int, messages []string, reasons []string, matched bool) {
+	// Track which rules have been matched to avoid counting the same rule multiple times
+	matchedRules := make(map[int]bool)
+
+	for _, addr := range addrs {
+		for ruleIdx, rule := range rules {
+			// Skip if this rule has already been matched
+			if matchedRules[ruleIdx] {
+				continue
+			}
+			
+			for _, respNet := range rule.Networks {
+				if respNet.Contains(addr.IP) {
+					score += rule.Score
+					if rule.Message != "" {
+						messages = append(messages, rule.Message)
+					}
+					reasons = append(reasons, addr.IP.String())
+					matchedRules[ruleIdx] = true
+					matched = true
+					break // Move to next rule
+				}
+			}
+		}
+	}
+	return
+}
+
 func checkIP(ctx context.Context, resolver dns.Resolver, cfg List, ip net.IP) error {
 	ipv6 := true
 	if ipv4 := ip.To4(); ipv4 != nil {
@@ -119,111 +147,72 @@ func checkIP(ctx context.Context, resolver dns.Resolver, cfg List, ip net.IP) er
 		return err
 	}
 
+	var filteredAddrs []net.IPAddr
+	var score int
+	var customMessage string
+
 	// If ResponseRules is configured, use new behavior
 	if len(cfg.ResponseRules) > 0 {
-		totalScore := 0
-		var matchedMessages []string
-		var matchedReasons []string
-
-		// Track which rules have been matched to avoid counting the same rule multiple times
-		matchedRules := make(map[int]bool)
-
-		for _, addr := range addrs {
-			for ruleIdx, rule := range cfg.ResponseRules {
-				// Skip if this rule has already been matched
-				if matchedRules[ruleIdx] {
-					continue
-				}
-				
-				for _, respNet := range rule.Networks {
-					if respNet.Contains(addr.IP) {
-						totalScore += rule.Score
-						if rule.Message != "" {
-							matchedMessages = append(matchedMessages, rule.Message)
-						}
-						matchedReasons = append(matchedReasons, addr.IP.String())
-						matchedRules[ruleIdx] = true
-						break // Move to next rule
-					}
-				}
-			}
-		}
-
-		if totalScore == 0 {
+		matchedScore, matchedMessages, matchedReasons, matched := matchResponseRules(addrs, cfg.ResponseRules)
+		if !matched {
 			return nil
 		}
-
-		// Attempt to extract explanation string from TXT records
-		txts, err := resolver.LookupTXT(ctx, query)
-		var reason string
-		if err == nil && len(txts) > 0 {
-			reason = strings.Join(txts, "; ")
-		} else {
-			reason = strings.Join(matchedReasons, "; ")
-		}
-
+		score = matchedScore
+		
 		// Use first matched message if available
-		message := ""
 		if len(matchedMessages) > 0 {
-			message = matchedMessages[0]
+			customMessage = matchedMessages[0]
 		}
-
-		return ListedErr{
-			Identity: ip.String(),
-			List:     cfg.Zone,
-			Reason:   reason,
-			Score:    totalScore,
-			Message:  message,
+		
+		// Build filteredAddrs from matched reasons for TXT lookup fallback
+		for _, reason := range matchedReasons {
+			filteredAddrs = append(filteredAddrs, net.IPAddr{IP: net.ParseIP(reason)})
 		}
-	}
-
-	// Legacy behavior: use flat Responses filter
-	filteredAddrs := make([]net.IPAddr, 0, len(addrs))
-addrsLoop:
-	for _, addr := range addrs {
-		// No responses whitelist configured - permit all.
-		if len(cfg.Responses) == 0 {
-			filteredAddrs = append(filteredAddrs, addr)
-			continue
-		}
-
-		for _, respNet := range cfg.Responses {
-			if respNet.Contains(addr.IP) {
+	} else {
+		// Legacy behavior: use flat Responses filter
+		filteredAddrs = make([]net.IPAddr, 0, len(addrs))
+	addrsLoop:
+		for _, addr := range addrs {
+			// No responses whitelist configured - permit all.
+			if len(cfg.Responses) == 0 {
 				filteredAddrs = append(filteredAddrs, addr)
-				continue addrsLoop
+				continue
+			}
+
+			for _, respNet := range cfg.Responses {
+				if respNet.Contains(addr.IP) {
+					filteredAddrs = append(filteredAddrs, addr)
+					continue addrsLoop
+				}
 			}
 		}
-	}
 
-	if len(filteredAddrs) == 0 {
-		return nil
+		if len(filteredAddrs) == 0 {
+			return nil
+		}
 	}
 
-	// Attempt to extract explanation string.
+	// Attempt to extract explanation string from TXT records (shared by both paths)
 	txts, err := resolver.LookupTXT(ctx, query)
-	if err != nil || len(txts) == 0 {
+	var reason string
+	if err == nil && len(txts) > 0 {
+		reason = strings.Join(txts, "; ")
+	} else {
 		// Not significant, include addresses as reason. Usually they are
 		// mapped to some predefined 'reasons' by BL.
-
 		reasonParts := make([]string, 0, len(filteredAddrs))
 		for _, addr := range filteredAddrs {
 			reasonParts = append(reasonParts, addr.IP.String())
 		}
-
-		return ListedErr{
-			Identity: ip.String(),
-			List:     cfg.Zone,
-			Reason:   strings.Join(reasonParts, "; "),
-		}
+		reason = strings.Join(reasonParts, "; ")
 	}
 
-	// Some BLs provide multiple reasons (meta-BLs such as Spamhaus Zen) so
-	// don't mangle them by joining with "", instead join with "; ".
-
 	return ListedErr{
 		Identity: ip.String(),
 		List:     cfg.Zone,
-		Reason:   strings.Join(txts, "; "),
+		Reason:   reason,
+		Score:    score,
+		Message:  customMessage,
 	}
 }
 

internal/check/dnsbl/dnsbl.go

@@ -134,6 +134,14 @@ func (bl *DNSBL) readListCfg(node config.Node) error {
 	cfg.Bool("mailfrom", false, defaultBL.EHLO, &listCfg.MAILFROM)
 	cfg.Int("score", false, false, 1, &listCfg.ScoreAdj)
 	cfg.StringList("responses", false, false, []string{"127.0.0.1/24"}, &responseNets)
+	cfg.Callback("response", func(_ *config.Map, node config.Node) error {
+		rule, err := parseResponseRule(node)
+		if err != nil {
+			return err
+		}
+		listCfg.ResponseRules = append(listCfg.ResponseRules, rule)
+		return nil
+	})
 	if _, err := cfg.Process(); err != nil {
 		return err
 	}
@@ -152,17 +160,9 @@ func (bl *DNSBL) readListCfg(node config.Node) error {
 		listCfg.Responses = append(listCfg.Responses, *ipNet)
 	}
 
-	// Parse response blocks from node children
-	for _, child := range node.Children {
-		if child.Name == "response" {
-			rule, err := parseResponseRule(child)
-			if err != nil {
-				return err
-			}
-			listCfg.ResponseRules = append(listCfg.ResponseRules, rule)
-		} else {
-			return config.NodeErr(child, "unknown directive: %s", child.Name)
-		}
+	// Warn if both response and responses are configured
+	if len(listCfg.ResponseRules) > 0 && len(responseNets) > 0 {
+		bl.log.Msg("both 'response' blocks and 'responses' directive are specified, 'response' blocks take precedence", "list", node.Name)
 	}
 
 	for _, zone := range append([]string{node.Name}, node.Args...) {