{"thread":{"id":"24987","subject":"[PATCH] Add ERR support to smart HTTP","startedAt":"2010-09-05T17:30:15Z","lastAt":"2010-09-08T14:36:34Z","messageCount":18,"participants":["Ilari Liusvaara","Jonathan Nieder","Ævar Arnfjörð Bjarmason","Jakub Narebski","Sitaram Chamarty","Joshua Juran","Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"149976","messageId":"1283707815-32038-1-git-send-email-ilari.liusvaara@elisanet.fi","threadId":"24987","inReplyTo":null,"subject":"[PATCH] Add ERR support to smart HTTP","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2010-09-05T17:30:15Z","receivedAt":"2010-09-05T17:30:15Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"All \"true smart transports\" support ERR packets, allowing server\nto send back error message explaining reasons for refusing the\nrequest instead of just rudely closing connection without any error.\n\nHowever, since smart HTTP isn't \"true smart transport\", but instead\ndumb one from git main executable perspective, smart HTTP needs to\nimplement its own version of this.\n\nNow that Gitolite supports HTTP too, it needs to be able to send\nerror messages for authorization failures back to client so that's\none probable user for this feature.\n\nThe error is sent as '# ERR \" and must be the\nfirst packet in response. The reason for putting the '#' there is that\nold git versions will interpret that as invalid server response and\nprint (at least the first line of) the error together with complaint\nof invalid response (mangling it a bit but it will still be understandable,\nin manner similar to existing smart transport ERR messages).\n\nThus for example server response:\n\n\"0031# ERR W access for foo/alice/a1 DENIED to bob\"\n\nWill cause the following to be printed:\n\n\"fatal: remote error: W access for foo/alice/a1 DENIED to bob\"\n\nIf the git version is old and doesn't support this feature, then the\nmessage will be:\n\n\"fatal: invalid server response; got '# ERR W access for foo/alice/a1\nDENIED to bob'\"\n\nWhich is at least undertandable.\n\nSigned-off-by: Ilari Liusvaara \n---\n remote-curl.c | 2 ++\n 1 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 24fbb9a..46fa971 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -153,6 +153,8 @@ static struct discovery* discover_refs(const char *service)\n \n \t\tif (packet_get_line(&buffer, &last->buf, &last->len) <= 0)\n \t\t\tdie(\"%s has invalid packet header\", refs_url);\n+\t\tif (buffer.len >= 6 && !strncmp(buffer.buf, \"# ERR \", 6))\n+\t\t\tdie(\"remote error: %s\", buffer.buf + 6);\n \t\tif (buffer.len && buffer.buf[buffer.len - 1] == '\\n')\n \t\t\tstrbuf_setlen(&buffer, buffer.len - 1);\n \n-- \n1.7.2.4.g27652\n"},{"id":"149978","messageId":"20100905174105.GB14020@burratino","threadId":"24987","inReplyTo":"1283707815-32038-1-git-send-email-ilari.liusvaara@elisanet.fi","subject":"Re: [PATCH] Add ERR support to smart HTTP","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-09-05T17:41:06Z","receivedAt":"2010-09-05T17:41:06Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ilari Liusvaara wrote:\n\n> Thus for example server response:\n> \n> \"0031# ERR W access for foo/alice/a1 DENIED to bob\"\n> \n> Will cause the following to be printed:\n> \n> \"fatal: remote error: W access for foo/alice/a1 DENIED to bob\"\n> \n> If the git version is old and doesn't support this feature, then the\n> message will be:\n> \n> \"fatal: invalid server response; got '# ERR W access for foo/alice/a1\n> DENIED to bob'\"\n\nYippee! Thanks, Ilari.\n\nFor this specific error, why can't gitolite use an HTTP response code?\nShould http-backend be using ERR is some places, too, a la [1]?\n\nJonathan\nwho would like to find time to write a test case for \"git daemon\" any\nday now\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/145456/focus=145573\n"},{"id":"149986","messageId":"20100905184929.GA32735@LK-Perkele-V2.elisa-laajakaista.fi","threadId":"24987","inReplyTo":"20100905174105.GB14020@burratino","subject":"Re: [PATCH] Add ERR support to smart HTTP","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2010-09-05T18:49:29Z","receivedAt":"2010-09-05T18:49:29Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"On Sun, Sep 05, 2010 at 12:41:06PM -0500, Jonathan Nieder wrote:\n> \n> For this specific error, why can't gitolite use an HTTP response code?\n> Should http-backend be using ERR is some places, too, a la [1]?\n\nI wanted this error to be sent in manner that causes old clients to print\nthe error, even if sightly mangled (the ERR of \"true smart transports\" does\nalso have this property).\n\nAFAIK, HTTP errors don't have descriptions printed.\n \n-Ilari\n"},{"id":"149988","messageId":"AANLkTinoEp55C3=hF6-LO5fwn2FpMxBZry-=2B6kvXc1@mail.gmail.com","threadId":"24987","inReplyTo":"20100905184929.GA32735@LK-Perkele-V2.elisa-laajakaista.fi","subject":"Re: [PATCH] Add ERR support to smart HTTP","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-09-05T19:27:30Z","receivedAt":"2010-09-05T19:27:30Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Sun, Sep 5, 2010 at 18:49, Ilari Liusvaara\n wrote:\n\n> AFAIK, HTTP errors don't have descriptions printed.\n\nI don't know if this applies here but HTTP error codes can come with\nany free-form \\n-delimited string:\n\n HTTP/1.1 402 You Must Build Additional Pylons\n"},{"id":"149996","messageId":"20100905201142.GE14497@burratino","threadId":"24987","inReplyTo":"20100905184929.GA32735@LK-Perkele-V2.elisa-laajakaista.fi","subject":"Re: [PATCH] Add ERR support to smart HTTP","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-09-05T20:11:43Z","receivedAt":"2010-09-05T20:11:43Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ilari Liusvaara wrote:\n> On Sun, Sep 05, 2010 at 12:41:06PM -0500, Jonathan Nieder wrote:\n \n>> For this specific error, why can't gitolite use an HTTP response code?\n>> Should http-backend be using ERR is some places, too, a la [1]?\n[...]\n> AFAIK, HTTP errors don't have descriptions printed.\n\nThanks for the explanation. Makes sense.\n\n $ git clone http://example.com/nonsense.git\n fatal: http://example.com/nonsense.git/info/refs not found: did you run git update-server-info on the server?\n"},{"id":"150001","messageId":"20100905212129.GA1419@LK-Perkele-V2.elisa-laajakaista.fi","threadId":"24987","inReplyTo":"AANLkTinoEp55C3=hF6-LO5fwn2FpMxBZry-=2B6kvXc1@mail.gmail.com","subject":"Re: [PATCH] Add ERR support to smart HTTP","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2010-09-05T21:21:29Z","receivedAt":"2010-09-05T21:21:29Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"On Sun, Sep 05, 2010 at 07:27:30PM +0000, Ævar Arnfjörð Bjarmason wrote:\n> On Sun, Sep 5, 2010 at 18:49, Ilari Liusvaara\n> wrote:\n> \n> > AFAIK, HTTP errors don't have descriptions printed.\n> \n> I don't know if this applies here but HTTP error codes can come with\n> any free-form \\n-delimited string:\n> \n> HTTP/1.1 402 You Must Build Additional Pylons\n\nYes, they can, but remote-curl doesn't print those error explanations\n(just tried).\n\n-Ilari\n"},{"id":"150003","messageId":"m3pqwrnay2.fsf@localhost.localdomain","threadId":"24987","inReplyTo":"AANLkTinoEp55C3=hF6-LO5fwn2FpMxBZry-=2B6kvXc1@mail.gmail.com","subject":"Re: [PATCH] Add ERR support to smart HTTP","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-09-05T21:22:23Z","receivedAt":"2010-09-05T21:22:23Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> On Sun, Sep 5, 2010 at 18:49, Ilari Liusvaara\n> wrote:\n> \n> > AFAIK, HTTP errors don't have descriptions printed.\n> \n> I don't know if this applies here but HTTP error codes can come with\n> any free-form \\n-delimited string:\n> \n> HTTP/1.1 402 You Must Build Additional Pylons\n\nAnd you can also send more detailed description in the *body* (and not\nonly HTTP headers) of HTTP response, though I don't know if git does\nthat.\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"150024","messageId":"AANLkTikGiekZGNffaraHK-waBt7wH84jujM_uh3cw46y@mail.gmail.com","threadId":"24987","inReplyTo":"m3pqwrnay2.fsf@localhost.localdomain","subject":"Re: [PATCH] Add ERR support to smart HTTP","fromName":"Sitaram Chamarty","fromEmail":"sitaramc@gmail.com","sentAt":"2010-09-06T01:04:54Z","receivedAt":"2010-09-06T01:04:54Z","isPatch":true,"sender":{"key":"sitaramc@gmail.com","avatar":"https://avatars.githubusercontent.com/u/43316?v=4"},"body":"On Mon, Sep 6, 2010 at 2:52 AM, Jakub Narebski wrote:\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> On Sun, Sep 5, 2010 at 18:49, Ilari Liusvaara\n>> wrote:\n>>\n>> > AFAIK, HTTP errors don't have descriptions printed.\n>>\n>> I don't know if this applies here but HTTP error codes can come with\n>> any free-form \\n-delimited string:\n>>\n>>     HTTP/1.1 402 You Must Build Additional Pylons\n>\n> And you can also send more detailed description in the *body* (and not\n> only HTTP headers) of HTTP response, though I don't know if git does\n> that.\n\nI'm going to try the patch that Ilari sent when I get to work but to\nanswer this sub-thread about HTTP status codes and messages, none of\nthat gets printed by the curl code, as Ilari pointed out. Here's a\ntranscript:\n\nNotice the 403 on this one... I do send that back:\n\n06:30:37 sitaram@sita-lt:http-test $ git clone `genurl alice foo/sitaram/try1`\nCloning into try1...\nerror: The requested URL returned error: 403 while accessing\nhttp://alice:alice@127.0.0.1/git/foo/sitaram/try1/info/refs\n\nfatal: HTTP request failed\n\nYou can see the actual message cleanly here:\n\n06:30:46 sitaram@sita-lt:http-test $ curl\nhttp://alice:alice@127.0.0.1/git/foo/sitaram/try1/info/refs\nERR R access for foo/sitaram/try1 DENIED to alice\n\n\nAnd here you can see the text part of the HTTP/1.1 NNN status line:\n\n06:31:04 sitaram@sita-lt:http-test $ curl -v\nhttp://alice:alice@127.0.0.1/git/foo/sitaram/try1/info/refs\n* About to connect() to 127.0.0.1 port 80 (#0)\n* Trying 127.0.0.1... connected\n* Connected to 127.0.0.1 (127.0.0.1) port 80 (#0)\n* Server auth using Basic with user 'alice'\n> GET /git/foo/sitaram/try1/info/refs HTTP/1.1\n> Authorization: Basic YWxpY2U6YWxpY2U=\n> User-Agent: curl/7.20.1 (i386-redhat-linux-gnu) libcurl/7.20.1 NSS/3.12.6.2 zlib/1.2.3 libidn/1.16 libssh2/1.2.4\n> Host: 127.0.0.1\n> Accept: */*\n>\n< HTTP/1.1 403 error - gitolite\n< Date: Mon, 06 Sep 2010 01:02:23 GMT\n< Server: Apache/2.2.16 (Fedora)\n< Expires: Fri, 01 Jan 1980 00:00:00 GMT\n< Pragma: no-cache\n< Cache-Control: no-cache, max-age=0, must-revalidate\n< Connection: close\n< Transfer-Encoding: chunked\n< Content-Type: text/plain; charset=UTF-8\n<\nERR R access for foo/sitaram/try1 DENIED to alice\n* Closing connection #0\n"},{"id":"150042","messageId":"AANLkTinTFWHWU1vCnDa-c3p5g+y7wnH9A8fieowQHU5z@mail.gmail.com","threadId":"24987","inReplyTo":"AANLkTikGiekZGNffaraHK-waBt7wH84jujM_uh3cw46y@mail.gmail.com","subject":"Re: [PATCH] Add ERR support to smart HTTP","fromName":"Sitaram Chamarty","fromEmail":"sitaramc@gmail.com","sentAt":"2010-09-06T05:45:57Z","receivedAt":"2010-09-06T05:45:57Z","isPatch":true,"sender":{"key":"sitaramc@gmail.com","avatar":"https://avatars.githubusercontent.com/u/43316?v=4"},"body":"On Mon, Sep 6, 2010 at 6:34 AM, Sitaram Chamarty wrote:\n> On Mon, Sep 6, 2010 at 2:52 AM, Jakub Narebski wrote:\n>> Ævar Arnfjörð Bjarmason writes:\n>>\n>>> On Sun, Sep 5, 2010 at 18:49, Ilari Liusvaara\n>>> wrote:\n>>>\n>>> > AFAIK, HTTP errors don't have descriptions printed.\n>>>\n>>> I don't know if this applies here but HTTP error codes can come with\n>>> any free-form \\n-delimited string:\n>>>\n>>>     HTTP/1.1 402 You Must Build Additional Pylons\n>>\n>> And you can also send more detailed description in the *body* (and not\n>> only HTTP headers) of HTTP response, though I don't know if git does\n>> that.\n\nturns out all this was moot. It was *because* I was using something\nother than \"200 OK\" that the user was not seeing the message. Ilari's\npatch just makes the message *look* better/cleaner, but I still have\nto send it out with a \"200 OK\" status.\n\nThat was... a surprise :-)\n\nThanks all\n\nsitaram\n"},{"id":"150055","messageId":"AANLkTim-yfjACM_VqR2oaOaB=mLtD=+3QiXiWpwcdH1z@mail.gmail.com","threadId":"24987","inReplyTo":"AANLkTinTFWHWU1vCnDa-c3p5g+y7wnH9A8fieowQHU5z@mail.gmail.com","subject":"Re: [PATCH] Add ERR support to smart HTTP","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-09-06T08:45:14Z","receivedAt":"2010-09-06T08:45:14Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Mon, Sep 6, 2010 at 05:45, Sitaram Chamarty wrote:\n> On Mon, Sep 6, 2010 at 6:34 AM, Sitaram Chamarty wrote:\n>> On Mon, Sep 6, 2010 at 2:52 AM, Jakub Narebski wrote:\n>>> Ævar Arnfjörð Bjarmason writes:\n>>>\n>>>> On Sun, Sep 5, 2010 at 18:49, Ilari Liusvaara\n>>>> wrote:\n>>>>\n>>>> > AFAIK, HTTP errors don't have descriptions printed.\n>>>>\n>>>> I don't know if this applies here but HTTP error codes can come with\n>>>> any free-form \\n-delimited string:\n>>>>\n>>>>     HTTP/1.1 402 You Must Build Additional Pylons\n>>>\n>>> And you can also send more detailed description in the *body* (and not\n>>> only HTTP headers) of HTTP response, though I don't know if git does\n>>> that.\n>\n> turns out all this was moot.  It was *because* I was using something\n> other than \"200 OK\" that the user was not seeing the message.  Ilari's\n> patch just makes the message *look* better/cleaner, but I still have\n> to send it out with a \"200 OK\" status.\n\nYou can still send it out with a \"200 \" if you\nwant to give a warning/error even on 200.\n"},{"id":"150056","messageId":"201009061049.38546.jnareb@gmail.com","threadId":"24987","inReplyTo":"AANLkTinTFWHWU1vCnDa-c3p5g+y7wnH9A8fieowQHU5z@mail.gmail.com","subject":"Re: [PATCH] Add ERR support to smart HTTP","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-09-06T08:49:36Z","receivedAt":"2010-09-06T08:49:36Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Mon, Sep 6, 2010, Sitaram Chamarty wrote:\n> On Mon, Sep 6, 2010 at 6:34 AM, Sitaram Chamarty wrote:\n>> On Mon, Sep 6, 2010 at 2:52 AM, Jakub Narebski wrote:\n>>> Ævar Arnfjörð Bjarmason writes:\n>>>\n>>>> On Sun, Sep 5, 2010 at 18:49, Ilari Liusvaara\n>>>> wrote:\n>>>>\n>>>>> AFAIK, HTTP errors don't have descriptions printed.\n>>>>\n>>>> I don't know if this applies here but HTTP error codes can come with\n>>>> any free-form \\n-delimited string:\n>>>>\n>>>>     HTTP/1.1 402 You Must Build Additional Pylons\n>>>\n>>> And you can also send more detailed description in the *body* (and not\n>>> only HTTP headers) of HTTP response, though I don't know if git does\n>>> that.\n> \n> turns out all this was moot. It was *because* I was using something\n> other than \"200 OK\" that the user was not seeing the message. Ilari's\n> patch just makes the message *look* better/cleaner, but I still have\n> to send it out with a \"200 OK\" status.\n> \n> That was... a surprise :-)\n\nFrom what I remember from smart HTTP discussion (during fleshing-out\nthe protocol/exchange details), the fact that errors from git are send\nwith \"200 OK\" HTTP status are very much conscious decision. But I don't\nremember *why* it was chosen this way. If I remember correctly it was\nsomething about transparent proxies and caches... Is it documented\nanywhere? Can anyone explain it?\n\nNevertheless I think it would be a good idea to make *client* more\naccepting, which means:\n1. Printing full HTTP status, and not only HTTP return / error code;\n perhaps only if it is non-standard, and perhaps only in --verbose\n mode.\n2. If message body contains ERR line, print error message even if the\n HTTP status was other than \"200 OK\". To be \"generous in what you\n receive\" (well, kind of).\n3. In verbose mode, if body of HTTP error message (not \"HTTP OK\")\n exists and does not contain ERR line (e.g. an error from web server),\n print it in full (perhaps indented).\n\nI think that neither of the above would lead to leaking sensitive \ninformation.\n\nWhat do you think?\n-- \nJakub Narebski\nPoland\n"},{"id":"150057","messageId":"EC704F6E-3075-459C-9210-10C234523D80@gmail.com","threadId":"24987","inReplyTo":"201009061049.38546.jnareb@gmail.com","subject":"Re: [PATCH] Add ERR support to smart HTTP","fromName":"Joshua Juran","fromEmail":"jjuran@gmail.com","sentAt":"2010-09-06T09:15:01Z","receivedAt":"2010-09-06T09:15:01Z","isPatch":true,"sender":{"key":"jjuran@gmail.com","avatar":null},"body":"On Sep 6, 2010, at 1:49 AM, Jakub Narebski wrote:\n\n> On Mon, Sep 6, 2010, Sitaram Chamarty wrote:\n>> On Mon, Sep 6, 2010 at 6:34 AM, Sitaram Chamarty \n>> wrote:\n>>> On Mon, Sep 6, 2010 at 2:52 AM, Jakub Narebski \n>>> wrote:\n>>>> Ævar Arnfjörð Bjarmason writes:\n>>>>\n>>>>> On Sun, Sep 5, 2010 at 18:49, Ilari Liusvaara\n>>>>> wrote:\n>>>>>\n>>>>>> AFAIK, HTTP errors don't have descriptions printed.\n>>>>>\n>>>>> I don't know if this applies here but HTTP error codes can come \n>>>>> with\n>>>>> any free-form \\n-delimited string:\n>>>>>\n>>>>> HTTP/1.1 402 You Must Build Additional Pylons\n>>>>\n>>>> And you can also send more detailed description in the *body* \n>>>> (and not\n>>>> only HTTP headers) of HTTP response, though I don't know if git \n>>>> does\n>>>> that.\n>>\n>> turns out all this was moot. It was *because* I was using something\n>> other than \"200 OK\" that the user was not seeing the message. \n>> Ilari's\n>> patch just makes the message *look* better/cleaner, but I still have\n>> to send it out with a \"200 OK\" status.\n>>\n>> That was... a surprise :-)\n>\n> From what I remember from smart HTTP discussion (during fleshing-out\n> the protocol/exchange details), the fact that errors from git are send\n> with \"200 OK\" HTTP status are very much conscious decision. But I \n> don't\n> remember *why* it was chosen this way. If I remember correctly it was\n> something about transparent proxies and caches... Is it documented\n> anywhere? Can anyone explain it?\n\nI wasn't involved in the decision process, but I suspect it's because \nHTTP is the transport layer to the Git application. It's the same \nlogic as trying to log in to a Web application with bogus credentials \nand getting back a page (HTTP 200 OK) stating that the login failed. \nAs far as HTTP is concerned, the transaction succeeded.\n\nJosh\n"},{"id":"150083","messageId":"AANLkTi=jqpspQvz6--CGfVEpP8raD7RpNGgMs6KabXfS@mail.gmail.com","threadId":"24987","inReplyTo":"201009061049.38546.jnareb@gmail.com","subject":"Re: [PATCH] Add ERR support to smart HTTP","fromName":"Sitaram Chamarty","fromEmail":"sitaramc@gmail.com","sentAt":"2010-09-06T14:24:31Z","receivedAt":"2010-09-06T14:24:31Z","isPatch":true,"sender":{"key":"sitaramc@gmail.com","avatar":"https://avatars.githubusercontent.com/u/43316?v=4"},"body":"On Mon, Sep 6, 2010 at 2:19 PM, Jakub Narebski wrote:\n> On Mon, Sep 6, 2010, Sitaram Chamarty wrote:\n>> On Mon, Sep 6, 2010 at 6:34 AM, Sitaram Chamarty wrote:\n>>> On Mon, Sep 6, 2010 at 2:52 AM, Jakub Narebski wrote:\n>>>> Ævar Arnfjörð Bjarmason writes:\n>>>>\n>>>>> On Sun, Sep 5, 2010 at 18:49, Ilari Liusvaara\n>>>>> wrote:\n>>>>>\n>>>>>> AFAIK, HTTP errors don't have descriptions printed.\n>>>>>\n>>>>> I don't know if this applies here but HTTP error codes can come with\n>>>>> any free-form \\n-delimited string:\n>>>>>\n>>>>>     HTTP/1.1 402 You Must Build Additional Pylons\n>>>>\n>>>> And you can also send more detailed description in the *body* (and not\n>>>> only HTTP headers) of HTTP response, though I don't know if git does\n>>>> that.\n>>\n>> turns out all this was moot.  It was *because* I was using something\n>> other than \"200 OK\" that the user was not seeing the message.  Ilari's\n>> patch just makes the message *look* better/cleaner, but I still have\n>> to send it out with a \"200 OK\" status.\n>>\n>> That was... a surprise :-)\n>\n> From what I remember from smart HTTP discussion (during fleshing-out\n> the protocol/exchange details), the fact that errors from git are send\n> with \"200 OK\" HTTP status are very much conscious decision.  But I don't\n> remember *why* it was chosen this way.  If I remember correctly it was\n> something about transparent proxies and caches...  Is it documented\n> anywhere?  Can anyone explain it?\n>\n> Nevertheless I think it would be a good idea to make *client* more\n> accepting, which means:\n> 1. Printing full HTTP status, and not only HTTP return / error code;\n>   perhaps only if it is non-standard, and perhaps only in --verbose\n>   mode.\n> 2. If message body contains ERR line, print error message even if the\n>   HTTP status was other than \"200 OK\".  To be \"generous in what you\n>   receive\" (well, kind of).\n> 3. In verbose mode, if body of HTTP error message (not \"HTTP OK\")\n>   exists and does not contain ERR line (e.g. an error from web server),\n>   print it in full (perhaps indented).\n>\n> I think that neither of the above would lead to leaking sensitive\n> information.\n\nI didn't understand this bit about leaking info. If the bits are\ncoming into my machine I know what they are anyway (or am able to find\nout easily enough, even if git itself isn't showing them to me).\nWhere's the leak?\n\nAnd I do see the point that Joshua made that the 200 reflects HTTP\nstatus, not git status. Makes sense, and answers my original\nquestion...\n\nregards\n\nsitaram\n"},{"id":"150084","messageId":"20100906145606.GM32601@spearce.org","threadId":"24987","inReplyTo":"EC704F6E-3075-459C-9210-10C234523D80@gmail.com","subject":"Re: [PATCH] Add ERR support to smart HTTP","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2010-09-06T14:56:06Z","receivedAt":"2010-09-06T14:56:06Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Joshua Juran wrote:\n> On Sep 6, 2010, at 1:49 AM, Jakub Narebski wrote:\n>>\n>> From what I remember from smart HTTP discussion (during fleshing-out\n>> the protocol/exchange details), the fact that errors from git are send\n>> with \"200 OK\" HTTP status are very much conscious decision. But I \n>> don't\n>> remember *why* it was chosen this way. If I remember correctly it was\n>> something about transparent proxies and caches... Is it documented\n>> anywhere? Can anyone explain it?\n>\n> I wasn't involved in the decision process, but I suspect it's because \n> HTTP is the transport layer to the Git application. It's the same logic \n> as trying to log in to a Web application with bogus credentials and \n> getting back a page (HTTP 200 OK) stating that the login failed. As far \n> as HTTP is concerned, the transaction succeeded.\n\nExactly correct.\n\nFWIW, I meant for the standard git:// ERR type error to be used\nhere under smart-HTTP. I'm not sure why we need Ilari's original\npatch at all.\n\nThat is, the following will trigger a correct error on the client:\n\n 200 OK\n Content-Type: application/x-git-upload-pack-advertisement\n\n 001e# service=git-upload-pack\n 0022ERR You shall not do this\n\nLikewise if you wanted to do this with receive-pack, replace upload\nwith receive above and adjust the pkt-line lengths.\n\nThe initial # service= packet is as much part of the \"transport\nlayer\" as the HTTP 200 OK response is. Its the server saying \"Yup,\nI understood your request correctly. Now here is your error.\"\n\nTranslation is, gitolite (or GitHub, or ...) should be sending back\ntwo pkt-lines under smart HTTP, not one.\n\n-- \nShawn.\n"},{"id":"150093","messageId":"201009061832.00512.jnareb@gmail.com","threadId":"24987","inReplyTo":"AANLkTi=jqpspQvz6--CGfVEpP8raD7RpNGgMs6KabXfS@mail.gmail.com","subject":"Re: [PATCH] Add ERR support to smart HTTP","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-09-06T16:31:58Z","receivedAt":"2010-09-06T16:31:58Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Sitaram Chamarty wrote:\n> On Mon, Sep 6, 2010 at 2:19 PM, Jakub Narebski wrote:\n\n> > Nevertheless I think it would be a good idea to make *client* more\n> > accepting, which means:\n> > 1. Printing full HTTP status, and not only HTTP return / error code;\n> >   perhaps only if it is non-standard, and perhaps only in --verbose\n> >   mode.\n> > 2. If message body contains ERR line, print error message even if the\n> >   HTTP status was other than \"200 OK\".  To be \"generous in what you\n> >   receive\" (well, kind of).\n> > 3. In verbose mode, if body of HTTP error message (not \"HTTP OK\")\n> >   exists and does not contain ERR line (e.g. an error from web server),\n> >   print it in full (perhaps indented).\n> >\n> > I think that neither of the above would lead to leaking sensitive\n> > information.\n> \n> I didn't understand this bit about leaking info. If the bits are\n> coming into my machine I know what they are anyway (or am able to find\n> out easily enough, even if git itself isn't showing them to me).\n> Where's the leak?\n\nI meant here that programs (including git) do not provide full details\nabout error condition, especially if it has to do womething with \nauthentication, to avoid leaking sensitive information (like e.g. \nsaying that username + password combination is invalid, instead of\ntelling which one is wrong, to avoid disclosing usernames).\n\n-- \nJakub Narebski\nPoland\n"},{"id":"150101","messageId":"AANLkTikmU9_Vg2+=73yjPyaaDSqk73Bvs1HyNjFDWqNY@mail.gmail.com","threadId":"24987","inReplyTo":"20100906145606.GM32601@spearce.org","subject":"Re: [PATCH] Add ERR support to smart HTTP","fromName":"Sitaram Chamarty","fromEmail":"sitaramc@gmail.com","sentAt":"2010-09-06T17:59:02Z","receivedAt":"2010-09-06T17:59:02Z","isPatch":true,"sender":{"key":"sitaramc@gmail.com","avatar":"https://avatars.githubusercontent.com/u/43316?v=4"},"body":"On Mon, Sep 6, 2010 at 8:26 PM, Shawn O. Pearce wrote:\n\n> That is, the following will trigger a correct error on the client:\n>\n>  200 OK\n>  Content-Type: application/x-git-upload-pack-advertisement\n>\n>  001e# service=git-upload-pack\n>  0022ERR You shall not do this\n\nare those counts accurate for the specific example you show or just made up?\n\nIt seems the first line has a count in hex that includes the newline\nat the end, and the second one has a count in decimal that does not\ninclude the newline nor even the 4-digits plus \"ERR\"\n\n> Likewise if you wanted to do this with receive-pack, replace upload\n> with receive above and adjust the pkt-line lengths.\n\nok... what about all the other service commands? like /info/refs?\nWhat should I put there?\n\nSorry if I'm being stupid but I couldn't find this info anywhere (my C\ngrokking isn't as good as it used to be anyway). I've tried all sorts\nof combinations of sending out two such lines -- variations on length,\n\\r, \\n, \\r\\n, neither, etc etc but I can't get the correct output.\n\nAlso, experimenting with making the update hook die similarly and\nwireshark-ing the responde does not show similar pattern coming\nthrough.\n\nIf you could point me to some place that says the precise format,\nincluding \\r\\n, I'd greatly appreciate it.\n\nThanks,\n\nSitaram\n"},{"id":"150106","messageId":"20100906181921.GN32601@spearce.org","threadId":"24987","inReplyTo":"AANLkTikmU9_Vg2+=73yjPyaaDSqk73Bvs1HyNjFDWqNY@mail.gmail.com","subject":"Re: [PATCH] Add ERR support to smart HTTP","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2010-09-06T18:19:21Z","receivedAt":"2010-09-06T18:19:21Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Sitaram Chamarty wrote:\n> On Mon, Sep 6, 2010 at 8:26 PM, Shawn O. Pearce wrote:\n> > That is, the following will trigger a correct error on the client:\n> >\n> >  200 OK\n> >  Content-Type: application/x-git-upload-pack-advertisement\n> >\n> >  001e# service=git-upload-pack\n> >  0022ERR You shall not do this\n> \n> are those counts accurate for the specific example you show or just made up?\n> \n> It seems the first line has a count in hex that includes the newline\n> at the end, and the second one has a count in decimal that does not\n> include the newline nor even the 4-digits plus \"ERR\"\n\nFeh. I can't count. The first count is correct. The second count\nshould also be 001e. I guess that should be obvious by just looking\nat the two lines, they are equal in length. :-)\n \n> > Likewise if you wanted to do this with receive-pack, replace upload\n> > with receive above and adjust the pkt-line lengths.\n> \n> ok... what about all the other service commands? like /info/refs?\n> What should I put there?\n\nThe only other command that matters is info/refs.\n\nFor smart clients, its what I said above.\n\nFor dumb clients, you have to use some sort of HTTP error status\nthat isn't 404. Dumb clients pre-1.6.6 use a curl error message\nbuffer to print out an error. But they don't check the format of\ninfo/refs at all, and skip over garbage and/or interpret garbage\nas valid input. So we can't use a hack like \"ERR blah\" to even\ntrigger a parsing failure.\n\n-- \nShawn.\n"},{"id":"150297","messageId":"AANLkTinPb+3rwUg5mwUN+HBkuj2SzLpiG=hCp+WOfu0S@mail.gmail.com","threadId":"24987","inReplyTo":"20100906181921.GN32601@spearce.org","subject":"Re: [PATCH] Add ERR support to smart HTTP","fromName":"Sitaram Chamarty","fromEmail":"sitaramc@gmail.com","sentAt":"2010-09-08T14:36:34Z","receivedAt":"2010-09-08T14:36:34Z","isPatch":true,"sender":{"key":"sitaramc@gmail.com","avatar":"https://avatars.githubusercontent.com/u/43316?v=4"},"body":"2010/9/6 Shawn O. Pearce :\n> Sitaram Chamarty wrote:\n>> On Mon, Sep 6, 2010 at 8:26 PM, Shawn O. Pearce wrote:\n>> > That is, the following will trigger a correct error on the client:\n>> >\n>> >  200 OK\n>> >  Content-Type: application/x-git-upload-pack-advertisement\n>> >\n>> >  001e# service=git-upload-pack\n>> >  0022ERR You shall not do this\n>>\n>> are those counts accurate for the specific example you show or just made up?\n>>\n>> It seems the first line has a count in hex that includes the newline\n>> at the end, and the second one has a count in decimal that does not\n>> include the newline nor even the 4-digits plus \"ERR\"\n>\n> Feh.  I can't count.  The first count is correct.  The second count\n> should also be 001e.  I guess that should be obvious by just looking\n> at the two lines, they are equal in length.  :-)\n\nSummary of offline discussion with Shawn, so that others can find it if needed:\n\nThe first packet (after the HTTP headers of course) should be\n\nXXXX# service=git-upload-pack\\n\n\n(or the same with upload replaced by receive). These are the service\nnames passed in the service query parameter (/info/refs?service=...).\n\nThe XXXX is a hex length of the whole thing. For these two specific\ncases, they will be 1E and 1F.\n\nThis should be followed by \"0000\" (with no \\n at the end). This is a\nspecial packet that means \"this sequence of messages is done\".\n\nAfter this you can send any error messages, as follows:\n\nXXXXERR your message\\n\n\nwhere again the XXXX is a hex count of the whole string (including 4\nfor the count itself, 4 for \"ERR \", and a newline if you add it).\n\n-- \nSitaram\n"}]}