[PATCH v2 0/5] mktag tests & fix for-each-ref segfault

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Thu, Apr 01 2021, Jeff King wrote:

> On Thu, Apr 01, 2021 at 03:54:56AM -0400, Jeff King wrote:
>
>> On Wed, Mar 31, 2021 at 10:46:22PM +0200, Ævar Arnfjörð Bjarmason wrote:
>> 
>> > > Neither of those types is the correct one. And the segfault is just a
>> > > bonus! :)
>> > >
>> > > I'd expect similar cases with parsing commit parents and tree pointers.
>> > > And probably tree entries whose modes are wrong.
>> > 
>> > So the segfault happens without my patches,
>> 
>> Yeah, sorry if that was unclear. It is definitely a pre-existing bug.
>
> Here's a patch to fix it. This is mostly orthogonal to your patch
> series. It happens to use a similar recipe to reproduce, but that is not
> the only way to do it, and the fix and the test shouldn't conflict
> textually or semantically.

Here's a proposed v2. We test the same case, but I thought it made
sense to test this more exhaustively.

The v1 will also leave t6300 in a bad state for whoever adds the next
test, trivial to fix with a test_create_repo, but this seems better.

Jeff King (1):
  ref-filter: fix NULL check for parse object failure

Ævar Arnfjörð Bjarmason (4):
  mktag tests: parse out options in helper
  mktag tests: invert --no-strict test
  mktag tests: do fsck on failure
  mktag tests: test for maybe segfaulting for-each-ref

 ref-filter.c     |  2 +-
 t/t3800-mktag.sh | 90 +++++++++++++++++++++++++++++++++++++++---------
 2 files changed, 75 insertions(+), 17 deletions(-)

Range-diff:
-:  ----------- > 1:  45e0f100613 mktag tests: parse out options in helper
-:  ----------- > 2:  dd71740447d mktag tests: invert --no-strict test
-:  ----------- > 3:  688d7456843 mktag tests: do fsck on failure
-:  ----------- > 4:  403024b1cca mktag tests: test for maybe segfaulting for-each-ref
1:  9358541ce1f ! 5:  2ffe8f9fe3c ref-filter: fix NULL check for parse object failure
    @@ Commit message
     
         There are many ways a parse could fail, but most of them are hard to set
         up in the tests (it's easy to make a bogus object, but update-ref will
    -    refuse to point to it). The test here uses a tag which points to a wrong
    -    object type. A parse of just the broken tag object will succeed, but
    -    seeing both tag objects in the same process will lead to a parse error
    -    (since we'll see the pointed-to object as both types).
    +    refuse to point to it).
    +
    +    A minimal stand-alone test can be found at, but let's use the newly
    +    amended t3800-mktag.sh tests to test these cases exhaustively on all
    +    sorts of bad tags.
    +
    +    1. http://lore.kernel.org/git/YGWFGMdGcKeaqCQF@coredump.intra.peff.net
     
         Signed-off-by: Jeff King <peff@peff.net>
    +    Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## ref-filter.c ##
     @@ ref-filter.c: static int get_object(struct ref_array_item *ref, int deref, struct object **obj
    @@ ref-filter.c: static int get_object(struct ref_array_item *ref, int deref, struc
      				free(oi->content);
      			return strbuf_addf_ret(err, -1, _("parse_object_buffer failed on %s for %s"),
     
    - ## t/t6300-for-each-ref.sh ##
    -@@ t/t6300-for-each-ref.sh: test_expect_success 'for-each-ref --ignore-case works on multiple sort keys' '
    - 	test_cmp expect actual
    - '
    + ## t/t3800-mktag.sh ##
    +@@ t/t3800-mktag.sh: check_verify_failure () {
    + 		git -C bad-tag for-each-ref "$tag_ref" >actual &&
    + 		test_cmp expected actual &&
    + 		# segfaults!
    +-		! git -C bad-tag for-each-ref --format="%(*objectname)"
    ++		test_must_fail git -C bad-tag for-each-ref --format="%(*objectname)"
    + 	'
    + }
      
    -+test_expect_success 'for-each-ref reports broken tags' '
    -+	git tag -m "good tag" broken-tag-good HEAD &&
    -+	git cat-file tag broken-tag-good >good &&
    -+	sed s/commit/blob/ <good >bad &&
    -+	bad=$(git hash-object -w -t tag bad) &&
    -+	git update-ref refs/tags/broken-tag-bad $bad &&
    -+	test_must_fail git for-each-ref --format="%(*objectname)" \
    -+		refs/tags/broken-tag-*
    -+'
    -+
    - test_done
-- 
2.31.1.474.g72d45d12706