dovecot-2.0: maildir_copy_preserve_filename=yes could have cause...

[dovecot at dovecot.org]

details:   http://hg.dovecot.org/dovecot-2.0/rev/2277467334fb
changeset: 10407:2277467334fb
user:      Timo Sirainen <tss at iki.fi>
date:      Thu Dec 03 10:38:54 2009 -0600
description:
maildir_copy_preserve_filename=yes could have caused crashes.

diffstat:

1 file changed, 30 insertions(+), 14 deletions(-)
src/lib-storage/index/maildir/maildir-copy.c |   44 +++++++++++++++++---------

diffs (61 lines):

diff -r ad3fb3f929fc -r 2277467334fb src/lib-storage/index/maildir/maildir-copy.c
--- a/src/lib-storage/index/maildir/maildir-copy.c	Wed Dec 02 15:35:51 2009 -0600
+++ b/src/lib-storage/index/maildir/maildir-copy.c	Thu Dec 03 10:38:54 2009 -0600
@@ -124,6 +124,34 @@ static int do_hardlink(struct maildir_ma
 	return 1;
 }
 
+static const char *
+maildir_copy_get_preserved_fname(struct maildir_mailbox *src_mbox,
+				 struct maildir_mailbox *dest_mbox,
+				 uint32_t uid)
+{
+	enum maildir_uidlist_rec_flag flags;
+	const char *fname;
+
+	/* see if the filename exists in destination maildir's
+	   uidlist. if it doesn't, we can use it. otherwise generate
+	   a new filename. FIXME: There's a race condition here if
+	   another process is just doing the same copy. */
+	if (maildir_uidlist_lookup(src_mbox->uidlist, uid, &flags,
+				   &fname) <= 0)
+		return NULL;
+
+	if (maildir_uidlist_refresh(dest_mbox->uidlist) <= 0)
+		return NULL;
+	if (maildir_uidlist_get_full_filename(dest_mbox->uidlist,
+					      fname) != NULL) {
+		/* already exists in destination */
+		return NULL;
+	}
+	/* fname may be freed by a later uidlist sync. make sure it gets
+	   strduped. */
+	return t_strcut(t_strdup(fname), ':');
+}
+
 static int
 maildir_copy_hardlink(struct mail_save_context *ctx, struct mail *mail)
 {
@@ -148,20 +176,8 @@ maildir_copy_hardlink(struct mail_save_c
 
 	if (dest_mbox->storage->set->maildir_copy_preserve_filename &&
 	    src_mbox != NULL) {
-		enum maildir_uidlist_rec_flag src_flags;
-		const char *src_fname;
-
-		/* see if the filename exists in destination maildir's
-		   uidlist. if it doesn't, we can use it. otherwise generate
-		   a new filename. FIXME: There's a race condition here if
-		   another process is just doing the same copy. */
-		if (maildir_uidlist_lookup(src_mbox->uidlist,
-					   mail->uid, &src_flags,
-					   &src_fname) > 0 &&
-		    maildir_uidlist_refresh(dest_mbox->uidlist) >= 0 &&
-		    maildir_uidlist_get_full_filename(dest_mbox->uidlist,
-						      src_fname) == NULL)
-			filename = t_strcut(src_fname, ':');
+		filename = maildir_copy_get_preserved_fname(src_mbox, dest_mbox,
+							    mail->uid);
 	}
 	if (filename == NULL) {
 		/* the generated filename is _always_ unique, so we don't