[PATCH 2/2] builtin-tag.c: use valid_ident for checking tagger field

[PATCH 2/2] builtin-tag.c: use valid_ident for checking tagger field

[drafnel]

From: Brandon Casey <drafnel@gmail.com>

Use valid_ident function for validating the tagger field
in generated tags rather than a tag specific version.

Also update tests.

Signed-off-by: Brandon Casey <drafnel@gmail.com>
---
 builtin-tag.c    |   39 +++++----------------------------------
 t/t3800-mktag.sh |   14 +++++++-------
 2 files changed, 12 insertions(+), 41 deletions(-)

diff --git a/builtin-tag.c b/builtin-tag.c
index 0f79d47..2dcb795 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -339,8 +339,7 @@ static int verify_tag_buffer(const char *buffer, size_t size)
 	int typelen;
 	char type[20];
 	unsigned char sha1[20];
-	const char *object, *type_line, *tag_line, *tagger_line, *lb, *rb;
-	size_t len;
+	const char *object, *type_line, *tag_line, *tagger_line, *rb;
 
 	if (size < 84)
 		return error("wanna fool me ? you obviously got the size wrong !");
@@ -396,42 +395,14 @@ static int verify_tag_buffer(const char *buffer, size_t size)
 		return error("char" PD_FMT ": could not find \"tagger \"",
 			tagger_line - buffer);
 
-	/*
-	 * Check for correct form for name and email
-	 * i.e. " <" followed by "> " on _this_ line
-	 * No angle brackets within the name or email address fields.
-	 * No spaces within the email address field.
-	 */
+	/* Validate the tagger identity field */
 	tagger_line += 7;
-	if (!(lb = strstr(tagger_line, " <")) || !(rb = strstr(lb+2, "> ")) ||
-		strpbrk(tagger_line, "<>\n") != lb+1 ||
-		strpbrk(lb+2, "><\n ") != rb)
+	rb = strchr(tagger_line, '\n');
+	if (!rb || !valid_ident(tagger_line, rb - tagger_line, 0))
 		return error("char" PD_FMT ": malformed tagger field",
 			tagger_line - buffer);
 
-	/* Check for author name, at least one character, space is acceptable */
-	if (lb == tagger_line)
-		return error("char" PD_FMT ": missing tagger name",
-			tagger_line - buffer);
-
-	/* timestamp, 1 or more digits followed by space */
-	tagger_line = rb + 2;
-	if (!(len = strspn(tagger_line, "0123456789")))
-		return error("char" PD_FMT ": missing tag timestamp",
-			tagger_line - buffer);
-	tagger_line += len;
-	if (*tagger_line != ' ')
-		return error("char" PD_FMT ": malformed tag timestamp",
-			tagger_line - buffer);
-	tagger_line++;
-
-	/* timezone, 5 digits [+-]hhmm, max. 1400 */
-	if (!((tagger_line[0] == '+' || tagger_line[0] == '-') &&
-	      strspn(tagger_line+1, "0123456789") == 4 &&
-	      tagger_line[5] == '\n' && atoi(tagger_line+1) <= 1400))
-		return error("char" PD_FMT ": malformed tag timezone",
-			tagger_line - buffer);
-	tagger_line += 6;
+	tagger_line = rb + 1;
 
 	/* Verify the blank line separating the header from the body */
 	if (*tagger_line != '\n')
diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh
index df1fd6f..1f62c05 100755
--- a/t/t3800-mktag.sh
+++ b/t/t3800-mktag.sh
@@ -192,7 +192,7 @@ This is filler
 EOF
 
 check_verify_failure 'disallow missing tag author name' \
-	'^error: char77: missing tagger name$'
+	'^error: char77: malformed tagger field$'
 
 ############################################################
 # 14. disallow missing tag author name
@@ -250,7 +250,7 @@ tagger T A Gger <tagger@example.com>
 EOF
 
 check_verify_failure 'disallow missing tag timestamp' \
-	'^error: char107: missing tag timestamp$'
+	'^error: char77: malformed tagger field$'
 
 ############################################################
 # 18. detect invalid tag timestamp1
@@ -264,7 +264,7 @@ tagger T A Gger <tagger@example.com> Tue Mar 25 15:47:44 2008
 EOF
 
 check_verify_failure 'detect invalid tag timestamp1' \
-	'^error: char107: missing tag timestamp$'
+	'^error: char77: malformed tagger field$'
 
 ############################################################
 # 19. detect invalid tag timestamp2
@@ -278,7 +278,7 @@ tagger T A Gger <tagger@example.com> 2008-03-31T12:20:15-0500
 EOF
 
 check_verify_failure 'detect invalid tag timestamp2' \
-	'^error: char111: malformed tag timestamp$'
+	'^error: char77: malformed tagger field$'
 
 ############################################################
 # 20. detect invalid tag timezone1
@@ -292,7 +292,7 @@ tagger T A Gger <tagger@example.com> 1206478233 GMT
 EOF
 
 check_verify_failure 'detect invalid tag timezone1' \
-	'^error: char118: malformed tag timezone$'
+	'^error: char77: malformed tagger field$'
 
 ############################################################
 # 21. detect invalid tag timezone2
@@ -306,7 +306,7 @@ tagger T A Gger <tagger@example.com> 1206478233 +  30
 EOF
 
 check_verify_failure 'detect invalid tag timezone2' \
-	'^error: char118: malformed tag timezone$'
+	'^error: char77: malformed tagger field$'
 
 ############################################################
 # 22. detect invalid tag timezone3
@@ -320,7 +320,7 @@ tagger T A Gger <tagger@example.com> 1206478233 -1430
 EOF
 
 check_verify_failure 'detect invalid tag timezone3' \
-	'^error: char118: malformed tag timezone$'
+	'^error: char77: malformed tagger field$'
 
 ############################################################
 # 23. detect invalid header entry
-- 
1.5.5.67.g9a49

Re: [PATCH 2/2] builtin-tag.c: use valid_ident for checking tagger field

[Junio C Hamano]

drafnel@gmail.com writes:

> From: Brandon Casey <drafnel@gmail.com>
>
> Use valid_ident function for validating the tagger field
> in generated tags rather than a tag specific version.
>
> Also update tests.
>
> Signed-off-by: Brandon Casey <drafnel@gmail.com>
> ---
>  builtin-tag.c    |   39 +++++----------------------------------
>  t/t3800-mktag.sh |   14 +++++++-------
>  2 files changed, 12 insertions(+), 41 deletions(-)
>
> diff --git a/builtin-tag.c b/builtin-tag.c
> index 0f79d47..2dcb795 100644
> --- a/builtin-tag.c
> +++ b/builtin-tag.c
> @@ -339,8 +339,7 @@ static int verify_tag_buffer(const char *buffer, size_t size)
>  	int typelen;
>  	char type[20];

Funny.

This [2/2] seems to depend on [3/5] that renames verify_tag to verify_tag_buffer().

What is going on?

Re: [PATCH 2/2] builtin-tag.c: use valid_ident for checking tagger field

[Brandon Casey]

Junio C Hamano wrote:
> drafnel@gmail.com writes:
> 
>> From: Brandon Casey <drafnel@gmail.com>
>>
>> Use valid_ident function for validating the tagger field
>> in generated tags rather than a tag specific version.
>>
>> Also update tests.
>>
>> Signed-off-by: Brandon Casey <drafnel@gmail.com>
>> ---
>>  builtin-tag.c    |   39 +++++----------------------------------
>>  t/t3800-mktag.sh |   14 +++++++-------
>>  2 files changed, 12 insertions(+), 41 deletions(-)
>>
>> diff --git a/builtin-tag.c b/builtin-tag.c
>> index 0f79d47..2dcb795 100644
>> --- a/builtin-tag.c
>> +++ b/builtin-tag.c
>> @@ -339,8 +339,7 @@ static int verify_tag_buffer(const char *buffer, size_t size)
>>  	int typelen;
>>  	char type[20];
> 
> Funny.
> 
> This [2/2] seems to depend on [3/5] that renames verify_tag to verify_tag_buffer().
> 
> What is going on?

Yes, sorry. I should have mentioned this.

I implemented this after the other series, but it could just as well be merged into one,
with these two coming before the 5 patch mktag stuff. I can resubmit if you like.

-brandon