Subject: Re: [PATCH v5 1/6] object-name tests: add tests for ambiguous object blind spots

From: Josh Steadmon <[email-masked]>
Date: Thu, 23 Dec 2021 13:51:40 -0800

On 2021.11.25 23:03, Ævar Arnfjörð Bjarmason wrote:
> Extend the tests for ambiguous objects to check how we handle objects
> where we return OBJ_BAD when trying to parse them. As noted in [1] we
> have a blindspot when it comes to this behavior.
> 
> Since we need to add new test data here let's extend these tests to be
> tested under SHA-256, in d7a2fc82491 (t1512: skip test if not using
> SHA-1, 2018-05-13) all of the existing tests were skipped, as they
> rely on specific SHA-1 object IDs.
> 
> For these tests it only matters that the first 4 characters of the OID
> prefix are the same for both SHA-1 and SHA-256. This uses strings that
> I mined, and have the same prefix when hashed with both.
> 
> 1. https://lore.kernel.org/git/[email-masked]/
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <[email-masked]>
> ---
>  t/t1512-rev-parse-disambiguation.sh | 84 +++++++++++++++++++++++++++++
>  1 file changed, 84 insertions(+)
> 
> diff --git a/t/t1512-rev-parse-disambiguation.sh b/t/t1512-rev-parse-disambiguation.sh
> index 7891a6becf3..ae1c0cf2b21 100755
> --- a/t/t1512-rev-parse-disambiguation.sh
> +++ b/t/t1512-rev-parse-disambiguation.sh
> @@ -25,6 +25,90 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>  
>  . ./test-lib.sh
>  
> +test_cmp_failed_rev_parse () {
> +	dir=$1
> +	rev=$2
> +	shift
> +
> +	test_must_fail git -C "$dir" rev-parse "$rev" 2>actual.raw &&
> +	sed "s/\($rev\)[0-9a-f]*/\1.../g" <actual.raw >actual &&
> +	test_cmp expect actual
> +}
> +
> +test_expect_success 'ambiguous blob output' '
> +	git init --bare blob.prefix &&
> +	(
> +		cd blob.prefix &&
> +
> +		# Both start with "dead..", under both SHA-1 and SHA-256
> +		echo brocdnra | git hash-object -w --stdin &&
> +		echo brigddsv | git hash-object -w --stdin &&

These "dead.." objects don't seem to be used later, unless I've missed
something.

> +		# Both start with "beef.."
> +		echo 1agllotbh | git hash-object -w --stdin &&
> +		echo 1bbfctrkc | git hash-object -w --stdin
> +	) &&
> +
> +	cat >expect <<-\EOF &&
> +	error: short object ID beef... is ambiguous
> +	hint: The candidates are:
> +	hint:   beef... blob
> +	hint:   beef... blob
> +	fatal: ambiguous argument '\''beef...'\'': unknown revision or path not in the working tree.
> +	Use '\''--'\'' to separate paths from revisions, like this:
> +	'\''git <command> [<revision>...] -- [<file>...]'\''
> +	EOF
> +	test_cmp_failed_rev_parse blob.prefix beef
> +'

Rather than comparing the entire output (which can be brittle), can we
just grep for the important parts of the error message and compare
those?

> +test_expect_success 'ambiguous loose blob parsed as OBJ_BAD' '
> +	git init --bare blob.bad &&
> +	(
> +		cd blob.bad &&
> +
> +		# Both have the prefix "bad0"
> +		echo xyzfaowcoh | git hash-object -t bad -w --stdin --literally &&
> +		echo xyzhjpyvwl | git hash-object -t bad -w --stdin --literally
> +	) &&
> +
> +	cat >expect <<-\EOF &&
> +	error: short object ID bad0... is ambiguous
> +	hint: The candidates are:
> +	fatal: invalid object type
> +	EOF
> +	test_cmp_failed_rev_parse blob.bad bad0
> +'
> +
> +test_expect_success POSIXPERM 'ambigous zlib corrupt loose blob' '
> +	git init --bare blob.corrupt &&
> +	(
> +		cd blob.corrupt &&
> +
> +		# Both have the prefix "cafe"
> +		echo bnkxmdwz | git hash-object -w --stdin &&
> +		oid=$(echo bmwsjxzi | git hash-object -w --stdin) &&
> +
> +		oidf=objects/$(test_oid_to_path "$oid") &&
> +		chmod 755 $oidf &&
> +		echo broken >$oidf
> +	) &&
> +
> +	cat >expect <<-\EOF &&
> +	error: short object ID cafe... is ambiguous
> +	hint: The candidates are:
> +	error: inflate: data stream error (incorrect header check)
> +	error: unable to unpack cafe... header
> +	error: inflate: data stream error (incorrect header check)
> +	error: unable to unpack cafe... header
> +	hint:   cafe... unknown type
> +	hint:   cafe... blob
> +	fatal: ambiguous argument '\''cafe...'\'': unknown revision or path not in the working tree.
> +	Use '\''--'\'' to separate paths from revisions, like this:
> +	'\''git <command> [<revision>...] -- [<file>...]'\''
> +	EOF
> +	test_cmp_failed_rev_parse blob.corrupt cafe
> +'
> +
>  if ! test_have_prereq SHA1
>  then
>  	skip_all='not using SHA-1 for objects'
> -- 
> 2.34.1.838.g779e9098efb
>