http: normalize curl results for dumb loose and alternates fetches

If the dumb-http walker encounters a 404 when fetching a loose object,
it then looks at any http-alternates for the object. The 404 check is
implemented by missing_target(), which checks not only the http code,
but also that we got an http error from the CURLcode.

That broke when we stopped using CURLOPT_FAILONERROR in 17966c0a63
(http: avoid disconnecting on 404s for loose objects, 2016-07-11), since
our CURLcode will now be CURLE_OK. As a result, fetching over dumb-http
from a repository with alternates could result in Git printing "Unable
to find abcd1234..." and aborting.

We could probably fix this just by loosening missing_target(). However,
there's other code which looks at the curl result, and it would have to
be tweaked as well. Instead, let's just normalize the result the same
way the smart-http code does.

There's a similar case in processing the alternates (where we failover
from "info/http-alternates" to "info/alternates"). We'll give it the
same treatment.

After this patch, we should be hitting all code paths that need this
normalization (notably absent here is the http_pack_request path, but it
does not use FAILONERROR, nor missing_target()).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/http-walker.c b/http-walker.c
index 8ae5d76c6aa216e92c26b875f72b7bb991b6e3cd..745436921d76d5111aec8842ab150e41bc16c5d0 100644 (file)
--- a/http-walker.c
+++ b/http-walker.c
@@ -98,6 +98,11 @@ static void process_object_response(void *callback_data)
        process_http_object_request(obj_req->req);
        obj_req->state = COMPLETE;
 
        process_http_object_request(obj_req->req);
        obj_req->state = COMPLETE;
 

+       normalize_curl_result(&obj_req->req->curl_result,
+                             obj_req->req->http_code,
+                             obj_req->req->errorstr,
+                             sizeof(obj_req->req->errorstr));
+

        /* Use alternates if necessary */
        if (missing_target(obj_req->req)) {
                fetch_alternates(walker, alt->base);
        /* Use alternates if necessary */
        if (missing_target(obj_req->req)) {
                fetch_alternates(walker, alt->base);


@@ -208,6 +213,9 @@ static void process_alternates_response(void *callback_data)
        char *data;
        int i = 0;
 
        char *data;
        int i = 0;
 

+       normalize_curl_result(&slot->curl_result, slot->http_code,
+                             curl_errorstr, sizeof(curl_errorstr));
+

        if (alt_req->http_specific) {
                if (slot->curl_result != CURLE_OK ||
                    !alt_req->buffer->len) {
        if (alt_req->http_specific) {
                if (slot->curl_result != CURLE_OK ||
                    !alt_req->buffer->len) {

diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
index 6d7d88ccc906a4c73285550ec80cd3fe67a764ee..694b77c8556d45ca594375683b1fd2209a346947 100755 (executable)
--- a/t/t5550-http-fetch-dumb.sh
+++ b/t/t5550-http-fetch-dumb.sh
@@ -408,5 +408,21 @@ test_expect_success 'print HTTP error when any intermediate redirect throws erro
        test_i18ngrep "unable to access.*/redir-to/502" stderr
 '
 
        test_i18ngrep "unable to access.*/redir-to/502" stderr
 '
 

+test_expect_success 'fetching via http alternates works' '
+       parent=$HTTPD_DOCUMENT_ROOT_PATH/alt-parent.git &&
+       git init --bare "$parent" &&
+       git -C "$parent" --work-tree=. commit --allow-empty -m foo &&
+       git -C "$parent" update-server-info &&
+       commit=$(git -C "$parent" rev-parse HEAD) &&
+
+       child=$HTTPD_DOCUMENT_ROOT_PATH/alt-child.git &&
+       git init --bare "$child" &&
+       echo "../../alt-parent.git/objects" >"$child/objects/info/alternates" &&
+       git -C "$child" update-ref HEAD $commit &&
+       git -C "$child" update-server-info &&
+
+       git -c http.followredirects=true clone "$HTTPD_URL/dumb/alt-child.git"
+'
+

 stop_httpd
 test_done
 stop_httpd
 test_done