Re: pack operation is thrashing my server

[Nicolas Pitre <nico@cam.org>]

On Sat, 6 Sep 2008, Junio C Hamano wrote:

> Linus Torvalds <torvalds@linux-foundation.org> writes:
> 
> > The reason? 
> >
> > Right now we depend on "avail_out" also making zlib understand to stop 
> > looking at the input stream. Sad, but true - we don't know or care about 
> > the compressed size of the object, only the uncompressed size. So in 
> > unpack_compressed_entry(), we simply set the output length, and expect 
> > zlib to stop when it's sufficient.
> >
> > Which it does - but the patch kind of violates that whole design.
> >
> > Now, it so happens that things seem to work, probably because the zlib 
> > format does have enough synchronization in it to not try to continue past 
> > the end _anyway_, but I think this makes the patch be of debatable value.
> 
> I thought the fact we do check the status with Z_STREAM_END means that we
> do already expect and rely on zlib to know where the end of input stream
> is, and stop there (otherwise we say something fishy is going on and we
> error out), and it was part of the design, not just "so happens" and "has
> enough synch ... _anyway_".
> 
> If input zlib stream were corrupted and it detected the end of stream too
> early, then check of "stream.total_out != size" would fail even though we
> would see "st == Z_STREAM_END".  If input stream were corrupted and it
> went past the end marker, we will read past the end and into some garbage
> that is the in-pack header of the next object representation, but zlib
> shouldn't go berserk even in that case, and would stop after filling the
> slop you allocated in the buffer --- we would detect the situation from
> stream.total_out != size and most likely st != Z_STREAM_END in such a
> case.

Unless I'm missing something, I think your analysis is right and 
everything should be safe.


Nicolas