From 704a96fa677763eef7ae62466e14e83a2f535427 Mon Sep 17 00:00:00 2001 From: Timo Sirainen Date: Sun, 5 Feb 2017 16:49:05 +0200 Subject: [PATCH] imap: Add imap_fetch_failure setting This controls what happens when FETCH fails for some mails. The possible values are: disconnect-immediately: This is the original behavior. Whenever FETCH fails for a mail, the FETCH is aborted and client is disconnected. disconnect-after: The FETCH runs for all the requested mails, skipping any mails that returned failures, but at the end the client is still disconnected. no-after: The FETCH runs for all the requested mails, skipping any mails that returned failures. At the end tagged NO reply is returned. If the client attempts to FETCH the same failed mail more than once, the client is disconnected. This is to avoid clients from going into infinite loops trying to FETCH a broken mail. --- src/imap/cmd-fetch.c | 48 +++++++++++++++++++++++++++++------ src/imap/imap-client.h | 1 + src/imap/imap-commands-util.c | 2 ++ src/imap/imap-fetch.c | 21 ++++++++++++--- src/imap/imap-fetch.h | 2 ++ src/imap/imap-settings.c | 14 ++++++++++ src/imap/imap-settings.h | 8 ++++++ 7 files changed, 85 insertions(+), 11 deletions(-) diff --git a/src/imap/cmd-fetch.c b/src/imap/cmd-fetch.c index e8baf28bd2..9b373a07ea 100644 --- a/src/imap/cmd-fetch.c +++ b/src/imap/cmd-fetch.c @@ -177,23 +177,42 @@ static bool cmd_fetch_finished(struct client_command_context *cmd ATTR_UNUSED) return TRUE; } +static bool imap_fetch_is_failed_retry(struct imap_fetch_context *ctx) +{ + if (!array_is_created(&ctx->client->fetch_failed_uids) || + !array_is_created(&ctx->fetch_failed_uids)) + return FALSE; + return seq_range_array_have_common(&ctx->client->fetch_failed_uids, + &ctx->fetch_failed_uids); + +} + +static void imap_fetch_add_failed_uids(struct imap_fetch_context *ctx) +{ + if (!array_is_created(&ctx->fetch_failed_uids)) + return; + if (!array_is_created(&ctx->client->fetch_failed_uids)) { + p_array_init(&ctx->client->fetch_failed_uids, ctx->client->pool, + array_count(&ctx->fetch_failed_uids)); + } + seq_range_array_merge(&ctx->client->fetch_failed_uids, + &ctx->fetch_failed_uids); +} + static bool cmd_fetch_finish(struct imap_fetch_context *ctx, struct client_command_context *cmd) { static const char *ok_message = "OK Fetch completed."; const char *tagged_reply = ok_message; enum mail_error error; - bool failed, seen_flags_changed = ctx->state.seen_flags_changed; + bool seen_flags_changed = ctx->state.seen_flags_changed; if (ctx->state.skipped_expunged_msgs) { tagged_reply = "OK ["IMAP_RESP_CODE_EXPUNGEISSUED"] " "Some messages were already expunged."; } - failed = imap_fetch_end(ctx) < 0; - imap_fetch_free(&ctx); - - if (failed) { + if (imap_fetch_end(ctx) < 0) { const char *errstr; if (cmd->client->output->closed) { @@ -205,6 +224,7 @@ static bool cmd_fetch_finish(struct imap_fetch_context *ctx, happen if we called client_disconnect() here directly). */ cmd->func = cmd_fetch_finished; + imap_fetch_free(&ctx); return cmd->cancel; } @@ -217,13 +237,25 @@ static bool cmd_fetch_finish(struct imap_fetch_context *ctx, /* Content was invalid */ tagged_reply = t_strdup_printf( "NO ["IMAP_RESP_CODE_PARSE"] %s", errstr); - } else { - /* We never want to reply NO to FETCH requests, - BYE is preferrable (see imap-ml for reasons). */ + } else if (cmd->client->set->parsed_fetch_failure != IMAP_CLIENT_FETCH_FAILURE_NO_AFTER || + imap_fetch_is_failed_retry(ctx)) { + /* By default we never want to reply NO to FETCH + requests, because many IMAP clients become confused + about what they should on NO. A disconnection causes + less confusion. */ client_disconnect_with_error(cmd->client, errstr); + imap_fetch_free(&ctx); return TRUE; + } else { + /* Use a tagged NO to FETCH failure, but only if client + hasn't repeated the FETCH to the same email (so that + we avoid infinitely retries from client.) */ + imap_fetch_add_failed_uids(ctx); + tagged_reply = t_strdup_printf( + "NO ["IMAP_RESP_CODE_SERVERBUG"] %s", errstr); } } + imap_fetch_free(&ctx); return cmd_sync(cmd, (seen_flags_changed ? 0 : MAILBOX_SYNC_FLAG_FAST) | diff --git a/src/imap/imap-client.h b/src/imap/imap-client.h index ec7978ec5e..9b9dd54585 100644 --- a/src/imap/imap-client.h +++ b/src/imap/imap-client.h @@ -157,6 +157,7 @@ struct client { uint64_t sync_last_full_modseq; uint64_t highest_fetch_modseq; + ARRAY_TYPE(seq_range) fetch_failed_uids; /* For imap_logout_format statistics: */ unsigned int fetch_hdr_count, fetch_body_count; diff --git a/src/imap/imap-commands-util.c b/src/imap/imap-commands-util.c index c8a783a57e..6886f4f43f 100644 --- a/src/imap/imap-commands-util.c +++ b/src/imap/imap-commands-util.c @@ -80,6 +80,8 @@ void imap_client_close_mailbox(struct client *client) i_assert(client->mailbox != NULL); + if (array_is_created(&client->fetch_failed_uids)) + array_clear(&client->fetch_failed_uids); client_search_updates_free(client); box = client->mailbox; diff --git a/src/imap/imap-fetch.c b/src/imap/imap-fetch.c index fc54043715..4ad59e0161 100644 --- a/src/imap/imap-fetch.c +++ b/src/imap/imap-fetch.c @@ -429,6 +429,19 @@ static int imap_fetch_send_nil_reply(struct imap_fetch_context *ctx) return 0; } +static bool imap_fetch_cur_failed(struct imap_fetch_context *ctx) +{ + ctx->failures = TRUE; + if (ctx->client->set->parsed_fetch_failure == + IMAP_CLIENT_FETCH_FAILURE_DISCONNECT_IMMEDIATELY) + return FALSE; + + if (!array_is_created(&ctx->fetch_failed_uids)) + p_array_init(&ctx->fetch_failed_uids, ctx->ctx_pool, 8); + seq_range_array_add(&ctx->fetch_failed_uids, ctx->state.cur_mail->uid); + return TRUE; +} + static int imap_fetch_more_int(struct imap_fetch_context *ctx, bool cancel) { struct imap_fetch_state *state = &ctx->state; @@ -452,7 +465,8 @@ static int imap_fetch_more_int(struct imap_fetch_context *ctx, bool cancel) if (imap_fetch_send_nil_reply(ctx) < 0) return -1; } else { - return -1; + if (!imap_fetch_cur_failed(ctx)) + return -1; } } @@ -517,7 +531,8 @@ static int imap_fetch_more_int(struct imap_fetch_context *ctx, bool cancel) } else { i_assert(ret < 0 || state->cont_handler != NULL); - return -1; + if (!imap_fetch_cur_failed(ctx)) + return -1; } } @@ -549,7 +564,7 @@ static int imap_fetch_more_int(struct imap_fetch_context *ctx, bool cancel) state->cur_handler = 0; } - return 1; + return ctx->failures ? -1 : 1; } int imap_fetch_more(struct imap_fetch_context *ctx, diff --git a/src/imap/imap-fetch.h b/src/imap/imap-fetch.h index 10e2c6f9b4..23b06618d1 100644 --- a/src/imap/imap-fetch.h +++ b/src/imap/imap-fetch.h @@ -84,8 +84,10 @@ struct imap_fetch_context { ARRAY_TYPE(keywords) tmp_keywords; struct imap_fetch_state state; + ARRAY_TYPE(seq_range) fetch_failed_uids; bool initialized:1; + bool failures:1; bool flags_have_handler:1; bool flags_update_seen:1; bool flags_show_only_seen_changes:1; diff --git a/src/imap/imap-settings.c b/src/imap/imap-settings.c index 645b8de5d4..4ded405ab2 100644 --- a/src/imap/imap-settings.c +++ b/src/imap/imap-settings.c @@ -72,6 +72,7 @@ static const struct setting_define imap_setting_defines[] = { DEF(SET_STR, imap_logout_format), DEF(SET_STR, imap_id_send), DEF(SET_STR, imap_id_log), + DEF(SET_ENUM, imap_fetch_failure), DEF(SET_BOOL, imap_metadata), DEF(SET_BOOL, imap_literal_minus), DEF(SET_TIME, imap_hibernate_timeout), @@ -99,6 +100,7 @@ static const struct imap_settings imap_default_settings = { "body_count=%{fetch_body_count} body_bytes=%{fetch_body_bytes}", .imap_id_send = "name *", .imap_id_log = "", + .imap_fetch_failure = "disconnect-immediately:disconnect-after:no-after", .imap_metadata = FALSE, .imap_literal_minus = FALSE, .imap_hibernate_timeout = 0, @@ -175,6 +177,18 @@ imap_settings_verify(void *_set, pool_t pool ATTR_UNUSED, const char **error_r) if (imap_settings_parse_workarounds(set, error_r) < 0) return FALSE; + + if (strcmp(set->imap_fetch_failure, "disconnect-immediately") == 0) + set->parsed_fetch_failure = IMAP_CLIENT_FETCH_FAILURE_DISCONNECT_IMMEDIATELY; + else if (strcmp(set->imap_fetch_failure, "disconnect-after") == 0) + set->parsed_fetch_failure = IMAP_CLIENT_FETCH_FAILURE_DISCONNECT_AFTER; + else if (strcmp(set->imap_fetch_failure, "no-after") == 0) + set->parsed_fetch_failure = IMAP_CLIENT_FETCH_FAILURE_NO_AFTER; + else { + *error_r = t_strdup_printf("Unknown imap_fetch_failure: %s", + set->imap_fetch_failure); + return FALSE; + } return TRUE; } /* */ diff --git a/src/imap/imap-settings.h b/src/imap/imap-settings.h index 7adb8e0931..f4ff7bce68 100644 --- a/src/imap/imap-settings.h +++ b/src/imap/imap-settings.h @@ -11,6 +11,12 @@ enum imap_client_workarounds { WORKAROUND_TB_EXTRA_MAILBOX_SEP = 0x08, WORKAROUND_TB_LSUB_FLAGS = 0x10 }; + +enum imap_client_fetch_failure { + IMAP_CLIENT_FETCH_FAILURE_DISCONNECT_IMMEDIATELY, + IMAP_CLIENT_FETCH_FAILURE_DISCONNECT_AFTER, + IMAP_CLIENT_FETCH_FAILURE_NO_AFTER, +}; /* */ struct imap_settings { @@ -25,6 +31,7 @@ struct imap_settings { const char *imap_logout_format; const char *imap_id_send; const char *imap_id_log; + const char *imap_fetch_failure; bool imap_metadata; bool imap_literal_minus; unsigned int imap_hibernate_timeout; @@ -34,6 +41,7 @@ struct imap_settings { in_port_t imap_urlauth_port; enum imap_client_workarounds parsed_workarounds; + enum imap_client_fetch_failure parsed_fetch_failure; }; extern const struct setting_parser_info imap_setting_parser_info;