From: Timo Sirainen Date: Sun, 22 Sep 2013 01:14:23 +0000 (+0300) Subject: Fixes and improvements to istream-attachment-extractor error handling. X-Git-Tag: 2.2.6~13 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=0482d891a6669537e10a1abc9866b45f2fc55fcc;p=thirdparty%2Fdovecot%2Fcore.git 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. --- diff --git a/src/lib-mail/istream-attachment-extractor.c b/src/lib-mail/istream-attachment-extractor.c index 4abab586d6..307262f463 100644 --- a/src/lib-mail/istream-attachment-extractor.c +++ b/src/lib-mail/istream-attachment-extractor.c @@ -59,6 +59,7 @@ struct attachment_istream { struct attachment_istream_part part; bool retry_read; + bool failed; }; static void stream_add_data(struct attachment_istream *astream, @@ -416,7 +417,8 @@ static int astream_decode_base64(struct attachment_istream *astream) 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 @@ static int astream_part_finish(struct attachment_istream *astream) 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 @@ static int astream_part_finish(struct attachment_istream *astream) 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 @@ static int astream_part_finish(struct attachment_istream *astream) } 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 void astream_part_reset(struct attachment_istream *astream) } 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 @@ astream_end_of_part(struct attachment_istream *astream) 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 @@ static int astream_read_next(struct attachment_istream *astream, bool *retry_r) 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 @@ static int astream_read_next(struct attachment_istream *astream, bool *retry_r) 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 @@ static int astream_read_next(struct attachment_istream *astream, bool *retry_r) 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 @@ static int astream_read_next(struct attachment_istream *astream, bool *retry_r) 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 --git a/src/lib-mail/istream-attachment-extractor.h b/src/lib-mail/istream-attachment-extractor.h index 4981fec5af..08201c91bf 100644 --- a/src/lib-mail/istream-attachment-extractor.h +++ b/src/lib-mail/istream-attachment-extractor.h @@ -40,10 +40,10 @@ struct istream_attachment_settings { /* 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 --git a/src/lib-mail/test-istream-attachment.c b/src/lib-mail/test-istream-attachment.c index da5e3c8f80..74663fc393 100644 --- a/src/lib-mail/test-istream-attachment.c +++ b/src/lib-mail/test-istream-attachment.c @@ -84,6 +84,7 @@ static int test_open_temp_fd(void *context ATTR_UNUSED) 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_open_attachment_ostream(struct istream_attachment_info *info, } 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 --git a/src/lib-storage/index/index-attachment.c b/src/lib-storage/index/index-attachment.c index b2c64d6b5b..d793f92aae 100644 --- a/src/lib-storage/index/index-attachment.c +++ b/src/lib-storage/index/index-attachment.c @@ -75,7 +75,8 @@ static int index_attachment_open_temp_fd(void *context) 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 @@ index_attachment_open_ostream(struct istream_attachment_info *info, } 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 @@ index_attachment_close_ostream(struct ostream *output, 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 @@ int index_attachment_save_continue(struct mail_save_context *ctx) 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 @@ int index_attachment_save_continue(struct mail_save_context *ctx) } } 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 @@ int index_attachment_save_finish(struct mail_save_context *ctx) (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)