Skip to content

Commit b81401c

Browse files
peffgitster
authored andcommitted
http: prompt for credentials on failed POST
All of the smart-http GET requests go through the http_get_* functions, which will prompt for credentials and retry if we see an HTTP 401. POST requests, however, do not go through any central point. Moreover, it is difficult to retry in the general case; we cannot assume the request body fits in memory or is even seekable, and we don't know how much of it was consumed during the attempt. Most of the time, this is not a big deal; for both fetching and pushing, we make a GET request before doing any POSTs, so typically we figure out the credentials during the first request, then reuse them during the POST. However, some servers may allow a client to get the list of refs from receive-pack without authentication, and then require authentication when the client actually tries to POST the pack. This is not ideal, as the client may do a non-trivial amount of work to generate the pack (e.g., delta-compressing objects). However, for a long time it has been the recommended example configuration in git-http-backend(1) for setting up a repository with anonymous fetch and authenticated push. This setup has always been broken without putting a username into the URL. Prior to commit 986bbc0, it did work with a username in the URL, because git would prompt for credentials before making any requests at all. However, post-986bbc0, it is totally broken. Since it has been advertised in the manpage for some time, we should make sure it works. Unfortunately, it is not as easy as simply calling post_rpc again when it fails, due to the input issue mentioned above. However, we can still make this specific case work by retrying in two specific instances: 1. If the request is large (bigger than LARGE_PACKET_MAX), we will first send a probe request with a single flush packet. Since this request is static, we can freely retry it. 2. If the request is small and we are not using gzip, then we have the whole thing in-core, and we can freely retry. That means we will not retry in some instances, including: 1. If we are using gzip. However, we only do so when calling git-upload-pack, so it does not apply to pushes. 2. If we have a large request, the probe succeeds, but then the real POST wants authentication. This is an extremely unlikely configuration and not worth worrying about. While it might be nice to cover those instances, doing so would be significantly more complex for very little real-world gain. In the long run, we will be much better off when curl learns to internally handle authentication as a callback, and we can cleanly handle all cases that way. Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 8809703 commit b81401c

File tree

2 files changed

+16
-9
lines changed

2 files changed

+16
-9
lines changed

remote-curl.c

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -362,16 +362,17 @@ static size_t rpc_in(char *ptr, size_t eltsize,
362362

363363
static int run_slot(struct active_request_slot *slot)
364364
{
365-
int err = 0;
365+
int err;
366366
struct slot_results results;
367367

368368
slot->results = &results;
369369
slot->curl_result = curl_easy_perform(slot->curl);
370370
finish_active_slot(slot);
371371

372-
if (results.curl_result != CURLE_OK) {
373-
err |= error("RPC failed; result=%d, HTTP code = %ld",
374-
results.curl_result, results.http_code);
372+
err = handle_curl_result(slot);
373+
if (err != HTTP_OK && err != HTTP_REAUTH) {
374+
error("RPC failed; result=%d, HTTP code = %ld",
375+
results.curl_result, results.http_code);
375376
}
376377

377378
return err;
@@ -436,9 +437,11 @@ static int post_rpc(struct rpc_state *rpc)
436437
}
437438

438439
if (large_request) {
439-
err = probe_rpc(rpc);
440-
if (err)
441-
return err;
440+
do {
441+
err = probe_rpc(rpc);
442+
} while (err == HTTP_REAUTH);
443+
if (err != HTTP_OK)
444+
return -1;
442445
}
443446

444447
slot = get_active_slot();
@@ -525,7 +528,11 @@ static int post_rpc(struct rpc_state *rpc)
525528
curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, rpc_in);
526529
curl_easy_setopt(slot->curl, CURLOPT_FILE, rpc);
527530

528-
err = run_slot(slot);
531+
do {
532+
err = run_slot(slot);
533+
} while (err == HTTP_REAUTH && !large_request && !use_gzip);
534+
if (err != HTTP_OK)
535+
err = -1;
529536

530537
curl_slist_free_all(headers);
531538
free(gzip_body);

t/t5541-http-push.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ test_expect_success 'push over smart http with auth' '
280280
test_cmp expect actual
281281
'
282282

283-
test_expect_failure 'push to auth-only-for-push repo' '
283+
test_expect_success 'push to auth-only-for-push repo' '
284284
cd "$ROOT_PATH/test_repo_clone" &&
285285
echo push-half-auth >expect &&
286286
test_commit push-half-auth &&

0 commit comments

Comments
 (0)