git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Brad Hein <linuxbrad@gmail.com>, git@vger.kernel.org
Subject: [PATCH 2/2] http: do not set up curl auth after a 401
Date: Fri, 12 Oct 2012 03:35:59 -0400	[thread overview]
Message-ID: <20121012073559.GB16441@sigill.intra.peff.net> (raw)
In-Reply-To: <20121012073053.GC17026@sigill.intra.peff.net>

When we get an http 401, we prompt for credentials and put
them in our global credential struct. We also feed them to
the curl handle that produced the 401, with the intent that
they will be used on a retry.

When the code was originally introduced in commit 42653c0,
this was a necessary step. However, since dfa1725, we always
feed our global credential into every curl handle when we
initialize the slot with get_active_slot. So every further
request already feeds the credential to curl.

Moreover, accessing the slot here is somewhat dubious. After
the slot has produced a response, we don't actually control
it any more.  If we are using curl_multi, it may even have
been re-initialized to handle a different request.

It just so happens that we will reuse the curl handle within
the slot in such a case, and that because we only keep one
global credential, it will be the one we want.  So the
current code is not buggy, but it is misleading.

By cleaning it up, we can remove the slot argument entirely
from handle_curl_result, making it much more obvious that
slots should not be accessed after they are marked as
finished.

Signed-off-by: Jeff King <peff@peff.net>
---
 http.c        | 6 ++----
 http.h        | 3 +--
 remote-curl.c | 2 +-
 3 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/http.c b/http.c
index 9334386..0a74345 100644
--- a/http.c
+++ b/http.c
@@ -744,8 +744,7 @@ char *get_remote_object_url(const char *url, const char *hex,
 	return strbuf_detach(&buf, NULL);
 }
 
-int handle_curl_result(struct active_request_slot *slot,
-		       struct slot_results *results)
+int handle_curl_result(struct slot_results *results)
 {
 	if (results->curl_result == CURLE_OK) {
 		credential_approve(&http_auth);
@@ -758,7 +757,6 @@ int handle_curl_result(struct active_request_slot *slot,
 			return HTTP_NOAUTH;
 		} else {
 			credential_fill(&http_auth);
-			init_curl_http_auth(slot->curl);
 			return HTTP_REAUTH;
 		}
 	} else {
@@ -817,7 +815,7 @@ static int http_request(const char *url, void *result, int target, int options)
 
 	if (start_active_slot(slot)) {
 		run_active_slot(slot);
-		ret = handle_curl_result(slot, &results);
+		ret = handle_curl_result(&results);
 	} else {
 		error("Unable to start HTTP request for %s", url);
 		ret = HTTP_START_FAILED;
diff --git a/http.h b/http.h
index 0bd1e84..0a80d30 100644
--- a/http.h
+++ b/http.h
@@ -78,8 +78,7 @@ extern void finish_all_active_slots(void);
 extern void run_active_slot(struct active_request_slot *slot);
 extern void finish_active_slot(struct active_request_slot *slot);
 extern void finish_all_active_slots(void);
-extern int handle_curl_result(struct active_request_slot *slot,
-			      struct slot_results *results);
+extern int handle_curl_result(struct slot_results *results);
 
 #ifdef USE_CURL_MULTI
 extern void fill_active_slots(void);
diff --git a/remote-curl.c b/remote-curl.c
index 4281262..aefafd3 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -369,7 +369,7 @@ static int run_slot(struct active_request_slot *slot)
 	slot->curl_result = curl_easy_perform(slot->curl);
 	finish_active_slot(slot);
 
-	err = handle_curl_result(slot, &results);
+	err = handle_curl_result(&results);
 	if (err != HTTP_OK && err != HTTP_REAUTH) {
 		error("RPC failed; result=%d, HTTP code = %ld",
 		      results.curl_result, results.http_code);
-- 
1.8.0.rc2.3.g303f317

  parent reply	other threads:[~2012-10-12  7:36 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CAJa+X0OkzAX9E2SnDmU=on0yzzVZ9OMa2dJZgKMK=gQu2Rhf_Q@mail.gmail.com>
2012-10-12  4:58 ` git fails: segfault at 0 ip 00000000004076d5 sp 00007fff7806ebc0 Brad Hein
2012-10-12  6:22   ` [PATCH] http: fix segfault in handle_curl_result Jeff King
2012-10-12  7:30     ` Jeff King
2012-10-12  7:35       ` [PATCH 1/2] remote-curl: do not call run_slot repeatedly Jeff King
2012-10-12  7:35       ` Jeff King [this message]
2012-10-12 13:39         ` [PATCH 2/2] http: do not set up curl auth after a 401 Brad Hein
2012-10-12 16:46     ` [PATCH] http: fix segfault in handle_curl_result Junio C Hamano
2012-10-12 16:29   ` git fails: segfault at 0 ip 00000000004076d5 sp 00007fff7806ebc0 Erik Faye-Lund
2012-10-12 16:34     ` Erik Faye-Lund
2012-10-12 17:12       ` 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=20121012073559.GB16441@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=linuxbrad@gmail.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).