net_imap: Fix misordering when traversing a maildir's new dir.

Previously, traversals of a maildir's cur dir and new dir both used
maildir_ordered_traverse, which uses uidsort to sort the directory
entries. However, this is invalid when traversing a maildir's new dir,
as no UIDs have been assigned; this leads to an arbitrary an invalid
sorting since uidsort assumes both inputs have a UID in the filename
in order to sort them. This led to the order in which messages were
added to the new dir not being preserved when assigned UIDs.

Instead, we should just be using the entire filename by itself for
sorting, since the filenames (when ordered) are already in the right
order to assigning UIDs. So, use alphasort when traversing a new dir,
which preserves the order in which messages were added to the mailbox.

include/mod_mail.h

@@ -474,9 +474,18 @@ int maildir_parse_uid_from_filename(const char *filename, unsigned int *uid) __a
  */
 int uidsort(const struct dirent **da, const struct dirent **db);
 
-/*! \brief Perform an ordered traversal of a maildir cur directory */
+/*!
+ * \brief Perform an ordered traversal (by UID) of a maildir cur directory
+ * \note This function MUST ONLY be used in the cur dir, NOT the new dir (use maildir_sequence_traverse for that)
+ */
 int maildir_ordered_traverse(const char *path, int (*on_file)(const char *dir_name, const char *filename, int seqno, void *obj), void *obj);
 
+/*!
+ * \brief Perform an ordered traversal (by filename) of a maildir new directory
+ * \note This function MUST ONLY be used in the new dir, NOT the cur dir (use maildir_ordered_traverse for that)
+ */
+int maildir_uidless_traverse(const char *path, int (*on_file)(const char *dir_name, const char *filename, int seqno, void *obj), void *obj);
+
 /* IMAP client */
 #define IMAP_CLIENT_EXPECT(client, s) if (bbs_tcp_client_expect(client, "\r\n", 1, 2000, s)) { bbs_debug(3, "Didn't receive expected '%s'\n", s); goto cleanup; }
 #define IMAP_CLIENT_EXPECT_EVENTUALLY(client, x, s) if (bbs_tcp_client_expect(client, "\r\n", x, 2000, s)) { bbs_debug(3, "Didn't receive expected '%s', got '%s'\n", s, (client)->rldata.buf); goto cleanup; }

modules/mod_mail.c

@@ -1989,15 +1989,15 @@ int uidsort(const struct dirent **da, const struct dirent **db)
 	return auid < buid ? -1 : 1;
 }
 
-int maildir_ordered_traverse(const char *path, int (*on_file)(const char *dir_name, const char *filename, int seqno, void *obj), void *obj)
+static int maildir_traverse(const char *path, int (*on_file)(const char *dir_name, const char *filename, int seqno, void *obj), void *obj, int (*sortfunc)(const struct dirent **da, const struct dirent **db))
 {
 	struct dirent *entry, **entries;
 	int files, fno = 0;
 	int res = 0;
 	int seqno = 0;
 
 	/* use scandir instead of opendir/readdir since we need ordering, even for message sequence numbers */
-	files = scandir(path, &entries, NULL, uidsort);
+	files = scandir(path, &entries, NULL, sortfunc);
 	if (files < 0) {
 		bbs_error("scandir(%s) failed: %s\n", path, strerror(errno));
 		return -1;
@@ -2016,6 +2016,20 @@ int maildir_ordered_traverse(const char *path, int (*on_file)(const char *dir_na
 	return res;
 }
 
+int maildir_ordered_traverse(const char *path, int (*on_file)(const char *dir_name, const char *filename, int seqno, void *obj), void *obj)
+{
+	return maildir_traverse(path, on_file, obj, uidsort);
+}
+
+int maildir_uidless_traverse(const char *path, int (*on_file)(const char *dir_name, const char *filename, int seqno, void *obj), void *obj)
+{
+	/* When traversing the new dir, UIDs have not yet been assigned,
+	 * so using uidsort is wrong and will result in random ordering.
+	 * However, there is still an ordering that must be preserved,
+	 * and it's simply the normal ordering by entire filename. */
+	return maildir_traverse(path, on_file, obj, alphasort);
+}
+
 static int cli_mailboxes(struct bbs_cli_args *a)
 {
 	struct mailbox *mbox;

nets/net_imap.c

@@ -313,10 +313,10 @@ static inline void reset_saved_search(struct imap_session *imap)
 	traversal->innew = 0; \
 	traversal->uidvalidity = 0; \
 	traversal->uidnext = 0; \
-	imap_traverse(traversal->curdir, callback, traversal); \
+	imap_traverse_cur(traversal->curdir, callback, traversal); \
 	traversal->innew = 1; \
 	traversal->minrecent = traversal->totalcur + 1; \
-	imap_traverse(traversal->newdir, callback, traversal); \
+	imap_traverse_new(traversal->newdir, callback, traversal); \
 	traversal->maxrecent = traversal->totalcur + traversal->totalnew; \
 	if (!traversal->uidvalidity || !traversal->uidnext) { \
 		mailbox_get_next_uid(traversal->mbox, traversal->imap->node, traversal->dir, 0, &traversal->uidvalidity, &traversal->uidnext); \
@@ -1073,11 +1073,16 @@ static int imap_expunge(struct imap_session *imap, int silent)
 	return 0;
 }
 
-static int imap_traverse(const char *path, int (*on_file)(const char *dir_name, const char *filename, int seqno, void *obj), struct imap_traversal *traversal)
+static int imap_traverse_cur(const char *path, int (*on_file)(const char *dir_name, const char *filename, int seqno, void *obj), struct imap_traversal *traversal)
 {
 	return maildir_ordered_traverse(path, on_file, traversal);
 }
 
+static int imap_traverse_new(const char *path, int (*on_file)(const char *dir_name, const char *filename, int seqno, void *obj), struct imap_traversal *traversal)
+{
+	return maildir_uidless_traverse(path, on_file, traversal);
+}
+
 static void do_qresync(struct imap_session *imap, unsigned long lastmodseq, const char *uidrange, char *seqrange)
 {
 	struct dirent *entry, **entries;