Re: Possible regression in git-http-backend

From: Jeff King <peff@peff.net>
Date: Apr 27, 2017, 09:31 UTC
Message-ID: <20170427093130.jnbuxuhksi33okry@sigill.intra.peff.net>
In-Reply-To: <CAA2+9hC4d0+igaSV6ziC55=KZGYrZN2A+8-Enhm0F4hExH3m_Q@mail.gmail.com>

On Tue, Apr 25, 2017 at 05:39:16PM -0600, Daniel Wallace wrote:

Show 8 quoted lines

> I am not sure if this is a regression or not, but I wanted to get feedback.
> 
> It looks like this commit changed some behavior in git-http-backend
> 
> https://git.kernel.org/pub/scm/git/git.git/commit/?id=6bc0cb5176a5e42ca4a74e3558e8f0790ed09bb1
> 
> The change that it has made is that it no git-upload-pack hangs when
> uwsgi doesn't close stdin.

Yeah, I think that could be considered an unintended regression. The original code _also_ didn't respect CONTENT_LENGTH and would potentially just keep reading. But because it was reading formatted data, as long as the data was well-formed, it would generally stop reading where the webserver expected it. So it mostly worked; even though there were cases that would hang, they were presumably rare.

People using IIS ran into a similar thing, I think. There was a patch, but it had a lot of problems. I do think it would be reasonable to:

  1. Respect CONTENT_LENGTH if it's set, and never read more than that
     many bytes.

  2. Handle sentinel values for CONTENT_LENGTH that tell us the data is
     streaming in and we must read until EOF (Apache leaves
     CONTENT_LENGTH unset in this case, and apparently IIS sets it to
     -1).

  3. Ideally, do both of those not just in the buffering case added by
     6bc0cb517, but for other input as well. That might be hard, though,
     because I think we literally hand off the descriptor to
     sub-programs for some operations. So even if we just handled the
     one case, that would at least fix any regressions caused by
     6bc0cb517.

Here are links to the previous discussion:

  http://public-inbox.org/git/F0F5A56A22F20D4CB4A03BB8D6658797E260E0E3@SERVER2011.CS-SOFTWARE.local/

  http://public-inbox.org/git/F0F5A56A22F20D4CB4A03BB8D6658797E261A3D6@SERVER2011.CS-SOFTWARE.local/

-Peff