Subject: Re: [PATCH v4 14/20] mktag: use fsck instead of custom verify_tag()

From: Junio C Hamano <[email-masked]>
Date: Wed, 23 Dec 2020 05:34:23 -0800

Ævar Arnfjörð Bjarmason  <[email-masked]> writes:

> +static int verify_object_in_tag(struct object_id *tagged_oid, int *tagged_type)
>  {
> +	int ret;
> +	enum object_type type;
> +	unsigned long size;
> +	void *buffer;
> +	const struct object_id *repl;
> +
> +	buffer = read_object_file(tagged_oid, &type, &size);
> +	if (!buffer)
> +		die("could not read tagged object '%s'",
> +		    oid_to_hex(tagged_oid));
> +	if (type != *tagged_type)
> +		die("object '%s' tagged as '%s', but is a '%s' type",
> +		    oid_to_hex(tagged_oid),
> +		    type_name(*tagged_type), type_name(type));
> +
> +	repl = lookup_replace_object(the_repository, tagged_oid);
> +	ret = check_object_signature(the_repository, repl,
> +				     buffer, size, type_name(*tagged_type));
> +	free(buffer);

This is not new, but I wonder if the object whose name is repl can
be of different type than the object whose name is tagged_oid, and
what happens when such a replacement is defined.

> +	return ret;
>  }
> ...
> +/*
> + * fsck a tag, and pass info about it back to the caller. This is
> + * exposed fsck_object() internals for git-mktag(1).
> + */
> +int fsck_tag_standalone(const struct object_id *oid, const char *buffer,
> +			unsigned long size, struct fsck_options *options,
> +			struct object_id *tagged_oid,
> +			int *tag_type);
> +

OK.

> diff --git a/t/t3800-mktag.sh b/t/t3800-mktag.sh
> index c6826762d9..d20adf0544 100755
> --- a/t/t3800-mktag.sh
> +++ b/t/t3800-mktag.sh
> @@ -47,7 +47,7 @@ too short for a tag
>  EOF

OK.  This seems to show rather nicely what checks got loosened
(and nothing got tightened it seems).

Thanks.  Looking good so far.

>  
>  check_verify_failure 'Tag object length check' \
> -	'^error: .*size wrong.*$'
> +	'^error:.* missingObject:'
>  
>  ############################################################
>  #  2. object line label check
> @@ -60,7 +60,7 @@ tagger . <> 0 +0000
>  
>  EOF
>  
> -check_verify_failure '"object" line label check' '^error: char0: .*"object "$'
> +check_verify_failure '"object" line label check' '^error:.* missingObject:'
>  
>  ############################################################
>  #  3. object line hash check
> @@ -73,7 +73,7 @@ tagger . <> 0 +0000
>  
>  EOF
>  
> -check_verify_failure '"object" line SHA1 check' '^error: char7: .*SHA1 hash$'
> +check_verify_failure '"object" line check' '^error:.* badObjectSha1:'
>  
>  ############################################################
>  #  4. type line label check
> @@ -86,7 +86,7 @@ tagger . <> 0 +0000
>  
>  EOF
>  
> -check_verify_failure '"type" line label check' '^error: char.*: .*"\\ntype "$'
> +check_verify_failure '"type" line label check' '^error:.* missingTypeEntry:'
>  
>  ############################################################
>  #  5. type line eol check
> @@ -94,7 +94,7 @@ check_verify_failure '"type" line label check' '^error: char.*: .*"\\ntype "$'
>  echo "object $head" >tag.sig
>  printf "type tagsssssssssssssssssssssssssssssss" >>tag.sig
>  
> -check_verify_failure '"type" line eol check' '^error: char.*: .*"\\n"$'
> +check_verify_failure '"type" line eol check' '^error:.* unterminatedHeader:'
>  
>  ############################################################
>  #  6. tag line label check #1
> @@ -108,7 +108,7 @@ tagger . <> 0 +0000
>  EOF
>  
>  check_verify_failure '"tag" line label check #1' \
> -	'^error: char.*: no "tag " found$'
> +	'^error:.* missingTagEntry:'
>  
>  ############################################################
>  #  7. tag line label check #2
> @@ -120,7 +120,7 @@ tag
>  EOF
>  
>  check_verify_failure '"tag" line label check #2' \
> -	'^error: char.*: no "tag " found$'
> +	'^error:.* badType:'
>  
>  ############################################################
>  #  8. type line type-name length check
> @@ -132,7 +132,7 @@ tag mytag
>  EOF
>  
>  check_verify_failure '"type" line type-name length check' \
> -	'^error: char.*: type too long$'
> +	'^error:.* badType:'
>  
>  ############################################################
>  #  9. verify object (hash/type) check
> @@ -146,7 +146,7 @@ tagger . <> 0 +0000
>  EOF
>  
>  check_verify_failure 'verify object (hash/type) check -- correct type, nonexisting object' \
> -	'^error: char7: could not verify object.*$'
> +	'^fatal: could not read tagged object'
>  
>  cat >tag.sig <<EOF
>  object $head
> @@ -157,7 +157,7 @@ tagger . <> 0 +0000
>  EOF
>  
>  check_verify_failure 'verify object (hash/type) check -- made-up type, nonexisting object' \
> -	'^fatal: invalid object type'
> +	'^error:.* badType:'
>  
>  cat >tag.sig <<EOF
>  object $(test_oid deadbeef)
> @@ -168,7 +168,7 @@ tagger . <> 0 +0000
>  EOF
>  
>  check_verify_failure 'verify object (hash/type) check -- incorrect type, valid object' \
> -	'^error: char7: could not verify object.*$'
> +	'^error:.* badType:'
>  
>  cat >tag.sig <<EOF
>  object $head
> @@ -179,7 +179,7 @@ tagger . <> 0 +0000
>  EOF
>  
>  check_verify_failure 'verify object (hash/type) check -- incorrect type, valid object' \
> -	'^error: char7: could not verify object'
> +	'^fatal: object.*tagged as.*tree.*but is.*commit'
>  
>  ############################################################
>  # 10. verify tag-name check
> @@ -193,7 +193,7 @@ tagger . <> 0 +0000
>  EOF
>  
>  check_verify_failure 'verify tag-name check' \
> -	'^error: char.*: could not verify tag name$'
> +	'^error:.* badTagName:'
>  
>  ############################################################
>  # 11. tagger line label check #1
> @@ -207,7 +207,7 @@ This is filler
>  EOF
>  
>  check_verify_failure '"tagger" line label check #1' \
> -	'^error: char.*: could not find "tagger "$'
> +	'^error:.* missingTaggerEntry:'
>  
>  ############################################################
>  # 12. tagger line label check #2
> @@ -222,10 +222,10 @@ This is filler
>  EOF
>  
>  check_verify_failure '"tagger" line label check #2' \
> -	'^error: char.*: could not find "tagger "$'
> +	'^error:.* missingTaggerEntry:'
>  
>  ############################################################
> -# 13. disallow missing tag author name
> +# 13. allow missing tag author name like fsck
>  
>  cat >tag.sig <<EOF
>  object $head
> @@ -236,8 +236,7 @@ tagger  <> 0 +0000
>  This is filler
>  EOF
>  
> -check_verify_failure 'disallow missing tag author name' \
> -	'^error: char.*: missing tagger name$'
> +test_expect_mktag_success 'allow missing tag author name'
>  
>  ############################################################
>  # 14. disallow missing tag author name
> @@ -252,7 +251,7 @@ tagger T A Gger <
>  EOF
>  
>  check_verify_failure 'disallow malformed tagger' \
> -	'^error: char.*: malformed tagger field$'
> +	'^error:.* badEmail:'
>  
>  ############################################################
>  # 15. allow empty tag email
> @@ -268,7 +267,7 @@ EOF
>  test_expect_mktag_success 'allow empty tag email'
>  
>  ############################################################
> -# 16. disallow spaces in tag email
> +# 16. allow spaces in tag email like fsck
>  
>  cat >tag.sig <<EOF
>  object $head
> @@ -278,8 +277,7 @@ tagger T A Gger <tag [email-masked]> 0 +0000
>  
>  EOF
>  
> -check_verify_failure 'disallow spaces in tag email' \
> -	'^error: char.*: malformed tagger field$'
> +test_expect_mktag_success 'allow spaces in tag email like fsck'
>  
>  ############################################################
>  # 17. disallow missing tag timestamp
> @@ -293,7 +291,7 @@ tagger T A Gger <[email-masked]>__
>  EOF
>  
>  check_verify_failure 'disallow missing tag timestamp' \
> -	'^error: char.*: missing tag timestamp$'
> +	'^error:.* badDate:'
>  
>  ############################################################
>  # 18. detect invalid tag timestamp1
> @@ -307,7 +305,7 @@ tagger T A Gger <[email-masked]> Tue Mar 25 15:47:44 2008
>  EOF
>  
>  check_verify_failure 'detect invalid tag timestamp1' \
> -	'^error: char.*: missing tag timestamp$'
> +	'^error:.* badDate:'
>  
>  ############################################################
>  # 19. detect invalid tag timestamp2
> @@ -321,7 +319,7 @@ tagger T A Gger <[email-masked]> 2008-03-31T12:20:15-0500
>  EOF
>  
>  check_verify_failure 'detect invalid tag timestamp2' \
> -	'^error: char.*: malformed tag timestamp$'
> +	'^error:.* badDate:'
>  
>  ############################################################
>  # 20. detect invalid tag timezone1
> @@ -335,7 +333,7 @@ tagger T A Gger <[email-masked]> 1206478233 GMT
>  EOF
>  
>  check_verify_failure 'detect invalid tag timezone1' \
> -	'^error: char.*: malformed tag timezone$'
> +	'^error:.* badTimezone:'
>  
>  ############################################################
>  # 21. detect invalid tag timezone2
> @@ -349,10 +347,10 @@ tagger T A Gger <[email-masked]> 1206478233 +  30
>  EOF
>  
>  check_verify_failure 'detect invalid tag timezone2' \
> -	'^error: char.*: malformed tag timezone$'
> +	'^error:.* badTimezone:'
>  
>  ############################################################
> -# 22. detect invalid tag timezone3
> +# 22. allow invalid tag timezone3 (the maximum is -1200/+1400)
>  
>  cat >tag.sig <<EOF
>  object $head
> @@ -362,8 +360,7 @@ tagger T A Gger <[email-masked]> 1206478233 -1430
>  
>  EOF
>  
> -check_verify_failure 'detect invalid tag timezone3' \
> -	'^error: char.*: malformed tag timezone$'
> +test_expect_mktag_success 'allow invalid tag timezone'
>  
>  ############################################################
>  # 23. detect invalid header entry
> @@ -378,7 +375,7 @@ this line should not be here
>  EOF
>  
>  check_verify_failure 'detect invalid header entry' \
> -	'^error: char.*: trailing garbage in tag header$'
> +	'^error:.* extraHeaderEntry:'
>  
>  cat >tag.sig <<EOF
>  object $head
> @@ -412,7 +409,7 @@ tagger T A Gger <[email-masked]> 1206478233 -0500$space
>  EOF
>  
>  check_verify_failure 'extra whitespace at end of headers' \
> -	'^error: char.*: malformed tag timezone$'
> +	'^error:.* badTimezone:'
>  
>  cat >tag.sig <<EOF
>  object $head
> @@ -422,7 +419,7 @@ tagger T A Gger <[email-masked]> 1206478233 -0500
>  EOF
>  
>  check_verify_failure 'disallow no header / body newline separator' \
> -	'^error: char.*: trailing garbage in tag header$'
> +	'^error:.* extraHeaderEntry:'
>  
>  ############################################################
>  # 24. create valid tag