lib-mail: message_address_parse() - Fix fill_missing==TRUE handling

Mainly MISSING_DOMAIN wasn't set in all situations. Also added unit tests.

diff --git a/src/lib-mail/message-address.c b/src/lib-mail/message-address.c
index c24564665c4d9ee0846a5536289ac49ffc4c584b..980004728e78df04f147f83cc5a07db7da7e4185 100644 (file)
--- 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 cfc5ea0942970e1633c179197ff7b2cd81b5901a..ebc84a0379115a43368fd2ad43838c4913bc3e7a 100644 (file)
--- 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", "<user@domain>",
+               { "user@domain", "<user@domain>", NULL,
+                 { NULL, NULL, NULL, "user", "domain", FALSE },
                  { NULL, NULL, NULL, "user", "domain", FALSE } },
-               { "\"user\"@domain", "<user@domain>",
+               { "\"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", "<user>",
-                 { NULL, NULL, NULL, "user", "", TRUE } },
-               { "@domain", "<\"\"@domain>",
-                 { NULL, NULL, NULL, "", "domain", TRUE } },
+               { "user", "<user>", "<user@MISSING_DOMAIN>",
+                 { NULL, NULL, NULL, "user", "", TRUE },
+                 { NULL, NULL, NULL, "user", "MISSING_DOMAIN", TRUE } },
+               { "@domain", "<\"\"@domain>", "<MISSING_MAILBOX@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\" <MISSING_MAILBOX@MISSING_DOMAIN>",
+                 { NULL, "Display Name", NULL, "", "", TRUE },
+                 { NULL, "Display Name", NULL, "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE } },
+               { "\"Display Name\"", "\"Display Name\"", "\"Display Name\" <MISSING_MAILBOX@MISSING_DOMAIN>",
+                 { NULL, "Display Name", NULL, "", "", TRUE },
+                 { NULL, "Display Name", NULL, "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE } },
+               { "Display \"Name\"", "\"Display Name\"", "\"Display Name\" <MISSING_MAILBOX@MISSING_DOMAIN>",
+                 { NULL, "Display Name", NULL, "", "", TRUE },
+                 { NULL, "Display Name", NULL, "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE } },
+               { "\"Display\" \"Name\"", "\"Display Name\"", "\"Display Name\" <MISSING_MAILBOX@MISSING_DOMAIN>",
+                 { NULL, "Display Name", NULL, "", "", TRUE },
+                 { NULL, "Display Name", NULL, "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE } },
+               { "\"\"", "", "<MISSING_MAILBOX@MISSING_DOMAIN>",
+                 { NULL, "", NULL, "", "", TRUE },
+                 { NULL, "", NULL, "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE } },
 
                /* <user@domain> -> <user@domain> */
-               { "<user@domain>", NULL,
+               { "<user@domain>", NULL, NULL,
+                 { NULL, NULL, NULL, "user", "domain", FALSE },
                  { NULL, NULL, NULL, "user", "domain", FALSE } },
-               { "<\"user\"@domain>", "<user@domain>",
+               { "<\"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 } },
-               { "<user>", NULL,
-                 { NULL, NULL, NULL, "user", "", TRUE } },
-               { "<@route>", "<@route:\"\">",
-                 { NULL, NULL, "@route", "", "", TRUE } },
+               { "<user>", NULL, "<user@MISSING_DOMAIN>",
+                 { NULL, NULL, NULL, "user", "", TRUE },
+                 { NULL, NULL, NULL, "user", "MISSING_DOMAIN", TRUE } },
+               { "<@route>", "<@route:\"\">", "<INVALID_ROUTE:MISSING_MAILBOX@MISSING_DOMAIN>",
+                 { NULL, NULL, "@route", "", "", TRUE },
+                 { NULL, NULL, "INVALID_ROUTE", "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE } },
 
                /* user@domain (Display Name) -> "Display Name" <user@domain> */
-               { "user@domain (DisplayName)", "DisplayName <user@domain>",
+               { "user@domain (DisplayName)", "DisplayName <user@domain>", NULL,
+                 { NULL, "DisplayName", NULL, "user", "domain", FALSE },
                  { NULL, "DisplayName", NULL, "user", "domain", FALSE } },
-               { "user@domain (Display Name)", "\"Display Name\" <user@domain>",
+               { "user@domain (Display Name)", "\"Display Name\" <user@domain>", NULL,
+                 { NULL, "Display Name", NULL, "user", "domain", FALSE },
                  { NULL, "Display Name", NULL, "user", "domain", FALSE } },
-               { "user@domain (Display\"Name)", "\"Display\\\"Name\" <user@domain>",
+               { "user@domain (Display\"Name)", "\"Display\\\"Name\" <user@domain>", NULL,
+                 { NULL, "Display\"Name", NULL, "user", "domain", FALSE },
                  { NULL, "Display\"Name", NULL, "user", "domain", FALSE } },
-               { "user (Display Name)", "\"Display Name\" <user>",
-                 { NULL, "Display Name", NULL, "user", "", TRUE } },
-               { "@domain (Display Name)", "\"Display Name\" <\"\"@domain>",
-                 { NULL, "Display Name", NULL, "", "domain", TRUE } },
-               { "user@domain ()", "<user@domain>",
+               { "user (Display Name)", "\"Display Name\" <user>", "\"Display Name\" <user@MISSING_DOMAIN>",
+                 { NULL, "Display Name", NULL, "user", "", TRUE },
+                 { NULL, "Display Name", NULL, "user", "MISSING_DOMAIN", TRUE } },
+               { "@domain (Display Name)", "\"Display Name\" <\"\"@domain>", "\"Display Name\" <MISSING_MAILBOX@domain>",
+                 { NULL, "Display Name", NULL, "", "domain", TRUE },
+                 { NULL, "Display Name", NULL, "MISSING_MAILBOX", "domain", TRUE } },
+               { "user@domain ()", "<user@domain>", NULL,
+                 { NULL, NULL, NULL, "user", "domain", FALSE },
                  { NULL, NULL, NULL, "user", "domain", FALSE } },
 
                /* Display Name <user@domain> -> "Display Name" <user@domain> */
-               { "DisplayName <user@domain>", NULL,
+               { "DisplayName <user@domain>", NULL, NULL,
+                 { NULL, "DisplayName", NULL, "user", "domain", FALSE },
                  { NULL, "DisplayName", NULL, "user", "domain", FALSE } },
-               { "Display Name <user@domain>", "\"Display Name\" <user@domain>",
+               { "Display Name <user@domain>", "\"Display Name\" <user@domain>", NULL,
+                 { NULL, "Display Name", NULL, "user", "domain", FALSE },
                  { NULL, "Display Name", NULL, "user", "domain", FALSE } },
-               { "\"Display Name\" <user@domain>", NULL,
+               { "\"Display Name\" <user@domain>", NULL, NULL,
+                 { NULL, "Display Name", NULL, "user", "domain", FALSE },
                  { NULL, "Display Name", NULL, "user", "domain", FALSE } },
-               { "\"Display\\\"Name\" <user@domain>", NULL,
+               { "\"Display\\\"Name\" <user@domain>", NULL, NULL,
+                 { NULL, "Display\"Name", NULL, "user", "domain", FALSE },
                  { NULL, "Display\"Name", NULL, "user", "domain", FALSE } },
-               { "Display Name <user>", "\"Display Name\" <user>",
-                 { NULL, "Display Name", NULL, "user", "", TRUE } },
-               { "\"\" <user@domain>", "<user@domain>",
+               { "Display Name <user>", "\"Display Name\" <user>", "\"Display Name\" <user@MISSING_DOMAIN>",
+                 { NULL, "Display Name", NULL, "user", "", TRUE },
+                 { NULL, "Display Name", NULL, "user", "MISSING_DOMAIN", TRUE } },
+               { "\"\" <user@domain>", "<user@domain>", 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: <a@b>;,\" <user@domain>", NULL,
+               { "\"foo: <a@b>;,\" <user@domain>", NULL, NULL,
+                 { NULL, "foo: <a@b>;,", NULL, "user", "domain", FALSE },
                  { NULL, "foo: <a@b>;,", NULL, "user", "domain", FALSE } },
-               { "<>", "",
-                 { NULL, NULL, NULL, "", "", TRUE } },
-               { "<@>", "",
-                 { NULL, NULL, NULL, "", "", TRUE } },
+               { "<>", "", "<MISSING_MAILBOX@MISSING_DOMAIN>",
+                 { NULL, NULL, NULL, "", "", TRUE },
+                 { NULL, NULL, NULL, "MISSING_MAILBOX", "MISSING_DOMAIN", TRUE } },
+               { "<@>", "", "<INVALID_ROUTE:MISSING_MAILBOX@MISSING_DOMAIN>",
+                 { 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));