Skip to content

Commit 3151a5f

Browse files
gitstergitster
committed
Merge branch 'jk/http-walker-status-fix'
dumb-http walker has been updated to share more error recovery strategy with the normal codepath. * jk/http-walker-status-fix: http: use normalize_curl_result() instead of manual conversion http: normalize curl results for dumb loose and alternates fetches http: factor out curl result code normalization
2 parents ec08c4f + 3d10f72 commit 3151a5f

File tree

4 files changed

+47
-17
lines changed

4 files changed

+47
-17
lines changed

‎http-walker.c‎

Lines changed: 10 additions & 11 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) {
@@ -518,17 +526,8 @@ static int fetch_object(struct walker *walker, unsigned char *sha1)
518526
req->localfile = -1;
519527
}
520528

521-
/*
522-
* we turned off CURLOPT_FAILONERROR to avoid losing a
523-
* persistent connection and got CURLE_OK.
524-
*/
525-
if (req->http_code >= 300 && req->curl_result == CURLE_OK &&
526-
(starts_with(req->url, "http://") ||
527-
starts_with(req->url, "https://"))) {
528-
req->curl_result = CURLE_HTTP_RETURNED_ERROR;
529-
xsnprintf(req->errorstr, sizeof(req->errorstr),
530-
"HTTP request failed");
531-
}
529+
normalize_curl_result(&req->curl_result, req->http_code,
530+
req->errorstr, sizeof(req->errorstr));
532531

533532
if (obj_req->state == ABORTED) {
534533
ret = error("Request for %s aborted", hex);

‎http.c‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1544,7 +1544,8 @@ char *get_remote_object_url(const char *url, const char *hex,
15441544
return strbuf_detach(&buf, NULL);
15451545
}
15461546

1547-
static int handle_curl_result(struct slot_results *results)
1547+
void normalize_curl_result(CURLcode *result, long http_code,
1548+
char *errorstr, size_t errorlen)
15481549
{
15491550
/*
15501551
* If we see a failing http code with CURLE_OK, we have turned off
@@ -1554,19 +1555,24 @@ static int handle_curl_result(struct slot_results *results)
15541555
* Likewise, if we see a redirect (30x code), that means we turned off
15551556
* redirect-following, and we should treat the result as an error.
15561557
*/
1557-
if (results->curl_result == CURLE_OK &&
1558-
results->http_code >= 300) {
1559-
results->curl_result = CURLE_HTTP_RETURNED_ERROR;
1558+
if (*result == CURLE_OK && http_code >= 300) {
1559+
*result = CURLE_HTTP_RETURNED_ERROR;
15601560
/*
15611561
* Normally curl will already have put the "reason phrase"
15621562
* from the server into curl_errorstr; unfortunately without
15631563
* FAILONERROR it is lost, so we can give only the numeric
15641564
* status code.
15651565
*/
1566-
xsnprintf(curl_errorstr, sizeof(curl_errorstr),
1566+
xsnprintf(errorstr, errorlen,
15671567
"The requested URL returned error: %ld",
1568-
results->http_code);
1568+
http_code);
15691569
}
1570+
}
1571+
1572+
static int handle_curl_result(struct slot_results *results)
1573+
{
1574+
normalize_curl_result(&results->curl_result, results->http_code,
1575+
curl_errorstr, sizeof(curl_errorstr));
15701576

15711577
if (results->curl_result == CURLE_OK) {
15721578
credential_approve(&http_auth);

‎http.h‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,15 @@ static inline int missing__target(int code, int result)
136136

137137
#define missing_target(a) missing__target((a)->http_code, (a)->curl_result)
138138

139+
/*
140+
* Normalize curl results to handle CURL_FAILONERROR (or lack thereof). Failing
141+
* http codes have their "result" converted to CURLE_HTTP_RETURNED_ERROR, and
142+
* an appropriate string placed in the errorstr buffer (pass curl_errorstr if
143+
* you don't have a custom buffer).
144+
*/
145+
void normalize_curl_result(CURLcode *result, long http_code, char *errorstr,
146+
size_t errorlen);
147+
139148
/* Helpers for modifying and creating URLs */
140149
extern void append_remote_object_url(struct strbuf *buf, const char *url,
141150
const char *hex,

‎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)