git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: Jann Horn <jannh@google.com>
Subject: [PATCH 6/6] http-walker: complain about non-404 loose object errors
Date: Thu, 1 Dec 2016 04:04:37 -0500	[thread overview]
Message-ID: <20161201090436.cwh2pxio7afowtfs@sigill.intra.peff.net> (raw)
In-Reply-To: <20161201090336.xjbb47bublfcpglo@sigill.intra.peff.net>

Since commit 17966c0a6 (http: avoid disconnecting on 404s
for loose objects, 2016-07-11), we turn off curl's
FAILONERROR option and instead manually deal with failing
HTTP codes.

However, the logic to do so only recognizes HTTP 404 as a
failure. This is probably the most common result, but if we
were to get another code, the curl result remains CURLE_OK,
and we treat it as success. We still end up detecting the
failure when we try to zlib-inflate the object (which will
fail), but instead of reporting the HTTP error, we just
claim that the object is corrupt.

Instead, let's catch anything in the 300's or above as an
error (300's are redirects which are not an error at the
HTTP level, but are an indication that we've explicitly
disabled redirects, so we should treat them as such; we
certainly don't have the resulting object content).

Note that we also fill in req->errorstr, which we didn't do
before. Without FAILONERROR, curl will not have filled this
in, and it will remain a blank string. This never mattered
for the 404 case, because in the logic below we hit the
"missing_target()" branch and print nothing. But for other
errors, we'd want to say _something_, if only to fill in the
blank slot in the error message.

Signed-off-by: Jeff King <peff@peff.net>
---
 http-walker.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/http-walker.c b/http-walker.c
index 25a8b1ad4..c2f81cd6a 100644
--- a/http-walker.c
+++ b/http-walker.c
@@ -482,10 +482,13 @@ static int fetch_object(struct walker *walker, unsigned char *sha1)
 	 * 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 &&
+	if (req->http_code >= 300 && req->curl_result == CURLE_OK &&
 			(starts_with(req->url, "http://") ||
-			 starts_with(req->url, "https://")))
+			 starts_with(req->url, "https://"))) {
 		req->curl_result = CURLE_HTTP_RETURNED_ERROR;
+		xsnprintf(req->errorstr, sizeof(req->errorstr),
+			  "HTTP request failed");
+	}
 
 	if (obj_req->state == ABORTED) {
 		ret = error("Request for %s aborted", hex);
-- 
2.11.0.319.g1f4e1e0

  parent reply	other threads:[~2016-12-01  9:11 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-01  9:03 [PATCH 0/6] restricting http redirects Jeff King
2016-12-01  9:03 ` [PATCH 1/6] http: simplify update_url_from_redirect Jeff King
2016-12-01  9:04 ` [PATCH 2/6] http: always update the base URL for redirects Jeff King
2016-12-01 16:02   ` Ramsay Jones
2016-12-01 22:53     ` Brandon Williams
2016-12-01 23:12       ` Philip Oakley
2016-12-01 23:43         ` Junio C Hamano
2016-12-02  0:07           ` Ramsay Jones
2016-12-02  0:18             ` Jeff King
2016-12-02  1:21               ` Ramsay Jones
2016-12-01  9:04 ` [PATCH 3/6] remote-curl: rename shadowed options variable Jeff King
2016-12-01  9:04 ` [PATCH 4/6] http: make redirects more obvious Jeff King
2016-12-01 16:06   ` Ramsay Jones
2016-12-01  9:04 ` [PATCH 5/6] http: treat http-alternates like redirects Jeff King
2016-12-01 23:02   ` Brandon Williams
2016-12-02  0:06     ` Jeff King
2016-12-01  9:04 ` Jeff King [this message]
2016-12-05 13:08 ` [PATCH 0/6] restricting http redirects Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161201090436.cwh2pxio7afowtfs@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=jannh@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).