git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Johannes Schindelin <johannes.schindelin@gmx.de>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: [PATCH] fsck: properly bound "invalid tag name" error message
Date: Mon, 8 Dec 2014 00:48:13 -0500	[thread overview]
Message-ID: <20141208054812.GA30154@peff.net> (raw)

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>
---
I'm generally nervous about adding too-specific stderr wording or
formatting to our tests, as I do not want them to be brittle. But I do
not actually think this is substantially different than what other fsck
tests do (i.e., they are already grepping for _half_ the wording
already, so if it changes, they are likely to break, too).

If we care, the test can check test_line_count or similar to make sure
there isn't extra data. But I think the way I have written it below is a
lot easier for a reader coming later to understand what is going on.

 fsck.c          | 3 ++-
 t/t1450-fsck.sh | 8 ++++++--
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/fsck.c b/fsck.c
index 2fffa43..88c92e8 100644
--- a/fsck.c
+++ b/fsck.c
@@ -423,7 +423,8 @@ static int fsck_tag_buffer(struct tag *tag, const char *data,
 	}
 	strbuf_addf(&sb, "refs/tags/%.*s", (int)(eol - buffer), buffer);
 	if (check_refname_format(sb.buf, 0))
-		error_func(&tag->object, FSCK_WARN, "invalid 'tag' name: %s", buffer);
+		error_func(&tag->object, FSCK_WARN, "invalid 'tag' name: %.*s",
+			   (int)(eol - buffer), buffer);
 	buffer = eol + 1;
 
 	if (!skip_prefix(buffer, "tagger ", &buffer))
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 019fddd..57ccce5 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -229,8 +229,12 @@ test_expect_success 'tag with incorrect tag name & missing tagger' '
 	echo $tag >.git/refs/tags/wrong &&
 	test_when_finished "git update-ref -d refs/tags/wrong" &&
 	git fsck --tags 2>out &&
-	grep "invalid .tag. name" out &&
-	grep "expected .tagger. line" out
+
+	cat >expect <<-EOF &&
+	warning in tag $tag: invalid '\''tag'\'' name: wrong name format
+	warning in tag $tag: invalid format - expected '\''tagger'\'' line
+	EOF
+	test_cmp expect out
 '
 
 test_expect_success 'tag with bad tagger' '
-- 
2.2.0.390.gf60752d

             reply	other threads:[~2014-12-08  5:48 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-08  5:48 Jeff King [this message]
2014-12-08  5:57 ` [PATCH] fsck: properly bound "invalid tag name" error message Jeff King
2014-12-08 11:17   ` Johannes Schindelin
2014-12-08 11:22     ` Jeff King
2014-12-08 11:28   ` Duy Nguyen
2014-12-08 11:35     ` Johannes Schindelin
2014-12-08 11:47       ` Jeff King
2014-12-08 13:46         ` Johannes Schindelin
2014-12-08 14:17     ` [PATCH v2] index-pack: terminate object buffers with NUL Johannes Schindelin
2014-12-08 11:01 ` [PATCH] fsck: properly bound "invalid tag name" error message Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141208054812.GA30154@peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johannes.schindelin@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).