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

[dovecot at dovecot.org]

details:   http://hg.dovecot.org/dovecot-1.2/rev/8472e487da49
changeset: 9493:8472e487da49
user:      Timo Sirainen <tss at iki.fi>
date:      Thu Dec 03 10:29:24 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 3efdbaab2960 -r 8472e487da49 src/lib-storage/index/maildir/maildir-copy.c
--- a/src/lib-storage/index/maildir/maildir-copy.c	Mon Nov 23 15:04:17 2009 -0500
+++ b/src/lib-storage/index/maildir/maildir-copy.c	Thu Dec 03 10:29:24 2009 -0600
@@ -125,6 +125,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 maildir_transaction_context *t, struct mail *mail,
 		      enum mail_flags flags, struct mail_keywords *keywords,
@@ -160,20 +188,8 @@ maildir_copy_hardlink(struct maildir_tra
 	do_ctx.dest_path = str_new(default_pool, 512);
 
 	if (dest_mbox->storage->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