Fix evaluate response-match validation

2026-04-13 20:28:32 +10:00 · 2026-04-01 18:50:19 +08:00
parent eb4c550da2
commit 5e97d5e296
4 changed files with 151 additions and 12 deletions
--- a/dns/router.go
+++ b/dns/router.go
@@ -1062,14 +1062,17 @@ func referencedDNSRuleSetTags(rules []option.DNSRule) []string {
 }

 func validateLegacyDNSModeDisabledRules(rules []option.DNSRule) error {
+	var seenEvaluate bool
 	for i, rule := range rules {
-		consumesResponse, err := validateLegacyDNSModeDisabledRuleTree(rule)
+		requiresPriorEvaluate, err := validateLegacyDNSModeDisabledRuleTree(rule)
 		if err != nil {
 			return E.Cause(err, "validate dns rule[", i, "]")
 		}
-		action := dnsRuleActionType(rule)
-		if action == C.RuleActionTypeEvaluate && consumesResponse {
-			return E.New("dns rule[", i, "]: evaluate action cannot be used with match_response in the same rule")
+		if requiresPriorEvaluate && !seenEvaluate {
+			return E.New("dns rule[", i, "]: response-based matching requires a preceding evaluate action")
+		}
+		if dnsRuleActionType(rule) == C.RuleActionTypeEvaluate {
+			seenEvaluate = true
 		}
 	}
 	return nil
@@ -1080,15 +1083,15 @@ func validateLegacyDNSModeDisabledRuleTree(rule option.DNSRule) (bool, error) {
 	case "", C.RuleTypeDefault:
 		return validateLegacyDNSModeDisabledDefaultRule(rule.DefaultOptions)
 	case C.RuleTypeLogical:
-		var consumesResponse bool
+		var requiresPriorEvaluate bool
 		for i, subRule := range rule.LogicalOptions.Rules {
-			subConsumesResponse, err := validateLegacyDNSModeDisabledRuleTree(subRule)
+			subRequiresPriorEvaluate, err := validateLegacyDNSModeDisabledRuleTree(subRule)
 			if err != nil {
 				return false, E.Cause(err, "sub rule[", i, "]")
 			}
-			consumesResponse = consumesResponse || subConsumesResponse
+			requiresPriorEvaluate = requiresPriorEvaluate || subRequiresPriorEvaluate
 		}
-		return consumesResponse, nil
+		return requiresPriorEvaluate, nil
 	default:
 		return false, nil
 	}
--- a/dns/router_test.go
+++ b/dns/router_test.go
@@ -2163,6 +2163,140 @@ func TestInitializeRejectsDNSRuleStrategyWhenLegacyDNSModeIsDisabledByMatchRespo
 	require.ErrorContains(t, err, "deprecated")
 }

+func TestInitializeRejectsDNSMatchResponseWithoutPrecedingEvaluate(t *testing.T) {
+	t.Parallel()
+
+	router := &Router{
+		ctx:                   context.Background(),
+		logger:                log.NewNOPFactory().NewLogger("dns"),
+		transport:             &fakeDNSTransportManager{},
+		client:                &fakeDNSClient{},
+		rawRules:              make([]option.DNSRule, 0, 1),
+		defaultDomainStrategy: C.DomainStrategyAsIS,
+	}
+	router.currentRules.Store(newRulesSnapshot(make([]adapter.DNSRule, 0, 1), false))
+	err := router.Initialize([]option.DNSRule{{
+		Type: C.RuleTypeDefault,
+		DefaultOptions: option.DefaultDNSRule{
+			RawDefaultDNSRule: option.RawDefaultDNSRule{
+				MatchResponse:  true,
+				ResponseAnswer: badoption.Listable[option.DNSRecordOptions]{mustRecord(t, "example.com. IN A 1.1.1.1")},
+			},
+			DNSRuleAction: option.DNSRuleAction{
+				Action:       C.RuleActionTypeRoute,
+				RouteOptions: option.DNSRouteActionOptions{Server: "default"},
+			},
+		},
+	}})
+	require.ErrorContains(t, err, "preceding evaluate action")
+}
+
+func TestInitializeRejectsEvaluateRuleWithResponseMatchWithoutPrecedingEvaluate(t *testing.T) {
+	t.Parallel()
+
+	router := &Router{
+		ctx:                   context.Background(),
+		logger:                log.NewNOPFactory().NewLogger("dns"),
+		transport:             &fakeDNSTransportManager{},
+		client:                &fakeDNSClient{},
+		rawRules:              make([]option.DNSRule, 0, 1),
+		defaultDomainStrategy: C.DomainStrategyAsIS,
+	}
+	router.currentRules.Store(newRulesSnapshot(make([]adapter.DNSRule, 0, 1), false))
+	err := router.Initialize([]option.DNSRule{{
+		Type: C.RuleTypeLogical,
+		LogicalOptions: option.LogicalDNSRule{
+			RawLogicalDNSRule: option.RawLogicalDNSRule{
+				Mode: C.LogicalTypeOr,
+				Rules: []option.DNSRule{
+					{
+						Type: C.RuleTypeDefault,
+						DefaultOptions: option.DefaultDNSRule{
+							RawDefaultDNSRule: option.RawDefaultDNSRule{
+								Domain: badoption.Listable[string]{"example.com"},
+							},
+						},
+					},
+					{
+						Type: C.RuleTypeDefault,
+						DefaultOptions: option.DefaultDNSRule{
+							RawDefaultDNSRule: option.RawDefaultDNSRule{
+								MatchResponse:  true,
+								ResponseAnswer: badoption.Listable[option.DNSRecordOptions]{mustRecord(t, "example.com. IN A 1.1.1.1")},
+							},
+						},
+					},
+				},
+			},
+			DNSRuleAction: option.DNSRuleAction{
+				Action:       C.RuleActionTypeEvaluate,
+				RouteOptions: option.DNSRouteActionOptions{Server: "default"},
+			},
+		},
+	}})
+	require.ErrorContains(t, err, "preceding evaluate action")
+}
+
+func TestInitializeAllowsEvaluateRuleWithResponseMatchAfterPrecedingEvaluate(t *testing.T) {
+	t.Parallel()
+
+	router := &Router{
+		ctx:                   context.Background(),
+		logger:                log.NewNOPFactory().NewLogger("dns"),
+		transport:             &fakeDNSTransportManager{},
+		client:                &fakeDNSClient{},
+		rawRules:              make([]option.DNSRule, 0, 2),
+		defaultDomainStrategy: C.DomainStrategyAsIS,
+	}
+	router.currentRules.Store(newRulesSnapshot(make([]adapter.DNSRule, 0, 2), false))
+	err := router.Initialize([]option.DNSRule{
+		{
+			Type: C.RuleTypeDefault,
+			DefaultOptions: option.DefaultDNSRule{
+				RawDefaultDNSRule: option.RawDefaultDNSRule{
+					Domain: badoption.Listable[string]{"bootstrap.example"},
+				},
+				DNSRuleAction: option.DNSRuleAction{
+					Action:       C.RuleActionTypeEvaluate,
+					RouteOptions: option.DNSRouteActionOptions{Server: "bootstrap"},
+				},
+			},
+		},
+		{
+			Type: C.RuleTypeLogical,
+			LogicalOptions: option.LogicalDNSRule{
+				RawLogicalDNSRule: option.RawLogicalDNSRule{
+					Mode: C.LogicalTypeOr,
+					Rules: []option.DNSRule{
+						{
+							Type: C.RuleTypeDefault,
+							DefaultOptions: option.DefaultDNSRule{
+								RawDefaultDNSRule: option.RawDefaultDNSRule{
+									Domain: badoption.Listable[string]{"example.com"},
+								},
+							},
+						},
+						{
+							Type: C.RuleTypeDefault,
+							DefaultOptions: option.DefaultDNSRule{
+								RawDefaultDNSRule: option.RawDefaultDNSRule{
+									MatchResponse:  true,
+									ResponseAnswer: badoption.Listable[option.DNSRecordOptions]{mustRecord(t, "example.com. IN A 1.1.1.1")},
+								},
+							},
+						},
+					},
+				},
+				DNSRuleAction: option.DNSRuleAction{
+					Action:       C.RuleActionTypeEvaluate,
+					RouteOptions: option.DNSRouteActionOptions{Server: "default"},
+				},
+			},
+		},
+	})
+	require.NoError(t, err)
+}
+
 func TestLookupLegacyDNSModeDisabledReturnsRejectedErrorForRejectAction(t *testing.T) {
 	t.Parallel()

--- a/docs/configuration/dns/rule_action.md
+++ b/docs/configuration/dns/rule_action.md
@@ -82,8 +82,9 @@ to match against using [`match_response`](/configuration/dns/rule/#match_respons
 Unlike `route`, it does **not** terminate rule evaluation.

 Only allowed on top-level DNS rules (not inside logical sub-rules).
-The rule itself must not use `match_response` or contain sub-rules with Response Match Fields,
-since `evaluate` populates the response for subsequent rules to consume.
+Rules that use [`match_response`](/configuration/dns/rule/#match_response) or Response Match Fields
+require a preceding top-level rule with `evaluate` action. A rule's own `evaluate` action
+does not satisfy this requirement, because matching happens before the action runs.

 #### server

--- a/docs/configuration/dns/rule_action.zh.md
+++ b/docs/configuration/dns/rule_action.zh.md
@@ -80,8 +80,9 @@ icon: material/new-box
 `evaluate` 向指定服务器发送 DNS 查询并保存响应，供后续规则通过 [`match_response`](/zh/configuration/dns/rule/#match_response) 和响应字段进行匹配。与 `route` 不同，它**不会**终止规则评估。

 仅允许在顶层 DNS 规则中使用（不可在逻辑子规则内部使用）。
-该规则本身不可使用 `match_response` 或包含带有响应匹配字段的子规则，
-因为 `evaluate` 是为后续规则填充响应数据。
+使用 [`match_response`](/zh/configuration/dns/rule/#match_response) 或响应匹配字段的规则，
+需要位于更早的顶层 `evaluate` 规则之后。规则自身的 `evaluate` 动作不能满足这个条件，
+因为匹配发生在动作执行之前。

 #### server