Re: [PATCH 2/7] t: introduce tests for unexpected object types

From: Jeff King <peff@peff.net>
Date: Apr 5, 2019, 18:31 UTC
Message-ID: <20190405183142.GD2284@sigill.intra.peff.net>
In-Reply-To: <ef6b4f380435d9743a56f47f68c18123bf0a0933.1554435033.git.me@ttaylorr.com>

On Thu, Apr 04, 2019 at 08:37:44PM -0700, Taylor Blau wrote:

Show 19 quoted lines

> 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