Jeff King <peff@xxxxxxxx> writes:
> 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.
>
> -- >8 --
> Subject: [PATCH] ref-filter: fix NULL check for parse object failure
>
> After we run parse_object_buffer() to get an object's contents, we try
> to check that the return value wasn't NULL. However, since our "struct
> object" is a pointer-to-pointer, and we assign like:
>
> *obj = parse_object_buffer(...);
>
> it's not correct to check:
>
> if (!obj)
>
> That will always be true, since our double pointer will continue to
> point to the single pointer (which is itself NULL). This is a regression
> that was introduced by aa46a0da30 (ref-filter: use oid_object_info() to
> get object, 2018-07-17); since that commit we'll segfault on a parse
> failure, as we try to look at the NULL object pointer.
>
> 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).
>
> Signed-off-by: Jeff King <peff@xxxxxxxx>
> ---
> ref-filter.c | 2 +-
> t/t6300-for-each-ref.sh | 10 ++++++++++
> 2 files changed, 11 insertions(+), 1 deletion(-)
Makes sense. Will queue.
> diff --git a/ref-filter.c b/ref-filter.c
> index f0bd32f714..a0adb4551d 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -1608,7 +1608,7 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj
>
> if (oi->info.contentp) {
> *obj = parse_object_buffer(the_repository, &oi->oid, oi->type, oi->size, oi->content, &eaten);
> - if (!obj) {
> + if (!*obj) {
> if (!eaten)
> free(oi->content);
> return strbuf_addf_ret(err, -1, _("parse_object_buffer failed on %s for %s"),
> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
> index cac7f443d0..2e7c32d50c 100755
> --- a/t/t6300-for-each-ref.sh
> +++ b/t/t6300-for-each-ref.sh
> @@ -1134,4 +1134,14 @@ test_expect_success 'for-each-ref --ignore-case works on multiple sort keys' '
> test_cmp expect actual
> '
>
> +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
