Skip to content

Commit

Permalink
mod_smtp_delivery_local: Fix notifications being generated for wrong …
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
InterLinked1 committed Jan 6, 2025
1 parent a433d7e commit 59b8f78
Show file tree
Hide file tree
Showing 4 changed files with 147 additions and 9 deletions.
7 changes: 7 additions & 0 deletions include/mod_mail.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
20 changes: 12 additions & 8 deletions modules/mod_mail.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
5 changes: 4 additions & 1 deletion modules/mod_smtp_delivery_local.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;
}
Expand Down
124 changes: 124 additions & 0 deletions tests/test_imap_filter.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
/*
* LBBS -- The Lightweight Bulletin Board System
*
* Copyright (C) 2025, Naveen Albert
*
* Naveen Albert <[email protected]>
*
* 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 <[email protected]>
*/

#include "test.h"

#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>
#include <string.h>
#include <sys/stat.h>

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");

0 comments on commit 59b8f78

Please sign in to comment.