From 181b3d6834619b9b9104e2ae16dbd44fb3e1f8a6 Mon Sep 17 00:00:00 2001 From: Timo Sirainen Date: Tue, 21 Feb 2017 12:38:10 +0200 Subject: [PATCH] lib-lda: Fix delivery logging when Sieve performs multiple actions Previous code assumed that it would work like: - save/copy - transaction commit - mail_deliver_ctx_get_log_var_expand_table() - repeat for transaction 2 While it really works: - transaction 1: save/copy - transaction 2: save/copy - transaction 1: commit - mail_deliver_ctx_get_log_var_expand_table() - transaction 2: commit - mail_deliver_ctx_get_log_var_expand_table() So the cache needs to be stored per transaction. This code still wouldn't work correctly if Sieve saved mails multiple times within the same transaction, but that doesn't happen (at least currently). --- src/lib-lda/mail-deliver.c | 66 ++++++++++++++++++++++++++++++++++---- 1 file changed, 60 insertions(+), 6 deletions(-) diff --git a/src/lib-lda/mail-deliver.c b/src/lib-lda/mail-deliver.c index 3db9b74eff..66c77cac0c 100644 --- a/src/lib-lda/mail-deliver.c +++ b/src/lib-lda/mail-deliver.c @@ -43,6 +43,13 @@ struct mail_deliver_cache { struct mail_deliver_mailbox { union mailbox_module_context module_ctx; + bool delivery_box; +}; + +struct mail_deliver_transaction { + union mailbox_transaction_module_context module_ctx; + + struct mail_deliver_cache cache; }; static const char *lda_log_wanted_headers[] = { @@ -223,6 +230,11 @@ int mail_deliver_save_open(struct mail_deliver_save_open_context *ctx, } *box_r = box = mailbox_alloc(ns->list, name, flags); + /* flag that this mailbox is used for delivering the mail. */ + struct mail_deliver_mailbox *mbox = MAIL_DELIVER_STORAGE_CONTEXT(box); + i_assert(mbox != NULL); + mbox->delivery_box = TRUE; + if (mailbox_open(box) == 0) return 0; @@ -514,13 +526,15 @@ static int mail_deliver_save_finish(struct mail_save_context *ctx) struct mail_deliver_mailbox *mbox = MAIL_DELIVER_STORAGE_CONTEXT(box); struct mail_deliver_user *muser = MAIL_DELIVER_USER_CONTEXT(box->storage->user); + struct mail_deliver_transaction *dt = + MAIL_DELIVER_STORAGE_CONTEXT(ctx->transaction); if (mbox->module_ctx.super.save_finish(ctx) < 0) return -1; /* initialize most of the fields from dest_mail */ - mail_deliver_log_update_cache(muser->deliver_ctx->cache, - muser->deliver_ctx->pool, ctx->dest_mail); + mail_deliver_log_update_cache(&dt->cache, muser->deliver_ctx->pool, + ctx->dest_mail); return 0; } @@ -530,13 +544,15 @@ static int mail_deliver_copy(struct mail_save_context *ctx, struct mail *mail) struct mail_deliver_mailbox *mbox = MAIL_DELIVER_STORAGE_CONTEXT(box); struct mail_deliver_user *muser = MAIL_DELIVER_USER_CONTEXT(box->storage->user); + struct mail_deliver_transaction *dt = + MAIL_DELIVER_STORAGE_CONTEXT(ctx->transaction); if (mbox->module_ctx.super.copy(ctx, mail) < 0) return -1; /* initialize most of the fields from dest_mail */ - mail_deliver_log_update_cache(muser->deliver_ctx->cache, - muser->deliver_ctx->pool, ctx->dest_mail); + mail_deliver_log_update_cache(&dt->cache, muser->deliver_ctx->pool, + ctx->dest_mail); return 0; } @@ -573,14 +589,51 @@ mail_deliver_cache_update_post_commit(struct mailbox *orig_box, uint32_t uid) mailbox_free(&box); } +static struct mailbox_transaction_context * +mail_deliver_transaction_begin(struct mailbox *box, + enum mailbox_transaction_flags flags) +{ + struct mail_deliver_mailbox *mbox = MAIL_DELIVER_STORAGE_CONTEXT(box); + struct mail_deliver_user *muser = + MAIL_DELIVER_USER_CONTEXT(box->storage->user); + struct mailbox_transaction_context *t; + struct mail_deliver_transaction *dt; + + i_assert(muser != NULL); + i_assert(muser->deliver_ctx != NULL); + + t = mbox->module_ctx.super.transaction_begin(box, flags); + dt = p_new(muser->deliver_ctx->pool, struct mail_deliver_transaction, 1); + + MODULE_CONTEXT_SET(t, mail_deliver_storage_module, dt); + return t; +} + static int mail_deliver_transaction_commit(struct mailbox_transaction_context *ctx, struct mail_transaction_commit_changes *changes_r) { struct mailbox *box = ctx->box; - union mailbox_module_context *mbox = MAIL_DELIVER_STORAGE_CONTEXT(box); + struct mail_deliver_mailbox *mbox = MAIL_DELIVER_STORAGE_CONTEXT(box); + struct mail_deliver_transaction *dt = MAIL_DELIVER_STORAGE_CONTEXT(ctx); + struct mail_deliver_user *muser = + MAIL_DELIVER_USER_CONTEXT(box->storage->user); + + i_assert(dt != NULL); + i_assert(muser != NULL); + i_assert(muser->deliver_ctx != NULL); + + /* sieve creates multiple transactions, saves the mails and + then commits all of them at the end. we'll need to keep + switching the deliver_ctx->cache for each commit. + + we also want to do this only for commits generated by sieve. + other plugins or storage backends may be creating transactions as + well, which we need to ignore. */ + if (mbox->delivery_box) + muser->deliver_ctx->cache = &dt->cache; - if (mbox->super.transaction_commit(ctx, changes_r) < 0) + if (mbox->module_ctx.super.transaction_commit(ctx, changes_r) < 0) return -1; if (array_count(&changes_r->saved_uids) > 0) { @@ -617,6 +670,7 @@ static void mail_deliver_mailbox_allocated(struct mailbox *box) box->vlast = &mbox->module_ctx.super; v->save_finish = mail_deliver_save_finish; v->copy = mail_deliver_copy; + v->transaction_begin = mail_deliver_transaction_begin; v->transaction_commit = mail_deliver_transaction_commit; MODULE_CONTEXT_SET(box, mail_deliver_storage_module, mbox);