The below patch fixes mutt's IMAP message number handling in case it is
started with a header cache in which a mail somewhere "in the middle"
is missing (i.e., the cache does contain a mail later than the one that
is missing).
This bug isn't just cosmetic; it can lead to unintended deletion of
large amounts of mails.
Let me give a step-by-step example of what went wrong and how that
could lead to losing mail:
0. Use mutt with IMAP, header caching enabled, but not CONDSTORE
nor QRESYNC.
1. Open an IMAP mailbox with e.g. 5 mails in it, with e.g. UIDs 11...15.
2. Mark the 3rd mail, UID=13, for deletion.
3. Give the sync-mailbox command; mutt immediately deletes the mail from
the header cache and tells the IMAP server to also delete it.
4. Assume that the server does not actually do the synchronization;
this could e.g. happen due to a network problem, or the server could
simply say "NO" (as the server I'm using sometimes does).
Now the mail with UID=13 is missing from the header cache, but it is
still on the server.
5. Exit and restart mutt.
6. Mutt will first populate its view of the mailbox from its header cache,
then request headers for the missing UID=13 mail, and _append_ the
UID=13 mail after the other mails.
So in mutt's view the mailbox contains UIDs in the order 11,12,14,15,13.
Note that this order violates the IMAP specification (and assumption
in the code) that UIDs are monotonically increasing.
7. The user sees that mail 13 has jumped to the end of the mailbox, if
not otherwise sorted; this may be confusing or surprising, but by
itself it's not a serious problem.
8. Assume a new mail arrives, with UID 16.
In mutt's view, the mailbox now contains UIDs 11,12,14,15,13,16.
9. Mark mails 13 and 16 for deletion.
10. Give the sync-mailbox command. In mutt's view, the two mails to be
deleted are consecutive in mailbox order, and therefore it will tell
the server to delete the entire UID range 13:16 .
11. Sadly, this inadvertently also deletes mails 14 and 15 !
This is not just theory. I've several times lost hundreds of mails due
to (presumably) this bug (though of course I wasn't yet aware of exactly
what was happening).
Fortunately, fixing this is trivial; at two places in imap/message.c,
replace
ctx->hdrs[idx]->index = idx;
by
ctx->hdrs[idx]->index = h.data->msn-1;
I.e., instead of writing the array index into the index field, write the
correct message number into it; this field is defined as "the absolute
(unsorted) message number" in mutt.h.
I also checked what happens if CONDSTORE and QRESYNC are enabled.
In that case, verify_qresync() notices that the quick resync didn't work
correctly, but the subsequent attempt at a normal resync fails in a
different way: it doesn't fetch the headers for the missing mail.
To the user, it then looks as if the mail is no longer in the mailbox,
even though the server still has it.
The below patch fixes this too.
Regards,
Pieter-Tjerk
---
imap/message.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/imap/message.c b/imap/message.c
index 103e48e5..42a9414d 100644
--- a/imap/message.c
+++ b/imap/message.c
@@ -240,6 +240,7 @@ int imap_read_headers (IMAP_DATA* idata, unsigned int
msn_begin, unsigned int ms
void *pmodseq = NULL;
unsigned long long hc_modseq = 0;
char *uid_seqset = NULL;
+ unsigned int msn_begin_original = msn_begin;
#endif /* USE_HCACHE */
ctx = idata->ctx;
@@ -357,6 +358,7 @@ retry:
FREE (&uid_seqset);
uid_validity = 0;
uidnext = 0;
+ msn_begin = msn_begin_original;
goto retry;
}
@@ -509,7 +511,7 @@ static int read_headers_normal_eval_cache (IMAP_DATA *idata,
idata->msn_index[h.data->msn - 1] = ctx->hdrs[idx];
int_hash_insert (idata->uid_hash, h.data->uid, ctx->hdrs[idx]);
- ctx->hdrs[idx]->index = idx;
+ ctx->hdrs[idx]->index = h.data->msn - 1;
/* messages which have not been expunged are ACTIVE (borrowed from mh
* folders) */
ctx->hdrs[idx]->active = 1;
@@ -943,7 +945,7 @@ static int read_headers_fetch_new (IMAP_DATA *idata,
unsigned int msn_begin,
idata->msn_index[h.data->msn - 1] = ctx->hdrs[idx];
int_hash_insert (idata->uid_hash, h.data->uid, ctx->hdrs[idx]);
- ctx->hdrs[idx]->index = idx;
+ ctx->hdrs[idx]->index = h.data->msn - 1;
/* messages which have not been expunged are ACTIVE (borrowed from mh
* folders) */
ctx->hdrs[idx]->active = 1;
--
2.20.1
