changeset 21667:bfa56df7a065

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).
author Timo Sirainen <timo.sirainen@dovecot.fi>
date Tue, 21 Feb 2017 12:38:10 +0200
parents b68299be012a
children e26f2729d135
files src/lib-lda/mail-deliver.c
diffstat 1 files changed, 60 insertions(+), 6 deletions(-) [+]
line wrap: on
line diff
--- a/src/lib-lda/mail-deliver.c	Tue Feb 21 12:36:52 2017 +0200
+++ b/src/lib-lda/mail-deliver.c	Tue Feb 21 12:38:10 2017 +0200
@@ -43,6 +43,13 @@
 
 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 @@
 	}
 
 	*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 @@
 	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 @@
 	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 @@
 	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);
 
-	if (mbox->super.transaction_commit(ctx, changes_r) < 0)
+	/* 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->module_ctx.super.transaction_commit(ctx, changes_r) < 0)
 		return -1;
 
 	if (array_count(&changes_r->saved_uids) > 0) {
@@ -617,6 +670,7 @@
 	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);