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

@@ -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) |

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;

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;

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,

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;

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> */

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;