Re: [PATCH 2/7] t: introduce tests for unexpected object types
- Date: Fri, 5 Apr 2019 14:31:42 -0400
- From: Jeff King <peff@xxxxxxxx>
- Subject: Re: [PATCH 2/7] t: introduce tests for unexpected object types
On Thu, Apr 04, 2019 at 08:37:44PM -0700, Taylor Blau wrote:
> Let A be the object referenced with an unexpected type, and B be the
> object doing the referencing. Do the following:
>
> - test 'git rev-list --objects A B'. This causes A to be "cached", and
> presents the above scenario.
>
> Likewise, if we have a tree entry that claims to be a tree (for example)
> but points to another object type (say, a blob), there are two ways we
> might find out:
>
> - when we call lookup_tree(), we might find that we've already seen
> the object referenced as another type, in which case we'd get NULL
>
> - we call lookup_tree() successfully, but when we try to read the
> object, we find out it's something else.
>
> We should check that we behave sensibly in both cases (especially
> because it is easy for a malicious actor to provoke one case or the
> other).
I think our pasting together of multiple commits adding the lone/seen
cases ended up in some redundancy in the description. In particular, I'm
not sure what the first paragraph/bullet quoted above is trying to say,
as it corresponds to the second bullet in the later list. Maybe collapse
them together like:
We might hit an unexpected type in two different ways (imagine we have
a tree entry that claims to be a tree but actually points to a blob):
- when we call lookup_tree(), we might find that we've already seen
the object referenced as a blob, in which case we'd get NULL. We
can exercise this with "git rev-list --objects $blob $tree", which
guarantees that the blob will have been parsed before we look in
the tree. These tests are marked as "seen" in the test script.
- we call lookup_tree() successfully, but when we try to read the
object, we find out it's something else. We construct our tests
such that $blob is not otherwise mentioned in $tree. These tests
are marked as "lone" in the script.
-Peff