On Sat, Dec 02, 2017 at 05:02:37PM -0800, Junio C Hamano wrote: > Jeff King <peff@xxxxxxxx> writes: > > > 3. For large inputs (like incoming packfiles), we connect the > > descriptor directly to index-pack or unpack-objects, and they try > > to read to EOF. > > > > For a well-formed pack, I _think_ this would work OK. We'd see the > > end of the pack and quit (there's a check for garbage at the end of > > the pack, but it triggers only for the non-pipe case). > > > > For a truncated input, we'd hang forever rather than report an > > error. > > Hmm. index-pack and/or unpack-objects would be fed directly from > the incoming pipe, they are not told how many bytes to expect (by > design), so they try to read to EOF, which may come after the end of > the pack-stream data, and they write the remaining junk to their > standard output IIRC. > > For a well-formed pack, the above is what should heppen. Yeah, there should be zero bytes of "remaining junk" in the normal well-formed case. And as long as the webserver does not mind us asking to read() a few extra bytes, we are fine (it tells us there are no more bytes available right now). The original problem report with IIS was that it would hang trying to read() that any final EOF, and I don't think that would happen here. I wouldn't be surprised if there are webservers or situations where that extra read() behaves badly (e.g., because it is connected directly to the client socket and the client is trying to pipeline requests or something). > I am having trouble trying to come up with a case where the input > stream is mangled and we cannot detect where the end of the > pack-stream is without reading more than we will be fed through the > pipe, and yet we do not trigger an "we tried to read because the data > we received so far is incomplete, and got an EOF" error. > > Wouldn't "early EOF" trigger in the fill() helper that these two > programs have (but not share X-<)? I think the original problem was the opposite of "early EOF": the other side of the pipe never gives us EOF at all. So imagine the pack is mangled so that the zlib stream of an object never ends, and just keeps asking for more data. Eventually our fill() will block trying to get data that is not there. On an Apache server, the webserver would know there is nothing left to send us and close() the pipe, and we'd get EOF. But on IIS, I think the pipe remains open and we'd just block indefinitely trying to read(). I don't have such a setup to test on, and it's possible I'm mis-interpreting or mis-remembering the discussion around the original patch. -Peff
