Wow, this is the thanks I get for doing sanity checks on files, find more bugs!
This one is due to delayed expunge, plain and simple. Cyrus decides what cache
records to copy during an IMAP COPY command by reading the cache offsets for
msgno and msgno+1 (or the end of the cache file if it's the last msgno).
Obviously if some intervening messages have already been deleted from the
cyrus.index then it will be copying all those cache records as well to the
destination folder. Oops.
The attached patch reworks mailbox_cache_size into two functions, the second
being mailbox_cache_size_detail that takes memory locations for the cache mmap
rather than a mailbox object (because imapd doesn't update the locations in
the object correctly according to my testing, *sigh*. Gotta love global
variables)
It then uses mailbox_cache_size_detail to calculate the ACTUAL record length
for this single cache item rather than blindly copying everything up to the
next index record's pointer.
Also note: in the event of cache corruption, mailbox_cache_size_detail returns
zero bytes, which correctly makes append_copy re-parse the message file. It's
all shiny :)
Wes/Ken - please apply to CVS :)
Thanks,
Bron.
--
Bron Gondwana
brong@xxxxxxxxxxx
Index: cyrus-imapd-2.3.13/imap/index.c
===================================================================
--- cyrus-imapd-2.3.13.orig/imap/index.c 2008-12-07 23:51:36.000000000 +0000
+++ cyrus-imapd-2.3.13/imap/index.c 2008-12-08 01:51:28.000000000 +0000
@@ -3574,13 +3574,9 @@ void *rock;
/* Force copy and re-parse of message */
copyargs->copymsg[copyargs->nummsg].cache_len = 0;
}
- else if (msgno < (unsigned) imapd_exists) {
- copyargs->copymsg[copyargs->nummsg].cache_len =
- CACHE_OFFSET(msgno+1) - CACHE_OFFSET(msgno);
- }
else {
copyargs->copymsg[copyargs->nummsg].cache_len =
- cache_end - CACHE_OFFSET(msgno);
+ mailbox_cache_size_detail(copyargs->copymsg[copyargs->nummsg].cache_begin, cache_base, cache_end);
}
copyargs->copymsg[copyargs->nummsg].seen = seenflag[msgno];
copyargs->copymsg[copyargs->nummsg].system_flags = SYSTEM_FLAGS(msgno);
Index: cyrus-imapd-2.3.13/imap/mailbox.c
===================================================================
--- cyrus-imapd-2.3.13.orig/imap/mailbox.c 2008-12-08 01:46:02.000000000 +0000
+++ cyrus-imapd-2.3.13/imap/mailbox.c 2008-12-08 02:04:34.000000000 +0000
@@ -301,8 +301,6 @@ unsigned long mailbox_cache_size(struct
{
const char *p;
unsigned long cache_offset;
- unsigned int cache_ent;
- const char *cacheitem, *cacheitembegin;
assert((msgno > 0) && (msgno <= mailbox->exists));
@@ -310,22 +308,33 @@ unsigned long mailbox_cache_size(struct
((msgno-1) * mailbox->record_size));
cache_offset = ntohl(*((bit32 *)(p+OFFSET_CACHE_OFFSET)));
- if (cache_offset > mailbox->cache_size) {
+
+ return mailbox_cache_size_detail(mailbox->cache_base + cache_offset,
+ mailbox->cache_base,
+ mailbox->cache_size);
+}
+
+unsigned long mailbox_cache_size_detail(const char *cache_item,
+ const char *cache_base,
+ unsigned long cache_size)
+{
+ unsigned int cache_ent;
+ const char *begin = cache_item;
+
+ if (begin < cache_base || begin >= cache_base + cache_size) {
+ /* already not in the area */
return 0;
}
/* Compute size of this record */
- cacheitembegin = cacheitem = mailbox->cache_base + cache_offset;
- if (cache_offset >= mailbox->cache_size)
- return 0;
for (cache_ent = 0; cache_ent < NUM_CACHE_FIELDS; cache_ent++) {
- cacheitem = CACHE_ITEM_NEXT(cacheitem);
- if (cacheitem < cacheitembegin ||
- cacheitem > cacheitembegin + mailbox->cache_size) {
+ cache_item = CACHE_ITEM_NEXT(cache_item);
+ if (cache_item < begin ||
+ cache_item > cache_base + cache_size) {
return 0; /* clearly bogus */
}
}
- return (cacheitem - cacheitembegin);
+ return (cache_item - begin);
}
/* function to be used for notification of mailbox changes/updates */
Index: cyrus-imapd-2.3.13/imap/mailbox.h
===================================================================
--- cyrus-imapd-2.3.13.orig/imap/mailbox.h 2008-12-08 01:51:37.000000000 +0000
+++ cyrus-imapd-2.3.13/imap/mailbox.h 2008-12-08 02:03:23.000000000 +0000
@@ -292,6 +292,9 @@ unsigned mailbox_cached_header(const cha
unsigned mailbox_cached_header_inline(const char *text);
unsigned long mailbox_cache_size(struct mailbox *mailbox, unsigned msgno);
+unsigned long mailbox_cache_size_detail(const char *cache_item,
+ const char *cache_base,
+ unsigned long cache_size);
typedef unsigned mailbox_decideproc_t(struct mailbox *mailbox, void *rock,
unsigned char *indexbuf,
----
Cyrus Home Page: http://cyrusimap.web.cmu.edu/
Cyrus Wiki/FAQ: http://cyrusimap.web.cmu.edu/twiki
List Archives/Info: http://asg.web.cmu.edu/cyrus/mailing-list.html
