From 693f72bb9c1fe68828e8b36839f985c08766a260 Mon Sep 17 00:00:00 2001 From: InterLinked1 <24227567+InterLinked1@users.noreply.github.com> Date: Sun, 13 Oct 2024 13:13:19 -0400 Subject: [PATCH] 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 | 11 ++++++++++- modules/mod_mail.c | 18 ++++++++++++++++-- nets/net_imap.c | 11 ++++++++--- 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/include/mod_mail.h b/include/mod_mail.h index 82ed62f..a5942c8 100644 --- a/include/mod_mail.h +++ b/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; } diff --git a/modules/mod_mail.c b/modules/mod_mail.c index 26786da..5af3ce3 100644 --- a/modules/mod_mail.c +++ b/modules/mod_mail.c @@ -1989,7 +1989,7 @@ 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; @@ -1997,7 +1997,7 @@ int maildir_ordered_traverse(const char *path, int (*on_file)(const char *dir_na 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; diff --git a/nets/net_imap.c b/nets/net_imap.c index 6ef9c5b..f2d40c0 100644 --- a/nets/net_imap.c +++ b/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;