ref-filter: detect broken tags when dereferencing them

Users can ask git-for-each-ref(1) to peel tags and return information of
the tagged object by adding an asterisk to the format, like for example
"%(*$objectname)". If so, git-for-each-ref(1) peels that object to the
first non-tag object and then returns its values.

As mentioned in preceding commits, it can happen that the tagged object
type and the claimed object type differ, effectively resulting in a
corrupt tag. git-for-each-ref(1) would notice this mismatch, print an
error and then bail out when trying to peel the tag.

But we only notice this corruption in some very specific edge cases!
While we have a test in "t/for-each-ref-tests.sh" that verifies the
above scenario, this test is specifically crafted to detect the issue at
hand. Namely, we create two tags:

  - One tag points to a specific object with the correct type.

  - The other tag points to the *same* object with a different type.

The fact that both tags point to the same object is important here:
`peel_object()` wouldn't notice the corruption if the tagged objects
were different.

The root cause is that `peel_object()` calls `lookup_${type}()`
eventually, where the type is the same type declared in the tag object.
Consequently, when we have two tags pointing to the same object but with
different declared types we'll call two different lookup functions. The
first lookup will store the object with an unverified type A, whereas
the second lookup will try to look up the object with a different
unverified type B. And it is only now that we notice the discrepancy in
object types, even though type A could've already been the wrong type.

Fix the issue by verifying the object type in `populate_value()`. With
this change we'll also notice type mismatches when only dereferencing a
tag once.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/ref-filter.c b/ref-filter.c
index 9a8ed8c8fc1f3b6bccb4ff8fe5a9123e43c1d9db..c54025d6b4c4cd2fd227fb3b690854635ccea025 100644 (file)
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -2581,7 +2581,8 @@ static int populate_value(struct ref_array_item *ref, struct strbuf *err)
        if (need_tagged) {
                if (!is_null_oid(&ref->peeled_oid)) {
                        oidcpy(&oi_deref.oid, &ref->peeled_oid);
-               } else if (!peel_object(the_repository, &oi.oid, &oi_deref.oid, 0)) {
+               } else if (!peel_object(the_repository, &oi.oid, &oi_deref.oid,
+                                       PEEL_OBJECT_VERIFY_OBJECT_TYPE)) {
                        /* We managed to peel the object ourselves. */
                } else {
                        die("bad tag");

diff --git a/t/for-each-ref-tests.sh b/t/for-each-ref-tests.sh
index e3ad19298accdeeb393b65fceb9e6bf97541cc8b..4593be5fd544a8359d458302b3807d6afee7b2de 100644 (file)
--- a/t/for-each-ref-tests.sh
+++ b/t/for-each-ref-tests.sh
@@ -1809,7 +1809,9 @@ test_expect_success "${git_for_each_ref} reports broken tags" '
        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-*
+               refs/tags/broken-tag-* &&
+       test_must_fail ${git_for_each_ref} --format="%(*objectname)" \
+               refs/tags/broken-tag-bad
 '
 
 test_expect_success 'set up tag with signature and no blank lines' '