dovecot-2.1: imapc: Added more checks to catch buggy IMAP server...
dovecot at dovecot.org
dovecot at dovecot.org
Tue Sep 20 16:44:43 EEST 2011
details: http://hg.dovecot.org/dovecot-2.1/rev/d174fa047d56
changeset: 13526:d174fa047d56
user: Timo Sirainen <tss at iki.fi>
date: Tue Sep 20 16:44:32 2011 +0300
description:
imapc: Added more checks to catch buggy IMAP server responses.
diffstat:
src/lib-storage/index/imapc/imapc-mailbox.c | 133 +++++++++++++++++++--------
src/lib-storage/index/imapc/imapc-msgmap.c | 7 +-
src/lib-storage/index/imapc/imapc-storage.h | 3 +
3 files changed, 101 insertions(+), 42 deletions(-)
diffs (237 lines):
diff -r 1dcc1ebe9516 -r d174fa047d56 src/lib-storage/index/imapc/imapc-mailbox.c
--- a/src/lib-storage/index/imapc/imapc-mailbox.c Tue Sep 20 16:42:58 2011 +0300
+++ b/src/lib-storage/index/imapc/imapc-mailbox.c Tue Sep 20 16:44:32 2011 +0300
@@ -103,10 +103,11 @@
}
static void
-imapc_untagged_exists(const struct imapc_untagged_reply *reply ATTR_UNUSED,
+imapc_untagged_exists(const struct imapc_untagged_reply *reply,
struct imapc_mailbox *mbox)
{
struct mail_index_view *view = mbox->delayed_sync_view;
+ uint32_t exists_count = reply->num;
const struct mail_index_header *hdr;
if (mbox == NULL)
@@ -122,6 +123,7 @@
hdr = mail_index_get_header(view);
mbox->sync_fetch_first_uid = hdr->next_uid;
}
+ mbox->exists_count = exists_count;
}
static void imapc_mailbox_idle_timeout(struct imapc_mailbox *mbox)
@@ -163,6 +165,82 @@
return TRUE;
}
+static int
+imapc_mailbox_msgmap_update(struct imapc_mailbox *mbox,
+ uint32_t rseq, uint32_t fetch_uid,
+ uint32_t *lseq_r, uint32_t *uid_r)
+{
+ struct imapc_msgmap *msgmap;
+ uint32_t uid, msg_count, rseq2;
+
+ *lseq_r = 0;
+ *uid_r = uid = fetch_uid;
+
+ if (rseq > mbox->exists_count) {
+ /* Receiving a FETCH for a message that EXISTS hasn't
+ announced yet. MS Exchange has a bug where our UID FETCH
+ request sometimes sends replies where sequences are above
+ EXISTS value, but their UIDs are for existing messages.
+ We'll just ignore these replies. */
+ return 0;
+ }
+ if (rseq < mbox->prev_skipped_rseq &&
+ fetch_uid > mbox->prev_skipped_uid) {
+ /* This was the initial attempt at catching the above
+ MS Exchange bug, but the above one appears to catch all
+ these cases. But keep it here just in case. */
+ imapc_mailbox_set_corrupted(mbox,
+ "FETCH sequence/UID order is mixed "
+ "(seq=%u,%u vs uid=%u,%u)",
+ mbox->prev_skipped_rseq, rseq,
+ mbox->prev_skipped_uid, fetch_uid);
+ return -1;
+ }
+
+ msgmap = imapc_client_mailbox_get_msgmap(mbox->client_box);
+ msg_count = imapc_msgmap_count(msgmap);
+ if (rseq <= msg_count) {
+ uid = imapc_msgmap_rseq_to_uid(msgmap, rseq);
+ if (uid != fetch_uid && fetch_uid != 0) {
+ imapc_mailbox_set_corrupted(mbox,
+ "FETCH UID mismatch (%u != %u)",
+ fetch_uid, uid);
+ return -1;
+ }
+ } else if (fetch_uid == 0 || rseq != msg_count+1) {
+ /* probably a flag update for a message we haven't yet
+ received our initial UID FETCH for. we should get
+ another one. */
+ if (fetch_uid == 0)
+ return 0;
+
+ if (imapc_msgmap_uid_to_rseq(msgmap, fetch_uid, &rseq2)) {
+ imapc_mailbox_set_corrupted(mbox,
+ "FETCH returned wrong sequence for UID %u "
+ "(%u != %u)", fetch_uid, rseq, rseq2);
+ return -1;
+ }
+ mbox->prev_skipped_rseq = rseq;
+ mbox->prev_skipped_uid = fetch_uid;
+ } else if (fetch_uid < imapc_msgmap_uidnext(msgmap)) {
+ imapc_mailbox_set_corrupted(mbox,
+ "Expunged message reappeared (uid=%u < next_uid=%u)",
+ fetch_uid, imapc_msgmap_uidnext(msgmap));
+ return -1;
+ } else {
+ /* newly seen message */
+ imapc_msgmap_append(msgmap, rseq, uid);
+ if (uid < mbox->min_append_uid) {
+ /* message is already added to index */
+ } else if (mbox->syncing) {
+ mail_index_append(mbox->delayed_sync_trans,
+ uid, lseq_r);
+ mbox->min_append_uid = uid + 1;
+ }
+ }
+ return 0;
+}
+
static void imapc_untagged_fetch(const struct imapc_untagged_reply *reply,
struct imapc_mailbox *mbox)
{
@@ -171,9 +249,8 @@
const struct imap_arg *list, *flags_list;
const char *atom;
const struct mail_index_record *rec = NULL;
- struct imapc_msgmap *msgmap;
enum mail_flags flags;
- uint32_t fetch_uid, uid, msg_count;
+ uint32_t fetch_uid, uid;
unsigned int i, j;
ARRAY_TYPE(const_string) keywords = ARRAY_INIT;
bool seen_flags = FALSE;
@@ -212,44 +289,10 @@
flags &= ~MAIL_RECENT;
imapc_mailbox_init_delayed_trans(mbox);
+ if (imapc_mailbox_msgmap_update(mbox, rseq, fetch_uid,
+ &lseq, &uid) < 0 || uid == 0)
+ return;
- msgmap = imapc_client_mailbox_get_msgmap(mbox->client_box);
- msg_count = imapc_msgmap_count(msgmap);
- if (rseq > msg_count) {
- /* newly seen message */
- if (fetch_uid == 0 || rseq != msg_count+1) {
- /* can't handle this one now. we should get another
- FETCH reply for it. */
- return;
- }
- uid = fetch_uid;
-
- if (uid < imapc_msgmap_uidnext(msgmap)) {
- imapc_mailbox_set_corrupted(mbox,
- "Expunged message reappeared "
- "(uid=%u < next_uid=%u)",
- uid, imapc_msgmap_uidnext(msgmap));
- return;
- }
-
- imapc_msgmap_append(msgmap, rseq, uid);
- if (uid < mbox->min_append_uid) {
- /* message is already added to index */
- lseq = 0;
- } else if (mbox->syncing) {
- mail_index_append(mbox->delayed_sync_trans, uid, &lseq);
- mbox->min_append_uid = uid + 1;
- }
- } else {
- uid = imapc_msgmap_rseq_to_uid(msgmap, rseq);
- if (uid != fetch_uid && fetch_uid != 0) {
- imapc_mailbox_set_corrupted(mbox,
- "FETCH UID mismatch (%u != %u)",
- fetch_uid, uid);
- return;
- }
- lseq = 0;
- }
/* if this is a reply to some FETCH request, update the mail's fields */
array_foreach(&mbox->fetch_mails, mailp) {
struct imapc_mail *mail = *mailp;
@@ -315,6 +358,16 @@
if (mbox == NULL || rseq == 0)
return;
+ mbox->prev_skipped_rseq = 0;
+ mbox->prev_skipped_uid = 0;
+
+ if (mbox->exists_count == 0) {
+ imapc_mailbox_set_corrupted(mbox,
+ "EXPUNGE received for empty mailbox");
+ return;
+ }
+ mbox->exists_count--;
+
msgmap = imapc_client_mailbox_get_msgmap(mbox->client_box);
if (rseq > imapc_msgmap_count(msgmap)) {
/* we haven't even seen this message yet */
diff -r 1dcc1ebe9516 -r d174fa047d56 src/lib-storage/index/imapc/imapc-msgmap.c
--- a/src/lib-storage/index/imapc/imapc-msgmap.c Tue Sep 20 16:42:58 2011 +0300
+++ b/src/lib-storage/index/imapc/imapc-msgmap.c Tue Sep 20 16:44:32 2011 +0300
@@ -6,6 +6,7 @@
struct imapc_msgmap {
ARRAY_TYPE(uint32_t) uids;
+ uint32_t uid_next;
};
struct imapc_msgmap *imapc_msgmap_init(void)
@@ -14,6 +15,7 @@
msgmap = i_new(struct imapc_msgmap, 1);
i_array_init(&msgmap->uids, 128);
+ msgmap->uid_next = 1;
return msgmap;
}
@@ -34,8 +36,7 @@
uint32_t imapc_msgmap_uidnext(struct imapc_msgmap *msgmap)
{
- return imapc_msgmap_count(msgmap) == 0 ? 1 :
- imapc_msgmap_rseq_to_uid(msgmap, 1) + 1;
+ return msgmap->uid_next;
}
uint32_t imapc_msgmap_rseq_to_uid(struct imapc_msgmap *msgmap, uint32_t rseq)
@@ -72,7 +73,9 @@
uint32_t rseq, uint32_t uid)
{
i_assert(rseq == imapc_msgmap_count(msgmap) + 1);
+ i_assert(uid >= msgmap->uid_next);
+ msgmap->uid_next = uid + 1;
array_append(&msgmap->uids, &uid, 1);
}
diff -r 1dcc1ebe9516 -r d174fa047d56 src/lib-storage/index/imapc/imapc-storage.h
--- a/src/lib-storage/index/imapc/imapc-storage.h Tue Sep 20 16:42:58 2011 +0300
+++ b/src/lib-storage/index/imapc/imapc-storage.h Tue Sep 20 16:44:32 2011 +0300
@@ -63,8 +63,11 @@
uint32_t sync_fetch_first_uid;
uint32_t sync_next_lseq;
uint32_t sync_next_rseq;
+ uint32_t exists_count;
uint32_t min_append_uid;
+ uint32_t prev_skipped_rseq, prev_skipped_uid;
+
unsigned int opening:1;
unsigned int syncing:1;
unsigned int initial_sync_done:1;
More information about the dovecot-cvs
mailing list