Re: SHA1 collision in production repo?! (probably not)

From: Jeff King <peff@peff.net>
Date: Mar 31, 2017, 18:42 UTC
Message-ID: <20170331184223.4im6sollr3vfahxw@sigill.intra.peff.net>
In-Reply-To: <xmqq4ly9z4tx.fsf@gitster.mtv.corp.google.com>

On Fri, Mar 31, 2017 at 11:19:54AM -0700, Junio C Hamano wrote:

Show 8 quoted lines

> > Er, no, that's totally wrong. "status' may be holding the type. It
> > should really be:
> >
> >   return status < 0 ? status : 0;
> 
> Sounds more like it.  The only caller will say "ah, that object is
> not available to us---let's try packs again", which is exactly what
> we want to happen.

Right. Callers cannot distinguish between "did not have it" and "corrupted", but that is no different than the rest of the sha1-file interface.

> There is another bug in the codepath: the assignment to *oi->typep
> in the pre-context must guard against negative status value.  By
> returning an error correctly like you do above, that bug becomes
> more or less irrelevant, though.

I think that is intentional. OBJ_BAD is -1 (though the callers are assuming that error() == OBJ_BAD). And that type gets relayed through sha1_object_info() as the combined type/status return value.

-Peff