BUGFIX: Cyrus 2.3.10 incorrect initial msgno in upload_messages_from

[Bron Gondwana]

On Tue, 23 Oct 2007 09:32:18 +0100 (BST), "David Carter" <dpc22 at cam.ac.uk> said:
> On Tue, 23 Oct 2007, Bron Gondwana wrote:
> 
> > Oct 23 03:47:06 imap4 slot407/sync_client[20103]: IOERROR: opening 
> > message file 10 of MAILBOXNAME: No such file or directory
> 
> I think that this error relates to the master rather than the replica.
> 
> It looks like an UPLOAD is failing because it can't open message with UID 
> 10 on the master. Errors from the replica will typically get reported as 
> UPLOAD received NO response: (whatever the error was).

And my guess that it was the MinorVersion field is vindicated,
VINDICATED I say.  So much for the doubters (hi Rob, added to
the CC this time)

A simple think about the offsets would have been a good hint too,
MinorVersion is bytes 9-12, conveniently 88 bytes (one index record)
before the first real index record.

An ideal place for "msgno == 0" to point to in fact.

  for (; (index_list->count <= max_count) &&
     (msgno <= mailbox->exists); msgno++) {
      r = mailbox_read_index_record(mailbox, msgno, &record);

      if (r) {
    syslog(LOG_ERR,
           "IOERROR: reading index entry for msgno %lu of %s: %m",
           record.uid, mailbox->name);
    return(IMAP_IOERROR);
      }

      if (record.uid <= old_last_uid) continue;

      /* Message with this GUID exists on client but not server */
      sync_index_list_add(index_list, msgno, &record);
  }

I guess that explains why a bogus initial record with "UID 10" would
be added to the index_list if it happened to be read.

Yes, a crc32 check would have caught this (unlikely that "spare4" will
contain a valid crc32).

Why was this rare?  It would only be noticed for a mailbox with fewer
than 10 messages where the conditions for upload_messages_from to be
called existed (append with no other mailbox events presumably)

I guess a more complete patch would contain an "assert(msgno > 0)" in
mailbox_read_index_record (and anything else that deals with msgno for
that matter), but this is the most important thing.

By the way, my cyrus-largemailboxappend-2.3.9.diff doesn't have this
bug - it crept in during the rewrite, so I don't feel guilty at all :)

Bron ( early adopters find all the cool bugs )

-- 
  Bron Gondwana
  brong at fastmail.fm

-------------- next part --------------
A non-text attachment was scrubbed...
Name: cyrus-appendfrom-bugfix-2.3.10.diff
Type: text/x-patch
Size: 540 bytes
Desc: not available
Url : http://lists.andrew.cmu.edu/pipermail/cyrus-devel/attachments/20071023/6a3608fa/attachment.bin