{"thread":{"id":"45801","subject":"Possible regression in git-http-backend","startedAt":"2017-04-25T23:39:24Z","lastAt":"2017-04-27T09:31:38Z","messageCount":2,"participants":["Daniel Wallace","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"317876","messageId":"CAA2+9hC4d0+igaSV6ziC55=KZGYrZN2A+8-Enhm0F4hExH3m_Q@mail.gmail.com","threadId":"45801","inReplyTo":null,"subject":"Possible regression in git-http-backend","fromName":"Daniel Wallace","fromEmail":"dwallace@saltstack.com","sentAt":"2017-04-25T23:39:16Z","receivedAt":"2017-04-25T23:39:24Z","isPatch":false,"sender":{"key":"dwallace@saltstack.com","avatar":null},"body":"I am not sure if this is a regression or not, but I wanted to get feedback.\n\nIt looks like this commit changed some behavior in git-http-backend\n\nhttps://git.kernel.org/pub/scm/git/git.git/commit/?id=6bc0cb5176a5e42ca4a74e3558e8f0790ed09bb1\n\nThe change that it has made is that it no git-upload-pack hangs when\nuwsgi doesn't close stdin.\n\nhttp://lists.unbit.it/pipermail/uwsgi/2016-April/008440.html\n\nThis was the only place I could find this discussed online for uwsgi\n(but I did find several blog posts comments mentioning the issue).  It\nlooks like it was worked around in uwsgi >= 2.0.13 by adding the\nsetting `cgi-close-stdin-on-eof = true`.  But there was some\ndiscussion last year that this shouldn't be needed based on the RFC\nfor cgi?\n\nhttp://lists.unbit.it/pipermail/uwsgi/2016-May/008462.html\nhttp://web.archive.org/web/20100212094332/http://hoohoo.ncsa.illinois.edu/cgi/in.html\n\n\"\"\"\nThe server will send CONTENT_LENGTH bytes on this file descriptor.\nRemember that it will give the CONTENT_TYPE of the data as well. The\nserver is in no way obligated to send end-of-file after the script reads\nCONTENT_LENGTH bytes.\n\"\"\"\n\nHere is a gist for providing a config setup, I used the latest nginx\nfrom the nginx repo for centos 7 and uwsgi 2.0.14 from epel.  The\nsetup requires starting nginx.service and uwsgi.service and creating a\nbare repo in /srv/git/ and running\n\nhttps://gist.github.com/gtmanfred/8b2e26b07db3c75094d0607048b2c828\n\nThanks,\nDaniel\n"},{"id":"318066","messageId":"20170427093130.jnbuxuhksi33okry@sigill.intra.peff.net","threadId":"45801","inReplyTo":"CAA2+9hC4d0+igaSV6ziC55=KZGYrZN2A+8-Enhm0F4hExH3m_Q@mail.gmail.com","subject":"Re: Possible regression in git-http-backend","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-04-27T09:31:30Z","receivedAt":"2017-04-27T09:31:38Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 25, 2017 at 05:39:16PM -0600, Daniel Wallace wrote:\n\n> I am not sure if this is a regression or not, but I wanted to get feedback.\n> \n> It looks like this commit changed some behavior in git-http-backend\n> \n> https://git.kernel.org/pub/scm/git/git.git/commit/?id=6bc0cb5176a5e42ca4a74e3558e8f0790ed09bb1\n> \n> The change that it has made is that it no git-upload-pack hangs when\n> uwsgi doesn't close stdin.\n\nYeah, I think that could be considered an unintended regression. The\noriginal code _also_ didn't respect CONTENT_LENGTH and would potentially\njust keep reading. But because it was reading formatted data, as long as\nthe data was well-formed, it would generally stop reading where the\nwebserver expected it. So it mostly worked; even though there were cases\nthat would hang, they were presumably rare.\n\nPeople using IIS ran into a similar thing, I think. There was a patch,\nbut it had a lot of problems. I do think it would be reasonable to:\n\n  1. Respect CONTENT_LENGTH if it's set, and never read more than that\n     many bytes.\n\n  2. Handle sentinel values for CONTENT_LENGTH that tell us the data is\n     streaming in and we must read until EOF (Apache leaves\n     CONTENT_LENGTH unset in this case, and apparently IIS sets it to\n     -1).\n\n  3. Ideally, do both of those not just in the buffering case added by\n     6bc0cb517, but for other input as well. That might be hard, though,\n     because I think we literally hand off the descriptor to\n     sub-programs for some operations. So even if we just handled the\n     one case, that would at least fix any regressions caused by\n     6bc0cb517.\n\nHere are links to the previous discussion:\n\n  http://public-inbox.org/git/F0F5A56A22F20D4CB4A03BB8D6658797E260E0E3@SERVER2011.CS-SOFTWARE.local/\n\n  http://public-inbox.org/git/F0F5A56A22F20D4CB4A03BB8D6658797E261A3D6@SERVER2011.CS-SOFTWARE.local/\n\n-Peff\n"}]}