From 8e4c42dbb3c770fcdbc396f2abcf1bc228ec548d Mon Sep 17 00:00:00 2001 From: Timo Sirainen Date: Fri, 9 Feb 2024 00:32:39 +0200 Subject: [PATCH 1/6] lib: test-llist - Fix dllist2 test name --- src/lib/test-llist.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/test-llist.c b/src/lib/test-llist.c index d57006ce2aa..ed584318fa3 100644 --- a/src/lib/test-llist.c +++ b/src/lib/test-llist.c @@ -71,7 +71,7 @@ static void test_dllist2(void) l2 = t_new(struct dllist, 1); l1 = t_new(struct dllist, 1); - test_begin("dllist"); + test_begin("dllist2"); /* prepend to empty */ DLLIST2_PREPEND(&head, &tail, l3); test_assert(head == l3 && tail == l3); From cee08202c759a3bdf185d998dcf888ebd1bc6e36 Mon Sep 17 00:00:00 2001 From: Timo Sirainen Date: Fri, 9 Feb 2024 00:33:00 +0200 Subject: [PATCH 2/6] lib: Add DLLIST2_JOIN() --- src/lib/llist.h | 14 ++++++++++++++ src/lib/test-llist.c | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/src/lib/llist.h b/src/lib/llist.h index 8a52e873352..5ad5d75c0df 100644 --- a/src/lib/llist.h +++ b/src/lib/llist.h @@ -78,4 +78,18 @@ #define DLLIST2_REMOVE(head, tail, item) \ DLLIST2_REMOVE_FULL(head, tail, item, prev, next) +#define DLLIST2_JOIN_FULL(head1, tail1, head2, tail2, prev, next) STMT_START { \ + if (*(head1) == NULL) { \ + *(head1) = *(head2); \ + *(tail1) = *(tail2); \ + } else if (*(head2) != NULL) { \ + (*(tail1))->next = *(head2); \ + (*(head2))->prev = *(tail1); \ + (*tail1) = (*tail2); \ + } \ + } STMT_END + +#define DLLIST2_JOIN(head1, tail1, head2, tail2) \ + DLLIST2_JOIN_FULL(head1, tail1, head2, tail2, prev, next) + #endif diff --git a/src/lib/test-llist.c b/src/lib/test-llist.c index ed584318fa3..e293eb6a603 100644 --- a/src/lib/test-llist.c +++ b/src/lib/test-llist.c @@ -131,8 +131,47 @@ static void test_dllist2(void) test_end(); } +static void test_dllist2_join(void) +{ + struct dllist *head, *tail, *elem[4]; + struct dllist *head2, *tail2, *elem2[N_ELEMENTS(elem)]; + + test_begin("dllist2 join"); + for (unsigned int i = 0; i < N_ELEMENTS(elem); i++) { + elem[i] = t_new(struct dllist, 1); + elem2[i] = t_new(struct dllist, 1); + } + for (unsigned int i = 0; i < N_ELEMENTS(elem); i++) { + for (unsigned int j = 0; j < N_ELEMENTS(elem2); j++) { + head = tail = head2 = tail2 = NULL; + for (unsigned int n = 0; n < i; n++) + DLLIST2_APPEND(&head, &tail, elem[n]); + for (unsigned int n = 0; n < j; n++) + DLLIST2_APPEND(&head2, &tail2, elem2[n]); + DLLIST2_JOIN(&head, &tail, &head2, &tail2); + + /* verify */ + struct dllist *tmp = head, *last = NULL; + for (unsigned int n = 0; n < i; n++) { + test_assert(tmp == elem[n]); + last = tmp; + tmp = tmp->next; + } + for (unsigned int n = 0; n < j; n++) { + test_assert(tmp == elem2[n]); + last = tmp; + tmp = tmp->next; + } + test_assert(tmp == NULL); + test_assert(tail == last); + } + } + test_end(); +} + void test_llist(void) { test_dllist(); test_dllist2(); + test_dllist2_join(); } From 0bae091859c905dc335f21eed01347e6b8338672 Mon Sep 17 00:00:00 2001 From: Timo Sirainen Date: Tue, 30 Jan 2024 22:42:50 +0200 Subject: [PATCH 3/6] lib-mail: test-imap-envelope - Use test_assert_idx() where possible --- src/lib-imap/test-imap-envelope.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib-imap/test-imap-envelope.c b/src/lib-imap/test-imap-envelope.c index 1f295e58bab..c9b92b4be2b 100644 --- a/src/lib-imap/test-imap-envelope.c +++ b/src/lib-imap/test-imap-envelope.c @@ -157,7 +157,7 @@ static void test_imap_envelope_write(void) envlp = msg_parse(pool, test->message); imap_envelope_write(envlp, str); - test_assert(strcmp(str_c(str), test->envelope) == 0); + test_assert_idx(strcmp(str_c(str), test->envelope) == 0, i); pool_unref(&pool); test_end(); @@ -179,12 +179,12 @@ static void test_imap_envelope_parse(void) test_begin(t_strdup_printf("imap envelope parser [%u]", i)); ret = imap_envelope_parse(test->envelope, pool, &envlp, &error); - test_assert(ret); + test_assert_idx(ret, i); if (ret) { str_truncate(str, 0); imap_envelope_write(envlp, str); - test_assert(strcmp(str_c(str), test->envelope) == 0); + test_assert_idx(strcmp(str_c(str), test->envelope) == 0, i); } else { i_error("Invalid envelope: %s", error); } From a1c9b0409454e45937bf7e9c3685f5e91d6a5a43 Mon Sep 17 00:00:00 2001 From: Timo Sirainen Date: Sun, 4 Feb 2024 00:26:57 +0200 Subject: [PATCH 4/6] lib-mail: Change message_address to be doubly linked list --- src/lib-imap/imap-envelope.c | 11 +- src/lib-mail/message-address.c | 8 +- src/lib-mail/message-address.h | 2 +- src/lib-mail/test-message-address.c | 226 ++++++++++++++-------------- 4 files changed, 121 insertions(+), 126 deletions(-) diff --git a/src/lib-imap/imap-envelope.c b/src/lib-imap/imap-envelope.c index 87297f4f691..1312eae2ff3 100644 --- a/src/lib-imap/imap-envelope.c +++ b/src/lib-imap/imap-envelope.c @@ -1,6 +1,7 @@ /* Copyright (c) 2002-2018 Dovecot authors, see the included COPYING file */ #include "lib.h" +#include "llist.h" #include "istream.h" #include "str.h" #include "message-address.h" @@ -127,7 +128,7 @@ static bool imap_envelope_parse_addresses(const struct imap_arg *arg, pool_t pool, struct message_address **addrs_r) { - struct message_address *first, *addr, *prev; + struct message_address *first, *last, *addr; const struct imap_arg *list_args; if (arg->type == IMAP_ARG_NIL) { @@ -138,16 +139,12 @@ imap_envelope_parse_addresses(const struct imap_arg *arg, if (!imap_arg_get_list(arg, &list_args)) return FALSE; - first = addr = prev = NULL; + first = last = addr = NULL; for (; !IMAP_ARG_IS_EOL(list_args); list_args++) { if (!imap_envelope_parse_address (list_args, pool, &addr)) return FALSE; - if (first == NULL) - first = addr; - if (prev != NULL) - prev->next = addr; - prev = addr; + DLLIST2_APPEND(&first, &last, addr); } *addrs_r = first; diff --git a/src/lib-mail/message-address.c b/src/lib-mail/message-address.c index fb06afae7b7..9d192799468 100644 --- a/src/lib-mail/message-address.c +++ b/src/lib-mail/message-address.c @@ -1,6 +1,7 @@ /* Copyright (c) 2002-2018 Dovecot authors, see the included COPYING file */ #include "lib.h" +#include "llist.h" #include "str.h" #include "strescape.h" #include "smtp-address.h" @@ -27,11 +28,7 @@ static void add_address(struct message_address_parser_context *ctx) memcpy(addr, &ctx->addr, sizeof(ctx->addr)); i_zero(&ctx->addr); - if (ctx->first_addr == NULL) - ctx->first_addr = addr; - else - ctx->last_addr->next = addr; - ctx->last_addr = addr; + DLLIST2_APPEND(&ctx->first_addr, &ctx->last_addr, addr); } /* quote with "" and escape all '\', '"' and "'" characters if need */ @@ -631,6 +628,7 @@ const char *message_address_first_to_string(const struct message_address *addr) struct message_address first_addr; first_addr = *addr; + first_addr.prev = NULL; first_addr.next = NULL; first_addr.route = NULL; return message_address_to_string(&first_addr); diff --git a/src/lib-mail/message-address.h b/src/lib-mail/message-address.h index 8370397741c..85cff3dcc6f 100644 --- a/src/lib-mail/message-address.h +++ b/src/lib-mail/message-address.h @@ -18,7 +18,7 @@ enum message_address_parse_flags { {name = NULL, NULL, "group", NULL}, ..., {NULL, NULL, NULL, NULL} */ struct message_address { - struct message_address *next; + struct message_address *prev, *next; /* display-name */ const char *name; diff --git a/src/lib-mail/test-message-address.c b/src/lib-mail/test-message-address.c index e6204bb0588..261cbfba70a 100644 --- a/src/lib-mail/test-message-address.c +++ b/src/lib-mail/test-message-address.c @@ -47,174 +47,174 @@ static void test_message_address(void) } tests[] = { /* user@domain -> */ { "user@domain", "", NULL, - { NULL, NULL, NULL, "user", "domain", FALSE }, - { NULL, NULL, NULL, "user", "domain", FALSE }, 0 }, + { NULL, NULL, NULL, NULL, "user", "domain", FALSE }, + { NULL, NULL, NULL, NULL, "user", "domain", FALSE }, 0 }, { "\"user\"@domain", "", NULL, - { NULL, NULL, NULL, "user", "domain", FALSE }, - { NULL, NULL, NULL, "user", "domain", FALSE }, 0 }, + { NULL, NULL, NULL, NULL, "user", "domain", FALSE }, + { NULL, NULL, NULL, NULL, "user", "domain", FALSE }, 0 }, { "\"user name\"@domain", "<\"user name\"@domain>", NULL, - { NULL, NULL, NULL, "user name", "domain", FALSE }, - { NULL, NULL, NULL, "user name", "domain", FALSE }, 0 }, + { NULL, NULL, NULL, NULL, "user name", "domain", FALSE }, + { NULL, NULL, NULL, NULL, "user name", "domain", FALSE }, 0 }, { "\"user@na\\\\me\"@domain", "<\"user@na\\\\me\"@domain>", NULL, - { NULL, NULL, NULL, "user@na\\me", "domain", FALSE }, - { NULL, NULL, NULL, "user@na\\me", "domain", FALSE }, 0 }, + { NULL, NULL, NULL, NULL, "user@na\\me", "domain", FALSE }, + { NULL, NULL, NULL, NULL, "user@na\\me", "domain", FALSE }, 0 }, { "\"user\\\"name\"@domain", "<\"user\\\"name\"@domain>", NULL, - { NULL, NULL, NULL, "user\"name", "domain", FALSE }, - { NULL, NULL, NULL, "user\"name", "domain", FALSE }, 0 }, + { NULL, NULL, NULL, NULL, "user\"name", "domain", FALSE }, + { NULL, NULL, NULL, NULL, "user\"name", "domain", FALSE }, 0 }, { "\"\"@domain", "<\"\"@domain>", NULL, - { NULL, NULL, NULL, "", "domain", FALSE }, - { NULL, NULL, NULL, "", "domain", FALSE }, 0 }, + { NULL, NULL, NULL, NULL, "", "domain", FALSE }, + { NULL, NULL, NULL, NULL, "", "domain", FALSE }, 0 }, { "user", "", "", - { NULL, NULL, NULL, "user", "", TRUE }, - { NULL, NULL, NULL, "user", "MISSING_DOMAIN", TRUE }, 0 }, + { NULL, NULL, NULL, NULL, "user", "", TRUE }, + { NULL, NULL, NULL, NULL, "user", "MISSING_DOMAIN", TRUE }, 0 }, { "@domain", "<\"\"@domain>", "", - { NULL, NULL, NULL, "", "domain", TRUE }, - { NULL, NULL, NULL, "MISSING_MAILBOX", "domain", TRUE }, 0 }, + { NULL, NULL, NULL, NULL, "", "domain", TRUE }, + { NULL, NULL, NULL, NULL, "MISSING_MAILBOX", "domain", TRUE }, 0 }, /* Display Name -> Display Name */ { "Display Name", "\"Display Name\"", "\"Display Name\" ", - { NULL, "Display Name", NULL, "", "", TRUE }, - { NULL, "Display Name", NULL, "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE }, 0 }, + { NULL, NULL, "Display Name", NULL, "", "", TRUE }, + { NULL, NULL, "Display Name", NULL, "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE }, 0 }, { "\"Display Name\"", "\"Display Name\"", "\"Display Name\" ", - { NULL, "Display Name", NULL, "", "", TRUE }, - { NULL, "Display Name", NULL, "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE }, 0 }, + { NULL, NULL, "Display Name", NULL, "", "", TRUE }, + { NULL, NULL, "Display Name", NULL, "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE }, 0 }, { "Display \"Name\"", "\"Display Name\"", "\"Display Name\" ", - { NULL, "Display Name", NULL, "", "", TRUE }, - { NULL, "Display Name", NULL, "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE }, 0 }, + { NULL, NULL, "Display Name", NULL, "", "", TRUE }, + { NULL, NULL, "Display Name", NULL, "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE }, 0 }, { "\"Display\" \"Name\"", "\"Display Name\"", "\"Display Name\" ", - { NULL, "Display Name", NULL, "", "", TRUE }, - { NULL, "Display Name", NULL, "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE }, 0 }, + { NULL, NULL, "Display Name", NULL, "", "", TRUE }, + { NULL, NULL, "Display Name", NULL, "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE }, 0 }, { "\"\"", "", "", - { NULL, "", NULL, "", "", TRUE }, - { NULL, "", NULL, "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE }, 0 }, + { NULL, NULL, "", NULL, "", "", TRUE }, + { NULL, NULL, "", NULL, "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE }, 0 }, /* -> */ { "", NULL, NULL, - { NULL, NULL, NULL, "user", "domain", FALSE }, - { NULL, NULL, NULL, "user", "domain", FALSE }, 0 }, + { NULL, NULL, NULL, NULL, "user", "domain", FALSE }, + { NULL, NULL, NULL, NULL, "user", "domain", FALSE }, 0 }, { "<\"user\"@domain>", "", NULL, - { NULL, NULL, NULL, "user", "domain", FALSE }, - { NULL, NULL, NULL, "user", "domain", FALSE }, 0 }, + { NULL, NULL, NULL, NULL, "user", "domain", FALSE }, + { NULL, NULL, NULL, NULL, "user", "domain", FALSE }, 0 }, { "<\"user name\"@domain>", NULL, NULL, - { NULL, NULL, NULL, "user name", "domain", FALSE }, - { NULL, NULL, NULL, "user name", "domain", FALSE }, 0 }, + { NULL, NULL, NULL, NULL, "user name", "domain", FALSE }, + { NULL, NULL, NULL, NULL, "user name", "domain", FALSE }, 0 }, { "<\"user@na\\\\me\"@domain>", NULL, NULL, - { NULL, NULL, NULL, "user@na\\me", "domain", FALSE }, - { NULL, NULL, NULL, "user@na\\me", "domain", FALSE }, 0 }, + { NULL, NULL, NULL, NULL, "user@na\\me", "domain", FALSE }, + { NULL, NULL, NULL, NULL, "user@na\\me", "domain", FALSE }, 0 }, { "<\"user\\\"name\"@domain>", NULL, NULL, - { NULL, NULL, NULL, "user\"name", "domain", FALSE }, - { NULL, NULL, NULL, "user\"name", "domain", FALSE }, 0 }, + { NULL, NULL, NULL, NULL, "user\"name", "domain", FALSE }, + { NULL, NULL, NULL, NULL, "user\"name", "domain", FALSE }, 0 }, { "<\"\"@domain>", NULL, NULL, - { NULL, NULL, NULL, "", "domain", FALSE }, - { NULL, NULL, NULL, "", "domain", FALSE }, 0 }, + { NULL, NULL, NULL, NULL, "", "domain", FALSE }, + { NULL, NULL, NULL, NULL, "", "domain", FALSE }, 0 }, { "", NULL, "", - { NULL, NULL, NULL, "user", "", TRUE }, - { NULL, NULL, NULL, "user", "MISSING_DOMAIN", TRUE }, 0 }, + { NULL, NULL, NULL, NULL, "user", "", TRUE }, + { NULL, NULL, NULL, NULL, "user", "MISSING_DOMAIN", TRUE }, 0 }, { "<@route>", "<@route:\"\">", "", - { NULL, NULL, "@route", "", "", TRUE }, - { NULL, NULL, "INVALID_ROUTE", "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE }, 0 }, + { NULL, NULL, NULL, "@route", "", "", TRUE }, + { NULL, NULL, NULL, "INVALID_ROUTE", "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE }, 0 }, /* user@domain (Display Name) -> "Display Name" */ { "user@domain (DisplayName)", "DisplayName ", NULL, - { NULL, "DisplayName", NULL, "user", "domain", FALSE }, - { NULL, "DisplayName", NULL, "user", "domain", FALSE }, 0 }, + { NULL, NULL, "DisplayName", NULL, "user", "domain", FALSE }, + { NULL, NULL, "DisplayName", NULL, "user", "domain", FALSE }, 0 }, { "user@domain (Display Name)", "\"Display Name\" ", NULL, - { NULL, "Display Name", NULL, "user", "domain", FALSE }, - { NULL, "Display Name", NULL, "user", "domain", FALSE }, 0 }, + { NULL, NULL, "Display Name", NULL, "user", "domain", FALSE }, + { NULL, NULL, "Display Name", NULL, "user", "domain", FALSE }, 0 }, { "user@domain (Display\"Name)", "\"Display\\\"Name\" ", NULL, - { NULL, "Display\"Name", NULL, "user", "domain", FALSE }, - { NULL, "Display\"Name", NULL, "user", "domain", FALSE }, 0 }, + { NULL, NULL, "Display\"Name", NULL, "user", "domain", FALSE }, + { NULL, NULL, "Display\"Name", NULL, "user", "domain", FALSE }, 0 }, { "user (Display Name)", "\"Display Name\" ", "\"Display Name\" ", - { NULL, "Display Name", NULL, "user", "", TRUE }, - { NULL, "Display Name", NULL, "user", "MISSING_DOMAIN", TRUE }, 0 }, + { NULL, NULL, "Display Name", NULL, "user", "", TRUE }, + { NULL, NULL, "Display Name", NULL, "user", "MISSING_DOMAIN", TRUE }, 0 }, { "@domain (Display Name)", "\"Display Name\" <\"\"@domain>", "\"Display Name\" ", - { NULL, "Display Name", NULL, "", "domain", TRUE }, - { NULL, "Display Name", NULL, "MISSING_MAILBOX", "domain", TRUE }, 0 }, + { NULL, NULL, "Display Name", NULL, "", "domain", TRUE }, + { NULL, NULL, "Display Name", NULL, "MISSING_MAILBOX", "domain", TRUE }, 0 }, { "user@domain ()", "", NULL, - { NULL, NULL, NULL, "user", "domain", FALSE }, - { NULL, NULL, NULL, "user", "domain", FALSE }, 0 }, + { NULL, NULL, NULL, NULL, "user", "domain", FALSE }, + { NULL, NULL, NULL, NULL, "user", "domain", FALSE }, 0 }, /* Display Name -> "Display Name" */ { "DisplayName ", NULL, NULL, - { NULL, "DisplayName", NULL, "user", "domain", FALSE }, - { NULL, "DisplayName", NULL, "user", "domain", FALSE }, 0 }, + { NULL, NULL, "DisplayName", NULL, "user", "domain", FALSE }, + { NULL, NULL, "DisplayName", NULL, "user", "domain", FALSE }, 0 }, { "Display Name ", "\"Display Name\" ", NULL, - { NULL, "Display Name", NULL, "user", "domain", FALSE }, - { NULL, "Display Name", NULL, "user", "domain", FALSE }, 0 }, + { NULL, NULL, "Display Name", NULL, "user", "domain", FALSE }, + { NULL, NULL, "Display Name", NULL, "user", "domain", FALSE }, 0 }, { "\"Display Name\" ", NULL, NULL, - { NULL, "Display Name", NULL, "user", "domain", FALSE }, - { NULL, "Display Name", NULL, "user", "domain", FALSE }, 0 }, + { NULL, NULL, "Display Name", NULL, "user", "domain", FALSE }, + { NULL, NULL, "Display Name", NULL, "user", "domain", FALSE }, 0 }, { "\"Display\\\"Name\" ", NULL, NULL, - { NULL, "Display\"Name", NULL, "user", "domain", FALSE }, - { NULL, "Display\"Name", NULL, "user", "domain", FALSE }, 0 }, + { NULL, NULL, "Display\"Name", NULL, "user", "domain", FALSE }, + { NULL, NULL, "Display\"Name", NULL, "user", "domain", FALSE }, 0 }, { "Display Name ", "\"Display Name\" ", "\"Display Name\" ", - { NULL, "Display Name", NULL, "user", "", TRUE }, - { NULL, "Display Name", NULL, "user", "MISSING_DOMAIN", TRUE }, 0 }, + { NULL, NULL, "Display Name", NULL, "user", "", TRUE }, + { NULL, NULL, "Display Name", NULL, "user", "MISSING_DOMAIN", TRUE }, 0 }, { "\"\" ", "", NULL, - { NULL, NULL, NULL, "user", "domain", FALSE }, - { NULL, NULL, NULL, "user", "domain", FALSE }, 0 }, + { NULL, NULL, NULL, NULL, "user", "domain", FALSE }, + { NULL, NULL, NULL, NULL, "user", "domain", FALSE }, 0 }, /* <@route:user@domain> -> <@route:user@domain> */ { "<@route:user@domain>", NULL, NULL, - { NULL, NULL, "@route", "user", "domain", FALSE }, - { NULL, NULL, "@route", "user", "domain", FALSE }, 0 }, + { NULL, NULL, NULL, "@route", "user", "domain", FALSE }, + { NULL, NULL, NULL, "@route", "user", "domain", FALSE }, 0 }, { "<@route,@route2:user@domain>", NULL, NULL, - { NULL, NULL, "@route,@route2", "user", "domain", FALSE }, - { NULL, NULL, "@route,@route2", "user", "domain", FALSE }, 0 }, + { NULL, NULL, NULL, "@route,@route2", "user", "domain", FALSE }, + { NULL, NULL, NULL, "@route,@route2", "user", "domain", FALSE }, 0 }, { "<@route@route2:user@domain>", "<@route,@route2:user@domain>", NULL, - { NULL, NULL, "@route,@route2", "user", "domain", FALSE }, - { NULL, NULL, "@route,@route2", "user", "domain", FALSE }, 0 }, + { NULL, NULL, NULL, "@route,@route2", "user", "domain", FALSE }, + { NULL, NULL, NULL, "@route,@route2", "user", "domain", FALSE }, 0 }, { "<@route@route2:user>", "<@route,@route2:user>", "<@route,@route2:user@MISSING_DOMAIN>", - { NULL, NULL, "@route,@route2", "user", "", TRUE }, - { NULL, NULL, "@route,@route2", "user", "MISSING_DOMAIN", TRUE }, 0 }, + { NULL, NULL, NULL, "@route,@route2", "user", "", TRUE }, + { NULL, NULL, NULL, "@route,@route2", "user", "MISSING_DOMAIN", TRUE }, 0 }, { "<@route@route2:\"\"@domain>", "<@route,@route2:\"\"@domain>", NULL, - { NULL, NULL, "@route,@route2", "", "domain", FALSE }, - { NULL, NULL, "@route,@route2", "", "domain", FALSE }, 0 }, + { NULL, NULL, NULL, "@route,@route2", "", "domain", FALSE }, + { NULL, NULL, NULL, "@route,@route2", "", "domain", FALSE }, 0 }, /* Display Name <@route:user@domain> -> "Display Name" <@route:user@domain> */ { "Display Name <@route:user@domain>", "\"Display Name\" <@route:user@domain>", NULL, - { NULL, "Display Name", "@route", "user", "domain", FALSE }, - { NULL, "Display Name", "@route", "user", "domain", FALSE }, 0 }, + { NULL, NULL, "Display Name", "@route", "user", "domain", FALSE }, + { NULL, NULL, "Display Name", "@route", "user", "domain", FALSE }, 0 }, { "Display Name <@route,@route2:user@domain>", "\"Display Name\" <@route,@route2:user@domain>", NULL, - { NULL, "Display Name", "@route,@route2", "user", "domain", FALSE }, - { NULL, "Display Name", "@route,@route2", "user", "domain", FALSE }, 0 }, + { NULL, NULL, "Display Name", "@route,@route2", "user", "domain", FALSE }, + { NULL, NULL, "Display Name", "@route,@route2", "user", "domain", FALSE }, 0 }, { "Display Name <@route@route2:user@domain>", "\"Display Name\" <@route,@route2:user@domain>", NULL, - { NULL, "Display Name", "@route,@route2", "user", "domain", FALSE }, - { NULL, "Display Name", "@route,@route2", "user", "domain", FALSE }, 0 }, + { NULL, NULL, "Display Name", "@route,@route2", "user", "domain", FALSE }, + { NULL, NULL, "Display Name", "@route,@route2", "user", "domain", FALSE }, 0 }, { "Display Name <@route@route2:user>", "\"Display Name\" <@route,@route2:user>", "\"Display Name\" <@route,@route2:user@MISSING_DOMAIN>", - { NULL, "Display Name", "@route,@route2", "user", "", TRUE }, - { NULL, "Display Name", "@route,@route2", "user", "MISSING_DOMAIN", TRUE }, 0 }, + { NULL, NULL, "Display Name", "@route,@route2", "user", "", TRUE }, + { NULL, NULL, "Display Name", "@route,@route2", "user", "MISSING_DOMAIN", TRUE }, 0 }, { "Display Name <@route@route2:\"\"@domain>", "\"Display Name\" <@route,@route2:\"\"@domain>", NULL, - { NULL, "Display Name", "@route,@route2", "", "domain", FALSE }, - { NULL, "Display Name", "@route,@route2", "", "domain", FALSE }, 0 }, + { NULL, NULL, "Display Name", "@route,@route2", "", "domain", FALSE }, + { NULL, NULL, "Display Name", "@route,@route2", "", "domain", FALSE }, 0 }, /* other tests: */ { "\"foo: ;,\" ", NULL, NULL, - { NULL, "foo: ;,", NULL, "user", "domain", FALSE }, - { NULL, "foo: ;,", NULL, "user", "domain", FALSE }, 0 }, + { NULL, NULL, "foo: ;,", NULL, "user", "domain", FALSE }, + { NULL, NULL, "foo: ;,", NULL, "user", "domain", FALSE }, 0 }, { "<>", "", "", - { NULL, NULL, NULL, "", "", TRUE }, - { NULL, NULL, NULL, "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE }, 0 }, + { NULL, NULL, NULL, NULL, "", "", TRUE }, + { NULL, NULL, NULL, NULL, "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE }, 0 }, { "<@>", "", "", - { NULL, NULL, NULL, "", "", TRUE }, - { NULL, NULL, "INVALID_ROUTE", "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE }, 0 }, + { NULL, NULL, NULL, NULL, "", "", TRUE }, + { NULL, NULL, NULL, "INVALID_ROUTE", "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE }, 0 }, /* Test against a out-of-bounds read bug - keep these two tests together in this same order: */ { "aaaa@", "", "", - { NULL, NULL, NULL, "aaaa", "", TRUE }, - { NULL, NULL, NULL, "aaaa", "MISSING_DOMAIN", TRUE }, 0 }, + { NULL, NULL, NULL, NULL, "aaaa", "", TRUE }, + { NULL, NULL, NULL, NULL, "aaaa", "MISSING_DOMAIN", TRUE }, 0 }, { "a(aa", "", "", - { NULL, NULL, NULL, "", "", TRUE }, - { NULL, NULL, NULL, "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE }, + { NULL, NULL, NULL, NULL, "", "", TRUE }, + { NULL, NULL, NULL, NULL, "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE }, TEST_MESSAGE_ADDRESS_FLAG_SKIP_LIST }, }; static struct message_address group_prefix = { - NULL, NULL, NULL, "group", NULL, FALSE + NULL, NULL, NULL, NULL, "group", NULL, FALSE }; static struct message_address group_suffix = { - NULL, NULL, NULL, NULL, NULL, FALSE + NULL, NULL, NULL, NULL, NULL, NULL, FALSE }; const struct message_address *addr; string_t *str, *group; @@ -327,7 +327,7 @@ static void test_message_address_nuls(void) const unsigned char input[] = "\"user\0nuls\\\0-esc\"@[domain\0nuls\\\0-esc] (comment\0nuls\\\0-esc)"; const struct message_address output = { - NULL, "comment\xEF\xBF\xBDnuls\\\xEF\xBF\xBD-esc", NULL, + NULL, NULL, "comment\xEF\xBF\xBDnuls\\\xEF\xBF\xBD-esc", NULL, "user\xEF\xBF\xBDnuls\\\xEF\xBF\xBD-esc", "[domain\xEF\xBF\xBDnuls\\\xEF\xBF\xBD-esc]", FALSE }; @@ -345,7 +345,7 @@ static void test_message_address_nuls_display_name(void) const unsigned char input[] = "\"displayname\0nuls\\\0-esc\" <\"user\0nuls\\\0-esc\"@[domain\0nuls\\\0-esc]>"; const struct message_address output = { - NULL, "displayname\xEF\xBF\xBDnuls\\\xEF\xBF\xBD-esc", NULL, + NULL, NULL, "displayname\xEF\xBF\xBDnuls\\\xEF\xBF\xBD-esc", NULL, "user\xEF\xBF\xBDnuls\\\xEF\xBF\xBD-esc", "[domain\xEF\xBF\xBDnuls\\\xEF\xBF\xBD-esc]", FALSE }; @@ -369,7 +369,7 @@ static void test_message_address_non_strict_dots(void) }; const struct message_address *addr; struct message_address output = { - NULL, NULL, NULL, "local-part", + NULL, NULL, NULL, NULL, "local-part", "example.com", FALSE }; @@ -421,29 +421,29 @@ static void test_message_address_path(void) struct message_address addr; } tests[] = { { "<>", NULL, - { NULL, NULL, NULL, NULL, NULL, FALSE } }, + { NULL, NULL, NULL, NULL, NULL, NULL, FALSE } }, { " < > ", "<>", - { NULL, NULL, NULL, NULL, NULL, FALSE } }, + { NULL, NULL, NULL, NULL, NULL, NULL, FALSE } }, { "", NULL, - { NULL, NULL, NULL, "user", "domain", FALSE } }, + { NULL, NULL, NULL, NULL, "user", "domain", FALSE } }, { " ", "", - { NULL, NULL, NULL, "user", "domain", FALSE } }, + { NULL, NULL, NULL, NULL, "user", "domain", FALSE } }, { "user@domain", "", - { NULL, NULL, NULL, "user", "domain", FALSE } }, + { NULL, NULL, NULL, NULL, "user", "domain", FALSE } }, { " user@domain ", "", - { NULL, NULL, NULL, "user", "domain", FALSE } }, + { NULL, NULL, NULL, NULL, "user", "domain", FALSE } }, { "<\"user\"@domain>", "", - { NULL, NULL, NULL, "user", "domain", FALSE } }, + { NULL, NULL, NULL, NULL, "user", "domain", FALSE } }, { "<\"user name\"@domain>", NULL, - { NULL, NULL, NULL, "user name", "domain", FALSE } }, + { NULL, NULL, NULL, NULL, "user name", "domain", FALSE } }, { "<\"user@na\\\\me\"@domain>", NULL, - { NULL, NULL, NULL, "user@na\\me", "domain", FALSE } }, + { NULL, NULL, NULL, NULL, "user@na\\me", "domain", FALSE } }, { "<\"user\\\"name\"@domain>", NULL, - { NULL, NULL, NULL, "user\"name", "domain", FALSE } }, + { NULL, NULL, NULL, NULL, "user\"name", "domain", FALSE } }, { "<\"\"@domain>", NULL, - { NULL, NULL, NULL, "", "domain", FALSE } }, + { NULL, NULL, NULL, NULL, "", "domain", FALSE } }, { "<@source", "<>", - { NULL, NULL, NULL, NULL, NULL, TRUE } }, + { NULL, NULL, NULL, NULL, NULL, NULL, TRUE } }, }; const struct message_address *addr; string_t *str; From da61d20311da34f22944c6111a0b97ea2a1f8a47 Mon Sep 17 00:00:00 2001 From: Timo Sirainen Date: Tue, 30 Jan 2024 22:17:38 +0200 Subject: [PATCH 5/6] lib-mail: Add message_address_parse_full() and struct message_address_list --- src/lib-mail/message-address.c | 37 +++++++++++++--------- src/lib-mail/message-address.h | 10 ++++++ src/lib-mail/test-message-address.c | 48 +++++++++++++++++++++++++---- 3 files changed, 75 insertions(+), 20 deletions(-) diff --git a/src/lib-mail/message-address.c b/src/lib-mail/message-address.c index 9d192799468..ae37014079a 100644 --- a/src/lib-mail/message-address.c +++ b/src/lib-mail/message-address.c @@ -13,7 +13,8 @@ struct message_address_parser_context { pool_t pool; struct rfc822_parser_context parser; - struct message_address *first_addr, *last_addr, addr; + struct message_address addr; + struct message_address_list addr_list; string_t *str; bool fill_missing, non_strict_dots; @@ -28,7 +29,7 @@ static void add_address(struct message_address_parser_context *ctx) memcpy(addr, &ctx->addr, sizeof(ctx->addr)); i_zero(&ctx->addr); - DLLIST2_APPEND(&ctx->first_addr, &ctx->last_addr, addr); + DLLIST2_APPEND(&ctx->addr_list.head, &ctx->addr_list.tail, addr); } /* quote with "" and escape all '\', '"' and "'" characters if need */ @@ -439,10 +440,11 @@ static int parse_path(struct message_address_parser_context *ctx) return ret; } -static struct message_address * +static void message_address_parse_real(pool_t pool, const unsigned char *data, size_t size, unsigned int max_addresses, - enum message_address_parse_flags flags) + enum message_address_parse_flags flags, + struct message_address_list *list_r) { struct message_address_parser_context ctx; @@ -461,7 +463,7 @@ message_address_parse_real(pool_t pool, const unsigned char *data, size_t size, (void)parse_address_list(&ctx, max_addresses); } rfc822_parser_deinit(&ctx.parser); - return ctx.first_addr; + *list_r = ctx.addr_list; } static int @@ -481,7 +483,7 @@ message_address_parse_path_real(pool_t pool, const unsigned char *data, ret = parse_path(&ctx); rfc822_parser_deinit(&ctx.parser); - *addr_r = ctx.first_addr; + *addr_r = ctx.addr_list.head; return (ret < 0 ? -1 : 0); } @@ -490,17 +492,24 @@ message_address_parse(pool_t pool, const unsigned char *data, size_t size, unsigned int max_addresses, enum message_address_parse_flags flags) { - struct message_address *addr; + struct message_address_list list; + message_address_parse_full(pool, data, size, max_addresses, flags, + &list); + return list.head; +} +void message_address_parse_full(pool_t pool, const unsigned char *data, + size_t size, unsigned int max_addresses, + enum message_address_parse_flags flags, + struct message_address_list *list_r) +{ if (pool->datastack_pool) { - return message_address_parse_real(pool, data, size, - max_addresses, flags); - } - T_BEGIN { - addr = message_address_parse_real(pool, data, size, - max_addresses, flags); + message_address_parse_real(pool, data, size, + max_addresses, flags, list_r); + } else T_BEGIN { + message_address_parse_real(pool, data, size, + max_addresses, flags, list_r); } T_END; - return addr; } int message_address_parse_path(pool_t pool, const unsigned char *data, diff --git a/src/lib-mail/message-address.h b/src/lib-mail/message-address.h index 85cff3dcc6f..224f7a75605 100644 --- a/src/lib-mail/message-address.h +++ b/src/lib-mail/message-address.h @@ -31,12 +31,22 @@ struct message_address { bool invalid_syntax; }; +struct message_address_list { + struct message_address *head, *tail; +}; + /* Parse message addresses from given data. Note that giving an empty string will return NULL since there are no addresses. */ struct message_address * message_address_parse(pool_t pool, const unsigned char *data, size_t size, unsigned int max_addresses, enum message_address_parse_flags flags); +/* Same as message_address_parse(), but return message_address_list containing + both the first and the last address in the linked list. */ +void message_address_parse_full(pool_t pool, const unsigned char *data, + size_t size, unsigned int max_addresses, + enum message_address_parse_flags flags, + struct message_address_list *list_r); /* Parse RFC 5322 "path" (Return-Path header) from given data. Returns -1 if the path is invalid and 0 otherwise. diff --git a/src/lib-mail/test-message-address.c b/src/lib-mail/test-message-address.c index 261cbfba70a..54aa9a83101 100644 --- a/src/lib-mail/test-message-address.c +++ b/src/lib-mail/test-message-address.c @@ -19,8 +19,9 @@ static bool cmp_addr(const struct message_address *a1, a1->invalid_syntax == a2->invalid_syntax; } -static const struct message_address * -test_parse_address(const char *input, bool fill_missing) +static void +test_parse_address_full(const char *input, bool fill_missing, + struct message_address_list *list_r) { const enum message_address_parse_flags flags = fill_missing ? MESSAGE_ADDRESS_PARSE_FLAG_FILL_MISSING : 0; @@ -28,11 +29,18 @@ test_parse_address(const char *input, bool fill_missing) if there's any out-of-bounds access */ size_t input_len = strlen(input); unsigned char *input_dup = i_memdup(input, input_len); - const struct message_address *addr = - message_address_parse(pool_datastack_create(), - input_dup, input_len, UINT_MAX, flags); + message_address_parse_full(pool_datastack_create(), + input_dup, input_len, UINT_MAX, flags, + list_r); i_free(input_dup); - return addr; +} + +static const struct message_address * +test_parse_address(const char *input, bool fill_missing) +{ + struct message_address_list list; + test_parse_address_full(input, fill_missing, &list); + return list.head; } static void test_message_address(void) @@ -322,6 +330,33 @@ static void test_message_address(void) test_end(); } +static void test_message_address_list(void) +{ + test_begin("message address list"); + + const char *test_input = + "user1@example1.com, user2@example2.com, user3@example3.com"; + const struct message_address wanted_addrs[] = { + { NULL, NULL, NULL, NULL, "user1", "example1.com", FALSE }, + { NULL, NULL, NULL, NULL, "user2", "example2.com", FALSE }, + { NULL, NULL, NULL, NULL, "user3", "example3.com", FALSE }, + }; + + struct message_address_list list; + struct message_address *addr, *scanned_last_addr; + test_parse_address_full(test_input, FALSE, &list); + addr = list.head; + for (unsigned int i = 0; i < N_ELEMENTS(wanted_addrs); i++) { + test_assert_idx(cmp_addr(addr, &wanted_addrs[i]), i); + scanned_last_addr = addr; + addr = addr->next; + } + test_assert(list.tail == scanned_last_addr); + test_assert(addr == NULL); + + test_end(); +} + static void test_message_address_nuls(void) { const unsigned char input[] = @@ -521,6 +556,7 @@ int main(void) { static void (*const test_functions[])(void) = { test_message_address, + test_message_address_list, test_message_address_nuls, test_message_address_nuls_display_name, test_message_address_non_strict_dots, From 1481c04f02df7647f520df65d63df7626bf0ee32 Mon Sep 17 00:00:00 2001 From: Timo Sirainen Date: Fri, 9 Feb 2024 00:57:12 +0200 Subject: [PATCH 6/6] lib-mail, lib-imap: Optimize parsing large number of address headers Every header was appended to a linked list by walking through the whole list, causing excessive CPU usage when the list became large enough. Fixed by changing struct message_part_envelope to use struct message_address_list, which stores also linked list tail pointers. This allows quickly appending to the end of the linked list. --- src/lib-imap/imap-envelope.c | 27 ++++++++++------------- src/lib-mail/message-part-data.c | 17 +++++++------- src/lib-mail/message-part-data.h | 6 +++-- src/lib-storage/index/index-search-mime.c | 4 ++-- 4 files changed, 27 insertions(+), 27 deletions(-) diff --git a/src/lib-imap/imap-envelope.c b/src/lib-imap/imap-envelope.c index 1312eae2ff3..da3177025a5 100644 --- a/src/lib-imap/imap-envelope.c +++ b/src/lib-imap/imap-envelope.c @@ -67,17 +67,17 @@ void imap_envelope_write(struct message_part_envelope *data, } str_append_c(str, ' '); - imap_write_address(str, data->from); + imap_write_address(str, data->from.head); str_append_c(str, ' '); - imap_write_address(str, NVL(data->sender, data->from)); + imap_write_address(str, NVL(data->sender.head, data->from.head)); str_append_c(str, ' '); - imap_write_address(str, NVL(data->reply_to, data->from)); + imap_write_address(str, NVL(data->reply_to.head, data->from.head)); str_append_c(str, ' '); - imap_write_address(str, data->to); + imap_write_address(str, data->to.head); str_append_c(str, ' '); - imap_write_address(str, data->cc); + imap_write_address(str, data->cc.head); str_append_c(str, ' '); - imap_write_address(str, data->bcc); + imap_write_address(str, data->bcc.head); str_append_c(str, ' '); imap_append_nstring_nolf(str, data->in_reply_to); @@ -126,28 +126,25 @@ imap_envelope_parse_address(const struct imap_arg *arg, static bool imap_envelope_parse_addresses(const struct imap_arg *arg, - pool_t pool, struct message_address **addrs_r) + pool_t pool, struct message_address_list *addrs_r) { - struct message_address *first, *last, *addr; + struct message_address *addr; const struct imap_arg *list_args; - if (arg->type == IMAP_ARG_NIL) { - *addrs_r = NULL; + i_zero(addrs_r); + if (arg->type == IMAP_ARG_NIL) return TRUE; - } if (!imap_arg_get_list(arg, &list_args)) return FALSE; - first = last = addr = NULL; + addr = NULL; for (; !IMAP_ARG_IS_EOL(list_args); list_args++) { if (!imap_envelope_parse_address (list_args, pool, &addr)) return FALSE; - DLLIST2_APPEND(&first, &last, addr); + DLLIST2_APPEND(&addrs_r->head, &addrs_r->tail, addr); } - - *addrs_r = first; return TRUE; } diff --git a/src/lib-mail/message-part-data.c b/src/lib-mail/message-part-data.c index a5771f87e2e..25019ab432d 100644 --- a/src/lib-mail/message-part-data.c +++ b/src/lib-mail/message-part-data.c @@ -4,6 +4,7 @@ #include "str.h" #include "wildcard-match.h" #include "array.h" +#include "llist.h" #include "rfc822-parser.h" #include "rfc2231-parser.h" #include "message-address.h" @@ -176,7 +177,7 @@ void message_part_envelope_parse_from_header(pool_t pool, { struct message_part_envelope *d; enum envelope_field field; - struct message_address **addr_p, *addr; + struct message_address_list *addr_p, new_addr; const char **str_p; if (*data == NULL) { @@ -234,18 +235,18 @@ void message_part_envelope_parse_from_header(pool_t pool, } if (addr_p != NULL) { - addr = message_address_parse(pool, hdr->full_value, - hdr->full_value_len, - UINT_MAX, - MESSAGE_ADDRESS_PARSE_FLAG_FILL_MISSING); + message_address_parse_full(pool, hdr->full_value, + hdr->full_value_len, + UINT_MAX, + MESSAGE_ADDRESS_PARSE_FLAG_FILL_MISSING, + &new_addr); /* Merge multiple headers the same as if they were comma separated in a single line. This is better from security point of view, because attacker could intentionally write addresses in a way that e.g. the first From header is validated while MUA only shows the second From header. */ - while (*addr_p != NULL) - addr_p = &(*addr_p)->next; - *addr_p = addr; + DLLIST2_JOIN(&addr_p->head, &addr_p->tail, + &new_addr.head, &new_addr.tail); } else if (str_p != NULL) { *str_p = message_header_strdup(pool, hdr->full_value, hdr->full_value_len); diff --git a/src/lib-mail/message-part-data.h b/src/lib-mail/message-part-data.h index 5ff9ffe1bc6..7ec878de68e 100644 --- a/src/lib-mail/message-part-data.h +++ b/src/lib-mail/message-part-data.h @@ -2,6 +2,7 @@ #define MESSAGE_PART_DATA_H #include "message-part.h" +#include "message-address.h" #define MESSAGE_PART_DEFAULT_CHARSET "us-ascii" @@ -14,8 +15,9 @@ struct message_part_param { struct message_part_envelope { const char *date, *subject; - struct message_address *from, *sender, *reply_to; - struct message_address *to, *cc, *bcc; + + struct message_address_list from, sender, reply_to; + struct message_address_list to, cc, bcc; const char *in_reply_to, *message_id; }; diff --git a/src/lib-storage/index/index-search-mime.c b/src/lib-storage/index/index-search-mime.c index da7e5e17092..3328ce98af1 100644 --- a/src/lib-storage/index/index-search-mime.c +++ b/src/lib-storage/index/index-search-mime.c @@ -205,7 +205,7 @@ seach_arg_mime_envelope_address_match( enum mail_search_mime_arg_type type, const char *key, const struct message_part_envelope *envelope) { - const struct message_address *addrs; + struct message_address_list addrs; string_t *addrs_enc; if (envelope == NULL) @@ -239,7 +239,7 @@ seach_arg_mime_envelope_address_match( probably be normalized directly in the struct message_address. */ addrs_enc = t_str_new(128); - message_address_write(addrs_enc, addrs); + message_address_write(addrs_enc, addrs.head); return (strstr(str_c(addrs_enc), key) != NULL ? 1 : 0); }