From 8ec1f77ac8636de24d7a175e245352183c10dd2d Mon Sep 17 00:00:00 2001 From: Ivan Kozlovic Date: Thu, 31 Aug 2017 18:48:45 -0600 Subject: [PATCH] [FIXED] Sublist Insert and Remove with wildcard characters in literals This is similar to #561 where `*` and `>` characters appear in tokens as literals, not wilcards. Both Insert() and Remove() were checking that the first character was `*` or `>` and consider it a wildcard node. This is wrong. Any token that is more than 1 character long must be treated as a literal. Only for token of size one should we check if the character is `*` or `>`. Added a test case for Insert and Remove with subject like `foo.*-` or `foo.>-`. --- server/sublist.go | 58 ++++++++++++++++++++++++++---------------- server/sublist_test.go | 34 +++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 22 deletions(-) diff --git a/server/sublist.go b/server/sublist.go index ae5b48e1..4df3927e 100644 --- a/server/sublist.go +++ b/server/sublist.go @@ -102,29 +102,38 @@ func (s *Sublist) Insert(sub *subscription) error { var n *node for _, t := range tokens { - if len(t) == 0 || sfwc { + lt := len(t) + if lt == 0 || sfwc { s.Unlock() return ErrInvalidSubject } - switch t[0] { - case pwc: - n = l.pwc - case fwc: - n = l.fwc - sfwc = true - default: + if lt > 1 { n = l.nodes[t] + } else { + switch t[0] { + case pwc: + n = l.pwc + case fwc: + n = l.fwc + sfwc = true + default: + n = l.nodes[t] + } } if n == nil { n = newNode() - switch t[0] { - case pwc: - l.pwc = n - case fwc: - l.fwc = n - default: + if lt > 1 { l.nodes[t] = n + } else { + switch t[0] { + case pwc: + l.pwc = n + case fwc: + l.fwc = n + default: + l.nodes[t] = n + } } } if n.next == nil { @@ -334,20 +343,25 @@ func (s *Sublist) Remove(sub *subscription) error { levels := lnts[:0] for _, t := range tokens { - if len(t) == 0 || sfwc { + lt := len(t) + if lt == 0 || sfwc { return ErrInvalidSubject } if l == nil { return ErrNotFound } - switch t[0] { - case pwc: - n = l.pwc - case fwc: - n = l.fwc - sfwc = true - default: + if lt > 1 { n = l.nodes[t] + } else { + switch t[0] { + case pwc: + n = l.pwc + case fwc: + n = l.fwc + sfwc = true + default: + n = l.nodes[t] + } } if n != nil { levels = append(levels, lnt{l, n, t}) diff --git a/server/sublist_test.go b/server/sublist_test.go index a5229133..a7df7b15 100644 --- a/server/sublist_test.go +++ b/server/sublist_test.go @@ -475,6 +475,40 @@ func TestSublistTwoTokenPubMatchSingleTokenSub(t *testing.T) { verifyLen(r.psubs, 0, t) } +func TestSublistInsertWithWildcardsAsLiterals(t *testing.T) { + s := NewSublist() + subjects := []string{"foo.*-", "foo.>-"} + for _, subject := range subjects { + sub := newSub(subject) + s.Insert(sub) + // Should find no match + r := s.Match("foo.bar") + verifyLen(r.psubs, 0, t) + // Should find a match + r = s.Match(subject) + verifyLen(r.psubs, 1, t) + } +} + +func TestSublistRemoveWithWildcardsAsLiterals(t *testing.T) { + s := NewSublist() + subjects := []string{"foo.*-", "foo.>-"} + for _, subject := range subjects { + sub := newSub(subject) + s.Insert(sub) + // Should find no match + rsub := newSub("foo.bar") + s.Remove(rsub) + if c := s.Count(); c != 1 { + t.Fatalf("Expected sublist to still contain sub, got %v", c) + } + s.Remove(sub) + if c := s.Count(); c != 0 { + t.Fatalf("Expected sublist to be empty, got %v", c) + } + } +} + // -- Benchmarks Setup -- var subs []*subscription