Re: [PATCH 0/7] fix transfer.hiderefs with smart http

From: Duy Nguyen <pclouds@gmail.com>
Date: Mar 13, 2015, 05:21 UTC
Message-ID: <CACsJy8B+dWoq+ide2_-zJuLqFzg2WM3XuyL2B77PrR=PdrBfeQ@mail.gmail.com>
In-Reply-To: <20150313045952.GA18853@peff.net>

On Fri, Mar 13, 2015 at 11:59 AM, Jeff King <peff@peff.net> wrote:

Show 14 quoted lines

> On Fri, Mar 13, 2015 at 12:41:01AM -0400, Jeff King wrote:
>
>> I'm experimenting with using transfer.hiderefs on a server, and it's
>> rather easy to cause a git client to hang when fetching from such a repo
>> over smart http. Details are in the first patch.
>
> A note on this hang. What happens is that upload-pack sees a bogus
> "want" line and calls die(). The client then sits there forever, but I'm
> not exactly sure what it's waiting for.
>
> This series fixes the bug that caused us to erroneously call die() in
> upload-pack, so the hang is "fixed". But there are other reasons to call
> die(); it would probably be a good thing if the client side noticed the
> problem and aborted.

Maybe we could install a custom die handler that also sends "ERR" line to the client before dying. Even with old clients where ERR lines are not recognized, they would see that as a sign of error and abort. The only thing to be careful is not sending ERR while we're in the middle of sending some pkt-line, and that only happens when die() is called inside packet_write() and we can catch that easily. This is for upload-pack only as the client side can also use packet_buf_write(), a bit harder to know if some pkt-line is being sent.

-- 
Duy