implement Message-Id based UIDVALIDITY recovery

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.

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.
 

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 );
 	}
 }

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,

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 ))
+					int off, in_msgid = 0;
-						tuid = tmp->val + 8;
+					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 *******************/

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 )) {
+				int off, in_msgid = 0;
-					if (!nbuf[0] || nbuf[0] == '\n')
+				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;
 		}

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
+OTOH, \fBmbsync\fR is susceptible to UID validity changes (but will recover
-never happen, but see "Compatibility" in the README).
+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

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) {
+		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] );
 			fails++;
-		}
+	if (fails == 2) {
-	if (fails) {
+		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;