net_imap: Fix false warning in __gen_keyword_names.

When moving messages from a mailbox (and possibly other operations)
with certain flags to a mailbox with different keyword mappings e.g.,

Mailbox A (test): a -> encrypted
Mailbox B (Trash): a -> NonJunk, b -> encrypted

there would be a warning like this:

File has 1 custom flags (b), but we only have mappings for 0 of them ()?

For one, this was misleading, since actually there was a mapping, just
not for the specific keyword, so this is a more appropriate warning:

'Sb' has 1 custom flag (b), but /home/bbs/maildir/1/.test/.keywords only
has mappings for 0 of them (a)?

Moreover, this warning is still not appropriate, because the keyword
mapping in question does exist as 'b' in the destination mailbox, but
we were using the source mailbox (since imap->dir is passed in to this
function by default). Add a few variants of calling APIs that allow us
to pass in the the destination maildir, which avoids this error.

Author: InterLinked1

nets/net_imap.c

@@ -1250,7 +1250,7 @@ static void do_qresync(struct imap_session *imap, unsigned long lastmodseq, cons
 				bbs_error("Message file %s contains no flags?\n", entry->d_name);
 				goto next;
 			}
-			generate_flag_names_full(imap, flags, flagsbuf, sizeof(flagsbuf), &buf, &len);
+			generate_flag_names_full(imap, entry->d_name, flags, flagsbuf, sizeof(flagsbuf), &buf, &len);
 			imap_send(imap, "%u FETCH (UID %u %s) MODSEQ (%lu)", seqno, uid, flagsbuf, modseq);
 		}
 next:
@@ -1307,7 +1307,7 @@ static int select_examine_response(struct imap_session *imap, enum select_type r
 	int condstore_just_enabled = 0;
 	unsigned int lastmodseq = 0;
 	char *uidrange = NULL, *seqrange = NULL;
-	int numkeywords = gen_keyword_names(imap, NULL, keywords, sizeof(keywords)); /* prepends a space before all of them, so this works out great */
+	int numkeywords = gen_keyword_names(imap, NULL, NULL, keywords, sizeof(keywords)); /* prepends a space before all of them, so this works out great */
 
 	if (!strlen_zero(s)) {
 		if (STARTS_WITH(s, "(QRESYNC")) {
@@ -2777,7 +2777,7 @@ static int copy_append_cb(const char *dir_name, const char *filename, int seqno,
 		bbs_error("Message file %s contains no flags?\n", filename);
 		return -1;
 	}
-	generate_flag_names_full(ca->imap, flags, flagnames, sizeof(flagnames), &pos, &len);
+	generate_flag_names_full(ca->imap, fullname, flags, flagnames, sizeof(flagnames), &pos, &len);
 	/* Here's something awkward: if you were to just send flagnames as is in the APPEND command,
 	 * then something like FLAGS (\Seen) might be sent as the flag contents,
 	 * and libetpan has issues with mailbox flags that contain parentheses,
@@ -3477,13 +3477,13 @@ static int process_flags(struct imap_session *imap, char *s, int usinguid, const
 			if (keywords[0]) { /* Current keywords */
 				size_t slen = strlen(flagstr);
 				/*! \todo We should not append a space before if we're at the beginning of the buffer */
-				gen_keyword_names(imap, keywords, flagstr + slen, sizeof(flagstr) - slen); /* Append keywords (include space before) */
+				gen_keyword_names(imap, entry->d_name, keywords, flagstr + slen, sizeof(flagstr) - slen); /* Append keywords (include space before) */
 			}
 			if (!silent) { /* Send the response if not silent */
 				if (imap->createdkeyword) {
 					/* Server SHOULD send untagged response when a new keyword is created */
 					char allkeywords[256] = "";
-					gen_keyword_names(imap, NULL, allkeywords, sizeof(allkeywords)); /* prepends a space before all of them, so this works out great */
+					gen_keyword_names(imap, NULL, NULL, allkeywords, sizeof(allkeywords)); /* prepends a space before all of them, so this works out great */
 					imap_send(imap, "FLAGS (%s%s)", IMAP_FLAGS, allkeywords);
 				}
 				/*! \todo This is repetitive, clean this up so we're not duplicating this log for UID/no UID, MODSEQ/no MODSEQ, silent/not silent */
@@ -3573,7 +3573,7 @@ static int process_flags(struct imap_session *imap, char *s, int usinguid, const
 		if (imap->numappendkeywords) {
 			size_t slen = strlen(changedflags);
 			/*! \todo We should not append a space before if we're at the beginning of the buffer */
-			gen_keyword_names(imap, imap->appendkeywords, changedflags + slen, sizeof(changedflags) - slen);
+			gen_keyword_names(imap, "", imap->appendkeywords, changedflags + slen, sizeof(changedflags) - slen);
 		}
 		mailbox_initialize_event(&e, flagop < 1 ? EVENT_FLAGS_CLEAR : EVENT_FLAGS_SET, imap->node, imap->mbox, imap->dir);
 		e.uids = aflagsset;

nets/net_imap/imap_server_fetch.c

@@ -60,7 +60,7 @@ static int process_fetch_flags(struct imap_session *imap, const char *filename,
 		safe_strncpy(inflags + c, flags, sizeof(inflags) - (size_t) c);
 		flags = inflags;
 	}
-	generate_flag_names_full(imap, flags, response, responselen, buf, len);
+	generate_flag_names_full(imap, filename, flags, response, responselen, buf, len);
 	return 0;
 }
 

nets/net_imap/imap_server_flags.c

@@ -21,6 +21,7 @@
 
 #include <string.h>
 #include <ctype.h>
+#include <libgen.h> /* use dirname */
 
 #if defined(linux) && defined(__GLIBC__)
 #include <bsd/string.h>
@@ -102,18 +103,27 @@ void parse_keyword(struct imap_session *imap, const char *s, const char *directo
 	fclose(fp);
 }
 
-int __gen_keyword_names(const char *s, char *inbuf, size_t inlen, const char *directory)
+int __gen_keyword_names(const char *msgfilename, const char *flagstr, char *inbuf, size_t inlen, const char *directory)
 {
 	FILE *fp;
 	char fbuf[32];
 	char filename[266];
 	char *buf = inbuf;
 	int matches = 0;
 	int left = (int) inlen;
-	const char *custom_start = s;
+	const char *custom_start = flagstr;
+	const char *tmp;
 
 	snprintf(filename, sizeof(filename), "%s/.keywords", directory);
 
+	/* If the string contains more than just keywords, skip to the start of the keywords.
+	 * Do this first to make future strchr calls and iterations more efficient. */
+	if (custom_start) {
+		while (*custom_start && !islower(*custom_start)) {
+			custom_start++;
+		}
+	}
+
 	*buf = '\0';
 	fp = fopen(filename, "r");
 	if (!fp) {
@@ -124,20 +134,22 @@ int __gen_keyword_names(const char *s, char *inbuf, size_t inlen, const char *di
 	}
 
 	while ((fgets(fbuf, sizeof(fbuf), fp))) {
-		if (!s || strchr(s, fbuf[0])) {
+		if (!custom_start || strchr(custom_start, fbuf[0])) { /* Either it's a keyword letter of interest or we have no filter, match */
 			matches++;
 			bbs_strterm(fbuf, '\n');
 			SAFE_FAST_COND_APPEND_NOSPACE(inbuf, inlen, buf, left, 1, " %s", fbuf + 2);
 		}
 	}
 
-	if (s) {
+	if (custom_start) {
 		int keywordslen = 0;
-		while (!strlen_zero(s)) {
-			if (islower(*s)) {
+		/* Count how many keyword letters there are (as opposed to normal flags) */
+		tmp = custom_start;
+		while (*tmp) {
+			if (islower(*tmp)) {
 				keywordslen++;
 			}
-			s++;
+			tmp++;
 		}
 
 		if (keywordslen > matches) {
@@ -146,19 +158,14 @@ int __gen_keyword_names(const char *s, char *inbuf, size_t inlen, const char *di
 			int mpos = 0;
 			rewind(fp);
 			while ((fgets(fbuf, sizeof(fbuf), fp))) {
-				if (!s || strchr(s, fbuf[0])) {
-					mappings[mpos++] = fbuf[0];
-					if (mpos >= (int) sizeof(mappings) - 1) {
-						break;
-					}
+				mappings[mpos++] = fbuf[0];
+				if (mpos >= (int) sizeof(mappings) - 1) {
+					break;
 				}
 			}
 			mappings[mpos] = '\0';
-			while (*custom_start && !islower(*custom_start)) {
-				custom_start++;
-			}
 
-			bbs_warning("File has %d custom flags (%s), but we only have mappings for %d of them (%s)?\n", keywordslen, custom_start, matches, mappings);
+			bbs_warning("'%s' has %d custom flag%s (%s), but %s only has mappings for %d of them (%s)?\n", msgfilename, keywordslen, ESS(keywordslen), custom_start, filename, matches, mappings);
 		}
 	}
 	fclose(fp);
@@ -340,7 +347,7 @@ int translate_maildir_flags(struct imap_session *imap, const char *oldmaildir, c
 	 * and abstracted away from the IMAP module (i.e. done automatically on any move or copy). */
 
 	/* Get the old keyword names themselves */
-	numkeywords = __gen_keyword_names(oldfilename, keywords, sizeof(keywords), oldmaildir); /* prepends a space before all of them, so this (usually) works out great */
+	numkeywords = __gen_keyword_names(oldfilenamefull, oldfilename, keywords, sizeof(keywords), oldmaildir); /* prepends a space before all of them, so this (usually) works out great */
 
 	if (numkeywords <= 0) {
 #ifdef EXTRA_DEBUG
@@ -376,27 +383,29 @@ int translate_maildir_flags(struct imap_session *imap, const char *oldmaildir, c
 	return maildir_msg_setflags(imap, 0, oldfilenamefull, newflagletters);
 }
 
-void generate_flag_names_full(struct imap_session *imap, const char *filename, char *bufstart, size_t bufsize, char **bufptr, int *lenptr)
+static void generate_flag_names_full_dir(struct imap_session *imap, const char *filename, const char *flagstr, char *bufstart, size_t bufsize, char **bufptr, int *lenptr, const char *keyworddir)
 {
 	char flagsbuf[256] = "";
 	int has_flags;
 	int custom_keywords;
+	const char *flagstart = flagstr;
 
 	char *buf = *bufptr;
 	size_t len = (size_t) *lenptr;
 
-	if (isdigit(*filename)) { /* We have an entire filename */
-		filename = strchr(filename, ':'); /* Skip everything before the flags, so we don't e.g. interpret ,S= as the Seen flag. */
-		if (!filename) {
-			filename = ""; /* There ain't no flags here */
+	if (isdigit(*flagstart)) { /* We have an entire filename */
+		flagstart = strchr(flagstart, ':'); /* Skip everything before the flags, so we don't e.g. interpret ,S= as the Seen flag. */
+		if (!flagstart) {
+			flagstart = ""; /* There ain't no flags here */
 		}
 	} /* else, must just have the "flags" portion of the filename to begin with */
 
-	gen_flag_names(filename, flagsbuf, sizeof(flagsbuf));
+	gen_flag_names(flagstart, flagsbuf, sizeof(flagsbuf));
 	has_flags = flagsbuf[0] ? 1 : 0;
 	SAFE_FAST_COND_APPEND(bufstart, bufsize, buf, len, 1, "FLAGS (%s", flagsbuf);
 	/* If there are any keywords (custom flags), include those as well */
-	custom_keywords = gen_keyword_names(imap, filename, flagsbuf, sizeof(flagsbuf));
+	UNUSED(imap);
+	custom_keywords = gen_keyword_names_dir(filename, flagstr, flagsbuf, sizeof(flagsbuf), keyworddir);
 	if (has_flags) {
 		SAFE_FAST_COND_APPEND_NOSPACE(bufstart, bufsize, buf, len, custom_keywords > 0, "%s", flagsbuf);
 	} else {
@@ -409,13 +418,20 @@ void generate_flag_names_full(struct imap_session *imap, const char *filename, c
 	*lenptr = (int) len;
 }
 
+void generate_flag_names_full(struct imap_session *imap, const char *filename, const char *flagstr, char *bufstart, size_t bufsize, char **bufptr, int *lenptr)
+{
+	/* By default, gen_keyword_names uses imap->dir, so pass that to gen_keyword_names_dir */
+	return generate_flag_names_full_dir(imap, filename, flagstr, bufstart, bufsize, bufptr, lenptr, imap->dir);
+}
+
 int maildir_msg_setflags_modseq(struct imap_session *imap, int seqno, const char *origname, const char *newflagletters, unsigned long *newmodseq)
 {
 	char fullfilename[524];
 	char newflags[512] = "";
 	char *newbuf = newflags;
 	int newlen = sizeof(newflags);
 	char dirpath[256];
+	char newmaildir[256];
 	char *tmp, *filename;
 	unsigned long modseq;
 
@@ -495,7 +511,10 @@ int maildir_msg_setflags_modseq(struct imap_session *imap, int seqno, const char
 	/* If newmodseq is not NULL, then we need to send responses as needed. XXX What if it's not? */
 
 	/* Send unilateral untagged FETCH responses to everyone except this session, to notify of the new flags */
-	generate_flag_names_full(imap, newflagletters, newflags, sizeof(newflags), &newbuf, &newlen);
+	safe_strncpy(newmaildir, dirpath, sizeof(newmaildir));
+	/* We pass the new maildir at this point, as the default would be to use imap->dir, i.e. the old maildir,
+	 * but the new maildir is what will have the correct keyword mapping. */
+	generate_flag_names_full_dir(imap, fullfilename, newflagletters, newflags, sizeof(newflags), &newbuf, &newlen, dirname(newmaildir));
 	if (seqno) { /* Skip for merely translating flag mappings between maildirs */
 		char *end;
 		unsigned int uid;

nets/net_imap/imap_server_flags.h

@@ -50,7 +50,7 @@
 /* The implementation of how keywords are stored is based on how Dovecot stores keywords:
  * https://doc.dovecot.org/admin_manual/mailbox_formats/maildir/
  * We use 26 lowercase letters, to differentiate from IMAP flags (uppercase letters).
- * However, we don't a uidlist file, and we store the keywords in a separate file.
+ * However, we don't use a uidlist file, and we store the keywords in a separate file.
  * The implementation is handled fully in net_imap, since other modules don't care about keywords.
  */
 
@@ -59,9 +59,18 @@
 /*! \brief Check filename for the mapping for keyword. If one does not exist and there is room ( < 26), it will be created. */
 void parse_keyword(struct imap_session *imap, const char *s, const char *directory, int create);
 
-#define gen_keyword_names(imap, s, inbuf, inlen) __gen_keyword_names(s, inbuf, inlen, imap->dir)
+#define gen_keyword_names(imap, filename, flagstr, inbuf, inlen) __gen_keyword_names(filename, flagstr, inbuf, inlen, imap->dir)
+#define gen_keyword_names_dir(filename, flagstr, inbuf, inlen, dir) __gen_keyword_names(filename, flagstr, inbuf, inlen, dir)
 
-int __gen_keyword_names(const char *s, char *inbuf, size_t inlen, const char *directory);
+/*!
+ * \brief Generate keyword names from the letters in a maildir file
+ * \param msgfilename Filename of message file, used only for logging. NULL to simply fetch all keywords.
+ * \param flagstr String containing keyword letters (e.g. filename). NULL to simply fetch all keywords.
+ * \param[out] inbuf Keyword names
+ * \param inlen Size of inbuf
+ * \param directory The maildir containing the keywords
+ */
+int __gen_keyword_names(const char *msgfilename, const char *flagstr, char *inbuf, size_t inlen, const char *directory);
 
 #define parse_flags_string(imap, s) __parse_flags_string(imap, s, imap->dir)
 
@@ -102,8 +111,8 @@ int restrict_flags(int acl, int *flags);
  */
 int translate_maildir_flags(struct imap_session *imap, const char *oldmaildir, const char *oldfilenamefull, const char *oldfilename, const char *newmaildir, int destacl);
 
-/*! \brief base filename The file name of the message file. Please do not provide the full filepath. */
-void generate_flag_names_full(struct imap_session *imap, const char *filename, char *bufstart, size_t bufsize, char **bufptr, int *lenptr);
+/*! \brief base flagstr The file name of the message file (importantly, we want the part containing the flags). Please do not provide the full filepath. */
+void generate_flag_names_full(struct imap_session *imap, const char *filename, const char *flagstr, char *bufstart, size_t bufsize, char **bufptr, int *lenptr);
 
 int maildir_msg_setflags_modseq(struct imap_session *imap, int seqno, const char *origname, const char *newflagletters, unsigned long *newmodseq);