From a6a68d990ce9dda5b26c9124c8e73e566f924cc0 Mon Sep 17 00:00:00 2001 From: Timo Sirainen Date: Wed, 7 Jun 2017 18:10:10 +0300 Subject: [PATCH] lib-mail: message_address_parse() - Fix fill_missing==TRUE handling Mainly MISSING_DOMAIN wasn't set in all situations. Also added unit tests. --- src/lib-mail/message-address.c | 2 +- src/lib-mail/test-message-address.c | 207 ++++++++++++++++++---------- 2 files changed, 133 insertions(+), 76 deletions(-) diff --git a/src/lib-mail/message-address.c b/src/lib-mail/message-address.c index c24564665c..980004728e 100644 --- a/src/lib-mail/message-address.c +++ b/src/lib-mail/message-address.c @@ -256,7 +256,7 @@ static void add_fixed_address(struct message_address_parser_context *ctx) ctx->addr.mailbox = !ctx->fill_missing ? "" : "MISSING_MAILBOX"; ctx->addr.invalid_syntax = TRUE; } - if (ctx->addr.domain == NULL) { + if (ctx->addr.domain == NULL || ctx->addr.domain[0] == '\0') { ctx->addr.domain = !ctx->fill_missing ? "" : "MISSING_DOMAIN"; ctx->addr.invalid_syntax = TRUE; } diff --git a/src/lib-mail/test-message-address.c b/src/lib-mail/test-message-address.c index cfc5ea0942..ebc84a0379 100644 --- a/src/lib-mail/test-message-address.c +++ b/src/lib-mail/test-message-address.c @@ -15,7 +15,8 @@ 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) +static const struct message_address * +test_parse_address(const char *input, bool fill_missing) { /* duplicate the input (without trailing NUL) so valgrind notices if there's any out-of-bounds access */ @@ -24,7 +25,7 @@ static const struct message_address *test_parse_address(const char *input) memcpy(input_dup, input, input_len); const struct message_address *addr = message_address_parse(pool_datastack_create(), - input_dup, input_len, UINT_MAX, FALSE); + input_dup, input_len, UINT_MAX, fill_missing); i_free(input_dup); return addr; } @@ -34,116 +35,164 @@ static void test_message_address(void) static const struct test { const char *input; const char *wanted_output; + const char *wanted_filled_output; struct message_address addr; + struct message_address filled_addr; } tests[] = { /* user@domain -> */ - { "user@domain", "", + { "user@domain", "", NULL, + { NULL, NULL, NULL, "user", "domain", FALSE }, { NULL, NULL, NULL, "user", "domain", FALSE } }, - { "\"user\"@domain", "", + { "\"user\"@domain", "", NULL, + { NULL, NULL, NULL, "user", "domain", FALSE }, { NULL, NULL, NULL, "user", "domain", FALSE } }, - { "\"user name\"@domain", "<\"user name\"@domain>", + { "\"user name\"@domain", "<\"user name\"@domain>", NULL, + { NULL, NULL, NULL, "user name", "domain", FALSE }, { NULL, NULL, NULL, "user name", "domain", FALSE } }, - { "\"user@na\\\\me\"@domain", "<\"user@na\\\\me\"@domain>", + { "\"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 } }, - { "\"user\\\"name\"@domain", "<\"user\\\"name\"@domain>", + { "\"user\\\"name\"@domain", "<\"user\\\"name\"@domain>", NULL, + { NULL, NULL, NULL, "user\"name", "domain", FALSE }, { NULL, NULL, NULL, "user\"name", "domain", FALSE } }, - { "\"\"@domain", "<\"\"@domain>", + { "\"\"@domain", "<\"\"@domain>", NULL, + { NULL, NULL, NULL, "", "domain", FALSE }, { NULL, NULL, NULL, "", "domain", FALSE } }, - { "user", "", - { NULL, NULL, NULL, "user", "", TRUE } }, - { "@domain", "<\"\"@domain>", - { NULL, NULL, NULL, "", "domain", TRUE } }, + { "user", "", "", + { NULL, NULL, NULL, "user", "", TRUE }, + { NULL, NULL, NULL, "user", "MISSING_DOMAIN", TRUE } }, + { "@domain", "<\"\"@domain>", "", + { NULL, NULL, NULL, "", "domain", TRUE }, + { NULL, NULL, NULL, "MISSING_MAILBOX", "domain", TRUE } }, /* Display Name -> Display Name */ - { "Display Name", "\"Display Name\"", - { NULL, "Display Name", NULL, "", "", TRUE } }, - { "\"Display Name\"", "\"Display Name\"", - { NULL, "Display Name", NULL, "", "", TRUE } }, - { "Display \"Name\"", "\"Display Name\"", - { NULL, "Display Name", NULL, "", "", TRUE } }, - { "\"Display\" \"Name\"", "\"Display Name\"", - { NULL, "Display Name", NULL, "", "", TRUE } }, - { "\"\"", "", - { NULL, "", NULL, "", "", TRUE } }, + { "Display Name", "\"Display Name\"", "\"Display Name\" ", + { NULL, "Display Name", NULL, "", "", TRUE }, + { NULL, "Display Name", NULL, "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE } }, + { "\"Display Name\"", "\"Display Name\"", "\"Display Name\" ", + { NULL, "Display Name", NULL, "", "", TRUE }, + { NULL, "Display Name", NULL, "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE } }, + { "Display \"Name\"", "\"Display Name\"", "\"Display Name\" ", + { NULL, "Display Name", NULL, "", "", TRUE }, + { NULL, "Display Name", NULL, "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE } }, + { "\"Display\" \"Name\"", "\"Display Name\"", "\"Display Name\" ", + { NULL, "Display Name", NULL, "", "", TRUE }, + { NULL, "Display Name", NULL, "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE } }, + { "\"\"", "", "", + { NULL, "", NULL, "", "", TRUE }, + { NULL, "", NULL, "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE } }, /* -> */ - { "", NULL, + { "", NULL, NULL, + { NULL, NULL, NULL, "user", "domain", FALSE }, { NULL, NULL, NULL, "user", "domain", FALSE } }, - { "<\"user\"@domain>", "", + { "<\"user\"@domain>", "", NULL, + { NULL, NULL, NULL, "user", "domain", FALSE }, { NULL, NULL, NULL, "user", "domain", FALSE } }, - { "<\"user name\"@domain>", NULL, + { "<\"user name\"@domain>", NULL, NULL, + { NULL, NULL, NULL, "user name", "domain", FALSE }, { NULL, NULL, NULL, "user name", "domain", FALSE } }, - { "<\"user@na\\\\me\"@domain>", NULL, + { "<\"user@na\\\\me\"@domain>", NULL, NULL, + { NULL, NULL, NULL, "user@na\\me", "domain", FALSE }, { NULL, NULL, NULL, "user@na\\me", "domain", FALSE } }, - { "<\"user\\\"name\"@domain>", NULL, + { "<\"user\\\"name\"@domain>", NULL, NULL, + { NULL, NULL, NULL, "user\"name", "domain", FALSE }, { NULL, NULL, NULL, "user\"name", "domain", FALSE } }, - { "<\"\"@domain>", NULL, + { "<\"\"@domain>", NULL, NULL, + { NULL, NULL, NULL, "", "domain", FALSE }, { NULL, NULL, NULL, "", "domain", FALSE } }, - { "", NULL, - { NULL, NULL, NULL, "user", "", TRUE } }, - { "<@route>", "<@route:\"\">", - { NULL, NULL, "@route", "", "", TRUE } }, + { "", NULL, "", + { NULL, NULL, NULL, "user", "", TRUE }, + { NULL, NULL, NULL, "user", "MISSING_DOMAIN", TRUE } }, + { "<@route>", "<@route:\"\">", "", + { NULL, NULL, "@route", "", "", TRUE }, + { NULL, NULL, "INVALID_ROUTE", "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE } }, /* user@domain (Display Name) -> "Display Name" */ - { "user@domain (DisplayName)", "DisplayName ", + { "user@domain (DisplayName)", "DisplayName ", NULL, + { NULL, "DisplayName", NULL, "user", "domain", FALSE }, { NULL, "DisplayName", NULL, "user", "domain", FALSE } }, - { "user@domain (Display Name)", "\"Display Name\" ", + { "user@domain (Display Name)", "\"Display Name\" ", NULL, + { NULL, "Display Name", NULL, "user", "domain", FALSE }, { NULL, "Display Name", NULL, "user", "domain", FALSE } }, - { "user@domain (Display\"Name)", "\"Display\\\"Name\" ", + { "user@domain (Display\"Name)", "\"Display\\\"Name\" ", NULL, + { NULL, "Display\"Name", NULL, "user", "domain", FALSE }, { NULL, "Display\"Name", NULL, "user", "domain", FALSE } }, - { "user (Display Name)", "\"Display Name\" ", - { NULL, "Display Name", NULL, "user", "", TRUE } }, - { "@domain (Display Name)", "\"Display Name\" <\"\"@domain>", - { NULL, "Display Name", NULL, "", "domain", TRUE } }, - { "user@domain ()", "", + { "user (Display Name)", "\"Display Name\" ", "\"Display Name\" ", + { NULL, "Display Name", NULL, "user", "", TRUE }, + { NULL, "Display Name", NULL, "user", "MISSING_DOMAIN", TRUE } }, + { "@domain (Display Name)", "\"Display Name\" <\"\"@domain>", "\"Display Name\" ", + { NULL, "Display Name", NULL, "", "domain", TRUE }, + { NULL, "Display Name", NULL, "MISSING_MAILBOX", "domain", TRUE } }, + { "user@domain ()", "", NULL, + { NULL, NULL, NULL, "user", "domain", FALSE }, { NULL, NULL, NULL, "user", "domain", FALSE } }, /* Display Name -> "Display Name" */ - { "DisplayName ", NULL, + { "DisplayName ", NULL, NULL, + { NULL, "DisplayName", NULL, "user", "domain", FALSE }, { NULL, "DisplayName", NULL, "user", "domain", FALSE } }, - { "Display Name ", "\"Display Name\" ", + { "Display Name ", "\"Display Name\" ", NULL, + { NULL, "Display Name", NULL, "user", "domain", FALSE }, { NULL, "Display Name", NULL, "user", "domain", FALSE } }, - { "\"Display Name\" ", NULL, + { "\"Display Name\" ", NULL, NULL, + { NULL, "Display Name", NULL, "user", "domain", FALSE }, { NULL, "Display Name", NULL, "user", "domain", FALSE } }, - { "\"Display\\\"Name\" ", NULL, + { "\"Display\\\"Name\" ", NULL, NULL, + { NULL, "Display\"Name", NULL, "user", "domain", FALSE }, { NULL, "Display\"Name", NULL, "user", "domain", FALSE } }, - { "Display Name ", "\"Display Name\" ", - { NULL, "Display Name", NULL, "user", "", TRUE } }, - { "\"\" ", "", + { "Display Name ", "\"Display Name\" ", "\"Display Name\" ", + { NULL, "Display Name", NULL, "user", "", TRUE }, + { NULL, "Display Name", NULL, "user", "MISSING_DOMAIN", TRUE } }, + { "\"\" ", "", NULL, + { NULL, NULL, NULL, "user", "domain", FALSE }, { NULL, NULL, NULL, "user", "domain", FALSE } }, /* <@route:user@domain> -> <@route:user@domain> */ - { "<@route:user@domain>", NULL, + { "<@route:user@domain>", NULL, NULL, + { NULL, NULL, "@route", "user", "domain", FALSE }, { NULL, NULL, "@route", "user", "domain", FALSE } }, - { "<@route,@route2:user@domain>", NULL, + { "<@route,@route2:user@domain>", NULL, NULL, + { NULL, NULL, "@route,@route2", "user", "domain", FALSE }, { NULL, NULL, "@route,@route2", "user", "domain", FALSE } }, - { "<@route@route2:user@domain>", "<@route,@route2:user@domain>", + { "<@route@route2:user@domain>", "<@route,@route2:user@domain>", NULL, + { NULL, NULL, "@route,@route2", "user", "domain", FALSE }, { NULL, NULL, "@route,@route2", "user", "domain", FALSE } }, - { "<@route@route2:user>", "<@route,@route2:user>", - { NULL, NULL, "@route,@route2", "user", "", TRUE } }, - { "<@route@route2:\"\"@domain>", "<@route,@route2:\"\"@domain>", + { "<@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 } }, + { "<@route@route2:\"\"@domain>", "<@route,@route2:\"\"@domain>", NULL, + { NULL, NULL, "@route,@route2", "", "domain", FALSE }, { NULL, NULL, "@route,@route2", "", "domain", FALSE } }, /* Display Name <@route:user@domain> -> "Display Name" <@route:user@domain> */ - { "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 } }, - { "Display Name <@route,@route2:user@domain>", "\"Display Name\" <@route,@route2:user@domain>", + { "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 } }, - { "Display Name <@route@route2:user@domain>", "\"Display Name\" <@route,@route2:user@domain>", + { "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 } }, - { "Display Name <@route@route2:user>", "\"Display Name\" <@route,@route2:user>", - { NULL, "Display Name", "@route,@route2", "user", "", TRUE } }, - { "Display Name <@route@route2:\"\"@domain>", "\"Display Name\" <@route,@route2:\"\"@domain>", + { "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 } }, + { "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 } }, /* other tests: */ - { "\"foo: ;,\" ", NULL, + { "\"foo: ;,\" ", NULL, NULL, + { NULL, "foo: ;,", NULL, "user", "domain", FALSE }, { NULL, "foo: ;,", NULL, "user", "domain", FALSE } }, - { "<>", "", - { NULL, NULL, NULL, "", "", TRUE } }, - { "<@>", "", - { NULL, NULL, NULL, "", "", TRUE } }, + { "<>", "", "", + { NULL, NULL, NULL, "", "", TRUE }, + { NULL, NULL, NULL, "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE } }, + { "<@>", "", "", + { NULL, NULL, NULL, "", "", TRUE }, + { NULL, NULL, "INVALID_ROUTE", "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE } }, }; static struct message_address group_prefix = { NULL, NULL, NULL, "group", NULL, FALSE @@ -160,18 +209,26 @@ static void test_message_address(void) str = t_str_new(128); group = t_str_new(256); - for (i = 0; i < N_ELEMENTS(tests); i++) { - const struct test *test = &tests[i]; + for (i = 0; i < N_ELEMENTS(tests)*2; i++) { + const struct test *test = &tests[i/2]; + const struct message_address *test_wanted_addr; + bool fill_missing = i%2 != 0; - addr = test_parse_address(test->input); + test_wanted_addr = !fill_missing ? + &test->addr : &test->filled_addr; + addr = test_parse_address(test->input, fill_missing); test_assert_idx(addr != NULL && addr->next == NULL && - cmp_addr(addr, &test->addr), i); + cmp_addr(addr, test_wanted_addr), i); /* test the address alone */ str_truncate(str, 0); message_address_write(str, addr); - wanted_string = test->wanted_output != NULL ? - test->wanted_output : test->input; + if (fill_missing && test->wanted_filled_output != NULL) + wanted_string = test->wanted_filled_output; + else if (test->wanted_output != NULL) + wanted_string = test->wanted_output; + else + wanted_string = test->input; test_assert_idx(strcmp(str_c(str), wanted_string) == 0, i); /* test the address as a list of itself */ @@ -186,10 +243,10 @@ static void test_message_address(void) str_append(group, test->input); } - addr = test_parse_address(str_c(group)); + addr = test_parse_address(str_c(group), fill_missing); for (unsigned int j = 0; j < list_length; j++) { test_assert_idx(addr != NULL && - cmp_addr(addr, &test->addr), i); + cmp_addr(addr, test_wanted_addr), i); if (addr != NULL) addr = addr->next; } @@ -209,12 +266,12 @@ static void test_message_address(void) } str_append_c(group, ';'); - addr = test_parse_address(str_c(group)); + addr = test_parse_address(str_c(group), fill_missing); test_assert(addr != NULL && cmp_addr(addr, &group_prefix)); addr = addr->next; for (unsigned int j = 0; j < list_length; j++) { test_assert_idx(addr != NULL && - cmp_addr(addr, &test->addr), i); + cmp_addr(addr, test_wanted_addr), i); if (addr != NULL) addr = addr->next; } @@ -227,7 +284,7 @@ static void test_message_address(void) test_begin("message address parsing with empty group"); str_truncate(group, 0); str_append(group, "group:;"); - addr = test_parse_address(str_c(group)); + addr = test_parse_address(str_c(group), FALSE); str_truncate(str, 0); message_address_write(str, addr); test_assert(addr != NULL && cmp_addr(addr, &group_prefix));