# HG changeset patch
# User Timo Sirainen <timo.sirainen@dovecot.fi>
# Date 1487507685 -7200
# Node ID 03b2c50c18af598e9a00c9bd9ccb14eac37a4fb5
# Parent  22f153275e33213e7a10c089da26e97e19a81c1a
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 -r 22f153275e33 -r 03b2c50c18af src/imap/imap-fetch.c
--- a/src/imap/imap-fetch.c	Thu Feb 16 20:39:12 2017 +0200
+++ b/src/imap/imap-fetch.c	Sun Feb 19 14:34:45 2017 +0200
@@ -429,6 +429,19 @@
 	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;
@@ -497,6 +510,7 @@
 			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;
 		}
@@ -543,15 +557,10 @@
 				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;
@@ -559,7 +568,8 @@
 
 		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;
@@ -617,15 +627,7 @@
 		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 -r 22f153275e33 -r 03b2c50c18af src/imap/imap-fetch.h
--- a/src/imap/imap-fetch.h	Thu Feb 16 20:39:12 2017 +0200
+++ b/src/imap/imap-fetch.h	Sun Feb 19 14:34:45 2017 +0200
@@ -56,6 +56,7 @@
 	uoff_t cur_size, cur_offset;
 	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 @@
 
 	unsigned int fetching:1;
 	unsigned int 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. */
 	unsigned int cur_first:1;
+	/* TRUE if the cur_str prefix has been flushed. More data may still
+	   be added to it. */
 	unsigned int cur_flushed:1;
 	unsigned int line_partial:1;
 	unsigned int line_finished:1;