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 ec7978ec5eece6e5f34a3951b1719fe65af1bca1..9b9dd545853342cc7029d577289306fdace2865e 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 fc54043715906c547a476b2d3a8fed4bd8181df1..4ad59e0161c80c368c07b132a193de24e4f6002a 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;
                        }
                }
 
@@ -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 10e2c6f9b415aeae3f0c6509b18619b39801c5d2..23b06618d1ebf2745d551e61afaa696561f55c13 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;
 
        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 645b8de5d4dc7d83f28ae5bb058fb1b24cb57ba5..4ded405ab2a5608d99c02f9db30a58cb4afa2e13 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_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;
 }
 /* </settings checks> */

diff --git a/src/imap/imap-settings.h b/src/imap/imap-settings.h
index 7adb8e093133d86b1f53484ff866005a9305f901..f4ff7bce683285276753c523f3d8fc14dfd65dee 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;
        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;