From 5e97d5e296b8f38d41e9bd3bdf15d7e2bac6fe81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E4=B8=96=E7=95=8C?= Date: Wed, 1 Apr 2026 18:50:19 +0800 Subject: [PATCH] Fix evaluate response-match validation --- dns/router.go | 19 ++-- dns/router_test.go | 134 +++++++++++++++++++++++ docs/configuration/dns/rule_action.md | 5 +- docs/configuration/dns/rule_action.zh.md | 5 +- 4 files changed, 151 insertions(+), 12 deletions(-) diff --git a/dns/router.go b/dns/router.go index f3167cfab..fd86319ab 100644 --- 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 } diff --git a/dns/router_test.go b/dns/router_test.go index 8d14ef138..e436db7f4 100644 --- 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() diff --git a/docs/configuration/dns/rule_action.md b/docs/configuration/dns/rule_action.md index 20471d3ee..b64f7c02d 100644 --- 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 diff --git a/docs/configuration/dns/rule_action.zh.md b/docs/configuration/dns/rule_action.zh.md index 8e0f8eff8..0ff9d0c08 100644 --- 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