Re: duplicate objects in packfile

On Wed, Aug 14, 2013 at 11:39:34AM -0700, Junio C Hamano wrote:

quoted

Older versions of JGit used to put duplicate objects in a pack, and
IIRC Shawn declared that a bug in the packer and fixed it, so from
that point of view, I think rejecting is probably the right thing,
even though I think warning and continuing is also acceptable and
indeed may be better.

Also repacking may have a funny corner case. I do not recall the
details as the above was a long time ago, but when I was tracking it
down, a delta was made against one copy of the base object, and
referred to it using delta-offset, while there was another copy of
the base object which was found by the bisection search, and from
there on, the inconsistencies between these two (they represent the
same payload, but they are at different offsets in the same pack and
with different in-pack sizes) led to some funky behaviour.

Thanks for the pointer. I found this commit:

  https://eclipse.googlesource.com/jgit/jgit/+/2fbf296fda205446eac11a13abd4fcdb182f28d9

which is presumably what you're thinking of.

I did not run into the problem described in my case, but presumably I
did not have a delta cycle between the multiple versions. In theory we
should find the same copy of the object each time we search, but there
are enough code paths to access the objects that I would not be
surprised if such funkiness is still triggerable, including infinite
loops.

That makes me inclined to teach index-pack to reject duplicate objects
in a single pack in order to prevent denial-of-service attacks. We can
potentially make them work in all code paths, but given that nobody
should be doing this legitimately, rejecting the duplicates outright
keeps our attack surface small, and nobody but attackers or users of
broken implementations should care.

-Peff