On Tue, Dec 6, 2011 at 07:30, Nguyen Thai Ngoc Duy <pclouds@xxxxxxxxx> wrote: > index-pack is called at server side as part of a push. So in theory > the sending side can generate very long delta chains and bring down > the server side. It is also called at client side during fetch. So in theory the server can produce very long delta chains and take down a client. >> Because if not, then the "creator" had better never be configured >> to use a chain depth that the "reader" cannot handle. > > Normal creators (i.e. C Git) use default depth 50 so we should be safe. JGit is also a "normal creator", and it sometimes produces chains deeper than 50. Junio identified a 255 deep chain a week or two ago. Some people have repacked their repositories very aggressively with deeper chains when they are trying to optimize for space and don't access historical revisions very often. I doubt anyone has packed deeper than 120ish intentionally... but we shouldn't assume that in the code. >> This in turn >> imply that there should be a common limit that is supported by all git >> clients and is a documented part of the protocol. (Or the code has to >> be rewritten to use an explicit stack instead of recursion.) > > It's the implementation limitation, not the protocol. If the server > propagates error messages from index-pack back to client (I'm not > sure), then users can adjust --depth to be accepted by server. We > could negotiate the limit over the protocol but not sure we would want > to go that route. What about clients fetching from a server? The client can't change the depth the server sends it. > The troubled code could be rewritten to avoid recursion. However long > delta chains may degrade performance, I'd rather have support to split > long chains (which can be done with --depth at client already) instead > of just recursion elimination. This patch is simpler, so it would be > easier to back port it if someone wants to do so. JGit long ago changed its IndexPack routine to use a manually managed heap based stack instead of recursion, making it immune from overrunning the stack due to a long delta chain. That is probably the cleaner route. But you also have to fix sha1_file.c and its recursion based unpacking of a delta chain... again which JGit fixed a long time ago. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html