git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 3/3] http: use normalize_curl_result() instead of manual conversion

From
Jeff King <peff@peff.net>
Date
Mar 24, 2019, 12:13 UTC
Message-ID
<20190324121316.GC312@sigill.intra.peff.net>
In-Reply-To
<20190324120757.GA18684@sigill.intra.peff.net>

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 <peff@peff.net>
---
I can't bring myself to care too much about whether git-over-ftp works
with alternates, but if somebody wants to dig into it, be my guest. I
suspect other bits may be broken, too, as we check http_code in several
places and assume it has some sensible http-based number in it (notably,
for 401 triggering authentication).

It may not even work at all. I didn't try (and I'd be kind of surprised if somebody is actually using it in practice). I'm content to let it bit-rot unless somebody using it shows up to report a bug.

(I'd also be fine with officially deprecating it. But then I kind of feel the same way about the dumb-http code).

 http-walker.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)
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);
-- 
2.21.0.705.g64cb90f133
Previous: Jeff King
Message 14 of 14 in “[BUG] Cloning with git HEAD fails for some repositories”
  1. Heinrich SchuchardtMar 22, 2019
  2. Jeff KingMar 22, 2019
  3. Wolfgang DenkMar 22, 2019
  4. Jeff KingMar 22, 2019
  5. Eric WongMar 22, 2019
  6. Heinrich SchuchardtMar 22, 2019
  7. Eric WongMar 22, 2019
  8. Heinrich SchuchardtMar 22, 2019
  9. Eric WongMar 22, 2019
  10. Jeff KingMar 23, 2019
  11. 0/3 fix dumb-http fetch with alternatesJeff King, Mar 24, 2019
  12. 1/3 http: factor out curl result code normalizationJeff King, Mar 24, 2019
  13. 2/3 http: normalize curl results for dumb loose and alternates fetchesJeff King, Mar 24, 2019
  14. 3/3 http: use normalize_curl_result() instead of manual conversionJeff King, Mar 24, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.