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@xxxxxxxxxxxxx-SOFTWARE.local/ http://public-inbox.org/git/F0F5A56A22F20D4CB4A03BB8D6658797E261A3D6@xxxxxxxxxxxxx-SOFTWARE.local/ -Peff