From 14aa85482e0e2baf2c515adb29ab6a7718150a44 Mon Sep 17 00:00:00 2001 From: Mechiel Lukkien Date: Tue, 23 Jan 2024 21:21:08 +0100 Subject: [PATCH] imapserver: fix interpreting the first "*" in sequence/uid patterns, like "*:123" or plain "*" in some cases, they were interpreted as meaning "the first sequence/uid", but it should always be "the last sequence/uid", just like patterns of the form "123:*". this wrong interpretation was used in the "fetch" command when combined with "changedsince", and in the search command for some parameters, and during expunge with an explicit uid range. the form "*" and "*:123" aren't very common. --- imapserver/protocol.go | 77 +++++++++++++++++-------------------- imapserver/protocol_test.go | 49 ++++++++++++++++------- 2 files changed, 71 insertions(+), 55 deletions(-) diff --git a/imapserver/protocol.go b/imapserver/protocol.go index c8535a7..716217f 100644 --- a/imapserver/protocol.go +++ b/imapserver/protocol.go @@ -12,6 +12,16 @@ type numSet struct { ranges []numRange } +type numRange struct { + first setNumber + last *setNumber // if nil, this numRange is just a setNumber in "first" and first.star will be false +} + +type setNumber struct { + number uint32 + star bool // References last message (max sequence number/uid). ../rfc/9051:799 +} + // containsSeq returns whether seq is in the numSet, given uids and (saved) searchResult. // uids and searchResult must be sorted. searchResult can have uids that are no longer in uids. func (ss numSet) containsSeq(seq msgseq, uids []store.UID, searchResult []store.UID) bool { @@ -24,19 +34,21 @@ func (ss numSet) containsSeq(seq msgseq, uids []store.UID, searchResult []store. } for _, r := range ss.ranges { first := r.first.number - if r.first.star { - first = 1 + if r.first.star || first > uint32(len(uids)) { + first = uint32(len(uids)) } + last := first if r.last != nil { last = r.last.number - if r.last.star { + if r.last.star || last > uint32(len(uids)) { last = uint32(len(uids)) } } - if last > uint32(len(uids)) { - last = uint32(len(uids)) + if first > last { + first, last = last, first } + if uint32(seq) >= first && uint32(seq) <= last { return true } @@ -53,27 +65,22 @@ func (ss numSet) containsUID(uid store.UID, uids []store.UID, searchResult []sto } for _, r := range ss.ranges { first := store.UID(r.first.number) - if r.first.star { - first = uids[0] + if r.first.star || first > uids[len(uids)-1] { + first = uids[len(uids)-1] } last := first // Num in :* can be larger than last, but it still matches the last... // Similar for *:. ../rfc/9051:4814 if r.last != nil { last = store.UID(r.last.number) - if r.last.star { + if r.last.star || last > uids[len(uids)-1] { last = uids[len(uids)-1] - if first > last { - first = last - } - } else if r.first.star && last < first { - last = first } } - if uid < first || uid > last { - continue + if first > last { + first, last = last, first } - if uidSearch(uids, uid) > 0 { + if uid >= first && uid <= last && uidSearch(uids, uid) > 0 { return true } } @@ -151,41 +158,24 @@ func (ss numSet) String() string { return l[0] } -type setNumber struct { - number uint32 - star bool -} - -type numRange struct { - first setNumber - last *setNumber // if nil, this numRange is just a setNumber in "first" and first.star will be false -} - // interpretStar returns a numset that interprets stars in a numset, returning a new // numset without stars with increasing first/last. func (s numSet) interpretStar(uids []store.UID) numSet { var ns numSet + if len(uids) == 0 { + return ns + } + for _, r := range s.ranges { first := r.first.number - if r.first.star { - if len(uids) == 0 { - continue - } - first = uint32(uids[0]) + if r.first.star || first > uint32(uids[len(uids)-1]) { + first = uint32(uids[len(uids)-1]) } last := first if r.last != nil { last = r.last.number - if r.last.star { - if len(uids) == 0 { - continue - } + if r.last.star || last > uint32(uids[len(uids)-1]) { last = uint32(uids[len(uids)-1]) - if first > last { - first = last - } - } else if r.first.star && last < first { - last = first } } if first > last { @@ -226,18 +216,18 @@ type numIter struct { // newIter must only be called on a numSet that is basic (no star/search) and ascending. func (s numSet) newIter() *numIter { - return &numIter{s: s, i: 0, r: s.ranges[0].newIter()} + return &numIter{s: s} } func (i *numIter) Next() (uint32, bool) { if v, ok := i.r.Next(); ok { return v, ok } - i.i++ if i.i >= len(i.s.ranges) { return 0, false } i.r = i.s.ranges[i.i].newIter() + i.i++ return i.r.Next() } @@ -252,6 +242,9 @@ func (r numRange) newIter() *rangeIter { } func (r *rangeIter) Next() (uint32, bool) { + if r == nil { + return 0, false + } if r.o == 0 { r.o++ return r.r.first.number, true diff --git a/imapserver/protocol_test.go b/imapserver/protocol_test.go index c53defe..86ab915 100644 --- a/imapserver/protocol_test.go +++ b/imapserver/protocol_test.go @@ -38,29 +38,37 @@ func TestNumSetContains(t *testing.T) { // 2:* ss2 := numSet{false, []numRange{{*num(2), star}}} - check(!ss2.containsSeq(1, []store.UID{2}, nil)) + check(ss2.containsSeq(1, []store.UID{2}, nil)) + check(!ss2.containsSeq(2, []store.UID{2}, nil)) check(ss2.containsSeq(2, []store.UID{4, 5}, nil)) check(ss2.containsSeq(3, []store.UID{4, 5, 6}, nil)) + check(!ss2.containsSeq(4, []store.UID{4, 5, 6}, nil)) check(ss2.containsUID(2, []store.UID{2}, nil)) + check(!ss2.containsUID(1, []store.UID{1, 2, 3}, nil)) check(ss2.containsUID(3, []store.UID{1, 2, 3}, nil)) - check(ss2.containsUID(2, []store.UID{2}, nil)) check(!ss2.containsUID(2, []store.UID{4, 5}, nil)) check(!ss2.containsUID(2, []store.UID{1}, nil)) check(ss2.containsUID(2, []store.UID{2, 6}, nil)) check(ss2.containsUID(6, []store.UID{2, 6}, nil)) - // *:2 + // *:2, same as 2:* ss3 := numSet{false, []numRange{{*star, num(2)}}} check(ss3.containsSeq(1, []store.UID{2}, nil)) + check(!ss3.containsSeq(2, []store.UID{2}, nil)) check(ss3.containsSeq(2, []store.UID{4, 5}, nil)) - check(!ss3.containsSeq(3, []store.UID{1, 2, 3}, nil)) + check(ss3.containsSeq(3, []store.UID{4, 5, 6}, nil)) + check(!ss3.containsSeq(4, []store.UID{4, 5, 6}, nil)) - check(ss3.containsUID(1, []store.UID{1}, nil)) - check(ss3.containsUID(2, []store.UID{1, 2, 3}, nil)) - check(!ss3.containsUID(1, []store.UID{2, 3}, nil)) - check(!ss3.containsUID(3, []store.UID{1, 2, 3}, nil)) + check(ss3.containsUID(2, []store.UID{2}, nil)) + check(!ss3.containsUID(1, []store.UID{1, 2, 3}, nil)) + check(ss3.containsUID(3, []store.UID{1, 2, 3}, nil)) + check(!ss3.containsUID(2, []store.UID{4, 5}, nil)) + check(!ss3.containsUID(2, []store.UID{1}, nil)) + + check(ss3.containsUID(2, []store.UID{2, 6}, nil)) + check(ss3.containsUID(6, []store.UID{2, 6}, nil)) } func TestNumSetInterpret(t *testing.T) { @@ -82,10 +90,25 @@ func TestNumSetInterpret(t *testing.T) { checkEqual([]store.UID{1}, "1:*", "1") checkEqual([]store.UID{1, 3}, "1:*", "1:3") checkEqual([]store.UID{1, 3}, "4:*", "3") - checkEqual([]store.UID{2, 3}, "*:4", "2:4") - checkEqual([]store.UID{2, 3}, "*:1", "2") + checkEqual([]store.UID{1, 3}, "*:4", "3") + checkEqual([]store.UID{2, 3}, "*:4", "3") + checkEqual([]store.UID{2, 3}, "*:1", "1:3") + checkEqual([]store.UID{2, 3}, "1:*", "1:3") checkEqual([]store.UID{1, 2, 3}, "1,2,3", "1,2,3") - checkEqual([]store.UID{}, "1,2,3", "1,2,3") - checkEqual([]store.UID{}, "1:3", "1:3") - checkEqual([]store.UID{}, "3:1", "1:3") + checkEqual([]store.UID{}, "1,2,3", "") + checkEqual([]store.UID{}, "1:3", "") + checkEqual([]store.UID{}, "3:1", "") + + iter := parseNumSet("1:3").interpretStar([]store.UID{}).newIter() + if _, ok := iter.Next(); ok { + t.Fatalf("expected immediate end for empty iter") + } + + iter = parseNumSet("3:1").interpretStar([]store.UID{1, 2}).newIter() + v0, _ := iter.Next() + v1, _ := iter.Next() + _, ok := iter.Next() + if v0 != 1 || v1 != 2 || ok { + t.Fatalf("got %v %v %v, expected 1, 2, false", v0, v1, ok) + } }