fsck: properly bound "invalid tag name" error message

When we detect an invalid tag-name header in a tag object, like, "tag foo bar\n", we feed the pointer starting at "foo bar" to a printf "%s" formatter. This shows the name, as we want, but then it keeps printing the rest of the tag buffer, rather than stopping at the end of the line. Our tests did not notice because they look only for the matching line, but the bug is that we print much more than we wanted to. So we also adjust the test to be more exact. Note that when fscking tags with "index-pack --strict", this is even worse. index-pack does not add a trailing NUL-terminator after the object, so we may actually read past the buffer and print uninitialized memory. Running t5302 with valgrind does notice the bug for that reason. Signed-off-by: Jeff King <peff@peff.net> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Dec 8, 2014 at 00:48 UTC 7add441984063d2c34fa8de252b8ceb803e7981a
2 files changed +8 -3
fsck.c
+2 -1
@@ -423,7 +423,8 @@ static int fsck_tag_buffer(struct tag *tag, const char *data,
423 }
424 strbuf_addf(&sb, "refs/tags/%.*s", (int)(eol - buffer), buffer);
425 if (check_refname_format(sb.buf, 0))
426 - error_func(&tag->object, FSCK_WARN, "invalid 'tag' name: %s", buffer);
426 + error_func(&tag->object, FSCK_WARN, "invalid 'tag' name: %.*s",
427 + (int)(eol - buffer), buffer);
428 buffer = eol + 1;
429
430 if (!skip_prefix(buffer, "tagger ", &buffer))
t/t1450-fsck.sh
+6 -2
@@ -209,8 +209,12 @@ test_expect_success 'tag with incorrect tag name & missing tagger' '
209 echo $tag >.git/refs/tags/wrong &&
210 test_when_finished "git update-ref -d refs/tags/wrong" &&
211 git fsck --tags 2>out &&
212 - grep "invalid .tag. name" out &&
213 - grep "expected .tagger. line" out
212 +
213 + cat >expect <<-EOF &&
214 + warning in tag $tag: invalid '\''tag'\'' name: wrong name format
215 + warning in tag $tag: invalid format - expected '\''tagger'\'' line
216 + EOF
217 + test_cmp expect out
218 '
219
220 test_expect_success 'cleaned up' '