From a21eb95579dd66ceac00febbc228e9fa998b526c Mon Sep 17 00:00:00 2001 From: Timo Sirainen Date: Tue, 10 Mar 2009 18:39:19 -0400 Subject: [PATCH] dbox: Reimplemented file cleanup in a safer way. Added some more error handling. --HG-- branch : HEAD --- src/lib-storage/index/dbox/dbox-file.c | 37 ++-- src/lib-storage/index/dbox/dbox-file.h | 5 +- src/lib-storage/index/dbox/dbox-map.c | 3 +- src/lib-storage/index/dbox/dbox-sync-file.c | 186 ++++++++++++------ .../index/dbox/dbox-sync-rebuild.c | 3 +- src/lib-storage/index/dbox/dbox-sync.c | 4 +- 6 files changed, 147 insertions(+), 91 deletions(-) diff --git a/src/lib-storage/index/dbox/dbox-file.c b/src/lib-storage/index/dbox/dbox-file.c index d0f014c490..bbe5bf4c1e 100644 --- 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 d29d8e541f..efe0d338aa 100644 --- 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 fb539bd59b..c7e443ca6a 100644 --- 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 c7166c4ea2..fb8ca4f2e7 100644 --- 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 + 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 25c9b16802..2cdd639372 100644 --- 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 8f56e35158..8aa0d54d1f 100644 --- 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; } -- 2.47.3