From ccbbd8bf66ca88385a34b16abcc1d5a800650d3a Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sun, 24 Mar 2019 08:09:46 -0400 Subject: 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 Signed-off-by: Junio C Hamano --- http-walker.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'http-walker.c') diff --git a/http-walker.c b/http-walker.c index 8ae5d76c6a..745436921d 100644 --- 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; + 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); @@ -208,6 +213,9 @@ static void process_alternates_response(void *callback_data) 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) { -- cgit v1.2.3 From 3d10f72ef8eaa229b285d39b4848aac41e5a8b02 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sun, 24 Mar 2019 08:13:16 -0400 Subject: http: use normalize_curl_result() instead of manual conversion When we switched off CURLOPT_FAILONERROR in 17966c0a63 (http: avoid disconnecting on 404s for loose objects, 2016-07-11), the fetch_object() function started manually handling 404's. Since we now have normalize_curl_result() for use elsewhere, we can use it here as well, shortening the code. Note that we lose the check for http/https in the URL here. None of the other result-normalizing code paths bother with this. Looking at missing_target(), which checks specifically for an FTP-specific CURLcode and "http" code 550, it seems likely that git-over-ftp has been subtly broken since 17966c0a63. This patch does nothing to fix that, but nor should it make anything worse (in fact, it may be slightly better because we'll actually recognize an error as such, rather than assuming CURLE_OK means we actually got some data). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- http-walker.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) (limited to 'http-walker.c') diff --git a/http-walker.c b/http-walker.c index 745436921d..48b1b3a193 100644 --- a/http-walker.c +++ b/http-walker.c @@ -526,17 +526,8 @@ static int fetch_object(struct walker *walker, unsigned char *sha1) req->localfile = -1; } - /* - * we turned off CURLOPT_FAILONERROR to avoid losing a - * persistent connection and got CURLE_OK. - */ - if (req->http_code >= 300 && req->curl_result == CURLE_OK && - (starts_with(req->url, "http://") || - starts_with(req->url, "https://"))) { - req->curl_result = CURLE_HTTP_RETURNED_ERROR; - xsnprintf(req->errorstr, sizeof(req->errorstr), - "HTTP request failed"); - } + normalize_curl_result(&req->curl_result, req->http_code, + req->errorstr, sizeof(req->errorstr)); if (obj_req->state == ABORTED) { ret = error("Request for %s aborted", hex); -- cgit v1.2.3