dovecot-2.2: Fixes and improvements to istream-attachment-extrac...

dovecot at dovecot.org dovecot at dovecot.org
Sun Sep 22 04:14:51 EEST 2013


details:   http://hg.dovecot.org/dovecot-2.2/rev/4f68ac02f46c
changeset: 16811:4f68ac02f46c
user:      Timo Sirainen <tss at iki.fi>
date:      Sun Sep 22 04:14:23 2013 +0300
description:
Fixes and improvements to istream-attachment-extractor error handling.
If an attachment saving failed, the mail was still saved to disk, just
without the attachments.

diffstat:

 src/lib-mail/istream-attachment-extractor.c |  38 +++++++++++++++++++---------
 src/lib-mail/istream-attachment-extractor.h |   4 +-
 src/lib-mail/test-istream-attachment.c      |   2 +
 src/lib-storage/index/index-attachment.c    |  22 +++++++++++-----
 4 files changed, 45 insertions(+), 21 deletions(-)

diffs (233 lines):

diff -r 1cf67db75455 -r 4f68ac02f46c src/lib-mail/istream-attachment-extractor.c
--- a/src/lib-mail/istream-attachment-extractor.c	Sun Sep 22 03:40:14 2013 +0300
+++ b/src/lib-mail/istream-attachment-extractor.c	Sun Sep 22 04:14:23 2013 +0300
@@ -59,6 +59,7 @@
 	struct attachment_istream_part part;
 
 	bool retry_read;
+	bool failed;
 };
 
 static void stream_add_data(struct attachment_istream *astream,
@@ -416,7 +417,8 @@
 	return 0;
 }
 
-static int astream_part_finish(struct attachment_istream *astream)
+static int
+astream_part_finish(struct attachment_istream *astream, const char **error_r)
 {
 	struct attachment_istream_part *part = &astream->part;
 	struct istream_attachment_info info;
@@ -428,8 +430,9 @@
 	int ret = 0;
 
 	if (o_stream_nfinish(part->temp_output) < 0) {
-		i_error("istream-attachment: write(%s) failed: %m",
-			o_stream_get_name(part->temp_output));
+		*error_r = t_strdup_printf("write(%s) failed: %s",
+					   o_stream_get_name(part->temp_output),
+					   o_stream_get_error(part->temp_output));
 		return -1;
 	}
 
@@ -477,7 +480,7 @@
 		   as attachment */
 		info.encoded_size = part->temp_output->offset;
 	}
-	if (astream->set.open_attachment_ostream(&info, &output,
+	if (astream->set.open_attachment_ostream(&info, &output, error_r,
 						 astream->context) < 0)
 		return -1;
 
@@ -489,13 +492,13 @@
 	}
 
 	if (input->stream_errno != 0) {
-		i_error("istream-attachment: read(%s) failed: %m",
-			i_stream_get_name(input));
+		*error_r = t_strdup_printf("read(%s) failed: %s",
+			i_stream_get_name(input), i_stream_get_error(input));
 		ret = -1;
 	}
 	i_stream_destroy(&input);
 
-	if (astream->set.close_attachment_ostream(output, ret == 0,
+	if (astream->set.close_attachment_ostream(output, ret == 0, error_r,
 						  astream->context) < 0)
 		ret = -1;
 	return ret;
@@ -521,7 +524,7 @@
 }
 
 static int
-astream_end_of_part(struct attachment_istream *astream)
+astream_end_of_part(struct attachment_istream *astream, const char **error_r)
 {
 	struct attachment_istream_part *part = &astream->part;
 	size_t old_size;
@@ -542,7 +545,7 @@
 		break;
 	case MAIL_ATTACHMENT_STATE_YES:
 		old_size = astream->istream.pos - astream->istream.skip;
-		if (astream_part_finish(astream) < 0)
+		if (astream_part_finish(astream, error_r) < 0)
 			ret = -1;
 		else {
 			/* finished base64 may have added a few more trailing
@@ -562,6 +565,7 @@
 	struct istream_private *stream = &astream->istream;
 	struct message_block block;
 	size_t old_size, new_size;
+	const char *error;
 	int ret;
 
 	*retry_r = FALSE;
@@ -569,11 +573,16 @@
 	if (stream->pos - stream->skip >= stream->max_buffer_size)
 		return -2;
 
+	if (astream->failed) {
+		stream->istream.stream_errno = EINVAL;
+		return -1;
+	}
+
 	old_size = stream->pos - stream->skip;
 	switch (message_parser_parse_next_block(astream->parser, &block)) {
 	case -1:
 		/* done / error */
-		ret = astream_end_of_part(astream);
+		ret = astream_end_of_part(astream, &error);
 		if (ret > 0) {
 			/* final data */
 			new_size = stream->pos - stream->skip;
@@ -582,8 +591,11 @@
 		stream->istream.eof = TRUE;
 		stream->istream.stream_errno = stream->parent->stream_errno;
 
-		if (ret < 0)
+		if (ret < 0) {
+			io_stream_set_error(&stream->iostream, "%s", error);
 			stream->istream.stream_errno = EINVAL;
+			astream->failed = TRUE;
+		}
 		astream->cur_part = NULL;
 		return -1;
 	case 0:
@@ -595,8 +607,10 @@
 
 	if (block.part != astream->cur_part && astream->cur_part != NULL) {
 		/* end of a MIME part */
-		if (astream_end_of_part(astream) < 0) {
+		if (astream_end_of_part(astream, &error) < 0) {
+			io_stream_set_error(&stream->iostream, "%s", error);
 			stream->istream.stream_errno = EINVAL;
+			astream->failed = TRUE;
 			return -1;
 		}
 	}
diff -r 1cf67db75455 -r 4f68ac02f46c src/lib-mail/istream-attachment-extractor.h
--- a/src/lib-mail/istream-attachment-extractor.h	Sun Sep 22 03:40:14 2013 +0300
+++ b/src/lib-mail/istream-attachment-extractor.h	Sun Sep 22 04:14:23 2013 +0300
@@ -40,10 +40,10 @@
 	/* Create output stream for attachment */
 	int (*open_attachment_ostream)(struct istream_attachment_info *info,
 				       struct ostream **output_r,
-				       void *context);
+				       const char **error_r, void *context);
 	/* Finish output stream */
 	int (*close_attachment_ostream)(struct ostream *output, bool success,
-					void *context);
+					const char **error_r, void *context);
 };
 
 struct istream *
diff -r 1cf67db75455 -r 4f68ac02f46c src/lib-mail/test-istream-attachment.c
--- a/src/lib-mail/test-istream-attachment.c	Sun Sep 22 03:40:14 2013 +0300
+++ b/src/lib-mail/test-istream-attachment.c	Sun Sep 22 04:14:23 2013 +0300
@@ -84,6 +84,7 @@
 
 static int test_open_attachment_ostream(struct istream_attachment_info *info,
 					struct ostream **output_r,
+					const char **error_r ATTR_UNUSED,
 					void *context ATTR_UNUSED)
 {
 	struct attachment *a;
@@ -106,6 +107,7 @@
 }
 
 static int test_close_attachment_ostream(struct ostream *output, bool success,
+					 const char **error_r ATTR_UNUSED,
 					 void *context ATTR_UNUSED)
 {
 	struct attachment *a;
diff -r 1cf67db75455 -r 4f68ac02f46c src/lib-storage/index/index-attachment.c
--- a/src/lib-storage/index/index-attachment.c	Sun Sep 22 03:40:14 2013 +0300
+++ b/src/lib-storage/index/index-attachment.c	Sun Sep 22 04:14:23 2013 +0300
@@ -75,7 +75,8 @@
 
 static int
 index_attachment_open_ostream(struct istream_attachment_info *info,
-			      struct ostream **output_r, void *context)
+			      struct ostream **output_r,
+			      const char **error_r ATTR_UNUSED, void *context)
 {
 	struct mail_save_context *ctx = context;
 	struct mail_save_attachment *attach = ctx->data.attach;
@@ -118,12 +119,11 @@
 }
 
 static int
-index_attachment_close_ostream(struct ostream *output,
-			       bool success, void *context)
+index_attachment_close_ostream(struct ostream *output, bool success,
+			       const char **error_r, void *context)
 {
 	struct mail_save_context *ctx = context;
 	struct mail_save_attachment *attach = ctx->data.attach;
-	struct mail_storage *storage = ctx->transaction->box->storage;
 	int ret = success ? 0 : -1;
 
 	i_assert(attach->cur_file != NULL);
@@ -131,8 +131,7 @@
 	if (ret < 0)
 		fs_write_stream_abort(attach->cur_file, &output);
 	else if (fs_write_stream_finish(attach->cur_file, &output) < 0) {
-		mail_storage_set_critical(storage, "%s",
-			fs_file_last_error(attach->cur_file));
+		*error_r = t_strdup(fs_file_last_error(attach->cur_file));
 		ret = -1;
 	}
 	fs_file_deinit(&attach->cur_file);
@@ -202,6 +201,9 @@
 	size_t size;
 	ssize_t ret;
 
+	if (attach->input->stream_errno != 0)
+		return -1;
+
 	do {
 		ret = i_stream_read(attach->input);
 		if (ret > 0) {
@@ -216,6 +218,12 @@
 		}
 	} while (ret != -1);
 
+	if (attach->input->stream_errno != 0) {
+		mail_storage_set_critical(storage, "read(%s) failed: %s",
+					  i_stream_get_name(attach->input),
+					  i_stream_get_error(attach->input));
+		return -1;
+	}
 	if (ctx->data.output != NULL) {
 		if (save_check_write_error(storage, ctx->data.output) < 0)
 			return -1;
@@ -229,7 +237,7 @@
 
 	(void)i_stream_read(attach->input);
 	i_assert(attach->input->eof);
-	return attach->input->stream_errno == 0;
+	return attach->input->stream_errno == 0 ? 0 : -1;
 }
 
 void index_attachment_save_free(struct mail_save_context *ctx)


More information about the dovecot-cvs mailing list