[PATCH 0/2] imap: search fixes

[PATCH 0/2] imap: search fixes

[Eric Wong]

Some bugs I noticed while retooling -watch to work as an
IMAP (and NNTP) client.

Eric Wong (2):
  imap: *SEARCH: return no results for out-of-range queries
  imap: refill_xap: remove needless loop

 lib/PublicInbox/IMAP.pm         | 23 ++++++++++-------------
 lib/PublicInbox/IMAPsearchqp.pm |  4 ++--
 t/imapd.t                       | 11 +++++++++--
 3 files changed, 21 insertions(+), 17 deletions(-)

[PATCH 1/2] imap: *SEARCH: return no results for out-of-range queries

[Eric Wong]

Unlike FETCH and "UID FETCH" where out-of-range queries can
will return the highest UID, SEARCH and "UID SEARCH" handle
ranges as-is for filtering.
---
 lib/PublicInbox/IMAP.pm         |  8 ++++----
 lib/PublicInbox/IMAPsearchqp.pm |  4 ++--
 t/imapd.t                       | 11 +++++++++--
 3 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/lib/PublicInbox/IMAP.pm b/lib/PublicInbox/IMAP.pm
index d8b1fce109e..0508ae381dd 100644
--- a/lib/PublicInbox/IMAP.pm
+++ b/lib/PublicInbox/IMAP.pm
@@ -689,8 +689,8 @@ sub uid_clamp ($$$) {
 	$$end = $uid_end if $$end > $uid_end;
 }
 
-sub range_step ($$) {
-	my ($self, $range_csv) = @_;
+sub range_step ($$;$) {
+	my ($self, $range_csv, $is_search) = @_;
 	my ($beg, $end, $range);
 	if ($$range_csv =~ s/\A([^,]+),//) {
 		$range = $1;
@@ -707,7 +707,7 @@ sub range_step ($$) {
 		$beg = $1 + 0;
 		$end = $self->{ibx}->over->max;
 		$end = $uid_end if $end > $uid_end;
-		$beg = $end if $beg > $end;
+		$beg = $end if !$is_search && $beg > $end;
 		uid_clamp($self, \$beg, \$end);
 	} elsif ($range =~ /\A[0-9]+\z/) {
 		$beg = $end = $range + 0;
@@ -772,7 +772,7 @@ sub refill_uids ($$$;$) {
 		} elsif (!$range_csv) {
 			return 0;
 		} else {
-			my $next_range = range_step($self, \$range_csv);
+			my $next_range = range_step($self, \$range_csv, !!$sql);
 			return $next_range if !ref($next_range); # error
 			($beg, $end, $range_csv) = @$range_info = @$next_range;
 			# continue looping
diff --git a/lib/PublicInbox/IMAPsearchqp.pm b/lib/PublicInbox/IMAPsearchqp.pm
index 4ea99ea500b..6d7ce32a84a 100644
--- a/lib/PublicInbox/IMAPsearchqp.pm
+++ b/lib/PublicInbox/IMAPsearchqp.pm
@@ -35,7 +35,7 @@ sub uid_set_xap ($$) {
 	my ($self, $seq_set) = @_;
 	my @u;
 	do {
-		my $u = $self->{imap}->range_step(\$seq_set);
+		my $u = $self->{imap}->range_step(\$seq_set, 1);
 		die $u unless ref($u); # break out of the parser on error
 		push @u, "uid:$u->[0]..$u->[1]";
 	} while ($seq_set);
@@ -265,7 +265,7 @@ sub parse {
 		if (my $uid = delete $q->{uid}) {
 			my @u;
 			for my $uid_set (@$uid) {
-				my $u = $q->{imap}->range_step(\$uid_set);
+				my $u = $q->{imap}->range_step(\$uid_set, 1);
 				return $u if !ref($u);
 				push @u, "num >= $u->[0] AND num <= $u->[1]";
 			}
diff --git a/t/imapd.t b/t/imapd.t
index ffa195d57ac..c17f59b5d8b 100644
--- a/t/imapd.t
+++ b/t/imapd.t
@@ -118,16 +118,23 @@ $ret = $mic->search('uid 1') or BAIL_OUT "SEARCH FAIL $@";
 is_deeply($ret, [ 1 ], 'search UID 1 works');
 $ret = $mic->search('uid 1:1') or BAIL_OUT "SEARCH FAIL $@";
 is_deeply($ret, [ 1 ], 'search UID 1:1 works');
-$ret = $mic->search('uid 1:*') or BAIL_OUT "SEARCH FAIL $@";
+$ret = $mic->search(\'uid 1:*') or BAIL_OUT "SEARCH FAIL $@";
 is_deeply($ret, [ 1 ], 'search UID 1:* works');
+$ret = $mic->search(\'uid 9:*') or BAIL_OUT "SEARCH FAIL $@";
+is_deeply($ret, [], 'search UID 9:* is empty');
 
 SKIP: {
-	skip 'Xapian missing', 7 if $level eq 'basic';
+	skip 'Xapian missing', 9 if $level eq 'basic';
 	my $x = $mic->search(qw(smaller 99999));
 	is_deeply($x, [1], 'SMALLER works with Xapian (hit)');
 	$x = $mic->search(qw(smaller 9));
 	is_deeply($x, [], 'SMALLER works with Xapian (miss)');
 
+	$x = $mic->search(\'smaller 9 UID 9:*');
+	is_deeply($x, [], 'SMALLER + UID works with Xapian (miss)');
+	$x = $mic->search(\'larger 9 UID 9:*');
+	is_deeply($x, [], 'LARGER + UID works with Xapian (miss)');
+
 	$x = $mic->search(qw(larger 99999));
 	is_deeply($x, [], 'LARGER works with Xapian (miss)');
 	$x = $mic->search(qw(larger 9));

[PATCH 2/2] imap: refill_xap: remove needless loop

[Eric Wong]

There's no need to loop when the first iteration guarantees
a `return'.
---
 lib/PublicInbox/IMAP.pm | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/lib/PublicInbox/IMAP.pm b/lib/PublicInbox/IMAP.pm
index 0508ae381dd..941ce3af495 100644
--- a/lib/PublicInbox/IMAP.pm
+++ b/lib/PublicInbox/IMAP.pm
@@ -1172,16 +1172,13 @@ sub refill_xap ($$$$) {
 	my $srch = $self->{ibx}->search;
 	my $opt = { mset => 2, limit => 1000 };
 	my $nshard = $srch->{nshard} // 1;
-	while (1) {
-		my $mset = $srch->query("$q uid:$beg..$end", $opt);
-		@$uids = map { mdocid($nshard, $_) } $mset->items;
-		if (@$uids) {
-			$range_info->[0] = $uids->[-1] + 1; # update $beg
-			return;
-		} else { # all done
-			return 0;
-		}
+	my $mset = $srch->query("$q uid:$beg..$end", $opt);
+	@$uids = map { mdocid($nshard, $_) } $mset->items;
+	if (@$uids) {
+		$range_info->[0] = $uids->[-1] + 1; # update $beg
+		return; # possibly more
 	}
+	0; # all done
 }
 
 sub search_xap_range { # long_response

[REJECT] imap: *SEARCH: return no results for out-of-range queries

[Eric Wong]

Eric Wong <e@yhbt.net> wrote:
> Unlike FETCH and "UID FETCH" where out-of-range queries can
> will return the highest UID, SEARCH and "UID SEARCH" handle
> ranges as-is for filtering.

Actually, no.  At least testing against other IMAP servers
says they still return the highest UID :x