Previously, if "b" was length 127, the loop would terminate after
copying bytes 0..125. The final for loop increment would set i=126.
Then, "tmp[i+1] = 0" would nul-terminate the string at byte 127. So
byte 126 would be unset.
Note that since none of the Capabilities mutt were passing are that
long, the bug wouldn't matter, but it was a confusing and buggy
function in any case. :D
The only caller of this function isn't actually interested in a
comparison, it's interested in whether the word "a" matches the first
word in "b". So rewrite the function to instead compare lengths and
check if they are equal. The function is much simpler and easier to
understand that way.
Thanks to Acts1631 for the bug report.
Thanks to Alejandro Colomar for his discussion and proposed patches,
which motivated the use of strcspn(), defining ASCII_WS, and renaming
the function to imap_wordcaseeq().
Also thanks to Kurt Hackenberg and Ian Collier for their discussion
and proposals, leading to the removal of the tmp[] array completely.
---
After thinking about it overnight, I agree with Alex that this API is
better. It also makes the code a tiny bit faster since it doesn't
need to compare unless the lengths actually match.
imap/command.c | 2 +-
imap/imap_private.h | 2 +-
imap/util.c | 27 +++++++++------------------
lib.h | 1 +
4 files changed, 12 insertions(+), 20 deletions(-)
diff --git a/imap/command.c b/imap/command.c
index 82e0ddb7..c9c4c0c2 100644
--- a/imap/command.c
+++ b/imap/command.c
@@ -630,7 +630,7 @@ static void cmd_parse_capability(IMAP_DATA *idata, char *s)
while (*s)
{
for (x = 0; x < CAPMAX; x++)
- if (imap_wordcasecmp(Capabilities[x], s) == 0)
+ if (imap_wordcaseeq(Capabilities[x], s))
{
mutt_bit_set(idata->capabilities, x);
break;
diff --git a/imap/imap_private.h b/imap/imap_private.h
index f417e327..19c1f4dc 100644
--- a/imap/imap_private.h
+++ b/imap/imap_private.h
@@ -328,7 +328,7 @@ void imap_quote_string_and_backquotes(char *dest, size_t
dlen, const char *src);
void imap_unquote_string(char *s);
void imap_munge_mbox_name(IMAP_DATA *idata, char *dest, size_t dlen, const
char *src);
void imap_unmunge_mbox_name(IMAP_DATA *idata, char *s);
-int imap_wordcasecmp(const char *a, const char *b);
+int imap_wordcaseeq(const char *a, const char *b);
SEQSET_ITERATOR *mutt_seqset_iterator_new(const char *seqset);
int mutt_seqset_iterator_next(SEQSET_ITERATOR *iter, unsigned int *next);
void mutt_seqset_iterator_free(SEQSET_ITERATOR **p_iter);
diff --git a/imap/util.c b/imap/util.c
index 9b6ef7b4..29f88a1b 100644
--- a/imap/util.c
+++ b/imap/util.c
@@ -880,26 +880,17 @@ void imap_unmunge_mbox_name(IMAP_DATA *idata, char *s)
FREE(&buf);
}
-/* imap_wordcasecmp: find word a in word list b */
-int imap_wordcasecmp(const char *a, const char *b)
+/* imap_wordcaseeq:
+ *
+ * Checks if word a is case-independently equal to the initial
+ * ASCII_WS delimited word in word list b. */
+int imap_wordcaseeq(const char *a, const char *b)
{
- char tmp[SHORT_STRING];
- const char *s = b;
- int i;
+ size_t a_len, b_len;
- tmp[SHORT_STRING-1] = 0;
- for (i=0;i < SHORT_STRING-2;i++,s++)
- {
- if (!*s || IS_ASCII_WS(*s))
- {
- tmp[i] = 0;
- break;
- }
- tmp[i] = *s;
- }
- tmp[i+1] = 0;
-
- return ascii_strcasecmp(a, tmp);
+ a_len = mutt_strlen(a);
+ b_len = b ? strcspn(b, ASCII_WS) : 0;
+ return (a_len == b_len) && !ascii_strncasecmp(a, b, a_len);
}
/*
diff --git a/lib.h b/lib.h
index 830459d6..72e3a498 100644
--- a/lib.h
+++ b/lib.h
@@ -113,6 +113,7 @@
* 0x09-0x0d (\t \n \v \f \r)
* 0x20 (space)
*/
+#define ASCII_WS " \t\n\v\f\r"
#define IS_ASCII_WS(c) ((9 <= (c) && (c) <= 13) || (c) == 32)
#define SKIP_ASCII_WS(c) while (IS_ASCII_WS(*(c))) c++;
--
2.54.0
