From: Brandon Casey Date: Wed, 26 Mar 2008 00:40:56 GMT Subject: [PATCH] mktag.c: improve verification of tagger field and tests Message-ID: <47E99B98.1060506@nrlssc.navy.mil> In-Reply-To: <1206490795-13247-1-git-send-email-casey@nrlssc.navy.mil> 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 --- 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 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 \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 < 0 + EOF check_verify_failure '"object" line label check' '^error: char0: .*"object "$' @@ -55,6 +57,8 @@ cat >tag.sig < 0 + EOF check_verify_failure '"object" line SHA1 check' '^error: char7: .*SHA1 hash$' @@ -66,6 +70,8 @@ cat >tag.sig < 0 + EOF check_verify_failure '"type" line label check' '^error: char47: .*"\\ntype "$' @@ -85,6 +91,8 @@ cat >tag.sig < 0 + EOF check_verify_failure '"tag" line label check #1' \ @@ -121,6 +129,8 @@ cat >tag.sig < 0 + EOF check_verify_failure 'verify object (SHA1/type) check' \ @@ -133,6 +143,8 @@ cat >tag.sig < 0 + EOF check_verify_failure 'verify tag-name check' \ @@ -145,10 +157,12 @@ cat >tag.sig <tag.sig < 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 < 0 + +EOF + +check_verify_failure 'detect malformed tagger' \ + '^error: char77: malformed tagger$' + +############################################################ +# 15. allow empty tag email + +cat >tag.sig < 0 + +EOF + +test_expect_success \ + 'allow empty tag email' \ + 'git-mktag .git/refs/tags/mytag 2>message' + +############################################################ +# 16. detect missing tag timestamp + +cat >tag.sig < + +EOF + +check_verify_failure 'detect missing tag timestamp' \ + '^error: char107: missing tag timestamp$' + +############################################################ +# 17. detect invalid tag timestamp + +cat >tag.sig < 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 < 1206478233 GMT + +EOF + +check_verify_failure 'detect invalid tag timezone' \ + '^error: char118: malformed tag timezone$' + +############################################################ +# 19. detect invalid header entry + +cat >tag.sig < 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 < 1206478233 -0500 + EOF test_expect_success \ @@ -178,7 +297,7 @@ test_expect_success \ 'git-mktag .git/refs/tags/mytag 2>message' ############################################################ -# 14. check mytag +# 21. check mytag test_expect_success \ 'check mytag' \ -- 1.5.4.4.481.g5075