Skip to content

Commit ccbbd8b

Browse files
peffgitster
authored andcommitted
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 17966c0 (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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent a3722bc commit ccbbd8b

File tree

2 files changed

+24
-0
lines changed

2 files changed

+24
-0
lines changed

http-walker.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,11 @@ static void process_object_response(void *callback_data)
9898
process_http_object_request(obj_req->req);
9999
obj_req->state = COMPLETE;
100100

101+
normalize_curl_result(&obj_req->req->curl_result,
102+
obj_req->req->http_code,
103+
obj_req->req->errorstr,
104+
sizeof(obj_req->req->errorstr));
105+
101106
/* Use alternates if necessary */
102107
if (missing_target(obj_req->req)) {
103108
fetch_alternates(walker, alt->base);
@@ -208,6 +213,9 @@ static void process_alternates_response(void *callback_data)
208213
char *data;
209214
int i = 0;
210215

216+
normalize_curl_result(&slot->curl_result, slot->http_code,
217+
curl_errorstr, sizeof(curl_errorstr));
218+
211219
if (alt_req->http_specific) {
212220
if (slot->curl_result != CURLE_OK ||
213221
!alt_req->buffer->len) {

t/t5550-http-fetch-dumb.sh

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,5 +408,21 @@ test_expect_success 'print HTTP error when any intermediate redirect throws erro
408408
test_i18ngrep "unable to access.*/redir-to/502" stderr
409409
'
410410

411+
test_expect_success 'fetching via http alternates works' '
412+
parent=$HTTPD_DOCUMENT_ROOT_PATH/alt-parent.git &&
413+
git init --bare "$parent" &&
414+
git -C "$parent" --work-tree=. commit --allow-empty -m foo &&
415+
git -C "$parent" update-server-info &&
416+
commit=$(git -C "$parent" rev-parse HEAD) &&
417+
418+
child=$HTTPD_DOCUMENT_ROOT_PATH/alt-child.git &&
419+
git init --bare "$child" &&
420+
echo "../../alt-parent.git/objects" >"$child/objects/info/alternates" &&
421+
git -C "$child" update-ref HEAD $commit &&
422+
git -C "$child" update-server-info &&
423+
424+
git -c http.followredirects=true clone "$HTTPD_URL/dumb/alt-child.git"
425+
'
426+
411427
stop_httpd
412428
test_done

0 commit comments

Comments
 (0)