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.
> 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
