Re: [PATCH] t1450: fix quoting of NUL byte when corrupting pack

[Junio C Hamano <gitster@pobox.com>]

Martin Ågren <martin.agren@gmail.com> writes:

> We use
>
>   printf '\0'
>
> to generate a NUL byte which we then `dd` into the packfile to ensure
> that we modify the first byte of the first object, thereby
> (probabilistically) invalidating the checksum. Except the single quotes
> we're using are interpreted to match with the ones we enclose the whole
> test in. So we actually execute
>
>   printf \0
>
> and end up injecting the ASCII code for "0", 0x30, instead.
>
> The comment right above this `printf` invocation says that "at least one
> of [the type bits] is not zero, so setting the first byte to 0 is
> sufficient". Substituting "0x30" for "0" in that comment won't do: we'd
> need to reason about which bits go where and just what the packfile
> looks like that we're modifying in this test.
>
> Let's avoid all of that by actually executing
>
>   printf "\0"
>
> to generate a NUL byte, as intended.

Thanks.  Very well explained.

I wonder if it is an easy way to find similar problems without too
much hand-parsing of the test scripts.  Inside a modern test_expect_*
that begins and ends the test body with a single quote, any line
that has a single quote that is not quoted could be suspect, but
that would probably give us too many false positive.


> Signed-off-by: Martin Ågren <martin.agren@gmail.com>
> ---
>  If my reading is correct, when we substitute 0x30, the type will be 3
>  (blob) and the size will be zero. So there might actually exist
>  formally valid packfiles where this byte that we're modifying is
>  already zero. What matters in the end is whether we might be using such
>  a packfile in this exact test and from what I can tell, no, we won't be
>  doing that.
>
>  t/t1450-fsck.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
> index 344a2aad82..af2a2c4682 100755
> --- a/t/t1450-fsck.sh
> +++ b/t/t1450-fsck.sh
> @@ -714,7 +714,7 @@ test_expect_success 'fsck fails on corrupt packfile' '
>  	# at least one of which is not zero, so setting the first byte to 0 is
>  	# sufficient.)
>  	chmod a+w .git/objects/pack/pack-$pack.pack &&
> -	printf '\0' | dd of=.git/objects/pack/pack-$pack.pack bs=1 conv=notrunc seek=12 &&
> +	printf "\0" | dd of=.git/objects/pack/pack-$pack.pack bs=1 conv=notrunc seek=12 &&
>  
>  	test_when_finished "rm -f .git/objects/pack/pack-$pack.*" &&
>  	remove_object $hsh &&