From: Timo Sirainen Date: Tue, 3 Dec 2013 15:29:38 +0000 (+0200) Subject: imap: If SELECT fails with "mailbox is inconsistent", disconnect client. (Plus relate... X-Git-Tag: 2.2.10~46 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=08837f59c1466ec0f533f120b167f2a3e87da738;p=thirdparty%2Fdovecot%2Fcore.git imap: If SELECT fails with "mailbox is inconsistent", disconnect client. (Plus related cleanups.) The inconsistency can also be used to indicate that something is badly wrong and nothing useful can be done before client reconnects. --- diff --git a/src/imap/cmd-append.c b/src/imap/cmd-append.c index b4099d7fe1..aaa3ebf6e7 100644 --- a/src/imap/cmd-append.c +++ b/src/imap/cmd-append.c @@ -26,7 +26,6 @@ struct cmd_append_context { struct client *client; struct client_command_context *cmd; - struct mail_storage *storage; struct mailbox *box; struct mailbox_transaction_context *t; time_t started; @@ -181,7 +180,7 @@ cmd_append_catenate_mpurl(struct client_command_context *cmd, /* catenate URL */ ret = imap_msgpart_url_read_part(mpurl, &mpresult, &error); if (ret < 0) { - client_send_storage_error(cmd, ctx->storage); + client_send_box_error(cmd, ctx->box); return -1; } if (ret == 0) { @@ -219,11 +218,11 @@ cmd_append_catenate_mpurl(struct client_command_context *cmd, "read(%s) failed: %s (for CATENATE URL %s)", i_stream_get_name(mpresult.input), i_stream_get_error(mpresult.input), caturl); - client_send_storage_error(cmd, ctx->storage); + client_send_box_error(cmd, ctx->box); ret = -1; } else if (!mpresult.input->eof) { /* save failed */ - client_send_storage_error(cmd, ctx->storage); + client_send_box_error(cmd, ctx->box); ret = -1; } else { /* all the input must be consumed, so istream-chain's read() @@ -248,7 +247,7 @@ cmd_append_catenate_url(struct client_command_context *cmd, const char *caturl) ret = imap_msgpart_url_parse(cmd->client->user, cmd->client->mailbox, caturl, &mpurl, &error); if (ret < 0) { - client_send_storage_error(cmd, ctx->storage); + client_send_box_error(cmd, ctx->box); return -1; } if (ret == 0) { @@ -356,7 +355,7 @@ static void cmd_append_finish_catenate(struct client_command_context *cmd) mailbox_save_cancel(&ctx->save_ctx); } else { if (mailbox_save_finish(&ctx->save_ctx) < 0) { - client_send_storage_error(cmd, ctx->storage); + client_send_box_error(cmd, ctx->box); ctx->failed = TRUE; } } @@ -532,7 +531,7 @@ cmd_append_handle_args(struct client_command_context *cmd, else if (mailbox_keywords_create(ctx->box, keywords_list, &keywords) < 0) { /* invalid keywords - delay failure */ - client_send_storage_error(cmd, ctx->storage); + client_send_box_error(cmd, ctx->box); ctx->failed = TRUE; } } @@ -594,7 +593,7 @@ cmd_append_handle_args(struct client_command_context *cmd, internal_date, timezone_offset); if (mailbox_save_begin(&ctx->save_ctx, ctx->input) < 0) { /* save initialization failed */ - client_send_storage_error(cmd, ctx->storage); + client_send_box_error(cmd, ctx->box); ctx->failed = TRUE; } } @@ -649,7 +648,7 @@ static bool cmd_append_finish_parsing(struct client_command_context *cmd) ret = mailbox_transaction_commit_get_changes(&ctx->t, &changes); if (ret < 0) { - client_send_storage_error(cmd, ctx->storage); + client_send_box_error(cmd, ctx->box); cmd_append_finish(ctx); return TRUE; } @@ -845,7 +844,7 @@ static bool cmd_append_continue_message(struct client_command_context *cmd) mailbox_save_cancel(&ctx->save_ctx); } else if (ctx->save_ctx == NULL) { /* failed above */ - client_send_storage_error(cmd, ctx->storage); + client_send_box_error(cmd, ctx->box); ctx->failed = TRUE; } else if (lit_offset != ctx->literal_size) { /* client disconnected before it finished sending the @@ -857,7 +856,7 @@ static bool cmd_append_continue_message(struct client_command_context *cmd) } else if (ctx->catenate) { /* CATENATE isn't finished yet */ } else if (mailbox_save_finish(&ctx->save_ctx) < 0) { - client_send_storage_error(cmd, ctx->storage); + client_send_box_error(cmd, ctx->box); ctx->failed = TRUE; } @@ -909,8 +908,6 @@ bool cmd_append(struct client_command_context *cmd) if (client_open_save_dest_box(cmd, mailbox, &ctx->box) < 0) ctx->failed = TRUE; else { - ctx->storage = mailbox_get_storage(ctx->box); - ctx->t = mailbox_transaction_begin(ctx->box, MAILBOX_TRANSACTION_FLAG_EXTERNAL | MAILBOX_TRANSACTION_FLAG_ASSIGN_UIDS); diff --git a/src/imap/cmd-create.c b/src/imap/cmd-create.c index 15580e4745..63394de905 100644 --- a/src/imap/cmd-create.c +++ b/src/imap/cmd-create.c @@ -39,7 +39,7 @@ bool cmd_create(struct client_command_context *cmd) box = mailbox_alloc(ns->list, mailbox, 0); if (mailbox_create(box, NULL, directory) < 0) - client_send_storage_error(cmd, mailbox_get_storage(box)); + client_send_box_error(cmd, box); else client_send_tagline(cmd, "OK Create completed."); mailbox_free(&box); diff --git a/src/imap/cmd-delete.c b/src/imap/cmd-delete.c index 420755ba27..33ab1160ba 100644 --- a/src/imap/cmd-delete.c +++ b/src/imap/cmd-delete.c @@ -41,7 +41,7 @@ bool cmd_delete(struct client_command_context *cmd) else { errstr = mailbox_get_last_error(box, &error); if (error != MAIL_ERROR_EXISTS) - client_send_storage_error(cmd, mailbox_get_storage(box)); + client_send_box_error(cmd, box); else { /* mailbox has children */ client_send_tagline(cmd, t_strdup_printf("NO %s", errstr)); diff --git a/src/imap/cmd-expunge.c b/src/imap/cmd-expunge.c index 50889d7a3b..1d25cd8bbb 100644 --- a/src/imap/cmd-expunge.c +++ b/src/imap/cmd-expunge.c @@ -35,8 +35,7 @@ cmd_expunge_finish(struct client_command_context *cmd, if (ret < 0) { errstr = mailbox_get_last_error(client->mailbox, &error); if (error != MAIL_ERROR_PERM) { - client_send_storage_error(cmd, - mailbox_get_storage(client->mailbox)); + client_send_box_error(cmd, client->mailbox); return TRUE; } else { return cmd_sync(cmd, 0, IMAP_SYNC_FLAG_SAFE, diff --git a/src/imap/cmd-getmetadata.c b/src/imap/cmd-getmetadata.c index 534df73102..487e216ca6 100644 --- a/src/imap/cmd-getmetadata.c +++ b/src/imap/cmd-getmetadata.c @@ -374,7 +374,7 @@ bool cmd_getmetadata(struct client_command_context *cmd) ctx->box = mailbox_alloc(ns->list, mailbox, MAILBOX_FLAG_READONLY); if (mailbox_open(ctx->box) < 0) { - client_send_storage_error(cmd, mailbox_get_storage(ctx->box)); + client_send_box_error(cmd, ctx->box); mailbox_free(&ctx->box); return TRUE; } diff --git a/src/imap/cmd-rename.c b/src/imap/cmd-rename.c index b2c6744dc3..83e0e24a63 100644 --- a/src/imap/cmd-rename.c +++ b/src/imap/cmd-rename.c @@ -38,7 +38,7 @@ bool cmd_rename(struct client_command_context *cmd) old_box = mailbox_alloc(old_ns->list, oldname, 0); new_box = mailbox_alloc(new_ns->list, newname, 0); if (mailbox_rename(old_box, new_box) < 0) - client_send_storage_error(cmd, mailbox_get_storage(old_box)); + client_send_box_error(cmd, old_box); else client_send_tagline(cmd, "OK Rename completed."); mailbox_free(&old_box); diff --git a/src/imap/cmd-resetkey.c b/src/imap/cmd-resetkey.c index 472fd45b79..fdb440ab65 100644 --- a/src/imap/cmd-resetkey.c +++ b/src/imap/cmd-resetkey.c @@ -50,7 +50,7 @@ cmd_resetkey_mailbox(struct client_command_context *cmd, /* open mailbox */ box = mailbox_alloc(ns->list, mailbox, flags); if (mailbox_open(box) < 0) { - client_send_storage_error(cmd, mailbox_get_storage(box)); + client_send_box_error(cmd, box); mailbox_free(&box); return TRUE; } diff --git a/src/imap/cmd-select.c b/src/imap/cmd-select.c index 19ce027116..f36dbf8b93 100644 --- a/src/imap/cmd-select.c +++ b/src/imap/cmd-select.c @@ -233,10 +233,8 @@ static bool cmd_select_continue(struct client_command_context *cmd) } ret = imap_fetch_end(ctx->fetch_ctx); - if (ret < 0) { - client_send_storage_error(ctx->cmd, - mailbox_get_storage(ctx->box)); - } + if (ret < 0) + client_send_box_error(ctx->cmd, ctx->box); imap_fetch_free(&ctx->fetch_ctx); cmd_select_finish(ctx, ret); return TRUE; @@ -302,8 +300,7 @@ select_open(struct imap_select_context *ctx, const char *mailbox, bool readonly) flags |= MAILBOX_FLAG_DROP_RECENT; ctx->box = mailbox_alloc(ctx->ns->list, mailbox, flags); if (mailbox_open(ctx->box) < 0) { - client_send_storage_error(ctx->cmd, - mailbox_get_storage(ctx->box)); + client_send_box_error(ctx->cmd, ctx->box); mailbox_free(&ctx->box); return -1; } @@ -312,8 +309,7 @@ select_open(struct imap_select_context *ctx, const char *mailbox, bool readonly) ret = mailbox_enable(ctx->box, client->enabled_features); if (ret < 0 || mailbox_sync(ctx->box, MAILBOX_SYNC_FLAG_FULL_READ) < 0) { - client_send_storage_error(ctx->cmd, - mailbox_get_storage(ctx->box)); + client_send_box_error(ctx->cmd, ctx->box); return -1; } mailbox_get_open_status(ctx->box, STATUS_MESSAGES | STATUS_RECENT | @@ -363,8 +359,7 @@ select_open(struct imap_select_context *ctx, const char *mailbox, bool readonly) if (ctx->qresync_uid_validity == status.uidvalidity && status.uidvalidity != 0) { if ((ret = select_qresync(ctx)) < 0) { - client_send_storage_error(ctx->cmd, - mailbox_get_storage(ctx->box)); + client_send_box_error(ctx->cmd, ctx->box); return -1; } } else { diff --git a/src/imap/cmd-setmetadata.c b/src/imap/cmd-setmetadata.c index 94cfe3638e..57880ddcca 100644 --- a/src/imap/cmd-setmetadata.c +++ b/src/imap/cmd-setmetadata.c @@ -244,9 +244,9 @@ static bool cmd_setmetadata_continue(struct client_command_context *cmd) if (ret < 0 || ctx->cmd_error_sent) /* already sent the error to client */ ; else if (ctx->storage_failure) - client_send_storage_error(cmd, mailbox_get_storage(ctx->box)); + client_send_box_error(cmd, ctx->box); else if (mailbox_transaction_commit(&ctx->trans) < 0) - client_send_storage_error(cmd, mailbox_get_storage(ctx->box)); + client_send_box_error(cmd, ctx->box); else client_send_tagline(cmd, "OK Setmetadata completed."); cmd_setmetadata_deinit(ctx); @@ -301,8 +301,7 @@ bool cmd_setmetadata(struct client_command_context *cmd) else { ctx->box = mailbox_alloc(ns->list, mailbox, 0); if (mailbox_open(ctx->box) < 0) { - client_send_storage_error(cmd, - mailbox_get_storage(ctx->box)); + client_send_box_error(cmd, ctx->box); mailbox_free(&ctx->box); return TRUE; } diff --git a/src/imap/cmd-store.c b/src/imap/cmd-store.c index 8f95cf6baf..7a187e475c 100644 --- a/src/imap/cmd-store.c +++ b/src/imap/cmd-store.c @@ -109,8 +109,7 @@ store_parse_args(struct imap_store_context *ctx, const struct imap_arg *args) if (mailbox_keywords_create(cmd->client->mailbox, keywords_list, &ctx->keywords) < 0) { /* invalid keywords */ - client_send_storage_error(cmd, - mailbox_get_storage(cmd->client->mailbox)); + client_send_box_error(cmd, cmd->client->mailbox); return FALSE; } } @@ -211,8 +210,7 @@ bool cmd_store(struct client_command_context *cmd) ret = mailbox_transaction_commit(&t); if (ret < 0) { array_free(&modified_set); - client_send_storage_error(cmd, - mailbox_get_storage(client->mailbox)); + client_send_box_error(cmd, client->mailbox); return TRUE; } diff --git a/src/imap/cmd-subscribe.c b/src/imap/cmd-subscribe.c index a8c89c96fc..7885f1ad78 100644 --- a/src/imap/cmd-subscribe.c +++ b/src/imap/cmd-subscribe.c @@ -12,7 +12,7 @@ subscribe_is_valid_name(struct client_command_context *cmd, struct mailbox *box) int ret; if ((ret = mailbox_exists(box, TRUE, &existence)) < 0) { - client_send_storage_error(cmd, mailbox_get_storage(box)); + client_send_box_error(cmd, box); return FALSE; } if (existence == MAILBOX_EXISTENCE_NONE) { @@ -71,7 +71,7 @@ bool cmd_subscribe_full(struct client_command_context *cmd, bool subscribe) if (mailbox_set_subscribed(box, subscribe) < 0 && !unsubscribed_mailbox2) { - client_send_storage_error(cmd, mailbox_get_storage(box)); + client_send_box_error(cmd, box); } else { client_send_tagline(cmd, subscribe ? "OK Subscribe completed." : diff --git a/src/imap/cmd-thread.c b/src/imap/cmd-thread.c index c48e6a590c..35b0fa3cd6 100644 --- a/src/imap/cmd-thread.c +++ b/src/imap/cmd-thread.c @@ -283,8 +283,7 @@ bool cmd_thread(struct client_command_context *cmd) ret = imap_thread_orderedsubject(cmd, sargs); mail_search_args_unref(&sargs); if (ret < 0) { - client_send_storage_error(cmd, - mailbox_get_storage(client->mailbox)); + client_send_box_error(cmd, client->mailbox); return TRUE; } diff --git a/src/imap/imap-commands-util.c b/src/imap/imap-commands-util.c index ebe70680d9..fb2b3d0112 100644 --- a/src/imap/imap-commands-util.c +++ b/src/imap/imap-commands-util.c @@ -98,14 +98,14 @@ int client_open_save_dest_box(struct client_command_context *cmd, client_send_tagline(cmd, t_strdup_printf( "NO [TRYCREATE] %s", error_string)); } else { - client_send_storage_error(cmd, mailbox_get_storage(box)); + client_send_box_error(cmd, box); } mailbox_free(&box); return -1; } if (cmd->client->enabled_features != 0) { if (mailbox_enable(box, cmd->client->enabled_features) < 0) { - client_send_storage_error(cmd, mailbox_get_storage(box)); + client_send_box_error(cmd, box); mailbox_free(&box); return -1; } @@ -170,6 +170,18 @@ void client_send_list_error(struct client_command_context *cmd, error)); } +void client_send_box_error(struct client_command_context *cmd, + struct mailbox *box) +{ + if (mailbox_is_inconsistent(box)) { + /* we can't do forced CLOSE, so have to disconnect */ + client_disconnect_with_error(cmd->client, + "IMAP session state is inconsistent, please relogin."); + return; + } + client_send_storage_error(cmd, mailbox_get_storage(box)); +} + void client_send_storage_error(struct client_command_context *cmd, struct mail_storage *storage) { diff --git a/src/imap/imap-commands-util.h b/src/imap/imap-commands-util.h index 304d0d35c2..2cf25fbaad 100644 --- a/src/imap/imap-commands-util.h +++ b/src/imap/imap-commands-util.h @@ -35,6 +35,9 @@ void client_send_list_error(struct client_command_context *cmd, /* Send last mail storage error message to client. */ void client_send_storage_error(struct client_command_context *cmd, struct mail_storage *storage); +/* Send last mailbox's storage error message to client. */ +void client_send_box_error(struct client_command_context *cmd, + struct mailbox *box); /* Send untagged error message to client. */ void client_send_untagged_storage_error(struct client *client, diff --git a/src/imap/imap-search.c b/src/imap/imap-search.c index bbeabab6bc..4c2a9f2e1a 100644 --- a/src/imap/imap-search.c +++ b/src/imap/imap-search.c @@ -491,8 +491,7 @@ static bool cmd_search_more(struct client_command_context *cmd) lost_data = mailbox_search_seen_lost_data(ctx->search_ctx); if (imap_search_deinit(ctx) < 0) { - client_send_storage_error(cmd, - mailbox_get_storage(cmd->client->mailbox)); + client_send_box_error(cmd, cmd->client->mailbox); return TRUE; }