From: Timo Sirainen <timo.sirainen@dovecot.fi>
Date: Sun, 19 Feb 2017 12:34:45 +0000 (+0200)
Subject: imap: Fix sending UID only when necessary on broken FETCHes.
X-Git-Tag: 2.3.0.rc1~2071
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=905627a760ce8bf4141b361f72858a99975ded3c;p=thirdparty%2Fdovecot%2Fcore.git

imap: Fix sending UID only when necessary on broken FETCHes.

b748f91d0677fffaa2208b39ebb6db3aeb2e937b changed UID to be sent for most
FETCH replies. There was also some existing code that attempted to do this,
but didn't fully work.

So now:

1) If there are no non-buffered replies, the entire FETCH response isn't
sent.

2) If the buffer was already flushed and nothing else was sent, add UID to
reply. The code paths for handling this are differently for
imap_fetch_failure = disconnect-immediately vs others (depending on
imap_fetch_cur_failed() return value).
---

diff --git a/src/imap/imap-fetch.c b/src/imap/imap-fetch.c
index 4ad59e0161..ee652d4f69 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 void imap_fetch_fix_empty_reply(struct imap_fetch_context *ctx)
+{
+	if (ctx->state.cur_flushed && ctx->state.cur_first) {
+		/* we've flushed an empty "FETCH (" reply so
+		   far. we can't take it back, but RFC 3501
+		   doesn't allow returning empty "FETCH ()"
+		   either, so just add the current message's
+		   UID there. */
+		str_printfa(ctx->state.cur_str, "UID %u ",
+			    ctx->state.cur_mail->uid);
+	}
+}
+
 static bool imap_fetch_cur_failed(struct imap_fetch_context *ctx)
 {
 	ctx->failures = TRUE;
@@ -496,6 +509,7 @@ static int imap_fetch_more_int(struct imap_fetch_context *ctx, bool cancel)
 			str_printfa(state->cur_str, "* %u FETCH (",
 				    state->cur_mail->seq);
 			state->cur_first = TRUE;
+			state->cur_str_prefix_size = str_len(state->cur_str);
 			state->cur_flushed = FALSE;
 			state->line_finished = FALSE;
 		}
@@ -541,15 +555,10 @@ static int imap_fetch_more_int(struct imap_fetch_context *ctx, bool cancel)
 				i_stream_unref(&state->cur_input);
 		}
 
-		if (state->cur_first) {
-			/* Writing FETCH () violates IMAP RFC. It's a bit
-			   troublesome to delay flushing of "FETCH (" with
-			   non-buffered data, so we'll just fix this by giving
-			   UID as the response. */
-			str_printfa(state->cur_str, "UID %u",
-				    state->cur_mail->uid);
-		}
-		if (str_len(state->cur_str) > 0) {
+		imap_fetch_fix_empty_reply(ctx);
+		if (str_len(state->cur_str) > 0 &&
+		    (state->cur_flushed ||
+		     str_len(state->cur_str) != state->cur_str_prefix_size)) {
 			/* no non-buffered handlers */
 			if (imap_fetch_flush_buffer(ctx) < 0)
 				return -1;
@@ -557,7 +566,8 @@ static int imap_fetch_more_int(struct imap_fetch_context *ctx, bool cancel)
 
 		state->line_finished = TRUE;
 		state->line_partial = FALSE;
-		o_stream_nsend(client->output, ")\r\n", 3);
+		if (state->cur_flushed)
+			o_stream_nsend(client->output, ")\r\n", 3);
 		client->last_output = ioloop_time;
 
 		state->cur_mail = NULL;
@@ -615,15 +625,7 @@ int imap_fetch_end(struct imap_fetch_context *ctx)
 		ctx->state.fetching = FALSE;
 		if (!state->line_finished &&
 		    (!state->cur_first || state->cur_flushed)) {
-			if (state->cur_first) {
-				/* we've flushed an empty "FETCH (" reply so
-				   far. we can't take it back, but RFC 3501
-				   doesn't allow returning empty "FETCH ()"
-				   either, so just add the current message's
-				   UID there. */
-				str_printfa(ctx->state.cur_str, "UID %u ",
-					    state->cur_mail->uid);
-			}
+			imap_fetch_fix_empty_reply(ctx);
 			if (imap_fetch_flush_buffer(ctx) < 0)
 				state->failed = TRUE;
 			if (o_stream_send(ctx->client->output, ")\r\n", 3) < 0)
diff --git a/src/imap/imap-fetch.h b/src/imap/imap-fetch.h
index 23b06618d1..a88f0e4204 100644
--- a/src/imap/imap-fetch.h
+++ b/src/imap/imap-fetch.h
@@ -56,6 +56,7 @@ struct imap_fetch_state {
 	uoff_t cur_size;
 	enum mail_fetch_field cur_size_field;
 	string_t *cur_str;
+	size_t cur_str_prefix_size;
 	struct istream *cur_input;
 	bool skip_cr;
 	int (*cont_handler)(struct imap_fetch_context *ctx);
@@ -63,7 +64,12 @@ struct imap_fetch_state {
 
 	bool fetching:1;
 	bool seen_flags_changed:1;
+	/* TRUE if the first FETCH parameter result hasn't yet been sent to
+	   the IMAP client. Note that this doesn't affect buffered content in
+	   cur_str until it gets flushed out. */
 	bool cur_first:1;
+	/* TRUE if the cur_str prefix has been flushed. More data may still
+	   be added to it. */
 	bool cur_flushed:1;
 	bool line_partial:1;
 	bool line_finished:1;