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 4abab586d6c105fc8b53d4399e6857937eaee374..307262f463299a23a0af9c9c83ffb4f8747e1ae0 100644 (file)
--- 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 4981fec5af2cb1f1012065f6760eaed92ccad3b5..08201c91bfbaf8c0d19a0d89d8f9c8b2929fdbfe 100644 (file)
--- 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 da5e3c8f80cd457f8ddb9175debf8a3a3664d9b6..74663fc3932901a842ca14e17d40c8118687a5ef 100644 (file)
--- 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 b2c64d6b5be1f8b02892b91b31da36669ed368fc..d793f92aae025e67307bb45fb0b060ead110fcb5 100644 (file)
--- 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)