Skip to content

Commit

Permalink
lib-mail: message_address_parse() - Fix fill_missing==TRUE handling
Browse files Browse the repository at this point in the history 
Mainly MISSING_DOMAIN wasn't set in all situations. Also added unit tests.
  • Loading branch information
@sirainen
sirainen committed Jun 12, 2017
1 parent e51455e commit a6a68d9
Show file tree
Hide file tree
Showing 2 changed files with 133 additions and 76 deletions.
2 changes: 1 addition & 1 deletion src/lib-mail/message-address.c
View file
Expand Up @@ -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;
}
Expand Down
207 changes: 132 additions & 75 deletions src/lib-mail/test-message-address.c
View file
Expand Up @@ -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 */
Expand All @@ -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;
}
Expand All @@ -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
Expand All @@ -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 */
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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));
Expand Down

0 comments on commit a6a68d9

Please sign in to comment.