Re: [PATCH] Add ERR support to smart HTTP

From: Shawn O. Pearce <spearce@spearce.org>
Date: Sep 6, 2010, 14:56 UTC
Message-ID: <20100906145606.GM32601@spearce.org>
In-Reply-To: <EC704F6E-3075-459C-9210-10C234523D80@gmail.com>

Joshua Juran <jjuran@gmail.com> wrote:

Show 15 quoted lines

> On Sep 6, 2010, at 1:49 AM, Jakub Narebski wrote:
>>
>> From what I remember from smart HTTP discussion (during fleshing-out
>> the protocol/exchange details), the fact that errors from git are send
>> with "200 OK" HTTP status are very much conscious decision.  But I  
>> don't
>> remember *why* it was chosen this way.  If I remember correctly it was
>> something about transparent proxies and caches...  Is it documented
>> anywhere?  Can anyone explain it?
>
> I wasn't involved in the decision process, but I suspect it's because  
> HTTP is the transport layer to the Git application.  It's the same logic 
> as trying to log in to a Web application with bogus credentials and 
> getting back a page (HTTP 200 OK) stating that the login failed.  As far 
> as HTTP is concerned, the transaction succeeded.

Exactly correct.

FWIW, I meant for the standard git:// ERR type error to be used here under smart-HTTP. I'm not sure why we need Ilari's original patch at all.

That is, the following will trigger a correct error on the client:

  200 OK
  Content-Type: application/x-git-upload-pack-advertisement

  001e# service=git-upload-pack
  0022ERR You shall not do this

Likewise if you wanted to do this with receive-pack, replace upload with receive above and adjust the pkt-line lengths.

The initial # service= packet is as much part of the "transport layer" as the HTTP 200 OK response is. Its the server saying "Yup, I understood your request correctly. Now here is your error."

Translation is, gitolite (or GitHub, or ...) should be sending back two pkt-lines under smart HTTP, not one.

-- 
Shawn.