From 59b8f789e3d37da3ed1c99e058798ac95be6cbb6 Mon Sep 17 00:00:00 2001 From: InterLinked1 <24227567+InterLinked1@users.noreply.github.com> Date: Sun, 5 Jan 2025 19:12:50 -0500 Subject: [PATCH] mod_smtp_delivery_local: Fix notifications being generated for wrong mailbox. When processing incoming messages, the root maildir for the entire mailbox, as opposed to the actual maildir of the message, was being passed to mailbox_notify_new_message. In the common case of messages being delivered to the INBOX, this would not make a difference, but if a filter moved it to a different folder, the mailbox's root maildir was still used, resulting in a "new message notification" for the INBOX, even if the new message was in a different folder. This manifested as receiving an untagged EXISTS in the INBOX even when there were no new messages. This is fixed to use the correct, specific maildir of the message, so that the notification is generated for the correct mailbox (such as using an untagged STATUS if needed). A test has also been added to capture this case. --- include/mod_mail.h | 7 ++ modules/mod_mail.c | 20 +++-- modules/mod_smtp_delivery_local.c | 5 +- tests/test_imap_filter.c | 124 ++++++++++++++++++++++++++++++ 4 files changed, 147 insertions(+), 9 deletions(-) create mode 100755 tests/test_imap_filter.c diff --git a/include/mod_mail.h b/include/mod_mail.h index 4f7071b..f183706 100644 --- a/include/mod_mail.h +++ b/include/mod_mail.h @@ -473,6 +473,13 @@ int maildir_copy_msg_filename(struct mailbox *mbox, struct bbs_node *node, const */ int maildir_parse_uid_from_filename(const char *filename, unsigned int *uid) __attribute__((nonnull (1, 2))); +/*! + * \brief Parse out the path to a message's maildir from the message filepath + * \param filename Full path to the message file. This should be in a subdirectory of new, cur, or tmp + * \note s is modified in-place, and it is assumed that filename is legitimate + */ +void maildir_extract_from_filename(char *restrict filename); + /*! * \brief Sort callback for scandir (for maildirs) * \note This function may not be specified as a callback parameter inside modules (outside of mod_mail) directly, diff --git a/modules/mod_mail.c b/modules/mod_mail.c index 359239b..7d1045b 100644 --- a/modules/mod_mail.c +++ b/modules/mod_mail.c @@ -1654,24 +1654,28 @@ static int gen_newname(struct mailbox *mbox, struct bbs_node *node, const char * return (int) uid; } -static int copy_move_untagged_exists(struct bbs_node *node, struct mailbox *mbox, const char *newpath, size_t size) +void maildir_extract_from_filename(char *restrict filename) { - char maildir[256]; char *tmp; - safe_strncpy(maildir, newpath, sizeof(maildir)); - - tmp = strrchr(maildir, '/'); /* newpath can be mutiliated at this point */ - if (!tmp) { - return -1; + tmp = strrchr(filename, '/'); /* filename can be mutiliated at this point */ + if (unlikely(!tmp)) { + return; } *tmp = '\0'; /* Truncating once gets rid of the filename, need to do it again to get the maildir without /new or /cur at the end */ /* Since we're already near the end of the string, it's more efficient to back up from here than start from the beginning a second time */ - while (tmp > maildir && *tmp != '/') { + while (tmp > filename && *tmp != '/') { tmp--; } *tmp = '\0'; +} + +static int copy_move_untagged_exists(struct bbs_node *node, struct mailbox *mbox, const char *newpath, size_t size) +{ + char maildir[256]; + safe_strncpy(maildir, newpath, sizeof(maildir)); + maildir_extract_from_filename(maildir); mailbox_notify_new_message(node, mbox, maildir, newpath, size); return 0; } diff --git a/modules/mod_smtp_delivery_local.c b/modules/mod_smtp_delivery_local.c index b978621..4413103 100644 --- a/modules/mod_smtp_delivery_local.c +++ b/modules/mod_smtp_delivery_local.c @@ -193,6 +193,7 @@ static int appendmsg(struct smtp_session *smtp, struct smtp_response *resp, stru bbs_error("rename %s -> %s failed: %s\n", tmpfile, newfile, strerror(errno)); return -1; } else { + char maildir[256]; /* Because the notification is delivered before we actually return success to the sending client, * this can result in the somewhat strange experience of receiving an email sent to yourself * before it seems that the email has been fully sent. @@ -203,7 +204,9 @@ static int appendmsg(struct smtp_session *smtp, struct smtp_response *resp, stru if (newfilebuf) { safe_strncpy(newfilebuf, newfile, len); } - mailbox_notify_new_message(smtp_node(smtp), mbox, mailbox_maildir(mbox), newfile, datalen); + safe_strncpy(maildir, newfile, sizeof(maildir)); + maildir_extract_from_filename(maildir); /* Strip everything beneath the maildir */ + mailbox_notify_new_message(smtp_node(smtp), mbox, maildir, newfile, datalen); } return 0; } diff --git a/tests/test_imap_filter.c b/tests/test_imap_filter.c new file mode 100755 index 0000000..517d981 --- /dev/null +++ b/tests/test_imap_filter.c @@ -0,0 +1,124 @@ +/* + * LBBS -- The Lightweight Bulletin Board System + * + * Copyright (C) 2025, Naveen Albert + * + * Naveen Albert + * + * This program is free software, distributed under the terms of + * the GNU General Public License Version 2. See the LICENSE file + * at the top of the source tree. + */ + +/*! \file + * + * \brief Misc. filtering-related IMAP tests + * + * \author Naveen Albert + */ + +#include "test.h" + +#include +#include +#include +#include +#include + +static int pre(void) +{ + test_preload_module("mod_auth_static.so"); + test_preload_module("mod_mail.so"); + test_preload_module("net_smtp.so"); + test_load_module("mod_smtp_delivery_local.so"); + test_load_module("mod_mailscript.so"); + test_load_module("net_imap.so"); + + TEST_ADD_CONFIG("mod_auth_static.conf"); + TEST_ADD_CONFIG("mod_mail.conf"); + TEST_ADD_CONFIG("net_smtp.conf"); + TEST_ADD_CONFIG("net_imap.conf"); + + system("rm -rf /tmp/test_lbbs/maildir"); /* Purge the contents of the directory, if it existed. */ + mkdir(TEST_MAIL_DIR, 0700); /* Make directory if it doesn't exist already (of course it won't due to the previous step) */ + system("cp before.rules " TEST_MAIL_DIR); /* Global before MailScript */ + return 0; +} + +#define STANDARD_ENVELOPE_BEGIN() \ + SWRITE(smtpfd, "RSET" ENDL); \ + CLIENT_EXPECT(smtpfd, "250"); \ + SWRITE(smtpfd, "EHLO " TEST_EXTERNAL_DOMAIN ENDL); \ + CLIENT_EXPECT_EVENTUALLY(smtpfd, "250 "); \ + SWRITE(smtpfd, "MAIL FROM:<" TEST_EMAIL_EXTERNAL ">\r\n"); \ + CLIENT_EXPECT(smtpfd, "250"); \ + SWRITE(smtpfd, "RCPT TO:<" TEST_EMAIL ">\r\n"); \ + CLIENT_EXPECT(smtpfd, "250"); \ + SWRITE(smtpfd, "DATA\r\n"); \ + CLIENT_EXPECT(smtpfd, "354"); \ + SWRITE(smtpfd, "Date: Sun, 1 Jan 2023 05:33:29 -0700" ENDL); + +#define STANDARD_DATA() \ + SWRITE(smtpfd, ENDL); \ + SWRITE(smtpfd, "This is a test email message." ENDL); \ + SWRITE(smtpfd, "From: Some string here" ENDL); /* This is not a header, but if we missed the EOH, it might think it was one. */ \ + SWRITE(smtpfd, "." ENDL); \ + +static int run(void) +{ + int smtpfd; + int imapfd = -1; + int res = -1; + + smtpfd = test_make_socket(25); + if (smtpfd < 0) { + return -1; + } + + imapfd = test_make_socket(143); + if (smtpfd < 0) { + goto cleanup; + } + + CLIENT_EXPECT_EVENTUALLY(smtpfd, "220 "); + + /* Log in and enable NOTIFY */ + CLIENT_EXPECT(imapfd, "OK"); + SWRITE(imapfd, "a1 LOGIN \"" TEST_USER "\" \"" TEST_PASS "\"" ENDL); + CLIENT_EXPECT(imapfd, "a1 OK"); + + /* Enable NOTIFY for all personal mailboxes */ + SWRITE(imapfd, "a2 NOTIFY SET (personal (MessageNew (FLAGS) MessageExpunge))" ENDL); + CLIENT_EXPECT(imapfd, "a2 OK"); + + SWRITE(imapfd, "a3 SELECT INBOX" ENDL); + CLIENT_EXPECT_EVENTUALLY(imapfd, "a3 OK"); + + SWRITE(imapfd, "a4 IDLE" ENDL); + CLIENT_EXPECT(imapfd, "+ idling"); + + /* Should be moved to .Trash */ + STANDARD_ENVELOPE_BEGIN(); + SWRITE(smtpfd, "From: " TEST_EMAIL_EXTERNAL ENDL); + SWRITE(smtpfd, "Subject: Test Subject 1" ENDL); + SWRITE(smtpfd, "To: " TEST_EMAIL ENDL); + STANDARD_DATA(); + CLIENT_EXPECT(smtpfd, "250"); + DIRECTORY_EXPECT_FILE_COUNT(TEST_MAIL_DIR "/1/new", 0); + DIRECTORY_EXPECT_FILE_COUNT(TEST_MAIL_DIR "/1/.Trash/new", 1); + + /* We're idling on the INBOX, with NOTIFY enabled. + * We should get an untagged STATUS for Junk, + * NOT an untagged EXISTS for INBOX. */ + CLIENT_EXPECT(imapfd, "* STATUS"); + + SWRITE(smtpfd, "QUIT"); + res = 0; + +cleanup: + close(smtpfd); + close_if(imapfd); + return res; +} + +TEST_MODULE_INFO_STANDARD("Mail Filtering IMAP Interaction Tests");