Mercurial > dovecot > core-2.2

changeset 22176:fc301d74f851

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
lib-mail: message_address_parse() - Fix fill_missing==TRUE handling Mainly MISSING_DOMAIN wasn't set in all situations. Also added unit tests.
author Timo Sirainen <timo.sirainen@dovecot.fi>
date Wed, 07 Jun 2017 18:10:10 +0300
parents 755d206606df
children 3001873cd962
files src/lib-mail/message-address.c src/lib-mail/test-message-address.c
diffstat 2 files changed, 133 insertions(+), 76 deletions(-) [+]
line wrap: on
line diff
--- a/src/lib-mail/message-address.c	Wed Jun 07 15:33:42 2017 +0300
+++ b/src/lib-mail/message-address.c	Wed Jun 07 18:10:10 2017 +0300
@@ -256,7 +256,7 @@
 		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;
 	}
--- a/src/lib-mail/test-message-address.c	Wed Jun 07 15:33:42 2017 +0300
+++ b/src/lib-mail/test-message-address.c	Wed Jun 07 18:10:10 2017 +0300
@@ -15,7 +15,8 @@
 		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 @@
 	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 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 @@
 	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 @@
 				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 @@
 			}
 			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 @@
 	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));