[PATCH] mktag.c: improve verification of tagger field and tests

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Since nearly its birth, git's tags have included a "tagger" field which
describe the name of tagger, email of tagger, and date and time of tagging.
But, this field was only loosely tested by git-mktag. Provide some thorough
testing for this field and also ensure that the tag header is separated
from the tag body by an empty line to reduce the convenience of creating
a flawed tag.

Signed-off-by: Brandon Casey <casey@xxxxxxxxxxxxxxx>
---


Well, since I looked at this code and used it in filter-branch, I figured I
should fix the verification code for the tagger field (even though it's
probably dieing soon).

I'm thinking this utility should be fairly strict about the format it accepts.

Some assumptions:
   -tagger field has form: A U Thor <author@xxxxxxxxxxx> SSSSSSSSS [[+]hhmm]
                           where the SSSS's are the time stamp and the time
                           zone is optional
   -author name must not be empty, but can be a single space
   -author email can be empty. i.e. '<>'
   -timestamp can have leading spaces
   -time zone can have leading spaces
   -no trailing spaces e.g. after the timezone field, or after the timestamp
    when timezone is not supplied

 Should leading spaces be accepted? The rest of git seems to handle them, I'm
 not sure if there was ever anything out there that would have created a tag
 with leading spaces. Possibly a fixed width printf of the timestamp?

 Should the timezone be optional? How about the prefixed +|-?

-brandon


 mktag.c          |   70 +++++++++++++++++++++++++----
 t/t3800-mktag.sh |  129 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 185 insertions(+), 14 deletions(-)

diff --git a/mktag.c b/mktag.c
index b05260c..49f4ca1 100644
--- a/mktag.c
+++ b/mktag.c
@@ -8,10 +8,11 @@
  * message and a signature block that git itself doesn't care about,
  * but that can be verified with gpg or similar.
  *
- * The first three lines are guaranteed to be at least 63 bytes:
+ * The first four lines are guaranteed to be at least 77 bytes:
  * "object <sha1>\n" is 48 bytes, "type tag\n" at 9 bytes is the
- * shortest possible type-line, and "tag .\n" at 6 bytes is the
- * shortest single-character-tag line.
+ * shortest possible type-line, "tag .\n" at 6 bytes is the shortest
+ * single-character-tag line, and "tagger . <> 0\n" at 14 bytes is
+ * the shortest possible tagger-line.
  */
 
 /*
@@ -43,9 +44,9 @@ static int verify_tag(char *buffer, unsigned long size)
 	int typelen;
 	char type[20];
 	unsigned char sha1[20];
-	const char *object, *type_line, *tag_line, *tagger_line;
+	const char *object, *type_line, *tag_line, *tagger_line, *lb, *rb;
 
-	if (size < 64)
+	if (size < 78)
 		return error("wanna fool me ? you obviously got the size wrong !");
 
 	buffer[size] = 0;
@@ -96,12 +97,63 @@ static int verify_tag(char *buffer, unsigned long size)
 
 	/* Verify the tagger line */
 	tagger_line = tag_line;
+	if (memcmp(tagger_line, "tagger ", 7) || (tagger_line[7] == '\n'))
+		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
+	 */
+	tagger_line += 7;
+	if (!(lb = strstr(tagger_line, " <")) || !(rb = strstr(lb, "> ")) ||
+		strchr(tagger_line, '\n') < rb)
+		return error("char" PD_FMT ": malformed tagger",
+			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 */
+	tagger_line = rb + 2;
+	while (*tagger_line == ' ')
+		tagger_line++;
+	if (*tagger_line == '\n')
+		return error("char" PD_FMT ": missing tag timestamp",
+			tagger_line - buffer);
+	for (;;) {
+		unsigned char c = *tagger_line++;
+		if (c == ' ' || c == '\n')
+			break;
+		if (c >= '0' && c <= '9')
+			continue;
+		return error("char" PD_FMT ": malformed tag timestamp",
+			tagger_line - buffer);
+	}
 
-	if (memcmp(tagger_line, "tagger", 6) || (tagger_line[6] == '\n'))
-		return error("char" PD_FMT ": could not find \"tagger\"", tagger_line - buffer);
+	/* optional timezone, 4 digits hhmm with optional leading +|- */
+	if (tagger_line[-1] != '\n') {
+		while (*tagger_line == ' ')
+			tagger_line++;
+		if ((*tagger_line == '+' || *tagger_line == '-') &&
+			tagger_line[1] != '\n')
+			tagger_line++;
+		if (!(tagger_line[0] >= '0' && tagger_line[0] <= '9' &&
+		      tagger_line[1] >= '0' && tagger_line[1] <= '9' &&
+		      tagger_line[2] >= '0' && tagger_line[2] <= '5' &&
+		      tagger_line[3] >= '0' && tagger_line[3] <= '9' &&
+		      tagger_line[4] == '\n'))
+			return error("char" PD_FMT ": malformed tag timezone",
+				tagger_line - buffer);
+		tagger_line += 5;
+	}
 
-	/* TODO: check for committer info + blank line? */
-	/* Also, the minimum length is probably + "tagger .", or 63+8=71 */
+	/* Verify the blank line separating the header from the body */
+	if (*tagger_line != '\n')
+		return error("char" PD_FMT ": trailing garbage in tag header",
+			tagger_line - buffer);
 
 	/* The actual stuff afterwards we don't care about.. */
 	return 0;
diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh
index bdc6e13..0098389 100755
--- a/t/t3800-mktag.sh
+++ b/t/t3800-mktag.sh
@@ -44,6 +44,8 @@ cat >tag.sig <<EOF
 xxxxxx 139e9b33986b1c2670fff52c5067603117b3e895
 type tag
 tag mytag
+tagger . <> 0
+
 EOF
 
 check_verify_failure '"object" line label check' '^error: char0: .*"object "$'
@@ -55,6 +57,8 @@ cat >tag.sig <<EOF
 object zz9e9b33986b1c2670fff52c5067603117b3e895
 type tag
 tag mytag
+tagger . <> 0
+
 EOF
 
 check_verify_failure '"object" line SHA1 check' '^error: char7: .*SHA1 hash$'
@@ -66,6 +70,8 @@ cat >tag.sig <<EOF
 object 779e9b33986b1c2670fff52c5067603117b3e895
 xxxx tag
 tag mytag
+tagger . <> 0
+
 EOF
 
 check_verify_failure '"type" line label check' '^error: char47: .*"\\ntype "$'
@@ -85,6 +91,8 @@ cat >tag.sig <<EOF
 object 779e9b33986b1c2670fff52c5067603117b3e895
 type tag
 xxx mytag
+tagger . <> 0
+
 EOF
 
 check_verify_failure '"tag" line label check #1' \
@@ -121,6 +129,8 @@ cat >tag.sig <<EOF
 object 779e9b33986b1c2670fff52c5067603117b3e895
 type tagggg
 tag mytag
+tagger . <> 0
+
 EOF
 
 check_verify_failure 'verify object (SHA1/type) check' \
@@ -133,6 +143,8 @@ cat >tag.sig <<EOF
 object $head
 type commit
 tag my	tag
+tagger . <> 0
+
 EOF
 
 check_verify_failure 'verify tag-name check' \
@@ -145,10 +157,12 @@ cat >tag.sig <<EOF
 object $head
 type commit
 tag mytag
+
+This is filler
 EOF
 
 check_verify_failure '"tagger" line label check #1' \
-	'^error: char70: could not find "tagger"$'
+	'^error: char70: could not find "tagger "$'
 
 ############################################################
 # 12. tagger line label check #2
@@ -158,19 +172,124 @@ object $head
 type commit
 tag mytag
 tagger
+
+This is filler
 EOF
 
 check_verify_failure '"tagger" line label check #2' \
-	'^error: char70: could not find "tagger"$'
+	'^error: char70: could not find "tagger "$'
 
 ############################################################
-# 13. create valid tag
+# 13. detect missing tag author name
 
 cat >tag.sig <<EOF
 object $head
 type commit
 tag mytag
-tagger another@xxxxxxxxxxx
+tagger  <> 0
+
+This is filler
+EOF
+
+check_verify_failure 'detect missing tag author name' \
+	'^error: char77: missing tagger name$'
+
+############################################################
+# 14. detect missing tag author name
+
+cat >tag.sig <<EOF
+object $head
+type commit
+tag mytag
+tagger T A Gger <
+ > 0
+
+EOF
+
+check_verify_failure 'detect malformed tagger' \
+	'^error: char77: malformed tagger$'
+
+############################################################
+# 15. allow empty tag email
+
+cat >tag.sig <<EOF
+object $head
+type commit
+tag mytag
+tagger T A Gger <> 0
+
+EOF
+
+test_expect_success \
+    'allow empty tag email' \
+    'git-mktag <tag.sig >.git/refs/tags/mytag 2>message'
+
+############################################################
+# 16. detect missing tag timestamp
+
+cat >tag.sig <<EOF
+object $head
+type commit
+tag mytag
+tagger T A Gger <tagger@xxxxxxxxxxx> 
+
+EOF
+
+check_verify_failure 'detect missing tag timestamp' \
+	'^error: char107: missing tag timestamp$'
+
+############################################################
+# 17. detect invalid tag timestamp
+
+cat >tag.sig <<EOF
+object $head
+type commit
+tag mytag
+tagger T A Gger <tagger@xxxxxxxxxxx> Tue Mar 25 15:47:44 2008
+
+EOF
+
+check_verify_failure 'detect invalid tag timestamp' \
+	'^error: char108: malformed tag timestamp$'
+
+############################################################
+# 18. detect invalid tag timezone
+
+cat >tag.sig <<EOF
+object $head
+type commit
+tag mytag
+tagger T A Gger <tagger@xxxxxxxxxxx> 1206478233 GMT
+
+EOF
+
+check_verify_failure 'detect invalid tag timezone' \
+	'^error: char118: malformed tag timezone$'
+
+############################################################
+# 19. detect invalid header entry
+
+cat >tag.sig <<EOF
+object $head
+type commit
+tag mytag
+tagger T A Gger <tagger@xxxxxxxxxxx> 1206478233 -0500
+this line should not be here
+
+EOF
+
+check_verify_failure 'detect invalid header entry' \
+	'^error: char124: trailing garbage in tag header$'
+
+############################################################
+# 20. create valid tag
+
+cat >tag.sig <<EOF
+object $head
+type commit
+tag mytag
+tagger T A Gger <tagger@xxxxxxxxxxx> 1206478233 -0500
+
 EOF
 
 test_expect_success \
@@ -178,7 +297,7 @@ test_expect_success \
     'git-mktag <tag.sig >.git/refs/tags/mytag 2>message'
 
 ############################################################
-# 14. check mytag
+# 21. check mytag
 
 test_expect_success \
     'check mytag' \
-- 
1.5.4.4.481.g5075

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux