Re: [RFC] t5300 considered dangerous

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Dec 27, 2005, 13:57 UTC
Message-ID: <Pine.LNX.4.63.0512271452470.17086@wbgn013.biozentrum.uni-wuerzburg.de>
In-Reply-To: <7vd5jjffxu.fsf@assigned-by-dhcp.cox.net>

Hi,

On Mon, 26 Dec 2005, Junio C Hamano wrote:

Show 19 quoted lines

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > The test case corrupts a pack, and then also the index of that pack, by 
> > writing "0" at certain offsets, and then tests if git-verify-pack fails.
> 
> These certain offsets are:
> 
>  . byte    2 in pack
>    This corrupts PACK_SIGNATURE ("A" in "PACK").
> 
>  . byte    7 in pack
>    This corrupts PACK_VERSION ("\002" in "\0\0\0\002").
> 
>  . byte   12 in pack
>    This corrupts the type/size byte of whatever the first object
>    happens to be (the type/size byte has non-zero upper nibble).
> 
>  . byte 1200 in idx that is currently 1208 bytes long.
>    This tries to corrupt the checksum of the index file itself,

Thank you for clarifying this.

Show 10 quoted lines

> diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
> index 7dfb1ab..dd719bb 100755
> --- a/t/t5300-pack-object.sh
> +++ b/t/t5300-pack-object.sh
> @@ -163,8 +163,10 @@ test_expect_success \
>       else :;
>       fi &&
>  
> +     l=`wc -c <test-3.idx` &&
> +     l=`expr "$l" - 20` &&

This needs to be

> +     l=`expr $l - 20` &&

since there are textutils out there (notably version 2.1) where "wc -c" prints a leading space.

Show 6 quoted lines

>       cp test-1-${packname_1}.pack test-3.pack &&
> -     dd if=/dev/zero of=test-3.idx count=1 bs=1 conv=notrunc seek=1200 &&
> +     dd if=/dev/zero of=test-3.idx count=20 bs=1 conv=notrunc seek=$l &&
>       if git-verify-pack test-3.pack
>       then false
>       else :;

When the commit is not skipped, I fail to see why this should not fail in one out of 256 cases: the input is (partially) pseudo-random. The last 20 bytes are the SHA-1 which should inherit this pseudo-randomness. So, the first byte should be pseudo-random, too.

Something I missed?

Ciao, Dscho