mirror of
https://github.com/shtorm-7/sing-box-extended.git
synced 2026-07-26 00:53:04 +03:00
route: formalize nested rule_set group-state semantics
Before795d1c289, nested rule-set evaluation reused the parent rule match cache. In practice, this meant these fields leaked across nested evaluation: - SourceAddressMatch - SourcePortMatch - DestinationAddressMatch - DestinationPortMatch - DidMatch That leak had two opposite effects. First, it made included rule-sets partially behave like the docs' "merged" semantics. For example, if an outer route rule had: rule_set = ["geosite-additional-!cn"] ip_cidr = 104.26.10.0/24 and the inline rule-set matched `domain_suffix = speedtest.net`, the inner match could set `DestinationAddressMatch = true` and the outer rule would then pass its destination-address group check. This is why some `rule_set + ip_cidr` combinations used to work. But the same leak also polluted sibling rules and sibling rule-sets. A branch could partially match one group, then fail later, and still leave that group cache set for the next branch. This broke cases such as gh-3485: with `rule_set = [test1, test2]`, `test1` could touch destination-address cache before an AdGuard `@@` exclusion made the whole branch fail, and `test2` would then run against dirty state.795d1c289fixed that by cloning metadata for nested rule-set/rule evaluation and resetting the rule match cache for each branch. That stopped sibling pollution, but it also removed the only mechanism by which a successful nested branch could affect the parent rule's grouped matching state. As a result, nested rule-sets became pure boolean sub-items against the outer rule. The previous example stopped working: the inner `domain_suffix = speedtest.net` still matched, but the outer rule no longer observed any destination-address-group success, so it fell through to `final`. This change makes the semantics explicit instead of relying on cache side effects: - `rule_set: ["a", "b"]` is OR - rules inside one rule-set are OR - each nested branch is evaluated in isolation - failed branches contribute no grouped match state - a successful branch contributes its grouped match state back to the parent rule - grouped state from different rule-sets must not be combined together to satisfy one outer rule In other words, rule-sets now behave as "OR branches whose successful group matches merge into the outer rule", which matches the documented intent without reintroducing cross-branch cache leakage.
This commit is contained in:
@@ -47,6 +47,10 @@ type DefaultDNSRule struct {
|
||||
abstractDefaultRule
|
||||
}
|
||||
|
||||
func (r *DefaultDNSRule) matchStates(metadata *adapter.InboundContext) ruleMatchStateSet {
|
||||
return r.abstractDefaultRule.matchStates(metadata)
|
||||
}
|
||||
|
||||
func NewDefaultDNSRule(ctx context.Context, logger log.ContextLogger, options option.DefaultDNSRule) (*DefaultDNSRule, error) {
|
||||
rule := &DefaultDNSRule{
|
||||
abstractDefaultRule: abstractDefaultRule{
|
||||
@@ -271,7 +275,7 @@ func NewDefaultDNSRule(ctx context.Context, logger log.ContextLogger, options op
|
||||
matchSource = true
|
||||
}
|
||||
item := NewRuleSetItem(router, options.RuleSet, matchSource, options.RuleSetIPCIDRAcceptEmpty)
|
||||
rule.items = append(rule.items, item)
|
||||
rule.ruleSetItem = item
|
||||
rule.allItems = append(rule.allItems, item)
|
||||
}
|
||||
return rule, nil
|
||||
@@ -285,12 +289,9 @@ func (r *DefaultDNSRule) WithAddressLimit() bool {
|
||||
if len(r.destinationIPCIDRItems) > 0 {
|
||||
return true
|
||||
}
|
||||
for _, rawRule := range r.items {
|
||||
ruleSet, isRuleSet := rawRule.(*RuleSetItem)
|
||||
if !isRuleSet {
|
||||
continue
|
||||
}
|
||||
if ruleSet.ContainsDestinationIPCIDRRule() {
|
||||
if r.ruleSetItem != nil {
|
||||
ruleSet, isRuleSet := r.ruleSetItem.(*RuleSetItem)
|
||||
if isRuleSet && ruleSet.ContainsDestinationIPCIDRRule() {
|
||||
return true
|
||||
}
|
||||
}
|
||||
@@ -302,11 +303,11 @@ func (r *DefaultDNSRule) Match(metadata *adapter.InboundContext) bool {
|
||||
defer func() {
|
||||
metadata.IgnoreDestinationIPCIDRMatch = false
|
||||
}()
|
||||
return r.abstractDefaultRule.Match(metadata)
|
||||
return !r.matchStates(metadata).isEmpty()
|
||||
}
|
||||
|
||||
func (r *DefaultDNSRule) MatchAddressLimit(metadata *adapter.InboundContext) bool {
|
||||
return r.abstractDefaultRule.Match(metadata)
|
||||
return !r.matchStates(metadata).isEmpty()
|
||||
}
|
||||
|
||||
var _ adapter.DNSRule = (*LogicalDNSRule)(nil)
|
||||
@@ -315,6 +316,10 @@ type LogicalDNSRule struct {
|
||||
abstractLogicalRule
|
||||
}
|
||||
|
||||
func (r *LogicalDNSRule) matchStates(metadata *adapter.InboundContext) ruleMatchStateSet {
|
||||
return r.abstractLogicalRule.matchStates(metadata)
|
||||
}
|
||||
|
||||
func NewLogicalDNSRule(ctx context.Context, logger log.ContextLogger, options option.LogicalDNSRule) (*LogicalDNSRule, error) {
|
||||
r := &LogicalDNSRule{
|
||||
abstractLogicalRule: abstractLogicalRule{
|
||||
@@ -362,29 +367,13 @@ func (r *LogicalDNSRule) WithAddressLimit() bool {
|
||||
}
|
||||
|
||||
func (r *LogicalDNSRule) Match(metadata *adapter.InboundContext) bool {
|
||||
if r.mode == C.LogicalTypeAnd {
|
||||
return common.All(r.rules, func(it adapter.HeadlessRule) bool {
|
||||
metadata.ResetRuleCache()
|
||||
return it.(adapter.DNSRule).Match(metadata)
|
||||
}) != r.invert
|
||||
} else {
|
||||
return common.Any(r.rules, func(it adapter.HeadlessRule) bool {
|
||||
metadata.ResetRuleCache()
|
||||
return it.(adapter.DNSRule).Match(metadata)
|
||||
}) != r.invert
|
||||
}
|
||||
metadata.IgnoreDestinationIPCIDRMatch = true
|
||||
defer func() {
|
||||
metadata.IgnoreDestinationIPCIDRMatch = false
|
||||
}()
|
||||
return !r.matchStates(metadata).isEmpty()
|
||||
}
|
||||
|
||||
func (r *LogicalDNSRule) MatchAddressLimit(metadata *adapter.InboundContext) bool {
|
||||
if r.mode == C.LogicalTypeAnd {
|
||||
return common.All(r.rules, func(it adapter.HeadlessRule) bool {
|
||||
metadata.ResetRuleCache()
|
||||
return it.(adapter.DNSRule).MatchAddressLimit(metadata)
|
||||
}) != r.invert
|
||||
} else {
|
||||
return common.Any(r.rules, func(it adapter.HeadlessRule) bool {
|
||||
metadata.ResetRuleCache()
|
||||
return it.(adapter.DNSRule).MatchAddressLimit(metadata)
|
||||
}) != r.invert
|
||||
}
|
||||
return !r.matchStates(metadata).isEmpty()
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user