changeset 16642:3c2e1879fdf6

imap: Various APPEND/CATENATE error handling bugfixes. Found using Apple's catenate.pl test script.
author Timo Sirainen <tss@iki.fi>
date Sun, 04 Aug 2013 18:34:43 +0300
parents 2d6497a4f124
children 3af0ae411b37
files src/imap/cmd-append.c
diffstat 1 files changed, 51 insertions(+), 33 deletions(-) [+]
line wrap: on
line diff
--- a/src/imap/cmd-append.c	Sun Aug 04 14:03:54 2013 +0300
+++ b/src/imap/cmd-append.c	Sun Aug 04 18:34:43 2013 +0300
@@ -102,7 +102,8 @@
 		   until newline is found. */
 		client->input_skip_line = TRUE;
 
-		client_send_command_error(cmd, "Too long argument.");
+		if (!ctx->failed)
+			client_send_command_error(cmd, "Too long argument.");
 		cmd->param_error = TRUE;
 		client_command_free(&cmd);
 		return;
@@ -125,11 +126,13 @@
 
 static void cmd_append_finish(struct cmd_append_context *ctx)
 {
-	imap_parser_unref(&ctx->save_parser);
+	if (ctx->save_parser != NULL)
+		imap_parser_unref(&ctx->save_parser);
 
 	i_assert(ctx->client->input_lock == ctx->cmd);
 
-	io_remove(&ctx->client->io);
+	if (ctx->client->io != NULL)
+		io_remove(&ctx->client->io);
 	/* we must put back the original flush callback before beginning to
 	   sync (the command is still unfinished at that point) */
 	o_stream_set_flush_callback(ctx->client->output,
@@ -147,12 +150,18 @@
 		mailbox_free(&ctx->box);
 }
 
-static void cmd_append_send_literal_continue(struct client *client)
+static bool cmd_append_send_literal_continue(struct cmd_append_context *ctx)
 {
-	o_stream_nsend(client->output, "+ OK\r\n", 6);
-	o_stream_nflush(client->output);
-	o_stream_uncork(client->output);
-	o_stream_cork(client->output);
+	if (ctx->failed) {
+		/* tagline was already sent, we can abort here */
+		return FALSE;
+	}
+
+	o_stream_nsend(ctx->client->output, "+ OK\r\n", 6);
+	o_stream_nflush(ctx->client->output);
+	o_stream_uncork(ctx->client->output);
+	o_stream_cork(ctx->client->output);
+	return TRUE;
 }
 
 static int
@@ -362,8 +371,10 @@
 		args++;
 		if (args->type == IMAP_ARG_LITERAL_SIZE ||
 		    args->type == IMAP_ARG_LITERAL_SIZE_NONSYNC) {
-			if (args->type == IMAP_ARG_LITERAL_SIZE)
-				cmd_append_send_literal_continue(ctx->client);
+			if (args->type == IMAP_ARG_LITERAL_SIZE) {
+				if (!cmd_append_send_literal_continue(ctx))
+					return TRUE;
+			}
 			imap_parser_read_last_literal(ctx->save_parser);
 			return FALSE;
 		}
@@ -431,12 +442,10 @@
 	/* TEXT <literal> */
 
 	if (!nonsync) {
-		if (ctx->failed) {
-			/* tagline was already sent, we can abort here */
+		if (!cmd_append_send_literal_continue(ctx)) {
 			cmd_append_finish(ctx);
 			return TRUE;
 		}
-		cmd_append_send_literal_continue(client);
 	}
 
 	i_assert(ctx->litinput != NULL);
@@ -549,8 +558,13 @@
 			if (!ctx->failed) {
 				client_send_tagline(cmd,
 					"NO Can't save a zero byte message.");
+				ctx->failed = TRUE;
 			}
-			return -1;
+			if (!*nonsync_r)
+				return -1;
+			/* {0+} used. although there isn't any point in using
+			   MULTIAPPEND here and adding more messages, it is
+			   technically valid so we'll continue parsing.. */
 		}
 		ctx->litinput = i_stream_create_limit(client->input, ctx->literal_size);
 		ctx->input = ctx->litinput;
@@ -583,7 +597,9 @@
 		return 1;
 	} else if (cat_list->type == IMAP_ARG_EOL) {
 		/* zero parts */
-		client_send_command_error(cmd, "Empty CATENATE list.");
+		if (!ctx->failed)
+			client_send_command_error(cmd, "Empty CATENATE list.");
+		client->input_skip_line = TRUE;
 		return -1;
 	} else if ((ret = cmd_append_catenate(cmd, cat_list, nonsync_r)) < 0) {
 		/* invalid parameters, abort immediately */
@@ -599,7 +615,6 @@
 
 static bool cmd_append_finish_parsing(struct client_command_context *cmd)
 {
-	struct client *client = cmd->client;
 	struct cmd_append_context *ctx = cmd->context;
 	enum mailbox_sync_flags sync_flags;
 	enum imap_sync_flags imap_flags;
@@ -609,7 +624,7 @@
 	int ret;
 
 	/* eat away the trailing CRLF */
-	client->input_skip_line = TRUE;
+	cmd->client->input_skip_line = TRUE;
 
 	if (ctx->failed) {
 		/* we failed earlier, error message is sent */
@@ -656,10 +671,12 @@
 }
 
 static bool cmd_append_args_can_stop(struct cmd_append_context *ctx,
-				     const struct imap_arg *args)
+				     const struct imap_arg *args,
+				     bool *last_literal_r)
 {
 	const struct imap_arg *cat_list;
 
+	*last_literal_r = FALSE;
 	if (args->type == IMAP_ARG_EOL)
 		return TRUE;
 
@@ -673,8 +690,11 @@
 	    args->type == IMAP_ARG_LITERAL_SIZE_NONSYNC)
 		return TRUE;
 	if (imap_arg_atom_equals(args, "CATENATE") &&
-	    imap_arg_get_list(&args[1], &cat_list))
-		return catenate_args_can_stop(ctx, cat_list);
+	    imap_arg_get_list(&args[1], &cat_list)) {
+		if (catenate_args_can_stop(ctx, cat_list))
+			return TRUE;
+		*last_literal_r = TRUE;
+	}
 	return FALSE;
 }
 
@@ -685,7 +705,7 @@
 	const struct imap_arg *args;
 	const char *msg;
 	unsigned int arg_min_count;
-	bool fatal, nonsync;
+	bool fatal, nonsync, last_literal;
 	int ret;
 
 	/* this function gets called 1) after parsing APPEND <mailbox> and
@@ -703,13 +723,19 @@
 	/* parse the entire line up to the first message literal, or in case
 	   the input buffer is full of MULTIAPPEND CATENATE URLs, parse at
 	   least until the beginning of the next message */
-	arg_min_count = 0;
+	arg_min_count = 0; last_literal = FALSE;
 	do {
-		ret = imap_parser_read_args(ctx->save_parser, ++arg_min_count,
+		if (!last_literal)
+			arg_min_count++;
+		else {
+			/* we only read the literal size. now we read the
+			   literal itself. */
+		}
+		ret = imap_parser_read_args(ctx->save_parser, arg_min_count,
 					    IMAP_PARSE_FLAG_LITERAL_SIZE |
 					    IMAP_PARSE_FLAG_LITERAL8, &args);
 	} while (ret >= (int)arg_min_count &&
-		 !cmd_append_args_can_stop(ctx, args));
+		 !cmd_append_args_can_stop(ctx, args, &last_literal));
 	if (ret == -1) {
 		if (!ctx->failed) {
 			msg = imap_parser_get_error(ctx->save_parser, &fatal);
@@ -745,19 +771,11 @@
 		return cmd_append_parse_new_msg(cmd);
 	}
 
-	if (!ctx->catenate) {
-		/* after literal comes CRLF, if we fail make sure
-		   we eat it away */
-		client->input_skip_line = TRUE;
-	}
-
 	if (!nonsync) {
-		if (ctx->failed) {
-			/* tagline was already sent, we can abort here */
+		if (!cmd_append_send_literal_continue(ctx)) {
 			cmd_append_finish(ctx);
 			return TRUE;
 		}
-		cmd_append_send_literal_continue(client);
 	}
 
 	i_assert(ctx->litinput != NULL);