# Possible regression in git-http-backend 2 messages from 2017-04-25 to 2017-04-27. Participants: Daniel Wallace, Jeff King. Thread: https://gitlist.dev/t/45801 ## Daniel Wallace, 2017-04-25 23:39 Subject: Possible regression in git-http-backend Message-ID: URL: https://gitlist.dev/e/CAA2%2B9hC4d0%2BigaSV6ziC55%3DKZGYrZN2A%2B8-Enhm0F4hExH3m_Q%40mail.gmail.com ``` 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. http://lists.unbit.it/pipermail/uwsgi/2016-April/008440.html This was the only place I could find this discussed online for uwsgi (but I did find several blog posts comments mentioning the issue). It looks like it was worked around in uwsgi >= 2.0.13 by adding the setting `cgi-close-stdin-on-eof = true`. But there was some discussion last year that this shouldn't be needed based on the RFC for cgi? http://lists.unbit.it/pipermail/uwsgi/2016-May/008462.html http://web.archive.org/web/20100212094332/http://hoohoo.ncsa.illinois.edu/cgi/in.html """ The server will send CONTENT_LENGTH bytes on this file descriptor. Remember that it will give the CONTENT_TYPE of the data as well. The server is in no way obligated to send end-of-file after the script reads CONTENT_LENGTH bytes. """ Here is a gist for providing a config setup, I used the latest nginx from the nginx repo for centos 7 and uwsgi 2.0.14 from epel. The setup requires starting nginx.service and uwsgi.service and creating a bare repo in /srv/git/ and running https://gist.github.com/gtmanfred/8b2e26b07db3c75094d0607048b2c828 Thanks, Daniel ``` ## Jeff King, 2017-04-27 09:31 Subject: Re: Possible regression in git-http-backend Message-ID: <20170427093130.jnbuxuhksi33okry@sigill.intra.peff.net> URL: https://gitlist.dev/e/20170427093130.jnbuxuhksi33okry%40sigill.intra.peff.net In-Reply-To: ``` On Tue, Apr 25, 2017 at 05:39:16PM -0600, Daniel Wallace wrote: > 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 ```