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

@@ -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 {
+		if r.first.star || first > uint32(len(uids)) {
-			first = 1
+			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)) {
+		if first > last {
-			last = uint32(len(uids))
+			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 {
+		if r.first.star || first > uids[len(uids)-1] {
-			first = uids[0]
+			first = uids[len(uids)-1]
 		}
 		last := first
 		// Num in <num>:* can be larger than last, but it still matches the last...
 		// Similar for *:<num>. ../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 {
+		if first > last {
-			continue
+			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 r.first.star || first > uint32(uids[len(uids)-1]) {
-			if len(uids) == 0 {
+			first = uint32(uids[len(uids)-1])
-				continue
-			}
-			first = uint32(uids[0])
 		}
 		last := first
 		if r.last != nil {
 			last = r.last.number
-			if r.last.star {
+			if r.last.star || last > uint32(uids[len(uids)-1]) {
-				if len(uids) == 0 {
-					continue
-				}
 				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

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{2}, nil))
-	check(ss3.containsUID(2, []store.UID{1, 2, 3}, nil))
+	check(!ss3.containsUID(1, []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(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{1, 3}, "*:4", "3")
-	checkEqual([]store.UID{2, 3}, "*:1", "2")
+	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,2,3", "")
-	checkEqual([]store.UID{}, "1:3", "1:3")
+	checkEqual([]store.UID{}, "1:3", "")
-	checkEqual([]store.UID{}, "3:1", "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)
+	}
 }