On Fri, Mar 22, 2019 at 07:02:41AM +0100, Heinrich Schuchardt wrote:

> Initial cloning via
> 
>     git clone http://git.denx.de/u-boot-efi.git
> 
> fails with git version 2.21.0.196.g041f5ea1cf
> 
> git version 2.1.4 works fine.
> 
> Bisection points to this first bad commit:
> 17966c0a63d25b1cc2dd1e98d30873e643bd581f
> http: avoid disconnecting on 404s for loose objects
> 
> Please, fix the regression.
I think 17966c0a63d needed to massage the CURLcode in more places, since
missing_target() cares about it. The patch below gets me past the error
you saw, but later dies with:

  error: Request for 283d73eda8011371f12daa2cd9fc3107705637dc aborted
  error: Unable to find 283d73eda8011371f12daa2cd9fc3107705637dc under 
https://git.denx.de/u-boot-efi.git
  Cannot obtain needed object 283d73eda8011371f12daa2cd9fc3107705637dc
  while processing commit d9af3c87df93e1a8126b1a52adf8db978e9a0d40.
  error: fetch failed.

However, so does the parent of 17966c0a63d. So I don't know if the
u-boot-efi repo is just broken, or if there's some other different bug
at play.

-Peff

diff --git a/http-walker.c b/http-walker.c
index 48f2df4d31..83e23e787c 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -93,6 +93,9 @@ 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);
+
        /* Use alternates if necessary */
        if (missing_target(obj_req->req)) {
                fetch_alternates(walker, alt->base);
@@ -189,6 +192,8 @@ static void process_alternates_response(void *callback_data)
        char *data;
        int i = 0;
 
+       normalize_curl_result(&slot->curl_result, slot->http_code);
+
        if (alt_req->http_specific) {
                if (slot->curl_result != CURLE_OK ||
                    !alt_req->buffer->len) {
@@ -488,14 +493,7 @@ 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 == 404 && req->curl_result == CURLE_OK &&
-                       (starts_with(req->url, "http://";) ||
-                        starts_with(req->url, "https://";)))
-               req->curl_result = CURLE_HTTP_RETURNED_ERROR;
+       normalize_curl_result(&req->curl_result, req->http_code);
 
        if (obj_req->state == ABORTED) {
                ret = error("Request for %s aborted", hex);
diff --git a/http.c b/http.c
index 6f12661a14..1d60a0064a 100644
--- a/http.c
+++ b/http.c
@@ -1116,16 +1116,15 @@ char *get_remote_object_url(const char *url, const char 
*hex,
        return strbuf_detach(&buf, NULL);
 }
 
-static int handle_curl_result(struct slot_results *results)
+void normalize_curl_result(CURLcode *result, long http_code)
 {
        /*
         * If we see a failing http code with CURLE_OK, we have turned off
         * FAILONERROR (to keep the server's custom error response), and should
         * translate the code into failure here.
         */
-       if (results->curl_result == CURLE_OK &&
-           results->http_code >= 400) {
-               results->curl_result = CURLE_HTTP_RETURNED_ERROR;
+       if (*result == CURLE_OK && http_code >= 400) {
+               *result = CURLE_HTTP_RETURNED_ERROR;
                /*
                 * Normally curl will already have put the "reason phrase"
                 * from the server into curl_errorstr; unfortunately without
@@ -1134,8 +1133,13 @@ static int handle_curl_result(struct slot_results 
*results)
                 */
                snprintf(curl_errorstr, sizeof(curl_errorstr),
                         "The requested URL returned error: %ld",
-                        results->http_code);
+                        http_code);
        }
+}
+
+static int handle_curl_result(struct slot_results *results)
+{
+       normalize_curl_result(&results->curl_result, results->http_code);
 
        if (results->curl_result == CURLE_OK) {
                credential_approve(&http_auth);
diff --git a/http.h b/http.h
index 36f558bfb3..1c75e99570 100644
--- a/http.h
+++ b/http.h
@@ -129,6 +129,12 @@ static inline int missing__target(int code, int result)
 
 #define missing_target(a) missing__target((a)->http_code, (a)->curl_result)
 
+/*
+ * Yuck, we cannot just pass the struct containing these because we store the
+ * results in various structs.
+ */
+void normalize_curl_result(CURLcode *result, long http_code);
+
 /* Helpers for modifying and creating URLs */
 extern void append_remote_object_url(struct strbuf *buf, const char *url,
                                     const char *hex,

Reply via email to