Skip to content

Commit

Permalink
lib-lda: Fix delivery logging when Sieve performs multiple actions
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
sirainen committed Feb 21, 2017
1 parent 9737f57 commit 181b3d6
Showing 1 changed file with 60 additions and 6 deletions.
66 changes: 60 additions & 6 deletions src/lib-lda/mail-deliver.c
Expand Up @@ -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[] = {
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 181b3d6

Please sign in to comment.