Re: [PATCH] remote-curl: don't hang when a server dies before any output

[Jeff King <peff@peff.net>]

On Mon, Nov 14, 2016 at 01:19:49PM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > So something like this. It turned out to be a lot uglier than I had
> > hoped because we get fed the data from curl in odd-sized chunks, so we
> > need a state machine.
> 
> It is unfortunate that we have to snoop the protocol like this to
> infer an error, but I do not think we can do better than that
> approach.  FWIW, I did not find the logic in update_pktline_state()
> you wrote ugly at all.
> 
> Having to assume that the end of each round from the other end must
> be a FLUSH does feel somewhat ugly and brittle, though.

Yeah, I agree. The other option is to signal to fetch-pack that we saw
EOF on the server request, and then we do not have to snoop; it knows
where in the protocol state it is at.

But doing that is a little tricky. We could send our own flush packet,
but that doesn't quite work. When fetch-pack sees a flush it does not
know if it is the server's flush and it should wait for our flush, or if
the server hung up prematurely and the flush is from us. :)

The "elegant" solution is to just wrap the server's data in another set
of pktlines. So the server flush becomes "00080000" (8 bytes including
our pktline, and then 4 bytes for the flush packet). But wrapping each
pktline gets a little hairy. What happens when the server sends us a
pktline of LARGE_PACKET_MAX? We can't wrap that without sending our own
pktline that's LARGE_PACKET_MAX+4.

One solution is that pktlines 0001-0003 are unused and impossible for
data. So we could send the server pktlines as-is, and use pktline 0001
as our special signal for "end of http request". A server shouldn't ever
send that, and if they did (perhaps to try something malicious), it
would just cause fetch-pack to think they prematurely ended the
conversation.

Hmm. I suppose that doesn't quite work, though. One of the problems is
that the server sends a partial pktline, in which case our special flush
packet would get gobbled up as data for that broken pktline.

So you really do need framing or some other out-of-band communication,
or resolve yourself to snooping in remote-curl.

-Peff