From c697dd10a1f433423734a9fc0d7be18cc1b7bfda Mon Sep 17 00:00:00 2001 From: Timo Sirainen Date: Fri, 29 Jan 2021 17:03:34 +0200 Subject: [PATCH] lib-index: Verify that keywords in dovecot.index header don't change unexpectedly Previously this was behind DEBUG for performance reasons, but that was only because dovecot.index file used to be modified directly. Nowadays it's rare that the index is being re-read within the same session (most changes are read via dovecot.index.log), so it's better to verify that things are working correctly. --- src/lib-index/mail-index-map-hdr.c | 40 ++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/src/lib-index/mail-index-map-hdr.c b/src/lib-index/mail-index-map-hdr.c index b55c9cf82a..3287a284d2 100644 --- a/src/lib-index/mail-index-map-hdr.c +++ b/src/lib-index/mail-index-map-hdr.c @@ -78,6 +78,12 @@ int mail_index_map_parse_keywords(struct mail_index_map *map) - struct mail_index_keyword_header - struct mail_index_keyword_header_rec * keywords_count - const char names[] * keywords_count + + The mail_index_keyword_header_rec are rather unnecessary nowadays. + They were originally an optimization when dovecot.index header kept + changing constantly, but nowadays the changes are usually read from + the .log changes, so re-reading dovecot.index header isn't common. + In a later version we could even remove it. */ i_assert(ext->hdr_offset < map->hdr.header_size); kw_hdr = MAIL_INDEX_MAP_HDR_OFFSET(map, ext->hdr_offset); @@ -87,13 +93,6 @@ int mail_index_map_parse_keywords(struct mail_index_map *map) old_count = !array_is_created(&map->keyword_idx_map) ? 0 : array_count(&map->keyword_idx_map); - /* Keywords can only be added into same mapping. Removing requires a - new mapping (recreating the index file) */ - if (kw_hdr->keywords_count == old_count) { - /* nothing changed */ - return 0; - } - /* make sure the header is valid */ if (kw_hdr->keywords_count < old_count) { mail_index_set_error(index, "Corrupted index file %s: " @@ -129,14 +128,23 @@ int mail_index_map_parse_keywords(struct mail_index_map *map) if (!array_is_created(&map->keyword_idx_map)) i_array_init(&map->keyword_idx_map, kw_hdr->keywords_count); -#ifdef DEBUG - /* Check that existing headers are still the same. It's behind DEBUG - since it's pretty useless waste of CPU normally. */ + size_t name_offset = 0; + /* Check that existing headers are still the same. */ for (i = 0; i < array_count(&map->keyword_idx_map); i++) { const char *keyword = name + kw_rec[i].name_offset; const unsigned int *old_idx; unsigned int kw_idx; + if (kw_rec[i].name_offset != name_offset) { + /* this shouldn't happen, but the old code didn't check + for this so for safety keep this as a warning. */ + e_warning(index->event, + "Corrupted index file %s: " + "Mismatching keyword name_offset", + index->filepath); + } + name_offset += strlen(keyword) + 1; + old_idx = array_idx(&map->keyword_idx_map, i); if (!mail_index_keyword_lookup(index, keyword, &kw_idx) || kw_idx != *old_idx) { @@ -146,13 +154,23 @@ int mail_index_map_parse_keywords(struct mail_index_map *map) return -1; } } -#endif + /* Register the newly seen keywords */ i = array_count(&map->keyword_idx_map); for (; i < kw_hdr->keywords_count; i++) { const char *keyword = name + kw_rec[i].name_offset; unsigned int kw_idx; + if (kw_rec[i].name_offset != name_offset) { + /* this shouldn't happen, but the old code didn't check + for this so for safety keep this as a warning. */ + e_warning(index->event, + "Corrupted index file %s: " + "Mismatching keyword name_offset", + index->filepath); + } + name_offset += strlen(keyword) + 1; + if (*keyword == '\0') { mail_index_set_error(index, "Corrupted index file %s: " "Empty keyword name in header", -- 2.47.3