fix loading of some messages' sizes in some partial sync scenarios

we need to pass a different "boundary" UID to driver_t::load_box() for
every OPEN_* flag that queries a partial range:
- OPEN_FIND refers to messages newer than all we know about
- OPEN_OLD_IDS refers to messages which are paired
- OPEN_{OLD,NEW}_SIZE refers to messages (not) above the committed
  boundary of already propagated messages

we treated the 3rd like the 2nd, which was just wrong - the actual
boundary may be lower or higher, so we'd produce wrong results when
MaxSize was set and only one of New and ReNew was requested.
This commit is contained in:
Oswald Buddenhagen 2019-12-29 11:52:26 +01:00
parent c8f402e43f
commit 395f802500
5 changed files with 29 additions and 36 deletions

View File

@ -206,12 +206,12 @@ struct driver {
* Consider only messages with UIDs between minuid and maxuid (inclusive) * Consider only messages with UIDs between minuid and maxuid (inclusive)
* and those named in the excs array (smaller than minuid). * and those named in the excs array (smaller than minuid).
* The driver takes ownership of the excs array. * The driver takes ownership of the excs array.
* Messages starting with newuid need to have the TUID populated when OPEN_FIND is set. * Messages starting with finduid 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 pairuid 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; * Messages up to newuid need to have the size populated when OPEN_OLD_SIZE is set;
* likewise messages above seenuid when OPEN_NEW_SIZE is set. * likewise messages above newuid when OPEN_NEW_SIZE is set.
* The returned message list remains owned by the driver. */ * The returned message list remains owned by the driver. */
void (*load_box)( store_t *ctx, uint minuid, uint maxuid, uint newuid, uint seenuid, uint_array_t excs, void (*load_box)( store_t *ctx, uint minuid, uint maxuid, uint finduid, uint pairuid, uint newuid, uint_array_t excs,
void (*cb)( int sts, message_t *msgs, int total_msgs, int recent_msgs, void *aux ), void *aux ); void (*cb)( int sts, message_t *msgs, int total_msgs, int recent_msgs, void *aux ), void *aux );
/* Fetch the contents and flags of the given message from the current mailbox. */ /* Fetch the contents and flags of the given message from the current mailbox. */

View File

@ -2683,7 +2683,7 @@ static void imap_submit_load( imap_store_t *, const char *, int, imap_load_box_s
static void imap_submit_load_p3( imap_store_t *ctx, imap_load_box_state_t * ); static void imap_submit_load_p3( imap_store_t *ctx, imap_load_box_state_t * );
static void static void
imap_load_box( store_t *gctx, uint minuid, uint maxuid, uint newuid, uint seenuid, uint_array_t excs, imap_load_box( store_t *gctx, uint minuid, uint maxuid, uint finduid, uint pairuid, uint newuid, uint_array_t excs,
void (*cb)( int sts, message_t *msgs, int total_msgs, int recent_msgs, void *aux ), void *aux ) void (*cb)( int sts, message_t *msgs, int total_msgs, int recent_msgs, void *aux ), void *aux )
{ {
imap_store_t *ctx = (imap_store_t *)gctx; imap_store_t *ctx = (imap_store_t *)gctx;
@ -2716,11 +2716,11 @@ imap_load_box( store_t *gctx, uint minuid, uint maxuid, uint newuid, uint seenui
uint nranges = 1; uint nranges = 1;
if (ctx->opts & (OPEN_OLD_SIZE | OPEN_NEW_SIZE)) if (ctx->opts & (OPEN_OLD_SIZE | OPEN_NEW_SIZE))
imap_set_range( ranges, &nranges, shifted_bit( ctx->opts, OPEN_OLD_SIZE, WantSize), imap_set_range( ranges, &nranges, shifted_bit( ctx->opts, OPEN_OLD_SIZE, WantSize),
shifted_bit( ctx->opts, OPEN_NEW_SIZE, WantSize), seenuid ); shifted_bit( ctx->opts, OPEN_NEW_SIZE, WantSize), newuid );
if (ctx->opts & OPEN_FIND) if (ctx->opts & OPEN_FIND)
imap_set_range( ranges, &nranges, 0, WantTuids, newuid - 1 ); imap_set_range( ranges, &nranges, 0, WantTuids, finduid - 1 );
if (ctx->opts & OPEN_OLD_IDS) if (ctx->opts & OPEN_OLD_IDS)
imap_set_range( ranges, &nranges, WantMsgids, 0, seenuid ); imap_set_range( ranges, &nranges, WantMsgids, 0, pairuid );
for (uint r = 0; r < nranges; r++) { for (uint r = 0; r < nranges; r++) {
sprintf( buf, "%u:%u", ranges[r].first, ranges[r].last ); sprintf( buf, "%u:%u", ranges[r].first, ranges[r].last );
imap_submit_load( ctx, buf, ranges[r].flags, sts ); imap_submit_load( ctx, buf, ranges[r].flags, sts );

View File

@ -70,7 +70,7 @@ typedef struct {
typedef struct { typedef struct {
store_t gen; store_t gen;
int uvfd, uvok, is_inbox, fresh[3]; int uvfd, uvok, is_inbox, fresh[3];
uint opts, minuid, maxuid, newuid, seenuid, uidvalidity, nuid; uint opts, minuid, maxuid, finduid, pairuid, newuid, uidvalidity, nuid;
uint_array_t excs; uint_array_t excs;
char *path; /* own */ char *path; /* own */
char *trash; char *trash;
@ -1139,9 +1139,9 @@ maildir_scan( maildir_store_t *ctx, msg_t_array_alloc_t *msglist )
free( entry->base ); free( entry->base );
entry->base = nfstrndup( buf + bl + 4, (size_t)fnl ); entry->base = nfstrndup( buf + bl + 4, (size_t)fnl );
} }
int want_size = (uid > ctx->seenuid) ? (ctx->opts & OPEN_NEW_SIZE) : (ctx->opts & OPEN_OLD_SIZE); int want_size = (uid > ctx->newuid) ? (ctx->opts & OPEN_NEW_SIZE) : (ctx->opts & OPEN_OLD_SIZE);
int want_tuid = ((ctx->opts & OPEN_FIND) && uid >= ctx->newuid); int want_tuid = ((ctx->opts & OPEN_FIND) && uid >= ctx->finduid);
int want_msgid = ((ctx->opts & OPEN_OLD_IDS) && uid <= ctx->seenuid); int want_msgid = ((ctx->opts & OPEN_OLD_IDS) && uid <= ctx->pairuid);
if (!want_size && !want_tuid && !want_msgid) if (!want_size && !want_tuid && !want_msgid)
continue; continue;
if (!fnl) if (!fnl)
@ -1357,7 +1357,7 @@ maildir_confirm_box_empty( store_t *gctx )
maildir_store_t *ctx = (maildir_store_t *)gctx; maildir_store_t *ctx = (maildir_store_t *)gctx;
msg_t_array_alloc_t msglist; msg_t_array_alloc_t msglist;
ctx->excs.size = ctx->minuid = ctx->maxuid = ctx->newuid = 0; ctx->excs.size = ctx->minuid = ctx->maxuid = ctx->finduid = 0;
if (maildir_scan( ctx, &msglist ) != DRV_OK) if (maildir_scan( ctx, &msglist ) != DRV_OK)
return DRV_BOX_BAD; return DRV_BOX_BAD;
@ -1435,7 +1435,7 @@ maildir_prepare_load_box( store_t *gctx, uint opts )
} }
static void static void
maildir_load_box( store_t *gctx, uint minuid, uint maxuid, uint newuid, uint seenuid, uint_array_t excs, maildir_load_box( store_t *gctx, uint minuid, uint maxuid, uint finduid, uint pairuid, uint newuid, uint_array_t excs,
void (*cb)( int sts, message_t *msgs, int total_msgs, int recent_msgs, void *aux ), void *aux ) void (*cb)( int sts, message_t *msgs, int total_msgs, int recent_msgs, void *aux ), void *aux )
{ {
maildir_store_t *ctx = (maildir_store_t *)gctx; maildir_store_t *ctx = (maildir_store_t *)gctx;
@ -1445,8 +1445,9 @@ maildir_load_box( store_t *gctx, uint minuid, uint maxuid, uint newuid, uint see
ctx->minuid = minuid; ctx->minuid = minuid;
ctx->maxuid = maxuid; ctx->maxuid = maxuid;
ctx->finduid = finduid;
ctx->pairuid = pairuid;
ctx->newuid = newuid; ctx->newuid = newuid;
ctx->seenuid = seenuid;
ARRAY_SQUEEZE( &excs ); ARRAY_SQUEEZE( &excs );
ctx->excs = excs; ctx->excs = excs;

View File

@ -198,8 +198,8 @@ proxy_@name@( store_t *gctx@decl_args@, void (*cb)( @decl_cb_args@void *aux ), v
//# DEFINE load_box_pre_print_args //# DEFINE load_box_pre_print_args
static char ubuf[12]; static char ubuf[12];
//# END //# END
//# DEFINE load_box_print_fmt_args , [%u,%s] (new >= %u, seen <= %u) //# DEFINE load_box_print_fmt_args , [%u,%s] (find >= %u, paired <= %u, new > %u)
//# DEFINE load_box_print_pass_args , minuid, (maxuid == UINT_MAX) ? "inf" : (nfsnprintf( ubuf, sizeof(ubuf), "%u", maxuid ), ubuf), newuid, seenuid //# DEFINE load_box_print_pass_args , minuid, (maxuid == UINT_MAX) ? "inf" : (nfsnprintf( ubuf, sizeof(ubuf), "%u", maxuid ), ubuf), finduid, pairuid, newuid
//# DEFINE load_box_print_args //# DEFINE load_box_print_args
if (excs.size) { if (excs.size) {
debugn( " excs:" ); debugn( " excs:" );

View File

@ -170,7 +170,7 @@ typedef struct {
uint newmaxuid[2]; // highest UID that is currently being propagated uint newmaxuid[2]; // highest UID that is currently being propagated
uint uidval[2]; // UID validity value uint uidval[2]; // UID validity value
uint newuidval[2]; // UID validity obtained from driver uint newuidval[2]; // UID validity obtained from driver
uint newuid[2]; // TUID lookup makes sense only for UIDs >= this uint finduid[2]; // TUID lookup makes sense only for UIDs >= this
uint maxxfuid; // highest expired UID on far side uint maxxfuid; // highest expired UID on far side
uchar good_flags[2], bad_flags[2]; uchar good_flags[2], bad_flags[2];
} sync_vars_t; } sync_vars_t;
@ -896,7 +896,7 @@ load_state( sync_vars_t *svars )
if (c == 'S') if (c == 'S')
svars->maxuid[t1] = svars->newmaxuid[t1]; svars->maxuid[t1] = svars->newmaxuid[t1];
else if (c == 'F') else if (c == 'F')
svars->newuid[t1] = t2; svars->finduid[t1] = t2;
else if (c == 'T') else if (c == 'T')
*uint_array_append( &svars->trashed_msgs[t1] ) = t2; *uint_array_append( &svars->trashed_msgs[t1] ) = t2;
else if (c == '!') else if (c == '!')
@ -1350,27 +1350,19 @@ static void box_loaded( int sts, message_t *msgs, int total_msgs, int recent_msg
static void static void
load_box( sync_vars_t *svars, int t, uint minwuid, uint_array_t mexcs ) load_box( sync_vars_t *svars, int t, uint minwuid, uint_array_t mexcs )
{ {
uint maxwuid, seenuid; uint maxwuid = 0, pairuid = UINT_MAX;
if (svars->opts[t] & OPEN_NEW) { if (svars->opts[t] & OPEN_NEW) {
if (minwuid > svars->maxuid[t] + 1) if (minwuid > svars->maxuid[t] + 1)
minwuid = svars->maxuid[t] + 1; minwuid = svars->maxuid[t] + 1;
maxwuid = UINT_MAX; maxwuid = UINT_MAX;
if (svars->opts[t] & (OPEN_OLD_IDS|OPEN_OLD_SIZE)) if (svars->opts[t] & OPEN_OLD_IDS) // Implies OPEN_OLD
seenuid = get_seenuid( svars, t ); pairuid = get_seenuid( svars, t );
else
seenuid = 0;
} else if (svars->opts[t] & OPEN_OLD) { } else if (svars->opts[t] & OPEN_OLD) {
maxwuid = seenuid = get_seenuid( svars, t ); maxwuid = get_seenuid( svars, t );
} else
maxwuid = seenuid = 0;
if (seenuid < svars->maxuid[t]) {
/* We cannot rely on the maxuid, as uni-directional syncing does not update it.
* But if it is there, use it to avoid a possible gap in the fetched range. */
seenuid = svars->maxuid[t];
} }
info( "Loading %s box...\n", str_fn[t] ); info( "Loading %s box...\n", str_fn[t] );
svars->drv[t]->load_box( svars->ctx[t], minwuid, maxwuid, svars->newuid[t], seenuid, mexcs, box_loaded, AUX ); svars->drv[t]->load_box( svars->ctx[t], minwuid, maxwuid, svars->finduid[t], pairuid, svars->maxuid[t], mexcs, box_loaded, AUX );
} }
typedef struct { typedef struct {
@ -1814,8 +1806,8 @@ box_loaded( int sts, message_t *msgs, int total_msgs, int recent_msgs, void *aux
if (UseFSync) if (UseFSync)
fdatasync( fileno( svars->jfp ) ); fdatasync( fileno( svars->jfp ) );
for (t = 0; t < 2; t++) { for (t = 0; t < 2; t++) {
svars->newuid[t] = svars->drv[t]->get_uidnext( svars->ctx[t] ); svars->finduid[t] = svars->drv[t]->get_uidnext( svars->ctx[t] );
JLOG( "F %d %u", (t, svars->newuid[t]), "save UIDNEXT of %s", str_fn[t] ); JLOG( "F %d %u", (t, svars->finduid[t]), "save UIDNEXT of %s", str_fn[t] );
svars->new_msgs[t] = svars->msgs[1-t]; svars->new_msgs[t] = svars->msgs[1-t];
msgs_copied( svars, t ); msgs_copied( svars, t );
if (check_cancel( svars )) if (check_cancel( svars ))
@ -1921,7 +1913,7 @@ msgs_copied( sync_vars_t *svars, int t )
if (svars->state[t] & ST_FIND_NEW) { if (svars->state[t] & ST_FIND_NEW) {
debug( "finding just copied messages on %s\n", str_fn[t] ); debug( "finding just copied messages on %s\n", str_fn[t] );
svars->drv[t]->find_new_msgs( svars->ctx[t], svars->newuid[t], msgs_found_new, AUX ); svars->drv[t]->find_new_msgs( svars->ctx[t], svars->finduid[t], msgs_found_new, AUX );
} else { } else {
msgs_new_done( svars, t ); msgs_new_done( svars, t );
} }