From: Eric Wong <e@yhbt.net>
Date: Thu, 25 Jun 2020 10:09:37 +0000 (+0000)
Subject: imap: EXAMINE: avoid potential race conditions
X-Git-Tag: v1.6.0~353
X-Git-Url: http://www.git.stargrave.org/?p=public-inbox.git;a=commitdiff_plain;h=89efc8f4e8d603e4fad3c4c754e23561d82de2c1

imap: EXAMINE: avoid potential race conditions

We need to rely on num_highwater for UIDNEXT since the
highest `num' stored in over.sqlite3 may be rolled back
if the most recent messages were spam.

We also need to load the uo2m immediately on EXAMINE to ensure
EXISTS responses are always consistent with regard to future
updates.
---

diff --git a/lib/PublicInbox/IMAP.pm b/lib/PublicInbox/IMAP.pm
index 888c9bec..9ad74de8 100644
--- a/lib/PublicInbox/IMAP.pm
+++ b/lib/PublicInbox/IMAP.pm
@@ -192,8 +192,8 @@ sub cmd_capability ($$) {
 
 # uo2m: UID Offset to MSN, this is an arrayref by default,
 # but uo2m_hibernate can compact and deduplicate it
-sub uo2m_ary_new ($) {
-	my ($self) = @_;
+sub uo2m_ary_new ($;$) {
+	my ($self, $exists) = @_;
 	my $base = $self->{uid_base};
 	my $uids = $self->{ibx}->over->uid_range($base + 1, $base + UID_SLICE);
 
@@ -202,6 +202,7 @@ sub uo2m_ary_new ($) {
 	my $msn = 0;
 	++$base;
 	$tmp[$_ - $base] = ++$msn for @$uids;
+	$$exists = $msn if $exists;
 	\@tmp;
 }
 
@@ -385,44 +386,48 @@ sub ensure_slices_exist ($$$) {
 	push @$l, map { qq[* LIST (\\HasNoChildren) "." $_\r\n] } @created;
 }
 
-sub inbox_lookup ($$) {
-	my ($self, $mailbox) = @_;
-	my ($ibx, $exists, $uidnext, $uid_base);
-	if ($mailbox =~ /\A(.+)\.([0-9]+)\z/) {
-		# old mail: inbox.comp.foo.$SLICE_IDX
-		my $mb_top = $1;
-		$uid_base = $2 * UID_SLICE;
-		$ibx = $self->{imapd}->{mailboxes}->{lc $mailbox} or return;
-		my $max;
-		($exists, $uidnext, $max) = $ibx->over->imap_status($uid_base,
-							$uid_base + UID_SLICE);
-		ensure_slices_exist($self->{imapd}, $ibx, $max);
-	} else { # check for dummy inboxes
-		$mailbox = lc $mailbox;
-		$ibx = $self->{imapd}->{mailboxes}->{$mailbox} or return;
-
+sub inbox_lookup ($$;$) {
+	my ($self, $mailbox, $examine) = @_;
+	my ($ibx, $exists, $uidmax, $uid_base) = (undef, 0, 0, 0);
+	$mailbox = lc $mailbox;
+	$ibx = $self->{imapd}->{mailboxes}->{$mailbox} or return;
+	my $over = $ibx->over;
+	if ($over != $ibx) { # not a dummy
+		$mailbox =~ /\.([0-9]+)\z/ or
+				die "BUG: unexpected dummy mailbox: $mailbox\n";
+		$uid_base = $1 * UID_SLICE;
+
+		# ->num_highwater caches for writers, so use ->meta_accessor
+		$uidmax = $ibx->mm->meta_accessor('num_highwater') // 0;
+		if ($examine) {
+			$self->{uid_base} = $uid_base;
+			$self->{ibx} = $ibx;
+			$self->{uo2m} = uo2m_ary_new($self, \$exists);
+		} else {
+			$exists = $over->imap_exists;
+		}
+		ensure_slices_exist($self->{imapd}, $ibx, $over->max);
+	} else {
+		if ($examine) {
+			$self->{uid_base} = $uid_base;
+			$self->{ibx} = $ibx;
+			delete $self->{uo2m};
+		}
 		# if "INBOX.foo.bar" is selected and "INBOX.foo.bar.0",
 		# check for new UID ranges (e.g. "INBOX.foo.bar.1")
 		if (my $z = $self->{imapd}->{mailboxes}->{"$mailbox.0"}) {
 			ensure_slices_exist($self->{imapd}, $z, $z->over->max);
 		}
-
-		$uid_base = $exists = 0;
-		$uidnext = 1;
 	}
-	($ibx, $exists, $uidnext, $uid_base);
+	($ibx, $exists, $uidmax + 1, $uid_base);
 }
 
 sub cmd_examine ($$$) {
 	my ($self, $tag, $mailbox) = @_;
-	my ($ibx, $exists, $uidnext, $base) = inbox_lookup($self, $mailbox);
-	return "$tag NO Mailbox doesn't exist: $mailbox\r\n" if !$ibx;
-	$self->{uid_base} = $base;
-	delete $self->{uo2m};
-
 	# XXX: do we need this? RFC 5162/7162
 	my $ret = $self->{ibx} ? "* OK [CLOSED] previous closed\r\n" : '';
-	$self->{ibx} = $ibx;
+	my ($ibx, $exists, $uidnext, $base) = inbox_lookup($self, $mailbox, 1);
+	return "$tag NO Mailbox doesn't exist: $mailbox\r\n" if !$ibx;
 	$ret .= <<EOF;
 * $exists EXISTS\r
 * $exists RECENT\r
diff --git a/lib/PublicInbox/Over.pm b/lib/PublicInbox/Over.pm
index 74c8fb86..e5a980d5 100644
--- a/lib/PublicInbox/Over.pm
+++ b/lib/PublicInbox/Over.pm
@@ -248,25 +248,13 @@ SELECT MAX(num) FROM over WHERE num > 0
 	$sth->fetchrow_array // 0;
 }
 
-sub imap_status {
+sub imap_exists {
 	my ($self, $uid_base, $uid_end) = @_;
-	my $dbh = $self->connect;
-	my $sth = $dbh->prepare_cached(<<'', undef, 1);
+	my $sth = $self->connect->prepare_cached(<<'', undef, 1);
 SELECT COUNT(num) FROM over WHERE num > ? AND num <= ?
 
 	$sth->execute($uid_base, $uid_end);
-	my $exists = $sth->fetchrow_array;
-
-	$sth = $dbh->prepare_cached(<<'', undef, 1);
-SELECT MAX(num) + 1 FROM over WHERE num <= ?
-
-	$sth->execute($uid_end);
-	my $uidnext = $sth->fetchrow_array;
-
-	$sth = $dbh->prepare_cached(<<'', undef, 1);
-SELECT MAX(num) FROM over WHERE num > 0
-
-	($exists, $uidnext, $sth->fetchrow_array // 0);
+	$sth->fetchrow_array;
 }
 
 sub check_inodes {