dbox: Reimplemented file cleanup in a safer way. Added some more error handling.

--HG--
branch : HEAD

diff --git a/src/lib-storage/index/dbox/dbox-file.c b/src/lib-storage/index/dbox/dbox-file.c
index d0f014c4901fc3404ed590bc6cb3b0ba527c951a..bbe5bf4c1ee9e84ae4c17b5403c8dbc099544805 100644 (file)
--- a/src/lib-storage/index/dbox/dbox-file.c
+++ b/src/lib-storage/index/dbox/dbox-file.c
@@ -41,12 +41,17 @@ void dbox_file_set_syscall_error(struct dbox_file *file, const char *function)
                                  function, file->current_path);
 }
 
-static void dbox_file_set_corrupted(struct dbox_file *file, const char *reason)
+void dbox_file_set_corrupted(struct dbox_file *file, const char *reason, ...)
 {
+       va_list args;
+
+       va_start(args, reason);
        mail_storage_set_critical(&file->storage->storage,
                "Corrupted dbox file %s (around offset=%"PRIuUOFF_T"): %s",
                file->current_path,
-               file->input == NULL ? 0 : file->input->v_offset, reason);
+               file->input == NULL ? 0 : file->input->v_offset,
+               t_strdup_vprintf(reason, args));
+       va_end(args);
 }
 
 static struct dbox_file *
@@ -557,10 +562,9 @@ dbox_file_read_mail_header(struct dbox_file *file, uoff_t *physical_size_r)
        if (ret <= 0) {
                if (file->input->stream_errno == 0) {
                        /* EOF, broken offset or file truncated */
-                       dbox_file_set_corrupted(file, t_strdup_printf(
-                               "EOF reading msg header "
-                               "(got %"PRIuSIZE_T"/%u bytes)",
-                               size, file->msg_header_size));
+                       dbox_file_set_corrupted(file, "EOF reading msg header "
+                                               "(got %"PRIuSIZE_T"/%u bytes)",
+                                               size, file->msg_header_size);
                        return 0;
                }
                dbox_file_set_syscall_error(file, "read()");
@@ -617,10 +621,10 @@ int dbox_file_get_mail_stream(struct dbox_file *file, uoff_t offset,
 }
 
 static int
-dbox_file_seek_next_at_metadata(struct dbox_file *file, uoff_t *offset,
-                               uoff_t *physical_size_r)
+dbox_file_seek_next_at_metadata(struct dbox_file *file, uoff_t *offset)
 {
        const char *line;
+       uoff_t physical_size;
        int ret;
 
        if ((ret = dbox_file_metadata_skip_header(file)) <= 0)
@@ -636,16 +640,13 @@ dbox_file_seek_next_at_metadata(struct dbox_file *file, uoff_t *offset,
        *offset = file->input->v_offset;
 
        (void)i_stream_read(file->input);
-       if (!i_stream_have_bytes_left(file->input)) {
-               *physical_size_r = 0;
+       if (!i_stream_have_bytes_left(file->input))
                return 1;
-       }
 
-       return dbox_file_read_mail_header(file, physical_size_r);
+       return dbox_file_read_mail_header(file, &physical_size);
 }
 
-int dbox_file_seek_next(struct dbox_file *file, uoff_t *offset,
-                       uoff_t *physical_size_r, bool *last_r)
+int dbox_file_seek_next(struct dbox_file *file, uoff_t *offset, bool *last_r)
 {
        uoff_t size;
        bool first = *offset == 0;
@@ -661,15 +662,13 @@ int dbox_file_seek_next(struct dbox_file *file, uoff_t *offset,
 
        if (deleted) {
                *last_r = TRUE;
-               return 1;
+               return 0;
        }
-       if (first) {
-               *physical_size_r = size;
+       if (first)
                return 1;
-       }
 
        i_stream_skip(file->input, size);
-       return dbox_file_seek_next_at_metadata(file, offset, physical_size_r);
+       return dbox_file_seek_next_at_metadata(file, offset);
 }
 
 static int

diff --git a/src/lib-storage/index/dbox/dbox-file.h b/src/lib-storage/index/dbox/dbox-file.h
index d29d8e541f9b5f6a866079bac206bd701f05d1ff..efe0d338aa8d1bc7b1402d883c2e74b875990afb 100644 (file)
--- a/src/lib-storage/index/dbox/dbox-file.h
+++ b/src/lib-storage/index/dbox/dbox-file.h
@@ -148,8 +148,7 @@ int dbox_file_get_mail_stream(struct dbox_file *file, uoff_t offset,
 /* Seek to next message after given offset, or to first message if offset=0.
    If there are no more messages, last_r is set to TRUE. Returns 1 if ok,
    0 if file/offset is corrupted, -1 if I/O error. */
-int dbox_file_seek_next(struct dbox_file *file, uoff_t *offset,
-                       uoff_t *physical_size_r, bool *last_r);
+int dbox_file_seek_next(struct dbox_file *file, uoff_t *offset, bool *last_r);
 
 /* Returns TRUE if mail_size bytes can be appended to the file. */
 bool dbox_file_can_append(struct dbox_file *file, uoff_t mail_size);
@@ -195,5 +194,7 @@ void dbox_msg_header_fill(struct dbox_message_header *dbox_msg_hdr,
 const char *dbox_file_get_primary_path(struct dbox_file *file);
 const char *dbox_file_get_alt_path(struct dbox_file *file);
 void dbox_file_set_syscall_error(struct dbox_file *file, const char *function);
+void dbox_file_set_corrupted(struct dbox_file *file, const char *reason, ...)
+       ATTR_FORMAT(2, 3);
 
 #endif

diff --git a/src/lib-storage/index/dbox/dbox-map.c b/src/lib-storage/index/dbox/dbox-map.c
index fb539bd59bb33693e3dbf4f5bcc46202ac3f1afe..c7e443ca6ab3b2d8933521a2ed2023bcbf4ec726 100644 (file)
--- a/src/lib-storage/index/dbox/dbox-map.c
+++ b/src/lib-storage/index/dbox/dbox-map.c
@@ -201,7 +201,8 @@ int dbox_map_get_file_msgs(struct dbox_map *map, uint32_t file_id,
        const void *data;
        bool expunged;
 
-       (void)dbox_map_refresh(map);
+       if (dbox_map_refresh(map) < 0)
+               return -1;
        hdr = mail_index_get_header(map->view);
 
        memset(&msg, 0, sizeof(msg));

diff --git a/src/lib-storage/index/dbox/dbox-sync-file.c b/src/lib-storage/index/dbox/dbox-sync-file.c
index c7166c4ea229f8962ae7e6462ebb0df41ddc0ce9..fb8ca4f2e7a884e024fb3e0867b0f294a9ae6986 100644 (file)
--- a/src/lib-storage/index/dbox/dbox-sync-file.c
+++ b/src/lib-storage/index/dbox/dbox-sync-file.c
@@ -10,6 +10,8 @@
 #include "dbox-map.h"
 #include "dbox-sync.h"
 
+#include <stdlib.h>
+
 struct dbox_mail_move {
        struct dbox_file *file;
        uint32_t offset;
@@ -42,22 +44,79 @@ static int dbox_sync_file_unlink(struct dbox_file *file)
        return 1;
 }
 
+static int dbox_map_file_msg_offset_cmp(const void *p1, const void *p2)
+{
+       const struct dbox_map_file_msg *m1 = p1, *m2 = p2;
+
+       if (m1->offset < m2->offset)
+               return -1;
+       else if (m1->offset > m2->offset)
+               return 1;
+       else
+               return 0;
+}
+
+static int
+dbox_sync_file_copy_metadata(struct dbox_file *file, struct ostream *output)
+{
+       struct dbox_metadata_header meta_hdr;
+       const char *line;
+       const unsigned char *data;
+       size_t size;
+       int ret;
+
+       ret = i_stream_read_data(file->input, &data, &size,
+                                sizeof(meta_hdr));
+       if (ret <= 0) {
+               i_assert(ret == -1);
+               if (file->input->stream_errno == 0) {
+                       dbox_file_set_corrupted(file, "missing metadata");
+                       return 0;
+               }
+               // FIXME
+               return -1;
+       }
+
+       memcpy(&meta_hdr, data, sizeof(meta_hdr));
+       if (memcmp(meta_hdr.magic_post, DBOX_MAGIC_POST,
+                  sizeof(meta_hdr.magic_post)) != 0) {
+               dbox_file_set_corrupted(file, "invalid metadata magic");
+               return 0;
+       }
+       i_stream_skip(file->input, sizeof(meta_hdr));
+       if (output != NULL)
+               o_stream_send(output, &meta_hdr, sizeof(meta_hdr));
+       while ((line = i_stream_read_next_line(file->input)) != NULL) {
+               if (*line == DBOX_METADATA_OLDV1_SPACE || *line == '\0') {
+                       /* end of metadata */
+                       break;
+               }
+               if (output != NULL) {
+                       o_stream_send_str(output, line);
+                       o_stream_send(output, "\n", 1);
+               }
+       }
+       if (line == NULL) {
+               dbox_file_set_corrupted(file, "missing end-of-metadata line");
+               return 0;
+       }
+       if (output != NULL)
+               o_stream_send(output, "\n", 1);
+       return 1;
+}
+
 int dbox_sync_file_cleanup(struct dbox_file *file)
 {
        struct dbox_file *out_file;
        struct stat st;
        struct istream *input;
        struct ostream *output = NULL;
-       struct dbox_metadata_header meta_hdr;
        struct dbox_map_append_context *append_ctx;
        ARRAY_TYPE(dbox_map_file_msg) msgs_arr;
-       const struct dbox_map_file_msg *msgs;
+       struct dbox_map_file_msg *msgs;
        ARRAY_TYPE(seq_range) copied_map_uids, expunged_map_uids;
        unsigned int i, count;
-       uoff_t physical_size, msg_size;
-       const unsigned char *data;
-       size_t size;
-       const char *line;
+       uoff_t offset, physical_size, msg_size;
        bool expunged;
        int ret;
 
@@ -85,83 +144,80 @@ int dbox_sync_file_cleanup(struct dbox_file *file)
                array_free(&msgs_arr);
                return -1;
        }
-       msgs = array_get(&msgs_arr, &count);
+       msgs = array_get_modifiable(&msgs_arr, &count);
+       /* sort messages by their offset */
+       qsort(msgs, count, sizeof(*msgs), dbox_map_file_msg_offset_cmp);
+
        i_array_init(&copied_map_uids, I_MIN(count, 1));
        i_array_init(&expunged_map_uids, I_MIN(count, 1));
+       offset = file->file_header_size;
        for (i = 0; i < count; i++) {
-               if (msgs[i].refcount == 0) {
-                       seq_range_array_add(&expunged_map_uids, 0,
-                                           msgs[i].map_uid);
-                       continue;
-               }
-               ret = dbox_file_get_mail_stream(file, msgs[i].offset,
-                                               &physical_size,
-                                               NULL, &expunged);
-               if (ret <= 0) {
-                       /* FIXME: handle corruption? */
-                       ret = -1;
+               if ((ret = dbox_file_get_mail_stream(file, offset,
+                                                    &physical_size,
+                                                    NULL, &expunged)) <= 0)
                        break;
-               }
+               msg_size = file->msg_header_size + physical_size;
 
-               /* non-expunged message. write it to output file. */
-               if (dbox_map_append_next(append_ctx, physical_size,
-                                        &out_file, &output) < 0) {
-                       // FIXME
-                       ret = -1;
+               if (msgs[i].offset != offset) {
+                       /* map doesn't match file's actual contents */
+                       dbox_file_set_corrupted(file,
+                               "cleanup found mismatched offsets "
+                               "(%"PRIuUOFF_T" vs %u, %u/%u)",
+                               offset, msgs[i].offset, i, count);
+                       ret = 0;
                        break;
                }
-               i_assert(file->file_id != out_file->file_id);
 
-               i_stream_seek(file->input, msgs[i].offset);
-               msg_size = file->msg_header_size + physical_size;
-               input = i_stream_create_limit(file->input, msg_size);
-               ret = o_stream_send_istream(output, input);
-               i_stream_unref(&input);
-               if (ret != (off_t)(file->msg_header_size + physical_size)) {
-                       // FIXME
-                       ret = -1;
-                       break;
-               }
+               if (msgs[i].refcount == 0) {
+                       seq_range_array_add(&expunged_map_uids, 0,
+                                           msgs[i].map_uid);
+                       output = NULL;
+               } else {
+                       /* non-expunged message. write it to output file. */
+                       if (dbox_map_append_next(append_ctx, physical_size,
+                                                &out_file, &output) < 0) {
+                               ret = -1;
+                               break;
+                       }
+                       i_assert(file->file_id != out_file->file_id);
 
-               /* copy metadata */
-               i_stream_seek(file->input, msgs[i].offset + msg_size);
-               ret = i_stream_read_data(file->input, &data, &size,
-                                        sizeof(meta_hdr));
-               if (ret <= 0) {
-                       // FIXME
-                       i_assert(ret != -2);
-                       ret = -1;
-                       break;
-               }
-               memcpy(&meta_hdr, data, sizeof(meta_hdr));
-               if (memcmp(meta_hdr.magic_post, DBOX_MAGIC_POST,
-                          sizeof(meta_hdr.magic_post)) != 0) {
-                       // FIXME
-                       ret = -1;
-                       break;
-               }
-               i_stream_skip(file->input, sizeof(meta_hdr));
-               o_stream_send(output, &meta_hdr, sizeof(meta_hdr));
-               while ((line = i_stream_read_next_line(file->input)) != NULL) {
-                       if (*line == DBOX_METADATA_OLDV1_SPACE || *line == '\0') {
-                               /* end of metadata */
+                       i_stream_seek(file->input, offset);
+                       input = i_stream_create_limit(file->input, msg_size);
+                       ret = o_stream_send_istream(output, input);
+                       i_stream_unref(&input);
+                       if (ret != (off_t)msg_size) {
+                               // FIXME
+                               ret = -1;
                                break;
                        }
-                       o_stream_send_str(output, line);
-                       o_stream_send(output, "\n", 1);
                }
-               if (line == NULL) {
-                       // FIXME
-                       ret = -1;
+
+               /* copy/skip metadata */
+               i_stream_seek(file->input, offset + msg_size);
+               if ((ret = dbox_sync_file_copy_metadata(file, output)) <= 0)
                        break;
-               }
-               o_stream_send(output, "\n", 1);
+
                dbox_map_append_finish_multi_mail(append_ctx);
                seq_range_array_add(&copied_map_uids, 0, msgs[i].map_uid);
+               offset = file->input->v_offset;
+       }
+       if (offset != (uoff_t)st.st_size && ret > 0) {
+               /* file has more messages than what map tells us */
+               dbox_file_set_corrupted(file,
+                       "more messages available than in map "
+                       "(%"PRIuUOFF_T" < %"PRIuUOFF_T")", offset, st.st_size);
+               ret = 0;
+               sleep(3600);
        }
        array_free(&msgs_arr); msgs = NULL;
 
-       if (ret < 0) {
+       if (ret == 0) {
+               // FIXME: ..?
+               i_error("dbox corrupted");
+               dbox_map_append_rollback(&append_ctx);
+               ret = -1;
+       } else if (ret < 0) {
+               i_error("dbox error");
                dbox_map_append_rollback(&append_ctx);
                ret = -1;
        } else if (output == NULL) {

diff --git a/src/lib-storage/index/dbox/dbox-sync-rebuild.c b/src/lib-storage/index/dbox/dbox-sync-rebuild.c
index 25c9b16802efb40908f4ac5a461120ce53ff89a1..2cdd6393729d5dea810e44c6871218069824c8e0 100644 (file)
--- a/src/lib-storage/index/dbox/dbox-sync-rebuild.c
+++ b/src/lib-storage/index/dbox/dbox-sync-rebuild.c
@@ -153,11 +153,10 @@ static int dbox_sync_index_file_next(struct dbox_sync_rebuild_context *ctx,
                                     struct dbox_file *file, uoff_t *offset)
 {
        uint32_t seq;
-       uoff_t physical_size;
        bool expunged, last;
        int ret;
 
-       ret = dbox_file_seek_next(file, offset, &physical_size, &last);
+       ret = dbox_file_seek_next(file, offset, &last);
        if (ret <= 0) {
                if (ret < 0)
                        return -1;

diff --git a/src/lib-storage/index/dbox/dbox-sync.c b/src/lib-storage/index/dbox/dbox-sync.c
index 8f56e351581a74d562e6bcbf5295517c8bede4e2..8aa0d54d1f296f81c30e193e2794aa9ebb1c42d0 100644 (file)
--- a/src/lib-storage/index/dbox/dbox-sync.c
+++ b/src/lib-storage/index/dbox/dbox-sync.c
@@ -326,12 +326,12 @@ void dbox_sync_cleanup(struct dbox_storage *storage)
 
        ref0_file_ids = dbox_map_get_zero_ref_files(storage->map);
        seq_range_array_iter_init(&iter, ref0_file_ids); i = 0;
-       while (seq_range_array_iter_nth(&iter, i++, &file_id)) {
+       while (seq_range_array_iter_nth(&iter, i++, &file_id)) T_BEGIN {
                file = dbox_file_init_multi(storage, file_id);
                if (dbox_file_open_or_create(file, &deleted) > 0 && !deleted)
                        (void)dbox_sync_file_cleanup(file);
                else
                        dbox_map_remove_file_id(storage->map, file_id);
                dbox_file_unref(&file);
-       }
+       } T_END;
 }