On Fri, Mar 31, 2017 at 02:16:29PM -0700, Junio C Hamano wrote: > Before we forget... > > -- >8 -- > From: Jeff King <peff@xxxxxxxx> > > When sha1_loose_object_info() finds that a loose object file cannot > be stat(2)ed or mmap(2)ed, it returns -1 to signal an error to the > caller. However, if it found that the loose object file is corrupt > and the object data cannot be used from it, it forgets to return -1. > > This can confuse the caller, who may be lead to mistakenly think > that there is a loose object and possibly gets an incorrect type and > size from the function. The SHA-1 collision detection codepath, for > example, when it gets an object over the wire and tries to see the > data is the same as what is available as a loose object locally, can > get confused when the loose object is correupted due to this bug. Unfortunately this isn't quite right. I was able to reproduce the problem that Lars saw, and this patch doesn't fix it. So here's a two-patch series. The first fixes the problem described above, along with a simpler test that demonstrates it. The second fixes Lars's problem on top. I know you're heading out for a week, so I'll post it now for review, and then hold and repost when you get back. [1/2]: sha1_loose_object_info: return error for corrupted objects [2/2]: index-pack: detect local corruption in collision check builtin/index-pack.c | 2 ++ sha1_file.c | 2 +- t/t1060-object-corruption.sh | 24 ++++++++++++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) -Peff