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.

diff --git a/src/imap/cmd-fetch.c b/src/imap/cmd-fetch.c
index e8baf28bd28c069575b733a420857355ab691b91..9b373a07ea3a83e5109979622cceabe004b3e2a4 100644 (file)
--- 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 95b843ed6fb2775b2b729e24178fd3f261e874d7..f554b6ef220c2c27a1d10fc3f6cd6fc60faadc92 100644 (file)
--- 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 c8a783a57e54f63998cb8ce3d0afab92c5c04d63..6886f4f43fb7ecaf8920a2b3adc741ca07415fc9 100644 (file)
--- 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 03191122243074b4ef6cbd861c84da0366da9f90..83d16968aaabbb774589d5687bd3362302f38c55 100644 (file)
--- 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;
                        }
                }
 
@@ -518,7 +532,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;
                                }
                        }
 
@@ -551,7 +566,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 aa37d779bb73e876197951444bccfabf9ec4ba62..fdf29774923f85e56c514dd613074b8058568375 100644 (file)
--- 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;
 
        unsigned int initialized:1;
+       unsigned int failures:1;
        unsigned int flags_have_handler:1;
        unsigned int flags_update_seen:1;
        unsigned int flags_show_only_seen_changes:1;

diff --git a/src/imap/imap-settings.c b/src/imap/imap-settings.c
index 47b1c8914c5487b39bf3d2f1e86f98c03c5334ae..526e16bb62c2cf66a7f0b4ffe207c4b6935ca5e1 100644 (file)
--- 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_TIME, imap_hibernate_timeout),
 
@@ -95,6 +96,7 @@ static const struct imap_settings imap_default_settings = {
        .imap_logout_format = "in=%i out=%o",
        .imap_id_send = "name *",
        .imap_id_log = "",
+       .imap_fetch_failure = "disconnect-immediately:disconnect-after:no-after",
        .imap_metadata = FALSE,
        .imap_hibernate_timeout = 0,
 
@@ -170,6 +172,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;
 }
 /* </settings checks> */

diff --git a/src/imap/imap-settings.h b/src/imap/imap-settings.h
index cfb9dccc58592aae527ab3a5ea54bee5729496da..2ce9cb496a1de0a9aa27ad8774711ef4250c894f 100644 (file)
--- 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,
+};
 /* </settings checks> */
 
 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;
        unsigned int imap_hibernate_timeout;
 
@@ -33,6 +40,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;