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.
158 lines
3.5 KiB
Go
158 lines
3.5 KiB
Go
package rule
|
|
|
|
import (
|
|
"context"
|
|
"testing"
|
|
|
|
"github.com/sagernet/sing-box/adapter"
|
|
C "github.com/sagernet/sing-box/constant"
|
|
"github.com/sagernet/sing/common/x/list"
|
|
|
|
"github.com/stretchr/testify/require"
|
|
"go4.org/netipx"
|
|
)
|
|
|
|
type fakeRuleSet struct {
|
|
matched bool
|
|
}
|
|
|
|
func (f *fakeRuleSet) Name() string {
|
|
return "fake-rule-set"
|
|
}
|
|
|
|
func (f *fakeRuleSet) StartContext(context.Context, *adapter.HTTPStartContext) error {
|
|
return nil
|
|
}
|
|
|
|
func (f *fakeRuleSet) PostStart() error {
|
|
return nil
|
|
}
|
|
|
|
func (f *fakeRuleSet) Metadata() adapter.RuleSetMetadata {
|
|
return adapter.RuleSetMetadata{}
|
|
}
|
|
|
|
func (f *fakeRuleSet) ExtractIPSet() []*netipx.IPSet {
|
|
return nil
|
|
}
|
|
|
|
func (f *fakeRuleSet) IncRef() {}
|
|
|
|
func (f *fakeRuleSet) DecRef() {}
|
|
|
|
func (f *fakeRuleSet) Cleanup() {}
|
|
|
|
func (f *fakeRuleSet) RegisterCallback(adapter.RuleSetUpdateCallback) *list.Element[adapter.RuleSetUpdateCallback] {
|
|
return nil
|
|
}
|
|
|
|
func (f *fakeRuleSet) UnregisterCallback(*list.Element[adapter.RuleSetUpdateCallback]) {}
|
|
|
|
func (f *fakeRuleSet) Close() error {
|
|
return nil
|
|
}
|
|
|
|
func (f *fakeRuleSet) Match(*adapter.InboundContext) bool {
|
|
return f.matched
|
|
}
|
|
|
|
func (f *fakeRuleSet) String() string {
|
|
return "fake-rule-set"
|
|
}
|
|
|
|
type fakeRuleItem struct {
|
|
matched bool
|
|
}
|
|
|
|
func (f *fakeRuleItem) Match(*adapter.InboundContext) bool {
|
|
return f.matched
|
|
}
|
|
|
|
func (f *fakeRuleItem) String() string {
|
|
return "fake-rule-item"
|
|
}
|
|
|
|
func newRuleSetOnlyRule(ruleSetMatched bool, invert bool) *DefaultRule {
|
|
ruleSetItem := &RuleSetItem{
|
|
setList: []adapter.RuleSet{&fakeRuleSet{matched: ruleSetMatched}},
|
|
}
|
|
return &DefaultRule{
|
|
abstractDefaultRule: abstractDefaultRule{
|
|
ruleSetItem: ruleSetItem,
|
|
allItems: []RuleItem{ruleSetItem},
|
|
invert: invert,
|
|
},
|
|
}
|
|
}
|
|
|
|
func newSingleItemRule(matched bool) *DefaultRule {
|
|
item := &fakeRuleItem{matched: matched}
|
|
return &DefaultRule{
|
|
abstractDefaultRule: abstractDefaultRule{
|
|
items: []RuleItem{item},
|
|
allItems: []RuleItem{item},
|
|
},
|
|
}
|
|
}
|
|
|
|
func TestAbstractDefaultRule_RuleSetOnly_InvertFalse(t *testing.T) {
|
|
t.Parallel()
|
|
require.True(t, newRuleSetOnlyRule(true, false).Match(&adapter.InboundContext{}))
|
|
require.False(t, newRuleSetOnlyRule(false, false).Match(&adapter.InboundContext{}))
|
|
}
|
|
|
|
func TestAbstractDefaultRule_RuleSetOnly_InvertTrue(t *testing.T) {
|
|
t.Parallel()
|
|
require.False(t, newRuleSetOnlyRule(true, true).Match(&adapter.InboundContext{}))
|
|
require.True(t, newRuleSetOnlyRule(false, true).Match(&adapter.InboundContext{}))
|
|
}
|
|
|
|
func TestAbstractLogicalRule_And_WithRuleSetInvert(t *testing.T) {
|
|
t.Parallel()
|
|
testCases := []struct {
|
|
name string
|
|
aMatched bool
|
|
ruleSetBMatch bool
|
|
expected bool
|
|
}{
|
|
{
|
|
name: "A true B true",
|
|
aMatched: true,
|
|
ruleSetBMatch: true,
|
|
expected: false,
|
|
},
|
|
{
|
|
name: "A true B false",
|
|
aMatched: true,
|
|
ruleSetBMatch: false,
|
|
expected: true,
|
|
},
|
|
{
|
|
name: "A false B true",
|
|
aMatched: false,
|
|
ruleSetBMatch: true,
|
|
expected: false,
|
|
},
|
|
{
|
|
name: "A false B false",
|
|
aMatched: false,
|
|
ruleSetBMatch: false,
|
|
expected: false,
|
|
},
|
|
}
|
|
for _, testCase := range testCases {
|
|
testCase := testCase
|
|
t.Run(testCase.name, func(t *testing.T) {
|
|
t.Parallel()
|
|
logicalRule := &abstractLogicalRule{
|
|
mode: C.LogicalTypeAnd,
|
|
rules: []adapter.HeadlessRule{
|
|
newSingleItemRule(testCase.aMatched),
|
|
newRuleSetOnlyRule(testCase.ruleSetBMatch, true),
|
|
},
|
|
}
|
|
require.Equal(t, testCase.expected, logicalRule.Match(&adapter.InboundContext{}))
|
|
})
|
|
}
|
|
}
|