Mercurial > dovecot > original-hg > dovecot-1.2

changeset 8171:60b8c2609087 HEAD

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
message address parser: More error handling improvements.
author Timo Sirainen <tss@iki.fi>
date Sun, 07 Sep 2008 20:34:20 +0300
parents 3e21ec854acc
children c0a80d6b8ef6
files src/lib-mail/message-address.c src/tests/test-mail.c
diffstat 2 files changed, 61 insertions(+), 48 deletions(-) [+]
line wrap: on
line diff
--- a/src/lib-mail/message-address.c	Sun Sep 07 20:03:00 2008 +0300
+++ b/src/lib-mail/message-address.c	Sun Sep 07 20:34:20 2008 +0300
@@ -40,8 +40,7 @@
 	   local-part      = dot-atom / quoted-string / obs-local-part
 	   obs-local-part  = word *("." word)
 	*/
-	if (ctx->parser.data == ctx->parser.end)
-		return 0;
+	i_assert(ctx->parser.data != ctx->parser.end);
 
 	str_truncate(ctx->str, 0);
 	if (*ctx->parser.data == '"')
@@ -157,15 +156,15 @@
 static int parse_addr_spec(struct message_address_parser_context *ctx)
 {
 	/* addr-spec       = local-part "@" domain */
-	int ret;
+	int ret, ret2;
 
 	str_truncate(ctx->parser.last_comment, 0);
 
-	if ((ret = parse_local_part(ctx)) < 0)
-		return ret;
-	if (ret > 0 && *ctx->parser.data == '@') {
-		if ((ret = parse_domain(ctx)) < 0)
-			return ret;
+	ret = parse_local_part(ctx);
+	if (ret != 0 && *ctx->parser.data == '@') {
+		ret2 = parse_domain(ctx);
+		if (ret2 <= 0)
+			ret = ret2;
 	}
 
 	if (str_len(ctx->parser.last_comment) > 0) {
@@ -193,18 +192,16 @@
 	const unsigned char *start;
 	int ret;
 
-	if (ctx->parser.data == ctx->parser.end)
-		return 0;
-
 	/* mailbox         = name-addr / addr-spec */
 	start = ctx->parser.data;
 	if ((ret = parse_name_addr(ctx)) < 0) {
 		/* nope, should be addr-spec */
 		ctx->parser.data = start;
-		if ((ret = parse_addr_spec(ctx)) < 0)
-			return -1;
+		ret = parse_addr_spec(ctx);
 	}
 
+	if (ret < 0)
+		ctx->addr.invalid_syntax = TRUE;
 	add_fixed_address(ctx);
 	return ret;
 }
@@ -214,17 +211,20 @@
 	int ret;
 
 	/* mailbox-list    = (mailbox *("," mailbox)) / obs-mbox-list */
-	while ((ret = parse_mailbox(ctx)) > 0) {
+	while ((ret = parse_mailbox(ctx)) != 0) {
 		if (*ctx->parser.data != ',')
 			break;
 		ctx->parser.data++;
-		rfc822_skip_lwsp(&ctx->parser);
+		if ((ret = rfc822_skip_lwsp(&ctx->parser)) <= 0)
+			break;
 	}
 	return ret;
 }
 
 static int parse_group(struct message_address_parser_context *ctx)
 {
+	int ret;
+
 	/*
 	   group           = display-name ":" [mailbox-list / CFWS] ";" [CFWS]
 	   display-name    = phrase
@@ -237,20 +237,25 @@
 	/* from now on don't return -1 even if there are problems, so that
 	   the caller knows this is a group */
 	ctx->parser.data++;
-	(void)rfc822_skip_lwsp(&ctx->parser);
+	if (rfc822_skip_lwsp(&ctx->parser) < 0)
+		ctx->addr.invalid_syntax = TRUE;
 
 	ctx->addr.mailbox = p_strdup(ctx->pool, str_c(ctx->str));
 	add_address(ctx);
 
-	if (parse_mailbox_list(ctx) > 0) {
-		if (*ctx->parser.data == ';') {
+	if ((ret = parse_mailbox_list(ctx)) > 0) {
+		if (*ctx->parser.data != ';')
+			ret = -1;
+		else {
 			ctx->parser.data++;
-			(void)rfc822_skip_lwsp(&ctx->parser);
+			ret = rfc822_skip_lwsp(&ctx->parser);
 		}
 	}
+	if (ret < 0)
+		ctx->addr.invalid_syntax = TRUE;
 
 	add_address(ctx);
-	return 1;
+	return ret == 0 ? 0 : 1;
 }
 
 static int parse_address(struct message_address_parser_context *ctx)
@@ -265,7 +270,6 @@
 		ctx->parser.data = start;
 		ret = parse_mailbox(ctx);
 	}
-
 	return ret;
 }
 
@@ -276,15 +280,20 @@
 
 	/* address-list    = (address *("," address)) / obs-addr-list */
 	while (max_addresses-- > 0) {
-		if ((ret = parse_address(ctx)) <= 0)
+		if ((ret = parse_address(ctx)) == 0)
 			break;
 		if (*ctx->parser.data != ',') {
 			ret = -1;
 			break;
 		}
 		ctx->parser.data++;
-		if ((ret = rfc822_skip_lwsp(&ctx->parser)) <= 0)
+		if ((ret = rfc822_skip_lwsp(&ctx->parser)) <= 0) {
+			if (ret < 0) {
+				/* ends with some garbage */
+				add_fixed_address(ctx);
+			}
 			break;
+		}
 	}
 	return ret;
 }
@@ -308,14 +317,7 @@
 		/* no addresses */
 		return NULL;
 	}
-	if (ret < 0 || parse_address_list(&ctx, max_addresses) < 0) {
-		if (ctx.first_addr == NULL) {
-			/* we had some input - we'll have to return something
-			   so that we can set invalid_syntax */
-			add_fixed_address(&ctx);
-		}
-		ctx.first_addr->invalid_syntax = TRUE;
-	}
+	(void)parse_address_list(&ctx, max_addresses);
 	return ctx.first_addr;
 }
 
--- a/src/tests/test-mail.c	Sun Sep 07 20:03:00 2008 +0300
+++ b/src/tests/test-mail.c	Sun Sep 07 20:34:20 2008 +0300
@@ -50,7 +50,8 @@
 	return null_strcmp(a1->name, a2->name) == 0 &&
 		null_strcmp(a1->route, a2->route) == 0 &&
 		null_strcmp(a1->mailbox, a2->mailbox) == 0 &&
-		null_strcmp(a1->domain, a2->domain) == 0;
+		null_strcmp(a1->domain, a2->domain) == 0 &&
+		a1->invalid_syntax == a2->invalid_syntax;
 }
 
 static void test_message_address(void)
@@ -62,22 +63,28 @@
 		"\"foo bar\" <user@domain>",
 		"<@route:user@domain>",
 		"<@route@route2:user@domain>",
-		"hello <@route ,@route2:user@domain>"
+		"hello <@route ,@route2:user@domain>",
+		"user (hello)",
+		"hello <user>",
+		"@domain"
 	};
 	static struct message_address group_prefix = {
-		NULL, NULL, NULL, "group", NULL
+		NULL, NULL, NULL, "group", NULL, FALSE
 	};
 	static struct message_address group_suffix = {
-		NULL, NULL, NULL, NULL, NULL
+		NULL, NULL, NULL, NULL, NULL, FALSE
 	};
 	static struct message_address output[] = {
-		{ NULL, NULL, NULL, "user", "domain" },
-		{ NULL, NULL, NULL, "user", "domain" },
-		{ NULL, "foo bar", NULL, "user", "domain" },
-		{ NULL, "foo bar", NULL, "user", "domain" },
-		{ NULL, NULL, "@route", "user", "domain" },
-		{ NULL, NULL, "@route,@route2", "user", "domain" },
-		{ NULL, "hello", "@route,@route2", "user", "domain" }
+		{ NULL, NULL, NULL, "user", "domain", FALSE },
+		{ NULL, NULL, NULL, "user", "domain", FALSE },
+		{ NULL, "foo bar", NULL, "user", "domain", FALSE },
+		{ NULL, "foo bar", NULL, "user", "domain", FALSE },
+		{ NULL, NULL, "@route", "user", "domain", FALSE },
+		{ NULL, NULL, "@route,@route2", "user", "domain", FALSE },
+		{ NULL, "hello", "@route,@route2", "user", "domain", FALSE },
+		{ NULL, "hello", NULL, "user", "", TRUE },
+		{ NULL, "hello", NULL, "user", "", TRUE },
+		{ NULL, NULL, NULL, "", "domain", TRUE }
 	};
 	struct message_address *addr;
 	string_t *group;
@@ -96,13 +103,15 @@
 		test_out(t_strdup_printf("message_address_parse(%d)", i),
 			 success);
 
-		if (i != 0) {
-			if ((i % 2) == 0)
-				str_append(group, ",");
-			else
-				str_append(group, " , \n ");
+		if (!output[i].invalid_syntax) {
+			if (i != 0) {
+				if ((i % 2) == 0)
+					str_append(group, ",");
+				else
+					str_append(group, " , \n ");
+			}
+			str_append(group, input[i]);
 		}
-		str_append(group, input[i]);
 	}
 	str_append_c(group, ';');
 
@@ -111,6 +120,8 @@
 	success = addr != NULL && cmp_addr(addr, &group_prefix);
 	addr = addr->next;
 	for (i = 0; i < N_ELEMENTS(input) && addr != NULL; i++) {
+		if (output[i].invalid_syntax)
+			continue;
 		if (!cmp_addr(addr, &output[i])) {
 			success = FALSE;
 			break;