Re: [PATCH] Document the HTTP transport protocols

From: Jeff King <peff@peff.net>
Date: Aug 21, 2013, 22:00 UTC
Message-ID: <20130821220021.GA32130@sigill.intra.peff.net>
In-Reply-To: <1377092713-25434-1-git-send-email-pclouds@gmail.com>

On Wed, Aug 21, 2013 at 08:45:13PM +0700, Nguyen Thai Ngoc Duy wrote:

>  On the topic, C Git's (maybe) violations on this spec are:
>  
>   - The client does not strip trailing slashes from $GIT_URL before
>     sending to the server, as described in section "URL Format".

Yeah. We get the basic gist right by not adding an extra "/" if there is already a trailing slash (so you do not have http://host/path//info/refs"). But we do not go out of our way to remove multiple slashes that the user hands out (either at the end or in the middle of the URL). I doubt that it matters in practice.

>   - The client does not check that HTTP status code is either 200 or
>     304 when receiving response in discovering references phase.

We rely on curl's CURLOPT_FAILONERROR to handle errors. And curl handles redirects internally. So yes, we could get a "204" or something weird, but it would almost certainly not pass the other checks (proper content-type, starting with pkt-line, etc). I doubt it's a problem in practice.

We also handle 401 these days, which is not in the document, but obviously makes sense to do (ditto for 407, but I cannot remember if we actually handle that or not; there were patches, but I think they may have been dropped).

>   - The client verifies the first 5 bytes against pattern
>     "^[0-9a-fA-F]{4}#" instead of "^[0-9a-f]{4}#" as described in
>     section "discovering references".

I think this could be counted as "be liberal in what you accept", although I do not know offhand of any implementations that use uppercase. But if it is not true pkt-line we would figure it out pretty quickly anyway.

> [...]

I read through the rest and did not see anything inaccurate. Thanks for working on this.

-Peff