commit 77acc268123b8233843ca9bc3dcf90669efde08f
parent f9fe75602e36878d92c8476ba2a3dac524712e1c
Author: Oswald Buddenhagen <ossi@users.sf.net>
Date: Sun, 18 Dec 2016 20:50:20 +0100
implement Message-Id based UIDVALIDITY recovery
Diffstat:
8 files changed, 150 insertions(+), 31 deletions(-)
diff --git a/NEWS b/NEWS
@@ -6,6 +6,8 @@ A Maildir sub-folder naming style without extra dots has been added.
Support for TLS client certificates was added.
+Support for recovering from baseless UID validity changes was added.
+
[1.2.0]
The 'isync' compatibility wrapper is now deprecated.
diff --git a/README b/README
@@ -45,16 +45,6 @@ isync executable still exists; it is a compatibility wrapper around mbsync.
Courier 1.4.3 is known to be buggy, version 1.7.3 works fine.
- c-client (UW-IMAP, Pine) is mostly fine, but versions less than 2004a.352
- tend to change UIDVALIDITY pretty often when used with unix/mbox mailboxes,
- making isync refuse synchronization.
- M$ Exchange (up to 2010 at least) occasionally exposes the same problem.
- The "cure" is to simply copy the new UIDVALIDITY from the affected
- mailbox to mbsync's state file. This is a Bad Hack (TM), but it works -
- use at your own risk (if the UIDVALIDITY change was genuine, this will
- delete all messages in the affected mailbox - not that this ever
- happened to me).
-
M$ Exchange (2013 at least) needs DisableExtension MOVE to be compatible
with the Trash functionality.
diff --git a/src/driver.c b/src/driver.c
@@ -34,6 +34,7 @@ free_generic_messages( message_t *msgs )
for (; msgs; msgs = tmsg) {
tmsg = msgs->next;
+ free( msgs->msgid );
free( msgs );
}
}
diff --git a/src/driver.h b/src/driver.h
@@ -63,6 +63,7 @@ typedef struct store_conf {
typedef struct message {
struct message *next;
struct sync_rec *srec;
+ char *msgid; /* owned */
/* string_list_t *keywords; */
size_t size; /* zero implies "not fetched" */
int uid;
@@ -80,6 +81,7 @@ typedef struct message {
#define OPEN_SETFLAGS (1<<6)
#define OPEN_APPEND (1<<7)
#define OPEN_FIND (1<<8)
+#define OPEN_OLD_IDS (1<<9)
typedef struct store {
struct store *next;
@@ -208,6 +210,7 @@ struct driver {
* and those named in the excs array (smaller than minuid).
* The driver takes ownership of the excs array.
* Messages starting with newuid need to have the TUID populated when OPEN_FIND is set.
+ * Messages up to seenuid need to have the Message-Id populated when OPEN_OLD_IDS is set.
* Messages up to seenuid need to have the size populated when OPEN_OLD_SIZE is set;
* likewise messages above seenuid when OPEN_NEW_SIZE is set. */
void (*load_box)( store_t *ctx, int minuid, int maxuid, int newuid, int seenuid, int_array_t excs,
diff --git a/src/drv_imap.c b/src/drv_imap.c
@@ -905,7 +905,7 @@ static int
parse_fetch_rsp( imap_store_t *ctx, list_t *list, char *s ATTR_UNUSED )
{
list_t *tmp, *flags;
- char *body = 0, *tuid = 0;
+ char *body = 0, *tuid = 0, *msgid = 0;
imap_message_t *cur;
msg_data_t *msgdata;
struct imap_cmd *cmdp;
@@ -983,8 +983,42 @@ parse_fetch_rsp( imap_store_t *ctx, list_t *list, char *s ATTR_UNUSED )
tmp = tmp->next;
if (!is_atom( tmp ))
goto bfail;
- if (starts_with_upper( tmp->val, tmp->len, "X-TUID: ", 8 ))
- tuid = tmp->val + 8;
+ int off, in_msgid = 0;
+ for (char *val = tmp->val, *end; (end = strchr( val, '\n' )); val = end + 1) {
+ int len = (int)(end - val);
+ if (len && end[-1] == '\r')
+ len--;
+ if (!len)
+ break;
+ if (starts_with_upper( val, len, "X-TUID: ", 8 )) {
+ if (len < 8 + TUIDL) {
+ error( "IMAP error: malformed X-TUID header (UID %d)\n", uid );
+ continue;
+ }
+ tuid = val + 8;
+ in_msgid = 0;
+ continue;
+ }
+ if (starts_with_upper( val, len, "MESSAGE-ID:", 11 )) {
+ off = 11;
+ } else if (in_msgid) {
+ if (!isspace( val[0] )) {
+ in_msgid = 0;
+ continue;
+ }
+ off = 1;
+ } else {
+ continue;
+ }
+ while (off < len && isspace( val[off] ))
+ off++;
+ if (off == len) {
+ in_msgid = 1;
+ continue;
+ }
+ msgid = nfstrndup( val + off, len - off );
+ in_msgid = 0;
+ }
} else {
bfail:
error( "IMAP error: unable to parse BODY[HEADER.FIELDS ...]\n" );
@@ -1018,6 +1052,7 @@ parse_fetch_rsp( imap_store_t *ctx, list_t *list, char *s ATTR_UNUSED )
cur->gen.status = status;
cur->gen.size = size;
cur->gen.srec = 0;
+ cur->gen.msgid = msgid;
if (tuid)
strncpy( cur->gen.tuid, tuid, TUIDL );
else
@@ -2322,7 +2357,7 @@ imap_prepare_load_box( store_t *gctx, int opts )
gctx->opts = opts;
}
-enum { WantSize = 1, WantTuids = 2 };
+enum { WantSize = 1, WantTuids = 2, WantMsgids = 4 };
typedef struct imap_range {
int first, last, flags;
} imap_range_t;
@@ -2375,7 +2410,7 @@ imap_load_box( store_t *gctx, int minuid, int maxuid, int newuid, int seenuid, i
if (i != j)
bl += sprintf( buf + bl, ":%d", excs.data[i] );
}
- imap_submit_load( ctx, buf, 0, sts );
+ imap_submit_load( ctx, buf, shifted_bit( ctx->gen.opts, OPEN_OLD_IDS, WantMsgids ), sts );
}
if (maxuid == INT_MAX)
maxuid = ctx->gen.uidnext ? ctx->gen.uidnext - 1 : 0x7fffffff;
@@ -2390,6 +2425,8 @@ imap_load_box( store_t *gctx, int minuid, int maxuid, int newuid, int seenuid, i
shifted_bit( ctx->gen.opts, OPEN_NEW_SIZE, WantSize), seenuid );
if (ctx->gen.opts & OPEN_FIND)
imap_set_range( ranges, &nranges, 0, WantTuids, newuid - 1 );
+ if (ctx->gen.opts & OPEN_OLD_IDS)
+ imap_set_range( ranges, &nranges, WantMsgids, 0, seenuid );
for (int r = 0; r < nranges; r++) {
sprintf( buf, "%d:%d", ranges[r].first, ranges[r].last );
imap_submit_load( ctx, buf, ranges[r].flags, sts );
@@ -2404,10 +2441,14 @@ static void
imap_submit_load( imap_store_t *ctx, const char *buf, int flags, struct imap_cmd_refcounted_state *sts )
{
imap_exec( ctx, imap_refcounted_new_cmd( sts ), imap_refcounted_done_box,
- "UID FETCH %s (UID%s%s%s)", buf,
+ "UID FETCH %s (UID%s%s%s%s%s%s%s)", buf,
(ctx->gen.opts & OPEN_FLAGS) ? " FLAGS" : "",
(flags & WantSize) ? " RFC822.SIZE" : "",
- (flags & WantTuids) ? " BODY.PEEK[HEADER.FIELDS (X-TUID)]" : "");
+ (flags & (WantTuids | WantMsgids)) ? " BODY.PEEK[HEADER.FIELDS (" : "",
+ (flags & WantTuids) ? "X-TUID" : "",
+ !(~flags & (WantTuids | WantMsgids)) ? " " : "",
+ (flags & WantMsgids) ? "MESSAGE-ID" : "",
+ (flags & (WantTuids | WantMsgids)) ? ")]" : "");
}
/******************* imap_fetch_msg *******************/
diff --git a/src/drv_maildir.c b/src/drv_maildir.c
@@ -442,6 +442,7 @@ static const char *subdirs[] = { "cur", "new", "tmp" };
typedef struct {
char *base;
+ char *msgid;
int size;
uint uid:31, recent:1;
char tuid[TUIDL];
@@ -926,6 +927,7 @@ maildir_scan( maildir_store_t *ctx, msg_t_array_alloc_t *msglist )
continue;
entry = msg_t_array_append( msglist );
entry->base = nfstrdup( e->d_name );
+ entry->msgid = 0;
entry->uid = uid;
entry->recent = i;
entry->size = 0;
@@ -1050,7 +1052,8 @@ maildir_scan( maildir_store_t *ctx, msg_t_array_alloc_t *msglist )
}
int want_size = (uid > ctx->seenuid) ? (ctx->gen.opts & OPEN_NEW_SIZE) : (ctx->gen.opts & OPEN_OLD_SIZE);
int want_tuid = ((ctx->gen.opts & OPEN_FIND) && uid >= ctx->newuid);
- if (!want_size && !want_tuid)
+ int want_msgid = ((ctx->gen.opts & OPEN_OLD_IDS) && uid <= ctx->seenuid);
+ if (!want_size && !want_tuid && !want_msgid)
continue;
if (!fnl)
nfsnprintf( buf + bl, sizeof(buf) - bl, "%s/%s", subdirs[entry->recent], entry->base );
@@ -1064,7 +1067,7 @@ maildir_scan( maildir_store_t *ctx, msg_t_array_alloc_t *msglist )
}
entry->size = st.st_size;
}
- if (want_tuid) {
+ if (want_tuid || want_msgid) {
if (!(f = fopen( buf, "r" ))) {
if (errno != ENOENT) {
sys_error( "Maildir error: cannot open %s", buf );
@@ -1072,13 +1075,45 @@ maildir_scan( maildir_store_t *ctx, msg_t_array_alloc_t *msglist )
}
goto retry;
}
- while (fgets( nbuf, sizeof(nbuf), f )) {
- if (!nbuf[0] || nbuf[0] == '\n')
+ int off, in_msgid = 0;
+ while ((want_tuid || want_msgid) && fgets( nbuf, sizeof(nbuf), f )) {
+ int bufl = strlen( nbuf );
+ if (bufl && nbuf[bufl - 1] == '\n')
+ --bufl;
+ if (bufl && nbuf[bufl - 1] == '\r')
+ --bufl;
+ if (!bufl)
break;
- if (starts_with( nbuf, -1, "X-TUID: ", 8 ) && nbuf[8 + TUIDL] == '\n') {
+ if (want_tuid && starts_with( nbuf, bufl, "X-TUID: ", 8 )) {
+ if (bufl < 8 + TUIDL) {
+ error( "Maildir error: malformed X-TUID header (UID %d)\n", uid );
+ continue;
+ }
memcpy( entry->tuid, nbuf + 8, TUIDL );
- break;
+ want_tuid = 0;
+ in_msgid = 0;
+ continue;
+ }
+ if (want_msgid && starts_with_upper( nbuf, bufl, "MESSAGE-ID:", 11 )) {
+ off = 11;
+ } else if (in_msgid) {
+ if (!isspace( nbuf[0] )) {
+ in_msgid = 0;
+ continue;
+ }
+ off = 1;
+ } else {
+ continue;
+ }
+ while (off < bufl && isspace( nbuf[off] ))
+ off++;
+ if (off == bufl) {
+ in_msgid = 1;
+ continue;
}
+ entry->msgid = nfstrndup( nbuf + off, bufl - off );
+ want_msgid = 0;
+ in_msgid = 0;
}
fclose( f );
}
@@ -1093,6 +1128,8 @@ maildir_init_msg( maildir_store_t *ctx, maildir_message_t *msg, msg_t *entry )
{
msg->base = entry->base;
entry->base = 0; /* prevent deletion */
+ msg->gen.msgid = entry->msgid;
+ entry->msgid = 0; /* prevent deletion */
msg->gen.size = entry->size;
msg->gen.srec = 0;
strncpy( msg->gen.tuid, entry->tuid, TUIDL );
@@ -1350,6 +1387,7 @@ maildir_rescan( maildir_store_t *ctx )
debug( "updating message %d\n", msg->gen.uid );
msg->gen.status &= ~(M_FLAGS|M_RECENT);
free( msg->base );
+ free( msg->gen.msgid );
maildir_init_msg( ctx, msg, msglist.array.data + i );
i++, msgapp = &msg->gen.next;
}
diff --git a/src/mbsync.1 b/src/mbsync.1
@@ -36,8 +36,8 @@ the operation set can be selected in a fine-grained manner.
.br
Synchronization is based on unique message identifiers (UIDs), so no
identification conflicts can occur (unlike with some other mail synchronizers).
-OTOH, \fBmbsync\fR is susceptible to UID validity changes (that \fIshould\fR
-never happen, but see "Compatibility" in the README).
+OTOH, \fBmbsync\fR is susceptible to UID validity changes (but will recover
+just fine if the change is unfounded).
Synchronization state is kept in one local text file per mailbox pair;
these files are protected against concurrent \fBmbsync\fR processes.
Mailboxes can be safely modified while \fBmbsync\fR operates
diff --git a/src/sync.c b/src/sync.c
@@ -1165,12 +1165,13 @@ box_opened2( sync_vars_t *svars, int t )
fails = 0;
for (t = 0; t < 2; t++)
- if (svars->uidval[t] >= 0 && svars->uidval[t] != ctx[t]->uidvalidity) {
- error( "Error: UIDVALIDITY of %s changed (got %d, expected %d)\n",
- str_ms[t], ctx[t]->uidvalidity, svars->uidval[t] );
+ if (svars->uidval[t] >= 0 && svars->uidval[t] != ctx[t]->uidvalidity)
fails++;
- }
- if (fails) {
+ if (fails == 2) {
+ error( "Error: channel %s: UIDVALIDITY of both master and slave changed\n"
+ "(master got %d, expected %d; slave got %d, expected %d).\n",
+ svars->chan->name,
+ ctx[M]->uidvalidity, svars->uidval[M], ctx[S]->uidvalidity, svars->uidval[S] );
bail:
svars->ret = SYNC_FAIL;
sync_bail( svars );
@@ -1193,6 +1194,8 @@ box_opened2( sync_vars_t *svars, int t )
Fprintf( svars->jfp, JOURNAL_VERSION "\n" );
opts[M] = opts[S] = 0;
+ if (fails)
+ opts[M] = opts[S] = OPEN_OLD|OPEN_OLD_IDS;
for (t = 0; t < 2; t++) {
if (chan->ops[t] & (OP_DELETE|OP_FLAGS)) {
opts[t] |= OPEN_SETFLAGS;
@@ -1323,7 +1326,7 @@ load_box( sync_vars_t *svars, int t, int minwuid, int_array_t mexcs )
if (minwuid > svars->maxuid[t] + 1)
minwuid = svars->maxuid[t] + 1;
maxwuid = INT_MAX;
- if (svars->ctx[t]->opts & OPEN_OLD_SIZE)
+ if (svars->ctx[t]->opts & (OPEN_OLD_IDS|OPEN_OLD_SIZE))
seenuid = get_seenuid( svars, t );
else
seenuid = 0;
@@ -1429,6 +1432,47 @@ box_loaded( int sts, void *aux )
if (!(svars->state[1-t] & ST_LOADED))
return;
+ for (t = 0; t < 2; t++) {
+ if (svars->uidval[t] >= 0 && svars->uidval[t] != svars->ctx[t]->uidvalidity) {
+ unsigned need = 0, got = 0;
+ debug( "trying to re-approve uid validity of %s\n", str_ms[t] );
+ for (srec = svars->srecs; srec; srec = srec->next) {
+ if (srec->status & S_DEAD)
+ continue;
+ if (!srec->msg[t])
+ continue; // Message disappeared.
+ need++; // Present paired messages require re-validation.
+ if (!srec->msg[t]->msgid)
+ continue; // Messages without ID are useless for re-validation.
+ if (!srec->msg[1-t])
+ continue; // Partner disappeared.
+ if (!srec->msg[1-t]->msgid || strcmp( srec->msg[M]->msgid, srec->msg[S]->msgid )) {
+ error( "Error: channel %s, %s %s: UIDVALIDITY genuinely changed (at UID %d).\n",
+ svars->chan->name, str_ms[t], svars->orig_name[t], srec->uid[t] );
+ uvchg:
+ svars->ret |= SYNC_FAIL;
+ cancel_sync( svars );
+ return;
+ }
+ got++;
+ }
+ if (got < 20 && got * 5 < need * 4) {
+ // Too few confirmed messages. This is very likely in the drafts folder.
+ // A proper fallback would be fetching more headers (which potentially need
+ // normalization) or the message body (which should be truncated for sanity)
+ // and comparing.
+ error( "Error: channel %s, %s %s: Unable to recover from UIDVALIDITY change\n"
+ "(got %d, expected %d).\n",
+ svars->chan->name, str_ms[t], svars->orig_name[t],
+ svars->ctx[t]->uidvalidity, svars->uidval[t] );
+ goto uvchg;
+ }
+ notice( "Notice: channel %s, %s %s: Recovered from change of UIDVALIDITY.\n",
+ svars->chan->name, str_ms[t], svars->orig_name[t] );
+ svars->uidval[t] = -1;
+ }
+ }
+
if (svars->uidval[M] < 0 || svars->uidval[S] < 0) {
svars->uidval[M] = svars->ctx[M]->uidvalidity;
svars->uidval[S] = svars->ctx[S]->uidvalidity;
