From cfaa4848dd7c72e628fcc81a4c4532c9be226144 Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Tue, 28 Jul 2020 16:14:00 +0200 Subject: [PATCH] actually implement imap_commit_cmds() delay reporting success of STORE FLAGS until a subsequent CHECK succeeds. this fixes (inverse flag change propagation) and (deletes not being propagated) after an interruption due to prematurely logged flag updates. --- src/drv_imap.c | 55 +++++++++++++++++++++++++++++++++++++++++++++---- src/drv_proxy.c | 8 ------- 2 files changed, 51 insertions(+), 12 deletions(-) diff --git a/src/drv_imap.c b/src/drv_imap.c index 4b9a429..9103e8a 100644 --- a/src/drv_imap.c +++ b/src/drv_imap.c @@ -129,9 +129,10 @@ struct imap_store { string_list_t *auth_mechs; parse_list_state_t parse_list_sts; /* command queue */ - int nexttag, num_in_progress; imap_cmd_t *pending, **pending_append; imap_cmd_t *in_progress, **in_progress_append; + imap_cmd_t *wait_check, **wait_check_append; + int nexttag, num_in_progress, num_wait_check; uint buffer_mem; /* memory currently occupied by buffers in the queue */ /* Used during sequential operations like connect */ @@ -169,6 +170,7 @@ struct imap_cmd { uint data_len; uint uid; /* to identify fetch responses */ char high_prio; /* if command is queued, put it at the front of the queue. */ + char wait_check; // Don't report success until subsequent CHECK success. char to_trash; /* we are storing to trash, not current. */ char create; /* create the mailbox if we get an error which suggests so. */ char failok; /* Don't complain about NO response. */ @@ -286,6 +288,8 @@ new_imap_cmd( uint size ) static void done_imap_cmd( imap_store_t *ctx, imap_cmd_t *cmd, int response ) { + if (cmd->param.wait_check) + ctx->num_wait_check--; cmd->param.done( ctx, cmd, response ); if (cmd->param.data) { free( cmd->param.data ); @@ -405,6 +409,18 @@ flush_imap_cmds( imap_store_t *ctx ) } } +static void +finalize_checked_imap_cmds( imap_store_t *ctx, int resp ) +{ + imap_cmd_t *cmd; + + while ((cmd = ctx->wait_check)) { + if (!(ctx->wait_check = cmd->next)) + ctx->wait_check_append = &ctx->wait_check; + done_imap_cmd( ctx, cmd, resp ); + } +} + static void cancel_pending_imap_cmds( imap_store_t *ctx ) { @@ -438,6 +454,8 @@ submit_imap_cmd( imap_store_t *ctx, imap_cmd_t *cmd ) assert( cmd ); assert( cmd->param.done ); + if (cmd->param.wait_check) + ctx->num_wait_check++; if ((ctx->pending && !cmd->param.high_prio) || !cmd_sendable( ctx, cmd )) { if (ctx->pending && cmd->param.high_prio) { cmd->next = ctx->pending; @@ -1600,7 +1618,13 @@ imap_socket_read( void *aux ) imap_ref( ctx ); if (resp == RESP_CANCEL) imap_invoke_bad_callback( ctx ); - done_imap_cmd( ctx, cmdp, resp ); + if (resp == RESP_OK && cmdp->param.wait_check) { + cmdp->next = NULL; + *ctx->wait_check_append = cmdp; + ctx->wait_check_append = &cmdp->next; + } else { + done_imap_cmd( ctx, cmdp, resp ); + } if (imap_deref( ctx )) return; if (ctx->canceling && !ctx->in_progress) { @@ -1623,6 +1647,7 @@ get_cmd_result_p2( imap_store_t *ctx, imap_cmd_t *cmd, int response ) if (response != RESP_OK) { done_imap_cmd( ctx, ocmd, response ); } else { + assert( !ocmd->param.wait_check ); ctx->uidnext = 1; if (ocmd->param.to_trash) ctx->trashnc = TrashKnown; @@ -1643,6 +1668,7 @@ imap_cancel_store( store_t *gctx ) sasl_dispose( &ctx->sasl ); #endif socket_close( &ctx->conn ); + finalize_checked_imap_cmds( ctx, RESP_CANCEL ); cancel_sent_imap_cmds( ctx ); cancel_pending_imap_cmds( ctx ); free( ctx->ns_prefix ); @@ -1699,6 +1725,8 @@ imap_free_store( store_t *gctx ) { imap_store_t *ctx = (imap_store_t *)gctx; + assert( !ctx->pending && !ctx->in_progress && !ctx->wait_check ); + free_generic_messages( ctx->msgs ); ctx->msgs = NULL; imap_set_bad_callback( gctx, imap_cancel_unowned, gctx ); @@ -1801,6 +1829,7 @@ imap_alloc_store( store_conf_t *conf, const char *label ) imap_socket_read, (void (*)(void *))flush_imap_cmds, ctx ); ctx->in_progress_append = &ctx->in_progress; ctx->pending_append = &ctx->pending; + ctx->wait_check_append = &ctx->wait_check; gotsrv: ctx->gen.driver = &imap_driver; @@ -2457,6 +2486,8 @@ imap_select_box( store_t *gctx, const char *name ) { imap_store_t *ctx = (imap_store_t *)gctx; + assert( !ctx->pending && !ctx->in_progress && !ctx->wait_check ); + free_generic_messages( ctx->msgs ); ctx->msgs = NULL; ctx->msgapp = &ctx->msgs; @@ -2873,7 +2904,9 @@ imap_flags_helper( imap_store_t *ctx, uint uid, char what, int flags, char buf[256]; buf[imap_make_flags( flags, buf )] = 0; - imap_exec( ctx, imap_refcounted_new_cmd( &sts->gen ), imap_set_flags_p2, + imap_cmd_t *cmd = imap_refcounted_new_cmd( &sts->gen ); + cmd->param.wait_check = 1; + imap_exec( ctx, cmd, imap_set_flags_p2, "UID STORE %u %cFLAGS.SILENT %s", uid, what, buf ); } @@ -2934,6 +2967,8 @@ imap_close_box( store_t *gctx, { imap_store_t *ctx = (imap_store_t *)gctx; + assert( !ctx->num_wait_check ); + if (ctx->gen.conf->trash && CAP(UIDPLUS)) { INIT_REFCOUNTED_STATE(imap_expunge_state_t, sts, cb, aux) message_t *msg, *fmsg, *nmsg; @@ -3216,6 +3251,7 @@ imap_cancel_cmds( store_t *gctx, { imap_store_t *ctx = (imap_store_t *)gctx; + finalize_checked_imap_cmds( ctx, RESP_CANCEL ); cancel_pending_imap_cmds( ctx ); if (ctx->in_progress) { ctx->canceling = 1; @@ -3228,10 +3264,21 @@ imap_cancel_cmds( store_t *gctx, /******************* imap_commit_cmds *******************/ +static void imap_commit_cmds_p2( imap_store_t *, imap_cmd_t *, int ); + static void imap_commit_cmds( store_t *gctx ) { - (void)gctx; + imap_store_t *ctx = (imap_store_t *)gctx; + + if (ctx->num_wait_check) + imap_exec( ctx, NULL, imap_commit_cmds_p2, "CHECK" ); +} + +static void +imap_commit_cmds_p2( imap_store_t *ctx, imap_cmd_t *cmd ATTR_UNUSED, int response ) +{ + finalize_checked_imap_cmds( ctx, response ); } /******************* imap_get_memory_usage *******************/ diff --git a/src/drv_proxy.c b/src/drv_proxy.c index 92c6b76..84597a3 100644 --- a/src/drv_proxy.c +++ b/src/drv_proxy.c @@ -288,14 +288,6 @@ proxy_@name@( store_t *gctx@decl_args@, void (*cb)( @decl_cb_args@void *aux ), v //# END #endif -//# SPECIAL commit_cmds -static void -proxy_commit_cmds( store_t *gctx ) -{ - // Currently a dummy in all real drivers. - (void) gctx; -} - //# SPECIAL set_bad_callback static void proxy_set_bad_callback( store_t *gctx, void (*cb)( void *aux ), void *aux )