Re: Cloning from sites with 404 overridden

[Nick Hengeveld <nickh@reactrix.com>]

On Wed, Mar 22, 2006 at 11:22:14AM -0800, Junio C Hamano wrote:

> You probably need only one bit here,...
> ... and note if that is an HTML document or not.

/me smacks self...

> However the patch would not help when such a server also did a
> "Sorry, did you mistype the URL?" HTML response, and I was
> wondering how typical that would be.

Seems like there are three cases to worry about:

1) the server returns a 200 status and a text/html response instead of a
   404, and the server's default content type is not text/html
2) the server returns a 200 status and a text/html response instead of a
   404, and the server's default content type is text/html
3) the server returns a corrupt object from the repository

I don't think there's a way to distinguish between #2 and #3, so all we
can really do is display as helpful an error message as possible.

We can detect #1 if there has been a previous successful loose object
transfer by tracking whether the repo's default content type is
text/html.  In such a case should http-fetch behave as if the server
returned 404?  If there have been no successful loose object transfers,
we'd have to respond as with #2.  This approach could potentially break
if requests are load-balanced to servers with different
misconfigurations - but I think trying to detect that is bending
backwards a little too far.

On a related note, I noticed that http-fetch will continue to try
inflating/sha1_updating the response after an inflate error has been
detected.  It's probably not a huge deal, but we could just error out
immediately at that point or at least stop the unnecessary processing.

Something like this?  Tested by cloning
http://digilander.libero.it/mcostalba/scm/qgit.git


[PATCH] http-fetch: try to detect 404s from misconfigured servers

Some HTTP server environments return a 200 status and text/html error
document or a redirect to one rather than a 404 status if a loose
object does not exist.  This patch tries to detect such a response
and treat it as a 404.

Signed-off-by: Nick Hengeveld <nickh@reactrix.com>


---

 http-fetch.c |   24 ++++++++++++++++++++++--
 1 files changed, 22 insertions(+), 2 deletions(-)

ab97429c5b0a4b4466ee0072f75706399e42b675
diff --git a/http-fetch.c b/http-fetch.c
index dc67218..bb75050 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -16,6 +16,7 @@ struct alt_base
 {
 	char *base;
 	int got_indices;
+	int default_html_content_type;
 	struct packed_git *packs;
 	struct alt_base *next;
 };
@@ -41,6 +42,7 @@ struct object_request
 	CURLcode curl_result;
 	char errorstr[CURL_ERROR_SIZE];
 	long http_code;
+	char html_content_type;
 	unsigned char real_sha1[20];
 	SHA_CTX c;
 	z_stream stream;
@@ -249,6 +251,9 @@ static void finish_object_request(struct
 		unlink(obj_req->tmpfile);
 		return;
 	}
+	if (obj_req->repo->default_html_content_type == -1)
+		obj_req->repo->default_html_content_type =
+			obj_req->html_content_type;
 	obj_req->rename =
 		move_temp_to_file(obj_req->tmpfile, obj_req->filename);
 
@@ -258,9 +263,15 @@ static void finish_object_request(struct
 
 static void process_object_response(void *callback_data)
 {
+	char *content_type;
 	struct object_request *obj_req =
 		(struct object_request *)callback_data;
 
+	curl_easy_getinfo(obj_req->slot->curl, CURLINFO_CONTENT_TYPE,
+			  &content_type);
+	if (content_type && !strcmp(content_type, "text/html"))
+		obj_req->html_content_type = 1;
+
 	obj_req->curl_result = obj_req->slot->curl_result;
 	obj_req->http_code = obj_req->slot->http_code;
 	obj_req->slot = NULL;
@@ -340,6 +351,7 @@ void prefetch(unsigned char *sha1)
 	memcpy(newreq->sha1, sha1, 20);
 	newreq->repo = alt;
 	newreq->url = NULL;
+	newreq->html_content_type = 0;
 	newreq->local = -1;
 	newreq->state = WAITING;
 	snprintf(newreq->filename, sizeof(newreq->filename), "%s", filename);
@@ -539,6 +551,7 @@ static void process_alternates_response(
 				newalt->next = NULL;
 				newalt->base = target;
 				newalt->got_indices = 0;
+				newalt->default_html_content_type = -1;
 				newalt->packs = NULL;
 				while (tail->next != NULL)
 					tail = tail->next;
@@ -835,8 +848,14 @@ static int fetch_object(struct alt_base 
 				    obj_req->errorstr, obj_req->curl_result,
 				    obj_req->http_code, hex);
 	} else if (obj_req->zret != Z_STREAM_END) {
-		corrupt_object_found++;
-		ret = error("File %s (%s) corrupt", hex, obj_req->url);
+		if (obj_req->html_content_type &&
+		    !obj_req->repo->default_html_content_type)
+			ret = -1; /* Be silent, looks like a 404 */
+		else {
+			corrupt_object_found++;
+			ret = error("File %s (%s) corrupt",
+				    sha1_to_hex(obj_req->sha1), obj_req->url);
+		}
 	} else if (memcmp(obj_req->sha1, obj_req->real_sha1, 20)) {
 		ret = error("File %s has bad hash", hex);
 	} else if (obj_req->rename < 0) {
@@ -985,6 +1004,7 @@ int main(int argc, char **argv)
 	alt = xmalloc(sizeof(*alt));
 	alt->base = url;
 	alt->got_indices = 0;
+	alt->default_html_content_type = -1;
 	alt->packs = NULL;
 	alt->next = NULL;
 
-- 
1.2.4.gb1bc1d-dirty