Skip to content

Commit 1960897

Browse files
peffgitster
authored andcommitted
http: do not set up curl auth after a 401
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent abf8df8 commit 1960897

File tree

3 files changed

+4
-7
lines changed

3 files changed

+4
-7
lines changed

http.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -744,8 +744,7 @@ char *get_remote_object_url(const char *url, const char *hex,
744744
return strbuf_detach(&buf, NULL);
745745
}
746746

747-
int handle_curl_result(struct active_request_slot *slot,
748-
struct slot_results *results)
747+
int handle_curl_result(struct slot_results *results)
749748
{
750749
if (results->curl_result == CURLE_OK) {
751750
credential_approve(&http_auth);
@@ -758,7 +757,6 @@ int handle_curl_result(struct active_request_slot *slot,
758757
return HTTP_NOAUTH;
759758
} else {
760759
credential_fill(&http_auth);
761-
init_curl_http_auth(slot->curl);
762760
return HTTP_REAUTH;
763761
}
764762
} else {
@@ -817,7 +815,7 @@ static int http_request(const char *url, void *result, int target, int options)
817815

818816
if (start_active_slot(slot)) {
819817
run_active_slot(slot);
820-
ret = handle_curl_result(slot, &results);
818+
ret = handle_curl_result(&results);
821819
} else {
822820
error("Unable to start HTTP request for %s", url);
823821
ret = HTTP_START_FAILED;

http.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,7 @@ extern int start_active_slot(struct active_request_slot *slot);
7878
extern void run_active_slot(struct active_request_slot *slot);
7979
extern void finish_active_slot(struct active_request_slot *slot);
8080
extern void finish_all_active_slots(void);
81-
extern int handle_curl_result(struct active_request_slot *slot,
82-
struct slot_results *results);
81+
extern int handle_curl_result(struct slot_results *results);
8382

8483
#ifdef USE_CURL_MULTI
8584
extern void fill_active_slots(void);

remote-curl.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,7 @@ static int run_slot(struct active_request_slot *slot)
369369
slot->curl_result = curl_easy_perform(slot->curl);
370370
finish_active_slot(slot);
371371

372-
err = handle_curl_result(slot, &results);
372+
err = handle_curl_result(&results);
373373
if (err != HTTP_OK && err != HTTP_REAUTH) {
374374
error("RPC failed; result=%d, HTTP code = %ld",
375375
results.curl_result, results.http_code);

0 commit comments

Comments
 (0)