Re: Bug in reflog of length 0x2BFF

[Jeff King <peff@peff.net>]

On Thu, Dec 04, 2014 at 09:37:34PM +0100, Christoph Mallon wrote:

> Am 04.12.14 21:18, schrieb Junio C Hamano:
> > Jonathan Nieder <jrnieder@gmail.com> writes:
> >> Could you make a test script that illustrates and reproduces the
> >> problem?  I.e., a patch to a file like t/t1410-reflog.sh, such that
> >> if I run
> >>
> >> 	cd git
> >> 	make
> >> 	cd t
> >> 	./t1410-reflog.sh
> >>
> >> then I can reproduce the bug?
> > 
> > Amen to that.  I am getting the same thing.
> 
> I ran reproduce it reliably on multiple machines (OS X, FreeBSD, ia32,
> amd64), a friend of mine can, too.

Thanks, I was able to reproduce this easily on an OS X machine.

Does this patch fix your problem?

diff --git a/refs.c b/refs.c
index f1afec5..42e3a30 100644
--- a/refs.c
+++ b/refs.c
@@ -3052,7 +3052,7 @@ static int show_one_reflog_ent(struct strbuf *sb, each_reflog_ent_fn fn, void *c
 	int tz;
 
 	/* old SP new SP name <email> SP time TAB msg LF */
-	if (sb->len < 83 || sb->buf[sb->len - 1] != '\n' ||
+	if (sb->len < 83 ||
 	    get_sha1_hex(sb->buf, osha1) || sb->buf[40] != ' ' ||
 	    get_sha1_hex(sb->buf + 41, nsha1) || sb->buf[81] != ' ' ||
 	    !(email_end = strchr(sb->buf + 82, '>')) ||


I think the bug is in the reverse-reflog reader in
for_each_reflog_ent_reverse. It reads BUFSIZ chunks of the file in
reverse order, and then parses them individually. If the trailing
newline for a line falls directly on the block boundary, we may not have
it in our current block, and pass the line to show_one_reflog_ent
without a trailing newline. That function is picky about making sure it
got a full line.

So this is a long-standing bug in for_each_reflog_ent_reverse. It just
showed up recently because we started using that function for
read_ref_at_ent.

I haven't confirmed yet, but I suspect the problem shows up on OS X and
FreeBSD but not Linux because of the definition of BUFSIZ (so it is
really probably glibc versus BSD libc). The same bug exists on Linux,
but you would need different input to stimulate the newline at the right
spot.

The above is a workaround. I think the right solution is probably to
teach for_each_reflog_ent_reverse to makes sure the trailing newline is
included (either by tweaking the reverse code, or conditionally adding
it to the parsed buffer).

-Peff