From a5bd828ec6a818372f7be42b2b5b123eca267f5e Mon Sep 17 00:00:00 2001 From: Vaidas Pilkauskas Date: Wed, 26 Nov 2025 12:06:57 +0200 Subject: [PATCH 01/15] http: add support for HTTP 429 rate limit retries Add retry logic for HTTP 429 (Too Many Requests) responses to handle server-side rate limiting gracefully. When Git's HTTP client receives a 429 response, it can now automatically retry the request after an appropriate delay, respecting the server's rate limits. The implementation supports the RFC-compliant Retry-After header in both delay-seconds (integer) and HTTP-date (RFC 2822) formats. If a past date is provided, Git retries immediately without waiting. Retry behavior is controlled by three new configuration options: * http.maxRetries: Maximum number of retry attempts (default: 0, meaning retries are disabled by default). Users must explicitly opt-in to retry behavior. * http.retryAfter: Default delay in seconds when the server doesn't provide a Retry-After header (default: -1, meaning fail if no header is provided). This serves as a fallback mechanism. * http.maxRetryTime: Maximum delay in seconds for a single retry (default: 300). If the server requests a delay exceeding this limit, Git fails immediately rather than waiting. This prevents indefinite blocking on unreasonable server requests. All three options can be overridden via environment variables: GIT_HTTP_MAX_RETRIES, GIT_HTTP_RETRY_AFTER, and GIT_HTTP_MAX_RETRY_TIME. The retry logic implements a fail-fast approach: if any delay (whether from server header or configuration) exceeds maxRetryTime, Git fails immediately with a clear error message rather than capping the delay. This provides better visibility into rate limiting issues. The implementation includes extensive test coverage for basic retry behavior, Retry-After header formats (integer and HTTP-date), configuration combinations, maxRetryTime limits, invalid header handling, environment variable overrides, and edge cases. Signed-off-by: Vaidas Pilkauskas --- Documentation/config/http.adoc | 24 ++ http-push.c | 8 + http-walker.c | 5 + http.c | 149 +++++++++++- http.h | 2 + remote-curl.c | 4 + t/meson.build | 1 + t/t5584-http-429-retry.sh | 429 +++++++++++++++++++++++++++++++++ 8 files changed, 618 insertions(+), 4 deletions(-) create mode 100755 t/t5584-http-429-retry.sh diff --git a/Documentation/config/http.adoc b/Documentation/config/http.adoc index 9da5c298cc1d5e..9e3c888df47867 100644 --- a/Documentation/config/http.adoc +++ b/Documentation/config/http.adoc @@ -315,6 +315,30 @@ http.keepAliveCount:: unset, curl's default value is used. Can be overridden by the `GIT_HTTP_KEEPALIVE_COUNT` environment variable. +http.retryAfter:: + Default wait time in seconds before retrying when a server returns + HTTP 429 (Too Many Requests) without a Retry-After header. If set + to -1 (the default), Git will fail immediately when encountering + a 429 response without a Retry-After header. When a Retry-After + header is present, its value takes precedence over this setting. + Can be overridden by the `GIT_HTTP_RETRY_AFTER` environment variable. + See also `http.maxRetries` and `http.maxRetryTime`. + +http.maxRetries:: + Maximum number of times to retry after receiving HTTP 429 (Too Many + Requests) responses. Set to 0 (the default) to disable retries. + Can be overridden by the `GIT_HTTP_MAX_RETRIES` environment variable. + See also `http.retryAfter` and `http.maxRetryTime`. + +http.maxRetryTime:: + Maximum time in seconds to wait for a single retry attempt when + handling HTTP 429 (Too Many Requests) responses. If the server + requests a delay (via Retry-After header) or if `http.retryAfter` + is configured with a value that exceeds this maximum, Git will fail + immediately rather than waiting. Default is 300 seconds (5 minutes). + Can be overridden by the `GIT_HTTP_MAX_RETRY_TIME` environment + variable. See also `http.retryAfter` and `http.maxRetries`. + http.noEPSV:: A boolean which disables using of EPSV ftp command by curl. This can be helpful with some "poor" ftp servers which don't diff --git a/http-push.c b/http-push.c index 60a9b756209071..4ec13ca4ecb9ab 100644 --- a/http-push.c +++ b/http-push.c @@ -716,6 +716,10 @@ static int fetch_indices(void) case HTTP_MISSING_TARGET: ret = 0; break; + case HTTP_RATE_LIMITED: + error("rate limited by '%s', please try again later", repo->url); + ret = -1; + break; default: ret = -1; } @@ -1548,6 +1552,10 @@ static int remote_exists(const char *path) case HTTP_MISSING_TARGET: ret = 0; break; + case HTTP_RATE_LIMITED: + error("rate limited by '%s', please try again later", url); + ret = -1; + break; case HTTP_ERROR: error("unable to access '%s': %s", url, curl_errorstr); /* fallthrough */ diff --git a/http-walker.c b/http-walker.c index e886e6486646d1..9f06f47de1c5c9 100644 --- a/http-walker.c +++ b/http-walker.c @@ -414,6 +414,11 @@ static int fetch_indices(struct walker *walker, struct alt_base *repo) repo->got_indices = 1; ret = 0; break; + case HTTP_RATE_LIMITED: + error("rate limited by '%s', please try again later", repo->base); + repo->got_indices = 0; + ret = -1; + break; default: repo->got_indices = 0; ret = -1; diff --git a/http.c b/http.c index 41f850db16d19f..212805cad56a59 100644 --- a/http.c +++ b/http.c @@ -22,6 +22,7 @@ #include "object-file.h" #include "odb.h" #include "tempfile.h" +#include "date.h" static struct trace_key trace_curl = TRACE_KEY_INIT(CURL); static int trace_curl_data = 1; @@ -149,6 +150,14 @@ static char *cached_accept_language; static char *http_ssl_backend; static int http_schannel_check_revoke = 1; + +/* Retry configuration */ +static long http_retry_after = -1; /* Default retry-after in seconds when header is missing (-1 means not set, exit with 128) */ +static long http_max_retries = 0; /* Maximum number of retry attempts (0 means retries are disabled) */ +static long http_max_retry_time = 300; /* Maximum time to wait for a single retry (default 5 minutes) */ + +/* Store retry_after value from 429 responses for retry logic (-1 = not set, 0 = retry immediately, >0 = delay in seconds) */ +static long last_retry_after = -1; /* * With the backend being set to `schannel`, setting sslCAinfo would override * the Certificate Store in cURL v7.60.0 and later, which is not what we want @@ -209,13 +218,14 @@ static inline int is_hdr_continuation(const char *ptr, const size_t size) return size && (*ptr == ' ' || *ptr == '\t'); } -static size_t fwrite_wwwauth(char *ptr, size_t eltsize, size_t nmemb, void *p UNUSED) +static size_t fwrite_wwwauth(char *ptr, size_t eltsize, size_t nmemb, void *p) { size_t size = eltsize * nmemb; struct strvec *values = &http_auth.wwwauth_headers; struct strbuf buf = STRBUF_INIT; const char *val; size_t val_len; + struct active_request_slot *slot = (struct active_request_slot *)p; /* * Header lines may not come NULL-terminated from libcurl so we must @@ -257,6 +267,47 @@ static size_t fwrite_wwwauth(char *ptr, size_t eltsize, size_t nmemb, void *p UN goto exit; } + /* Parse Retry-After header for rate limiting */ + if (skip_iprefix_mem(ptr, size, "retry-after:", &val, &val_len)) { + strbuf_add(&buf, val, val_len); + strbuf_trim(&buf); + + if (slot && slot->results) { + /* Parse the retry-after value (delay-seconds or HTTP-date) */ + char *endptr; + long retry_after; + + errno = 0; + retry_after = strtol(buf.buf, &endptr, 10); + + /* Check if it's a valid integer (delay-seconds format) */ + if (endptr != buf.buf && *endptr == '\0' && + errno != ERANGE && retry_after > 0) { + slot->results->retry_after = retry_after; + } else { + /* Try parsing as HTTP-date format */ + timestamp_t timestamp; + int offset; + if (!parse_date_basic(buf.buf, ×tamp, &offset)) { + /* Successfully parsed as date, calculate delay from now */ + timestamp_t now = time(NULL); + if (timestamp > now) { + slot->results->retry_after = (long)(timestamp - now); + } else { + /* Past date means retry immediately */ + slot->results->retry_after = 0; + } + } else { + /* Failed to parse as either delay-seconds or HTTP-date */ + warning(_("unable to parse Retry-After header value: '%s'"), buf.buf); + } + } + } + + http_auth.header_is_last_match = 1; + goto exit; + } + /* * This line could be a continuation of the previously matched header * field. If this is the case then we should append this value to the @@ -575,6 +626,21 @@ static int http_options(const char *var, const char *value, return 0; } + if (!strcmp("http.retryafter", var)) { + http_retry_after = git_config_int(var, value, ctx->kvi); + return 0; + } + + if (!strcmp("http.maxretries", var)) { + http_max_retries = git_config_int(var, value, ctx->kvi); + return 0; + } + + if (!strcmp("http.maxretrytime", var)) { + http_max_retry_time = git_config_int(var, value, ctx->kvi); + return 0; + } + /* Fall back on the default ones */ return git_default_config(var, value, ctx, data); } @@ -1422,6 +1488,10 @@ void http_init(struct remote *remote, const char *url, int proactive_auth) set_long_from_env(&curl_tcp_keepintvl, "GIT_TCP_KEEPINTVL"); set_long_from_env(&curl_tcp_keepcnt, "GIT_TCP_KEEPCNT"); + set_long_from_env(&http_retry_after, "GIT_HTTP_RETRY_AFTER"); + set_long_from_env(&http_max_retries, "GIT_HTTP_MAX_RETRIES"); + set_long_from_env(&http_max_retry_time, "GIT_HTTP_MAX_RETRY_TIME"); + curl_default = get_curl_handle(); } @@ -1871,6 +1941,10 @@ static int handle_curl_result(struct slot_results *results) } return HTTP_REAUTH; } + } else if (results->http_code == 429) { + /* Store the retry_after value for use in retry logic */ + last_retry_after = results->retry_after; + return HTTP_RATE_LIMITED; } else { if (results->http_connectcode == 407) credential_reject(the_repository, &proxy_auth); @@ -1886,6 +1960,8 @@ int run_one_slot(struct active_request_slot *slot, struct slot_results *results) { slot->results = results; + /* Initialize retry_after to -1 (not set) */ + results->retry_after = -1; if (!start_active_slot(slot)) { xsnprintf(curl_errorstr, sizeof(curl_errorstr), "failed to start HTTP request"); @@ -2149,6 +2225,7 @@ static int http_request(const char *url, } curl_easy_setopt(slot->curl, CURLOPT_HEADERFUNCTION, fwrite_wwwauth); + curl_easy_setopt(slot->curl, CURLOPT_HEADERDATA, slot); accept_language = http_get_accept_language_header(); @@ -2253,19 +2330,36 @@ static int update_url_from_redirect(struct strbuf *base, return 1; } +/* + * Sleep for the specified number of seconds before retrying. + */ +static void sleep_for_retry(long retry_after) +{ + if (retry_after > 0) { + unsigned int remaining; + warning(_("rate limited, waiting %ld seconds before retry"), retry_after); + remaining = sleep(retry_after); + while (remaining > 0) { + /* Sleep was interrupted, continue sleeping */ + remaining = sleep(remaining); + } + } +} + static int http_request_reauth(const char *url, void *result, int target, struct http_get_options *options) { int i = 3; int ret; + int rate_limit_retries = http_max_retries; if (always_auth_proactively()) credential_fill(the_repository, &http_auth, 1); ret = http_request(url, result, target, options); - if (ret != HTTP_OK && ret != HTTP_REAUTH) + if (ret != HTTP_OK && ret != HTTP_REAUTH && ret != HTTP_RATE_LIMITED) return ret; if (options && options->effective_url && options->base_url) { @@ -2276,7 +2370,7 @@ static int http_request_reauth(const char *url, } } - while (ret == HTTP_REAUTH && --i) { + while ((ret == HTTP_REAUTH || ret == HTTP_RATE_LIMITED) && --i) { /* * The previous request may have put cruft into our output stream; we * should clear it out before making our next request. @@ -2302,7 +2396,54 @@ static int http_request_reauth(const char *url, BUG("Unknown http_request target"); } - credential_fill(the_repository, &http_auth, 1); + if (ret == HTTP_RATE_LIMITED) { + /* Handle rate limiting with retry logic */ + int retry_attempt = http_max_retries - rate_limit_retries + 1; + + if (rate_limit_retries <= 0) { + /* Retries are disabled or exhausted */ + if (http_max_retries > 0) { + error(_("too many rate limit retries, giving up")); + } + return HTTP_ERROR; + } + + /* Decrement retries counter */ + rate_limit_retries--; + + /* Use the stored retry_after value or configured default */ + if (last_retry_after >= 0) { + /* Check if retry delay exceeds maximum allowed */ + if (last_retry_after > http_max_retry_time) { + error(_("rate limited (HTTP 429) requested %ld second delay, " + "exceeds http.maxRetryTime of %ld seconds"), + last_retry_after, http_max_retry_time); + last_retry_after = -1; /* Reset after use */ + return HTTP_ERROR; + } + sleep_for_retry(last_retry_after); + last_retry_after = -1; /* Reset after use */ + } else { + /* No Retry-After header provided */ + if (http_retry_after < 0) { + /* Not configured - exit with error */ + error(_("rate limited (HTTP 429) and no Retry-After header provided. " + "Configure http.retryAfter or set GIT_HTTP_RETRY_AFTER.")); + return HTTP_ERROR; + } + /* Check if configured default exceeds maximum allowed */ + if (http_retry_after > http_max_retry_time) { + error(_("configured http.retryAfter (%ld seconds) exceeds " + "http.maxRetryTime (%ld seconds)"), + http_retry_after, http_max_retry_time); + return HTTP_ERROR; + } + /* Use configured default retry-after value */ + sleep_for_retry(http_retry_after); + } + } else if (ret == HTTP_REAUTH) { + credential_fill(the_repository, &http_auth, 1); + } ret = http_request(url, result, target, options); } diff --git a/http.h b/http.h index f9d459340476e4..eb404564502165 100644 --- a/http.h +++ b/http.h @@ -20,6 +20,7 @@ struct slot_results { long http_code; long auth_avail; long http_connectcode; + long retry_after; }; struct active_request_slot { @@ -167,6 +168,7 @@ struct http_get_options { #define HTTP_REAUTH 4 #define HTTP_NOAUTH 5 #define HTTP_NOMATCHPUBLICKEY 6 +#define HTTP_RATE_LIMITED 7 /* * Requests a URL and stores the result in a strbuf. diff --git a/remote-curl.c b/remote-curl.c index 69f919454a4565..5959461cd34220 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -529,6 +529,10 @@ static struct discovery *discover_refs(const char *service, int for_push) show_http_message(&type, &charset, &buffer); die(_("unable to access '%s' with http.pinnedPubkey configuration: %s"), transport_anonymize_url(url.buf), curl_errorstr); + case HTTP_RATE_LIMITED: + show_http_message(&type, &charset, &buffer); + die(_("rate limited by '%s', please try again later"), + transport_anonymize_url(url.buf)); default: show_http_message(&type, &charset, &buffer); die(_("unable to access '%s': %s"), diff --git a/t/meson.build b/t/meson.build index d3d0be28224b9c..856fb0633c6358 100644 --- a/t/meson.build +++ b/t/meson.build @@ -699,6 +699,7 @@ integration_tests = [ 't5581-http-curl-verbose.sh', 't5582-fetch-negative-refspec.sh', 't5583-push-branches.sh', + 't5584-http-429-retry.sh', 't5600-clone-fail-cleanup.sh', 't5601-clone.sh', 't5602-clone-remote-exec.sh', diff --git a/t/t5584-http-429-retry.sh b/t/t5584-http-429-retry.sh new file mode 100755 index 00000000000000..8bcc382763037b --- /dev/null +++ b/t/t5584-http-429-retry.sh @@ -0,0 +1,429 @@ +#!/bin/sh + +test_description='test HTTP 429 Too Many Requests retry logic' + +. ./test-lib.sh + +. "$TEST_DIRECTORY"/lib-httpd.sh + +start_httpd + +test_expect_success 'setup test repository' ' + test_commit initial && + git clone --bare . "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" && + git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" config http.receivepack true +' + +test_expect_success 'HTTP 429 with retries disabled (maxRetries=0) fails immediately' ' + write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF && + printf "Status: 429 Too Many Requests\r\n" + printf "Retry-After: 1\r\n" + printf "Content-Type: text/plain\r\n" + printf "\r\n" + printf "Rate limited\n" + cat "$1" >/dev/null + EOF + + # Set maxRetries to 0 (disabled) + test_config http.maxRetries 0 && + test_config http.retryAfter 1 && + + # Should fail immediately without any retry attempt + test_must_fail git ls-remote "$HTTPD_URL/one_time_script/repo.git" 2>err && + + # Verify no retry happened (no "waiting" message in stderr) + ! grep -i "waiting.*retry" err && + + # The one-time script will be consumed on first request (not a retry) + test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script" +' + +test_expect_success 'HTTP 429 permanent should fail after max retries' ' + # Install a permanent error script to prove retries are limited + write_script "$HTTPD_ROOT_PATH/http-429-permanent.sh" <<-\EOF && + printf "Status: 429 Too Many Requests\r\n" + printf "Retry-After: 1\r\n" + printf "Content-Type: text/plain\r\n" + printf "\r\n" + printf "Permanently rate limited\n" + EOF + + # Enable retries with a limit + test_config http.maxRetries 2 && + + # Git should retry but eventually fail when 429 persists + test_must_fail git ls-remote "$HTTPD_URL/error/http-429-permanent.sh/repo.git" 2>err +' + +test_expect_success 'HTTP 429 with Retry-After is retried and succeeds' ' + # Create a one-time script that returns 429 with Retry-After header + # on the first request. Subsequent requests will succeed. + # This contrasts with the permanent 429 above - proving retry works + write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF && + # Return HTTP 429 response instead of git response + printf "Status: 429 Too Many Requests\r\n" + printf "Retry-After: 1\r\n" + printf "Content-Type: text/plain\r\n" + printf "\r\n" + printf "Rate limited - please retry after 1 second\n" + # Output something different from input so the script gets removed + cat "$1" >/dev/null + EOF + + # Enable retries + test_config http.maxRetries 3 && + + # Git should retry after receiving 429 and eventually succeed + git ls-remote "$HTTPD_URL/one_time_script/repo.git" >output 2>err && + test_grep "refs/heads/" output && + + # The one-time script should have been consumed (proving retry happened) + test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script" +' + +test_expect_success 'HTTP 429 without Retry-After uses configured default' ' + write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF && + printf "Status: 429 Too Many Requests\r\n" + printf "Content-Type: text/plain\r\n" + printf "\r\n" + printf "Rate limited - no retry info\n" + cat "$1" >/dev/null + EOF + + # Enable retries and configure default delay + test_config http.maxRetries 3 && + test_config http.retryAfter 1 && + + # Git should retry using configured default and succeed + git ls-remote "$HTTPD_URL/one_time_script/repo.git" >output 2>err && + test_grep "refs/heads/" output && + test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script" +' + +test_expect_success 'HTTP 429 retry delays are respected' ' + write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF && + printf "Status: 429 Too Many Requests\r\n" + printf "Retry-After: 2\r\n" + printf "Content-Type: text/plain\r\n" + printf "\r\n" + printf "Rate limited\n" + cat "$1" >/dev/null + EOF + + # Enable retries + test_config http.maxRetries 3 && + + # Time the operation - it should take at least 2 seconds due to retry delay + start=$(date +%s) && + git ls-remote "$HTTPD_URL/one_time_script/repo.git" >output 2>err && + end=$(date +%s) && + duration=$((end - start)) && + + # Verify it took at least 2 seconds (allowing some tolerance) + test "$duration" -ge 1 && + test_grep "refs/heads/" output && + test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script" +' + +test_expect_success 'HTTP 429 fails immediately if Retry-After exceeds http.maxRetryTime' ' + write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF && + printf "Status: 429 Too Many Requests\r\n" + printf "Retry-After: 100\r\n" + printf "Content-Type: text/plain\r\n" + printf "\r\n" + printf "Rate limited with long delay\n" + cat "$1" >/dev/null + EOF + + # Configure max retry time to 3 seconds (much less than requested 100) + test_config http.maxRetries 3 && + test_config http.maxRetryTime 3 && + + # Should fail immediately without waiting + start=$(date +%s) && + test_must_fail git ls-remote "$HTTPD_URL/one_time_script/repo.git" 2>err && + end=$(date +%s) && + duration=$((end - start)) && + + # Should fail quickly (less than 2 seconds, no 100 second wait) + test "$duration" -lt 2 && + test_grep "exceeds http.maxRetryTime" err && + + # The one-time script will be consumed on first request + test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script" +' + +test_expect_success 'HTTP 429 fails if configured http.retryAfter exceeds http.maxRetryTime' ' + # Test misconfiguration: retryAfter > maxRetryTime + write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF && + printf "Status: 429 Too Many Requests\r\n" + printf "Content-Type: text/plain\r\n" + printf "\r\n" + printf "Rate limited without header\n" + cat "$1" >/dev/null + EOF + + # Configure retryAfter larger than maxRetryTime + test_config http.maxRetries 3 && + test_config http.retryAfter 100 && + test_config http.maxRetryTime 5 && + + # Should fail immediately with configuration error + start=$(date +%s) && + test_must_fail git ls-remote "$HTTPD_URL/one_time_script/repo.git" 2>err && + end=$(date +%s) && + duration=$((end - start)) && + + # Should fail quickly + test "$duration" -lt 2 && + test_grep "configured http.retryAfter.*exceeds.*http.maxRetryTime" err +' + +test_expect_success 'HTTP 429 with Retry-After HTTP-date format' ' + # Test HTTP-date format (RFC 2822) in Retry-After header + # Generate a date 2 seconds in the future + future_date=$(TZ=GMT date -d "+2 seconds" "+%a, %d %b %Y %H:%M:%S GMT" 2>/dev/null || \ + TZ=GMT date -v+2S "+%a, %d %b %Y %H:%M:%S GMT" 2>/dev/null || \ + echo "skip") && + + if test "$future_date" = "skip" + then + skip_all="date command does not support required format" && + test_done + fi && + + write_script "$HTTPD_ROOT_PATH/one-time-script" <<-EOF && + printf "Status: 429 Too Many Requests\\r\\n" + printf "Retry-After: $future_date\\r\\n" + printf "Content-Type: text/plain\\r\\n" + printf "\\r\\n" + printf "Rate limited with HTTP-date\\n" + cat "\$1" >/dev/null + EOF + + # Enable retries + test_config http.maxRetries 3 && + + # Git should parse the HTTP-date and retry after the delay + start=$(date +%s) && + git ls-remote "$HTTPD_URL/one_time_script/repo.git" >output 2>err && + end=$(date +%s) && + duration=$((end - start)) && + + # Should take at least 1 second (allowing tolerance for processing time) + test "$duration" -ge 1 && + test_grep "refs/heads/" output && + test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script" +' + +test_expect_success 'HTTP 429 with HTTP-date exceeding maxRetryTime fails immediately' ' + # Generate a date 200 seconds in the future + future_date=$(TZ=GMT date -d "+200 seconds" "+%a, %d %b %Y %H:%M:%S GMT" 2>/dev/null || \ + TZ=GMT date -v+200S "+%a, %d %b %Y %H:%M:%S GMT" 2>/dev/null || \ + echo "skip") && + + if test "$future_date" = "skip" + then + skip_all="date command does not support required format" && + test_done + fi && + + write_script "$HTTPD_ROOT_PATH/one-time-script" <<-EOF && + printf "Status: 429 Too Many Requests\\r\\n" + printf "Retry-After: $future_date\\r\\n" + printf "Content-Type: text/plain\\r\\n" + printf "\\r\\n" + printf "Rate limited with long HTTP-date\\n" + cat "\$1" >/dev/null + EOF + + # Configure max retry time much less than the 200 second delay + test_config http.maxRetries 3 && + test_config http.maxRetryTime 10 && + + # Should fail immediately without waiting 200 seconds + start=$(date +%s) && + test_must_fail git ls-remote "$HTTPD_URL/one_time_script/repo.git" 2>err && + end=$(date +%s) && + duration=$((end - start)) && + + # Should fail quickly (not wait 200 seconds) + test "$duration" -lt 2 && + test_grep "exceeds http.maxRetryTime" err && + + # The one-time script will be consumed on first request + test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script" +' + +test_expect_success 'HTTP 429 with past HTTP-date should not wait' ' + past_date=$(TZ=GMT date -d "-10 seconds" "+%a, %d %b %Y %H:%M:%S GMT" 2>/dev/null || \ + TZ=GMT date -v-10S "+%a, %d %b %Y %H:%M:%S GMT" 2>/dev/null || \ + echo "skip") && + + if test "$past_date" = "skip" + then + skip_all="date command does not support required format" && + test_done + fi && + + write_script "$HTTPD_ROOT_PATH/one-time-script" <<-EOF && + printf "Status: 429 Too Many Requests\\r\\n" + printf "Retry-After: $past_date\\r\\n" + printf "Content-Type: text/plain\\r\\n" + printf "\\r\\n" + printf "Rate limited with past date\\n" + cat "\$1" >/dev/null + EOF + + # Enable retries + test_config http.maxRetries 3 && + + # Git should retry immediately without waiting + start=$(date +%s) && + git ls-remote "$HTTPD_URL/one_time_script/repo.git" >output 2>err && + end=$(date +%s) && + duration=$((end - start)) && + + # Should complete quickly (less than 2 seconds) + test "$duration" -lt 2 && + test_grep "refs/heads/" output && + test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script" +' + +test_expect_success 'HTTP 429 with invalid Retry-After format uses configured default' ' + write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF && + printf "Status: 429 Too Many Requests\r\n" + printf "Retry-After: invalid-format-123abc\r\n" + printf "Content-Type: text/plain\r\n" + printf "\r\n" + printf "Rate limited with malformed header\n" + cat "$1" >/dev/null + EOF + + # Configure default retry-after + test_config http.maxRetries 3 && + test_config http.retryAfter 1 && + + # Should use configured default (1 second) since header is invalid + start=$(date +%s) && + git ls-remote "$HTTPD_URL/one_time_script/repo.git" >output 2>err && + end=$(date +%s) && + duration=$((end - start)) && + + # Should take at least 1 second (the configured default) + test "$duration" -ge 1 && + test_grep "refs/heads/" output && + test_grep "waiting.*retry" err && + test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script" +' + +test_expect_success 'HTTP 429 will not be retried without config' ' + # Default config means http.maxRetries=0 (retries disabled) + # When 429 is received, it should fail immediately without retry + write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF && + printf "Status: 429 Too Many Requests\r\n" + printf "Retry-After: 1\r\n" + printf "Content-Type: text/plain\r\n" + printf "\r\n" + printf "Rate limited\n" + cat "$1" >/dev/null + EOF + + # Do NOT configure anything - use defaults (http.maxRetries defaults to 0) + + # Should fail immediately without retry + test_must_fail git ls-remote "$HTTPD_URL/one_time_script/repo.git" 2>err && + + # Verify no retry happened (no "waiting" message) + ! grep -i "waiting.*retry" err && + + # Should get 429 error + test_grep "429" err && + + # The one-time script should be consumed on first request + test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script" +' + +test_expect_success 'GIT_HTTP_RETRY_AFTER overrides http.retryAfter config' ' + write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF && + printf "Status: 429 Too Many Requests\r\n" + printf "Content-Type: text/plain\r\n" + printf "\r\n" + printf "Rate limited - no Retry-After header\n" + cat "$1" >/dev/null + EOF + + # Configure retryAfter to 10 seconds + test_config http.maxRetries 3 && + test_config http.retryAfter 10 && + + # Override with environment variable to 1 second + start=$(date +%s) && + GIT_HTTP_RETRY_AFTER=1 git ls-remote "$HTTPD_URL/one_time_script/repo.git" >output 2>err && + end=$(date +%s) && + duration=$((end - start)) && + + # Should use env var (1 second), not config (10 seconds) + test "$duration" -ge 1 && + test "$duration" -lt 5 && + test_grep "refs/heads/" output && + test_grep "waiting.*retry" err && + test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script" +' + +test_expect_success 'GIT_HTTP_MAX_RETRIES overrides http.maxRetries config' ' + write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF && + printf "Status: 429 Too Many Requests\r\n" + printf "Retry-After: 1\r\n" + printf "Content-Type: text/plain\r\n" + printf "\r\n" + printf "Rate limited\n" + cat "$1" >/dev/null + EOF + + # Configure maxRetries to 0 (disabled) + test_config http.maxRetries 0 && + test_config http.retryAfter 1 && + + # Override with environment variable to enable retries + GIT_HTTP_MAX_RETRIES=3 git ls-remote "$HTTPD_URL/one_time_script/repo.git" >output 2>err && + + # Should retry (env var enables it despite config saying disabled) + test_grep "refs/heads/" output && + test_grep "waiting.*retry" err && + test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script" +' + +test_expect_success 'GIT_HTTP_MAX_RETRY_TIME overrides http.maxRetryTime config' ' + write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF && + printf "Status: 429 Too Many Requests\r\n" + printf "Retry-After: 50\r\n" + printf "Content-Type: text/plain\r\n" + printf "\r\n" + printf "Rate limited with long delay\n" + cat "$1" >/dev/null + EOF + + # Configure maxRetryTime to 100 seconds (would accept 50 second delay) + test_config http.maxRetries 3 && + test_config http.maxRetryTime 100 && + + # Override with environment variable to 10 seconds (should reject 50 second delay) + start=$(date +%s) && + test_must_fail env GIT_HTTP_MAX_RETRY_TIME=10 \ + git ls-remote "$HTTPD_URL/one_time_script/repo.git" 2>err && + end=$(date +%s) && + duration=$((end - start)) && + + # Should fail quickly (not wait 50 seconds) because env var limits to 10 + test "$duration" -lt 5 && + test_grep "exceeds http.maxRetryTime" err && + test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script" +' + +test_expect_success 'verify normal repository access still works' ' + git ls-remote "$HTTPD_URL/smart/repo.git" >output && + test_grep "refs/heads/" output +' + +test_done From 7639fa3ae895d7121d190a08a8223a9ecdc47703 Mon Sep 17 00:00:00 2001 From: Vaidas Pilkauskas Date: Tue, 25 Nov 2025 12:29:55 +0200 Subject: [PATCH 02/15] remote-curl: fix memory leak in show_http_message() Fix a memory leak in show_http_message() that was triggered when displaying HTTP error messages before die(). The function would call strbuf_reencode() which modifies the caller's strbuf in place, allocating new memory for the re-encoded string. Since this function is only called immediately before die(), the allocated memory was never explicitly freed, causing leak detectors to report it. The leak became visible when HTTP 429 rate limit retry support was added, which introduced the HTTP_RATE_LIMITED error case. However, the issue existed in pre-existing error paths as well (HTTP_MISSING_TARGET, HTTP_NOAUTH, HTTP_NOMATCHPUBLICKEY) - the new retry logic just made it more visible in tests because retries exercise the error paths more frequently. The leak was detected by LeakSanitizer in t5584 tests that enable retries (maxRetries > 0). Tests with retries disabled passed because they took a different code path or timing. Fix this by making show_http_message() work on a local copy of the message buffer instead of modifying the caller's buffer in place: 1. Create a local strbuf and copy the message into it 2. Perform re-encoding on the local copy if needed 3. Display the message from the local copy 4. Properly release the local copy before returning This ensures all memory allocated by strbuf_reencode() is freed before the function returns, even though die() is called immediately after, eliminating the leak. Signed-off-by: Vaidas Pilkauskas --- remote-curl.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/remote-curl.c b/remote-curl.c index 5959461cd34220..dd0680e5ae2461 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -371,6 +371,7 @@ static int show_http_message(struct strbuf *type, struct strbuf *charset, struct strbuf *msg) { const char *p, *eol; + struct strbuf msgbuf = STRBUF_INIT; /* * We only show text/plain parts, as other types are likely @@ -378,19 +379,24 @@ static int show_http_message(struct strbuf *type, struct strbuf *charset, */ if (strcmp(type->buf, "text/plain")) return -1; + + strbuf_addbuf(&msgbuf, msg); if (charset->len) - strbuf_reencode(msg, charset->buf, get_log_output_encoding()); + strbuf_reencode(&msgbuf, charset->buf, get_log_output_encoding()); - strbuf_trim(msg); - if (!msg->len) + strbuf_trim(&msgbuf); + if (!msgbuf.len) { + strbuf_release(&msgbuf); return -1; + } - p = msg->buf; + p = msgbuf.buf; do { eol = strchrnul(p, '\n'); fprintf(stderr, "remote: %.*s\n", (int)(eol - p), p); p = eol + 1; } while(*eol); + strbuf_release(&msgbuf); return 0; } From 0cd3dfd4bb632ef80e17369a70241b4b1fd041b5 Mon Sep 17 00:00:00 2001 From: Vaidas Pilkauskas Date: Wed, 26 Nov 2025 13:33:08 +0200 Subject: [PATCH 03/15] http: add trace2 logging for retry operations Add trace2 instrumentation to HTTP 429 retry operations to enable monitoring and debugging of rate limit scenarios in production environments. The trace2 logging captures: * Retry attempt numbers (http/429-retry-attempt) to track retry progression and identify how many attempts were needed * Retry-After header values (http/429-retry-after) from server responses to understand server-requested delays * Actual sleep durations (http/retry-sleep-seconds) within trace2 regions (http/retry-sleep) to measure time spent waiting * Error conditions (http/429-error) such as "retries-exhausted", "exceeds-max-retry-time", "no-retry-after-config", and "config-exceeds-max-retry-time" for diagnosing failures * Retry source (http/429-retry-source) indicating whether delay came from server header or config default This instrumentation provides complete visibility into retry behavior, enabling operators to monitor rate limiting patterns, diagnose retry failures, and optimize retry configuration based on real-world data. Signed-off-by: Vaidas Pilkauskas --- http.c | 40 +++++++++++++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/http.c b/http.c index 212805cad56a59..f318e2fbe829dc 100644 --- a/http.c +++ b/http.c @@ -23,6 +23,7 @@ #include "odb.h" #include "tempfile.h" #include "date.h" +#include "trace2.h" static struct trace_key trace_curl = TRACE_KEY_INIT(CURL); static int trace_curl_data = 1; @@ -1944,6 +1945,8 @@ static int handle_curl_result(struct slot_results *results) } else if (results->http_code == 429) { /* Store the retry_after value for use in retry logic */ last_retry_after = results->retry_after; + trace2_data_intmax("http", the_repository, "http/429-retry-after", + last_retry_after); return HTTP_RATE_LIMITED; } else { if (results->http_connectcode == 407) @@ -2338,11 +2341,15 @@ static void sleep_for_retry(long retry_after) if (retry_after > 0) { unsigned int remaining; warning(_("rate limited, waiting %ld seconds before retry"), retry_after); + trace2_region_enter("http", "retry-sleep", the_repository); + trace2_data_intmax("http", the_repository, "http/retry-sleep-seconds", + retry_after); remaining = sleep(retry_after); while (remaining > 0) { /* Sleep was interrupted, continue sleeping */ remaining = sleep(remaining); } + trace2_region_leave("http", "retry-sleep", the_repository); } } @@ -2400,10 +2407,15 @@ static int http_request_reauth(const char *url, /* Handle rate limiting with retry logic */ int retry_attempt = http_max_retries - rate_limit_retries + 1; + trace2_data_intmax("http", the_repository, "http/429-retry-attempt", + retry_attempt); + if (rate_limit_retries <= 0) { /* Retries are disabled or exhausted */ if (http_max_retries > 0) { error(_("too many rate limit retries, giving up")); + trace2_data_string("http", the_repository, + "http/429-error", "retries-exhausted"); } return HTTP_ERROR; } @@ -2418,6 +2430,10 @@ static int http_request_reauth(const char *url, error(_("rate limited (HTTP 429) requested %ld second delay, " "exceeds http.maxRetryTime of %ld seconds"), last_retry_after, http_max_retry_time); + trace2_data_string("http", the_repository, + "http/429-error", "exceeds-max-retry-time"); + trace2_data_intmax("http", the_repository, + "http/429-requested-delay", last_retry_after); last_retry_after = -1; /* Reset after use */ return HTTP_ERROR; } @@ -2429,17 +2445,23 @@ static int http_request_reauth(const char *url, /* Not configured - exit with error */ error(_("rate limited (HTTP 429) and no Retry-After header provided. " "Configure http.retryAfter or set GIT_HTTP_RETRY_AFTER.")); + trace2_data_string("http", the_repository, + "http/429-error", "no-retry-after-config"); return HTTP_ERROR; } - /* Check if configured default exceeds maximum allowed */ - if (http_retry_after > http_max_retry_time) { - error(_("configured http.retryAfter (%ld seconds) exceeds " - "http.maxRetryTime (%ld seconds)"), - http_retry_after, http_max_retry_time); - return HTTP_ERROR; - } - /* Use configured default retry-after value */ - sleep_for_retry(http_retry_after); + /* Check if configured default exceeds maximum allowed */ + if (http_retry_after > http_max_retry_time) { + error(_("configured http.retryAfter (%ld seconds) exceeds " + "http.maxRetryTime (%ld seconds)"), + http_retry_after, http_max_retry_time); + trace2_data_string("http", the_repository, + "http/429-error", "config-exceeds-max-retry-time"); + return HTTP_ERROR; + } + /* Use configured default retry-after value */ + trace2_data_string("http", the_repository, + "http/429-retry-source", "config-default"); + sleep_for_retry(http_retry_after); } } else if (ret == HTTP_REAUTH) { credential_fill(the_repository, &http_auth, 1); From 380be469f273b1021512386c6e1121583d98ee5f Mon Sep 17 00:00:00 2001 From: Vaidas Pilkauskas Date: Fri, 12 Dec 2025 15:29:00 +0200 Subject: [PATCH 04/15] non-blocking retry handling --- http.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/http.c b/http.c index f318e2fbe829dc..ece64a9c5df2e4 100644 --- a/http.c +++ b/http.c @@ -2369,6 +2369,10 @@ static int http_request_reauth(const char *url, if (ret != HTTP_OK && ret != HTTP_REAUTH && ret != HTTP_RATE_LIMITED) return ret; + /* If retries are disabled and we got a 429, fail immediately */ + if (ret == HTTP_RATE_LIMITED && http_max_retries == 0) + return HTTP_ERROR; + if (options && options->effective_url && options->base_url) { if (update_url_from_redirect(options->base_url, url, options->effective_url)) { @@ -2405,11 +2409,6 @@ static int http_request_reauth(const char *url, if (ret == HTTP_RATE_LIMITED) { /* Handle rate limiting with retry logic */ - int retry_attempt = http_max_retries - rate_limit_retries + 1; - - trace2_data_intmax("http", the_repository, "http/429-retry-attempt", - retry_attempt); - if (rate_limit_retries <= 0) { /* Retries are disabled or exhausted */ if (http_max_retries > 0) { @@ -2420,6 +2419,11 @@ static int http_request_reauth(const char *url, return HTTP_ERROR; } + int retry_attempt = http_max_retries - rate_limit_retries + 1; + + trace2_data_intmax("http", the_repository, "http/429-retry-attempt", + retry_attempt); + /* Decrement retries counter */ rate_limit_retries--; From 041beb715854910848897b48eb533227e3efa8ef Mon Sep 17 00:00:00 2001 From: Vaidas Pilkauskas Date: Mon, 15 Dec 2025 12:03:49 +0200 Subject: [PATCH 05/15] draft-refactor-test-retry-state-handling --- t/lib-httpd.sh | 1 + t/lib-httpd/apache.conf | 8 ++ t/lib-httpd/http-429.sh | 98 ++++++++++++++++ t/t5584-http-429-retry.sh | 227 +++++++------------------------------- 4 files changed, 149 insertions(+), 185 deletions(-) create mode 100644 t/lib-httpd/http-429.sh diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh index 5091db949b7f99..8a43261ffc1ae2 100644 --- a/t/lib-httpd.sh +++ b/t/lib-httpd.sh @@ -167,6 +167,7 @@ prepare_httpd() { install_script error.sh install_script apply-one-time-script.sh install_script nph-custom-auth.sh + install_script http-429.sh ln -s "$LIB_HTTPD_MODULE_PATH" "$HTTPD_ROOT_PATH/modules" diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf index e631ab0eb5ef05..6bdef603cdb393 100644 --- a/t/lib-httpd/apache.conf +++ b/t/lib-httpd/apache.conf @@ -139,6 +139,10 @@ SetEnv PERL_PATH ${PERL_PATH} SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH} SetEnv GIT_HTTP_EXPORT_ALL + + SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH} + SetEnv GIT_HTTP_EXPORT_ALL + SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH} SetEnv GIT_HTTP_EXPORT_ALL @@ -160,6 +164,7 @@ ScriptAlias /broken_smart/ broken-smart-http.sh/ ScriptAlias /error_smart/ error-smart-http.sh/ ScriptAlias /error/ error.sh/ ScriptAliasMatch /one_time_script/(.*) apply-one-time-script.sh/$1 +ScriptAliasMatch /http_429/(.*) http-429.sh/$1 ScriptAliasMatch /custom_auth/(.*) nph-custom-auth.sh/$1 Options FollowSymlinks @@ -185,6 +190,9 @@ ScriptAliasMatch /custom_auth/(.*) nph-custom-auth.sh/$1 Options ExecCGI + + Options ExecCGI + Options ExecCGI diff --git a/t/lib-httpd/http-429.sh b/t/lib-httpd/http-429.sh new file mode 100644 index 00000000000000..c97b16145b7f92 --- /dev/null +++ b/t/lib-httpd/http-429.sh @@ -0,0 +1,98 @@ +#!/bin/sh + +# Script to return HTTP 429 Too Many Requests responses for testing retry logic. +# Usage: /http_429/// +# +# The test-context is a unique identifier for each test to isolate state files. +# The retry-after-value can be: +# - A number (e.g., "1", "2", "100") - sets Retry-After header to that many seconds +# - "none" - no Retry-After header +# - "invalid" - invalid Retry-After format +# - "permanent" - always return 429 (never succeed) +# - An HTTP-date string (RFC 2822 format) - sets Retry-After to that date +# +# On first call, returns 429. On subsequent calls (after retry), forwards to git-http-backend +# unless retry-after-value is "permanent". + +# Extract test context, retry-after value and repo path from PATH_INFO +# PATH_INFO format: /// +path_info="${PATH_INFO#/}" # Remove leading slash +test_context="${path_info%%/*}" # Get first component (test context) +remaining="${path_info#*/}" # Get rest +retry_after="${remaining%%/*}" # Get second component (retry-after value) +repo_path="${remaining#*/}" # Get rest (repo path) + +# Extract repository name from repo_path (e.g., "repo.git" from "repo.git/info/refs") +# The repo name is the first component before any "/" +repo_name="${repo_path%%/*}" + +# Use current directory (HTTPD_ROOT_PATH) for state file +# Create a safe filename from test_context, retry_after and repo_name +# This ensures all requests for the same test context share the same state file +safe_name=$(echo "${test_context}-${retry_after}-${repo_name}" | tr '/' '_' | tr -cd 'a-zA-Z0-9_-') +state_file="http-429-state-${safe_name}" + +# Check if this is the first call (no state file exists) +if test -f "$state_file" +then + # Already returned 429 once, forward to git-http-backend + # Set PATH_INFO to just the repo path (without retry-after value) + # Set GIT_PROJECT_ROOT so git-http-backend can find the repository + # Use exec to replace this process so git-http-backend gets the updated environment + PATH_INFO="/$repo_path" + export PATH_INFO + # GIT_PROJECT_ROOT points to the document root where repositories are stored + # The script runs from HTTPD_ROOT_PATH, and www/ is the document root + if test -z "$GIT_PROJECT_ROOT" + then + # Construct path: current directory (HTTPD_ROOT_PATH) + /www + GIT_PROJECT_ROOT="$(pwd)/www" + export GIT_PROJECT_ROOT + fi + exec "$GIT_EXEC_PATH/git-http-backend" +fi + +# Mark that we've returned 429 +touch "$state_file" + +# Output HTTP 429 response +printf "Status: 429 Too Many Requests\r\n" + +# Set Retry-After header based on retry_after value +case "$retry_after" in + none) + # No Retry-After header + ;; + invalid) + printf "Retry-After: invalid-format-123abc\r\n" + ;; + permanent) + # Always return 429, don't set state file for success + rm -f "$state_file" + printf "Retry-After: 1\r\n" + printf "Content-Type: text/plain\r\n" + printf "\r\n" + printf "Permanently rate limited\n" + exit 0 + ;; + *) + # Check if it's a number + case "$retry_after" in + [0-9]*) + # Numeric value + printf "Retry-After: %s\r\n" "$retry_after" + ;; + *) + # Assume it's an HTTP-date format (passed as-is, URL decoded) + # Apache may URL-encode the path, so decode common URL-encoded characters + # %20 = space, %2C = comma, %3A = colon + retry_value=$(echo "$retry_after" | sed -e 's/%20/ /g' -e 's/%2C/,/g' -e 's/%3A/:/g') + printf "Retry-After: %s\r\n" "$retry_value" + ;; + esac + ;; +esac + +printf "Content-Type: text/plain\r\n" +printf "\r\n" +printf "Rate limited\n" diff --git a/t/t5584-http-429-retry.sh b/t/t5584-http-429-retry.sh index 8bcc382763037b..095b4244e8175f 100755 --- a/t/t5584-http-429-retry.sh +++ b/t/t5584-http-429-retry.sh @@ -14,155 +14,86 @@ test_expect_success 'setup test repository' ' git --git-dir="$HTTPD_DOCUMENT_ROOT_PATH/repo.git" config http.receivepack true ' -test_expect_success 'HTTP 429 with retries disabled (maxRetries=0) fails immediately' ' - write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF && - printf "Status: 429 Too Many Requests\r\n" - printf "Retry-After: 1\r\n" - printf "Content-Type: text/plain\r\n" - printf "\r\n" - printf "Rate limited\n" - cat "$1" >/dev/null - EOF +# This test suite uses a special HTTP 429 endpoint at /http_429/ that simulates +# rate limiting. The endpoint format is: +# /http_429/// +# The http-429.sh script (in t/lib-httpd) returns a 429 response with the +# specified Retry-After header on the first request for each test context, +# then forwards subsequent requests to git-http-backend. Each test context +# is isolated, allowing multiple tests to run independently. +test_expect_success 'HTTP 429 with retries disabled (maxRetries=0) fails immediately' ' # Set maxRetries to 0 (disabled) test_config http.maxRetries 0 && test_config http.retryAfter 1 && # Should fail immediately without any retry attempt - test_must_fail git ls-remote "$HTTPD_URL/one_time_script/repo.git" 2>err && + test_must_fail git ls-remote "$HTTPD_URL/http_429/retries-disabled/1/repo.git" 2>err && # Verify no retry happened (no "waiting" message in stderr) - ! grep -i "waiting.*retry" err && - - # The one-time script will be consumed on first request (not a retry) - test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script" + ! grep -i "waiting.*retry" err ' test_expect_success 'HTTP 429 permanent should fail after max retries' ' - # Install a permanent error script to prove retries are limited - write_script "$HTTPD_ROOT_PATH/http-429-permanent.sh" <<-\EOF && - printf "Status: 429 Too Many Requests\r\n" - printf "Retry-After: 1\r\n" - printf "Content-Type: text/plain\r\n" - printf "\r\n" - printf "Permanently rate limited\n" - EOF - # Enable retries with a limit test_config http.maxRetries 2 && # Git should retry but eventually fail when 429 persists - test_must_fail git ls-remote "$HTTPD_URL/error/http-429-permanent.sh/repo.git" 2>err + test_must_fail git ls-remote "$HTTPD_URL/http_429/permanent-fail/permanent/repo.git" 2>err ' test_expect_success 'HTTP 429 with Retry-After is retried and succeeds' ' - # Create a one-time script that returns 429 with Retry-After header - # on the first request. Subsequent requests will succeed. - # This contrasts with the permanent 429 above - proving retry works - write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF && - # Return HTTP 429 response instead of git response - printf "Status: 429 Too Many Requests\r\n" - printf "Retry-After: 1\r\n" - printf "Content-Type: text/plain\r\n" - printf "\r\n" - printf "Rate limited - please retry after 1 second\n" - # Output something different from input so the script gets removed - cat "$1" >/dev/null - EOF - # Enable retries test_config http.maxRetries 3 && # Git should retry after receiving 429 and eventually succeed - git ls-remote "$HTTPD_URL/one_time_script/repo.git" >output 2>err && - test_grep "refs/heads/" output && - - # The one-time script should have been consumed (proving retry happened) - test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script" + git ls-remote "$HTTPD_URL/http_429/retry-succeeds/1/repo.git" >output 2>err && + test_grep "refs/heads/" output ' test_expect_success 'HTTP 429 without Retry-After uses configured default' ' - write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF && - printf "Status: 429 Too Many Requests\r\n" - printf "Content-Type: text/plain\r\n" - printf "\r\n" - printf "Rate limited - no retry info\n" - cat "$1" >/dev/null - EOF - # Enable retries and configure default delay test_config http.maxRetries 3 && test_config http.retryAfter 1 && # Git should retry using configured default and succeed - git ls-remote "$HTTPD_URL/one_time_script/repo.git" >output 2>err && - test_grep "refs/heads/" output && - test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script" + git ls-remote "$HTTPD_URL/http_429/no-retry-after-header/none/repo.git" >output 2>err && + test_grep "refs/heads/" output ' test_expect_success 'HTTP 429 retry delays are respected' ' - write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF && - printf "Status: 429 Too Many Requests\r\n" - printf "Retry-After: 2\r\n" - printf "Content-Type: text/plain\r\n" - printf "\r\n" - printf "Rate limited\n" - cat "$1" >/dev/null - EOF - # Enable retries test_config http.maxRetries 3 && # Time the operation - it should take at least 2 seconds due to retry delay start=$(date +%s) && - git ls-remote "$HTTPD_URL/one_time_script/repo.git" >output 2>err && + git ls-remote "$HTTPD_URL/http_429/retry-delays-respected/2/repo.git" >output 2>err && end=$(date +%s) && duration=$((end - start)) && # Verify it took at least 2 seconds (allowing some tolerance) test "$duration" -ge 1 && - test_grep "refs/heads/" output && - test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script" + test_grep "refs/heads/" output ' test_expect_success 'HTTP 429 fails immediately if Retry-After exceeds http.maxRetryTime' ' - write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF && - printf "Status: 429 Too Many Requests\r\n" - printf "Retry-After: 100\r\n" - printf "Content-Type: text/plain\r\n" - printf "\r\n" - printf "Rate limited with long delay\n" - cat "$1" >/dev/null - EOF - # Configure max retry time to 3 seconds (much less than requested 100) test_config http.maxRetries 3 && test_config http.maxRetryTime 3 && # Should fail immediately without waiting start=$(date +%s) && - test_must_fail git ls-remote "$HTTPD_URL/one_time_script/repo.git" 2>err && + test_must_fail git ls-remote "$HTTPD_URL/http_429/retry-after-exceeds-max-time/100/repo.git" 2>err && end=$(date +%s) && duration=$((end - start)) && # Should fail quickly (less than 2 seconds, no 100 second wait) test "$duration" -lt 2 && - test_grep "exceeds http.maxRetryTime" err && - - # The one-time script will be consumed on first request - test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script" + test_grep "exceeds http.maxRetryTime" err ' test_expect_success 'HTTP 429 fails if configured http.retryAfter exceeds http.maxRetryTime' ' # Test misconfiguration: retryAfter > maxRetryTime - write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF && - printf "Status: 429 Too Many Requests\r\n" - printf "Content-Type: text/plain\r\n" - printf "\r\n" - printf "Rate limited without header\n" - cat "$1" >/dev/null - EOF - # Configure retryAfter larger than maxRetryTime test_config http.maxRetries 3 && test_config http.retryAfter 100 && @@ -170,7 +101,7 @@ test_expect_success 'HTTP 429 fails if configured http.retryAfter exceeds http.m # Should fail immediately with configuration error start=$(date +%s) && - test_must_fail git ls-remote "$HTTPD_URL/one_time_script/repo.git" 2>err && + test_must_fail git ls-remote "$HTTPD_URL/http_429/config-retry-after-exceeds-max-time/none/repo.git" 2>err && end=$(date +%s) && duration=$((end - start)) && @@ -192,28 +123,21 @@ test_expect_success 'HTTP 429 with Retry-After HTTP-date format' ' test_done fi && - write_script "$HTTPD_ROOT_PATH/one-time-script" <<-EOF && - printf "Status: 429 Too Many Requests\\r\\n" - printf "Retry-After: $future_date\\r\\n" - printf "Content-Type: text/plain\\r\\n" - printf "\\r\\n" - printf "Rate limited with HTTP-date\\n" - cat "\$1" >/dev/null - EOF + # URL-encode the date (replace spaces with %20) + future_date_encoded=$(echo "$future_date" | sed "s/ /%20/g") && # Enable retries test_config http.maxRetries 3 && # Git should parse the HTTP-date and retry after the delay start=$(date +%s) && - git ls-remote "$HTTPD_URL/one_time_script/repo.git" >output 2>err && + git ls-remote "$HTTPD_URL/http_429/http-date-format/$future_date_encoded/repo.git" >output 2>err && end=$(date +%s) && duration=$((end - start)) && # Should take at least 1 second (allowing tolerance for processing time) test "$duration" -ge 1 && - test_grep "refs/heads/" output && - test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script" + test_grep "refs/heads/" output ' test_expect_success 'HTTP 429 with HTTP-date exceeding maxRetryTime fails immediately' ' @@ -228,14 +152,8 @@ test_expect_success 'HTTP 429 with HTTP-date exceeding maxRetryTime fails immedi test_done fi && - write_script "$HTTPD_ROOT_PATH/one-time-script" <<-EOF && - printf "Status: 429 Too Many Requests\\r\\n" - printf "Retry-After: $future_date\\r\\n" - printf "Content-Type: text/plain\\r\\n" - printf "\\r\\n" - printf "Rate limited with long HTTP-date\\n" - cat "\$1" >/dev/null - EOF + # URL-encode the date (replace spaces with %20) + future_date_encoded=$(echo "$future_date" | sed "s/ /%20/g") && # Configure max retry time much less than the 200 second delay test_config http.maxRetries 3 && @@ -243,16 +161,13 @@ test_expect_success 'HTTP 429 with HTTP-date exceeding maxRetryTime fails immedi # Should fail immediately without waiting 200 seconds start=$(date +%s) && - test_must_fail git ls-remote "$HTTPD_URL/one_time_script/repo.git" 2>err && + test_must_fail git ls-remote "$HTTPD_URL/http_429/http-date-exceeds-max-time/$future_date_encoded/repo.git" 2>err && end=$(date +%s) && duration=$((end - start)) && # Should fail quickly (not wait 200 seconds) test "$duration" -lt 2 && - test_grep "exceeds http.maxRetryTime" err && - - # The one-time script will be consumed on first request - test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script" + test_grep "exceeds http.maxRetryTime" err ' test_expect_success 'HTTP 429 with past HTTP-date should not wait' ' @@ -266,100 +181,63 @@ test_expect_success 'HTTP 429 with past HTTP-date should not wait' ' test_done fi && - write_script "$HTTPD_ROOT_PATH/one-time-script" <<-EOF && - printf "Status: 429 Too Many Requests\\r\\n" - printf "Retry-After: $past_date\\r\\n" - printf "Content-Type: text/plain\\r\\n" - printf "\\r\\n" - printf "Rate limited with past date\\n" - cat "\$1" >/dev/null - EOF + # URL-encode the date (replace spaces with %20) + past_date_encoded=$(echo "$past_date" | sed "s/ /%20/g") && # Enable retries test_config http.maxRetries 3 && # Git should retry immediately without waiting start=$(date +%s) && - git ls-remote "$HTTPD_URL/one_time_script/repo.git" >output 2>err && + git ls-remote "$HTTPD_URL/http_429/past-http-date/$past_date_encoded/repo.git" >output 2>err && end=$(date +%s) && duration=$((end - start)) && # Should complete quickly (less than 2 seconds) test "$duration" -lt 2 && - test_grep "refs/heads/" output && - test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script" + test_grep "refs/heads/" output ' test_expect_success 'HTTP 429 with invalid Retry-After format uses configured default' ' - write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF && - printf "Status: 429 Too Many Requests\r\n" - printf "Retry-After: invalid-format-123abc\r\n" - printf "Content-Type: text/plain\r\n" - printf "\r\n" - printf "Rate limited with malformed header\n" - cat "$1" >/dev/null - EOF - # Configure default retry-after test_config http.maxRetries 3 && test_config http.retryAfter 1 && # Should use configured default (1 second) since header is invalid start=$(date +%s) && - git ls-remote "$HTTPD_URL/one_time_script/repo.git" >output 2>err && + git ls-remote "$HTTPD_URL/http_429/invalid-retry-after-format/invalid/repo.git" >output 2>err && end=$(date +%s) && duration=$((end - start)) && # Should take at least 1 second (the configured default) test "$duration" -ge 1 && test_grep "refs/heads/" output && - test_grep "waiting.*retry" err && - test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script" + test_grep "waiting.*retry" err ' test_expect_success 'HTTP 429 will not be retried without config' ' # Default config means http.maxRetries=0 (retries disabled) # When 429 is received, it should fail immediately without retry - write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF && - printf "Status: 429 Too Many Requests\r\n" - printf "Retry-After: 1\r\n" - printf "Content-Type: text/plain\r\n" - printf "\r\n" - printf "Rate limited\n" - cat "$1" >/dev/null - EOF - # Do NOT configure anything - use defaults (http.maxRetries defaults to 0) # Should fail immediately without retry - test_must_fail git ls-remote "$HTTPD_URL/one_time_script/repo.git" 2>err && + test_must_fail git ls-remote "$HTTPD_URL/http_429/no-retry-without-config/1/repo.git" 2>err && # Verify no retry happened (no "waiting" message) ! grep -i "waiting.*retry" err && # Should get 429 error - test_grep "429" err && - - # The one-time script should be consumed on first request - test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script" + test_grep "429" err ' test_expect_success 'GIT_HTTP_RETRY_AFTER overrides http.retryAfter config' ' - write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF && - printf "Status: 429 Too Many Requests\r\n" - printf "Content-Type: text/plain\r\n" - printf "\r\n" - printf "Rate limited - no Retry-After header\n" - cat "$1" >/dev/null - EOF - # Configure retryAfter to 10 seconds test_config http.maxRetries 3 && test_config http.retryAfter 10 && # Override with environment variable to 1 second start=$(date +%s) && - GIT_HTTP_RETRY_AFTER=1 git ls-remote "$HTTPD_URL/one_time_script/repo.git" >output 2>err && + GIT_HTTP_RETRY_AFTER=1 git ls-remote "$HTTPD_URL/http_429/env-retry-after-override/none/repo.git" >output 2>err && end=$(date +%s) && duration=$((end - start)) && @@ -367,43 +245,23 @@ test_expect_success 'GIT_HTTP_RETRY_AFTER overrides http.retryAfter config' ' test "$duration" -ge 1 && test "$duration" -lt 5 && test_grep "refs/heads/" output && - test_grep "waiting.*retry" err && - test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script" + test_grep "waiting.*retry" err ' test_expect_success 'GIT_HTTP_MAX_RETRIES overrides http.maxRetries config' ' - write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF && - printf "Status: 429 Too Many Requests\r\n" - printf "Retry-After: 1\r\n" - printf "Content-Type: text/plain\r\n" - printf "\r\n" - printf "Rate limited\n" - cat "$1" >/dev/null - EOF - # Configure maxRetries to 0 (disabled) test_config http.maxRetries 0 && test_config http.retryAfter 1 && # Override with environment variable to enable retries - GIT_HTTP_MAX_RETRIES=3 git ls-remote "$HTTPD_URL/one_time_script/repo.git" >output 2>err && + GIT_HTTP_MAX_RETRIES=3 git ls-remote "$HTTPD_URL/http_429/env-max-retries-override/1/repo.git" >output 2>err && # Should retry (env var enables it despite config saying disabled) test_grep "refs/heads/" output && - test_grep "waiting.*retry" err && - test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script" + test_grep "waiting.*retry" err ' test_expect_success 'GIT_HTTP_MAX_RETRY_TIME overrides http.maxRetryTime config' ' - write_script "$HTTPD_ROOT_PATH/one-time-script" <<-\EOF && - printf "Status: 429 Too Many Requests\r\n" - printf "Retry-After: 50\r\n" - printf "Content-Type: text/plain\r\n" - printf "\r\n" - printf "Rate limited with long delay\n" - cat "$1" >/dev/null - EOF - # Configure maxRetryTime to 100 seconds (would accept 50 second delay) test_config http.maxRetries 3 && test_config http.maxRetryTime 100 && @@ -411,14 +269,13 @@ test_expect_success 'GIT_HTTP_MAX_RETRY_TIME overrides http.maxRetryTime config' # Override with environment variable to 10 seconds (should reject 50 second delay) start=$(date +%s) && test_must_fail env GIT_HTTP_MAX_RETRY_TIME=10 \ - git ls-remote "$HTTPD_URL/one_time_script/repo.git" 2>err && + git ls-remote "$HTTPD_URL/http_429/env-max-retry-time-override/50/repo.git" 2>err && end=$(date +%s) && duration=$((end - start)) && # Should fail quickly (not wait 50 seconds) because env var limits to 10 test "$duration" -lt 5 && - test_grep "exceeds http.maxRetryTime" err && - test_path_is_missing "$HTTPD_ROOT_PATH/one-time-script" + test_grep "exceeds http.maxRetryTime" err ' test_expect_success 'verify normal repository access still works' ' From 869d97c3415d68d38da138bbd52abfdda64b0be4 Mon Sep 17 00:00:00 2001 From: Vaidas Pilkauskas Date: Mon, 15 Dec 2025 13:51:17 +0200 Subject: [PATCH 06/15] fix C90 decalration error --- http.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/http.c b/http.c index ece64a9c5df2e4..98433211291bcb 100644 --- a/http.c +++ b/http.c @@ -2409,6 +2409,8 @@ static int http_request_reauth(const char *url, if (ret == HTTP_RATE_LIMITED) { /* Handle rate limiting with retry logic */ + int retry_attempt = http_max_retries - rate_limit_retries + 1; + if (rate_limit_retries <= 0) { /* Retries are disabled or exhausted */ if (http_max_retries > 0) { @@ -2419,8 +2421,6 @@ static int http_request_reauth(const char *url, return HTTP_ERROR; } - int retry_attempt = http_max_retries - rate_limit_retries + 1; - trace2_data_intmax("http", the_repository, "http/429-retry-attempt", retry_attempt); From 83e8eae11a4779ed5b080dcbac8b0927e52cc00e Mon Sep 17 00:00:00 2001 From: Vaidas Pilkauskas Date: Mon, 15 Dec 2025 14:21:50 +0200 Subject: [PATCH 07/15] make strings translatable --- http-push.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/http-push.c b/http-push.c index 4ec13ca4ecb9ab..ddb99483524c4c 100644 --- a/http-push.c +++ b/http-push.c @@ -717,7 +717,7 @@ static int fetch_indices(void) ret = 0; break; case HTTP_RATE_LIMITED: - error("rate limited by '%s', please try again later", repo->url); + error(_("rate limited by '%s', please try again later"), repo->url); ret = -1; break; default: @@ -1553,7 +1553,7 @@ static int remote_exists(const char *path) ret = 0; break; case HTTP_RATE_LIMITED: - error("rate limited by '%s', please try again later", url); + error(_("rate limited by '%s', please try again later"), url); ret = -1; break; case HTTP_ERROR: From 9b887e451c13266b152e79b3db9b62f45f1668f7 Mon Sep 17 00:00:00 2001 From: Vaidas Pilkauskas Date: Mon, 15 Dec 2025 14:29:55 +0200 Subject: [PATCH 08/15] Remove redundand comments as per review --- http.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/http.c b/http.c index 98433211291bcb..3f1acfb2122978 100644 --- a/http.c +++ b/http.c @@ -152,13 +152,15 @@ static char *http_ssl_backend; static int http_schannel_check_revoke = 1; -/* Retry configuration */ -static long http_retry_after = -1; /* Default retry-after in seconds when header is missing (-1 means not set, exit with 128) */ -static long http_max_retries = 0; /* Maximum number of retry attempts (0 means retries are disabled) */ -static long http_max_retry_time = 300; /* Maximum time to wait for a single retry (default 5 minutes) */ - -/* Store retry_after value from 429 responses for retry logic (-1 = not set, 0 = retry immediately, >0 = delay in seconds) */ +static long http_retry_after = -1; +static long http_max_retries = 0; +static long http_max_retry_time = 300; +/* + * Store retry_after value from 429 responses for retry logic + * (-1 = not set, 0 = retry immediately, >0 = delay in seconds). + */ static long last_retry_after = -1; + /* * With the backend being set to `schannel`, setting sslCAinfo would override * the Certificate Store in cURL v7.60.0 and later, which is not what we want From c4bf891164c65e150617dc0b10c9e2717a482e0e Mon Sep 17 00:00:00 2001 From: Vaidas Pilkauskas Date: Mon, 15 Dec 2025 15:12:15 +0200 Subject: [PATCH 09/15] rename fwrite_wwwauth --- http.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/http.c b/http.c index 3f1acfb2122978..aa9b3015b9b8e4 100644 --- a/http.c +++ b/http.c @@ -221,7 +221,7 @@ static inline int is_hdr_continuation(const char *ptr, const size_t size) return size && (*ptr == ' ' || *ptr == '\t'); } -static size_t fwrite_wwwauth(char *ptr, size_t eltsize, size_t nmemb, void *p) +static size_t fwrite_headers(char *ptr, size_t eltsize, size_t nmemb, void *p) { size_t size = eltsize * nmemb; struct strvec *values = &http_auth.wwwauth_headers; @@ -2229,7 +2229,7 @@ static int http_request(const char *url, fwrite_buffer); } - curl_easy_setopt(slot->curl, CURLOPT_HEADERFUNCTION, fwrite_wwwauth); + curl_easy_setopt(slot->curl, CURLOPT_HEADERFUNCTION, fwrite_headers); curl_easy_setopt(slot->curl, CURLOPT_HEADERDATA, slot); accept_language = http_get_accept_language_header(); From b28e82a4e3fd66f334e88624ebdc4d81e3a778e5 Mon Sep 17 00:00:00 2001 From: Vaidas Pilkauskas Date: Mon, 15 Dec 2025 15:54:16 +0200 Subject: [PATCH 10/15] refactor http_request_reauth --- http.c | 136 +++++++++++++++++++++++++++++++-------------------------- 1 file changed, 74 insertions(+), 62 deletions(-) diff --git a/http.c b/http.c index aa9b3015b9b8e4..3afb5cc575d31c 100644 --- a/http.c +++ b/http.c @@ -2355,7 +2355,75 @@ static void sleep_for_retry(long retry_after) } } -static int http_request_reauth(const char *url, +/* + * Handle rate limiting retry logic for HTTP 429 responses. + * Returns HTTP_ERROR if retries are exhausted or configuration is invalid, + * otherwise returns 0 to indicate the retry should proceed. + */ +static int handle_rate_limit_retry(int *rate_limit_retries) +{ + int retry_attempt = http_max_retries - *rate_limit_retries + 1; + + if (*rate_limit_retries <= 0) { + /* Retries are disabled or exhausted */ + if (http_max_retries > 0) { + error(_("too many rate limit retries, giving up")); + trace2_data_string("http", the_repository, + "http/429-error", "retries-exhausted"); + } + return HTTP_ERROR; + } + + trace2_data_intmax("http", the_repository, "http/429-retry-attempt", + retry_attempt); + + /* Decrement retries counter */ + (*rate_limit_retries)--; + + /* Use the stored retry_after value or configured default */ + if (last_retry_after >= 0) { + /* Check if retry delay exceeds maximum allowed */ + if (last_retry_after > http_max_retry_time) { + error(_("rate limited (HTTP 429) requested %ld second delay, " + "exceeds http.maxRetryTime of %ld seconds"), + last_retry_after, http_max_retry_time); + trace2_data_string("http", the_repository, + "http/429-error", "exceeds-max-retry-time"); + trace2_data_intmax("http", the_repository, + "http/429-requested-delay", last_retry_after); + last_retry_after = -1; /* Reset after use */ + return HTTP_ERROR; + } + sleep_for_retry(last_retry_after); + last_retry_after = -1; /* Reset after use */ + } else { + /* No Retry-After header provided */ + if (http_retry_after < 0) { + /* Not configured - exit with error */ + error(_("rate limited (HTTP 429) and no Retry-After header provided. " + "Configure http.retryAfter or set GIT_HTTP_RETRY_AFTER.")); + trace2_data_string("http", the_repository, + "http/429-error", "no-retry-after-config"); + return HTTP_ERROR; + } + /* Check if configured default exceeds maximum allowed */ + if (http_retry_after > http_max_retry_time) { + error(_("configured http.retryAfter (%ld seconds) exceeds " + "http.maxRetryTime (%ld seconds)"), + http_retry_after, http_max_retry_time); + trace2_data_string("http", the_repository, + "http/429-error", "config-exceeds-max-retry-time"); + return HTTP_ERROR; + } + /* Use configured default retry-after value */ + trace2_data_string("http", the_repository, + "http/429-retry-source", "config-default"); + sleep_for_retry(http_retry_after); + } + return 0; +} + +static int http_request_recoverable(const char *url, void *result, int target, struct http_get_options *options) { @@ -2410,65 +2478,9 @@ static int http_request_reauth(const char *url, } if (ret == HTTP_RATE_LIMITED) { - /* Handle rate limiting with retry logic */ - int retry_attempt = http_max_retries - rate_limit_retries + 1; - - if (rate_limit_retries <= 0) { - /* Retries are disabled or exhausted */ - if (http_max_retries > 0) { - error(_("too many rate limit retries, giving up")); - trace2_data_string("http", the_repository, - "http/429-error", "retries-exhausted"); - } - return HTTP_ERROR; - } - - trace2_data_intmax("http", the_repository, "http/429-retry-attempt", - retry_attempt); - - /* Decrement retries counter */ - rate_limit_retries--; - - /* Use the stored retry_after value or configured default */ - if (last_retry_after >= 0) { - /* Check if retry delay exceeds maximum allowed */ - if (last_retry_after > http_max_retry_time) { - error(_("rate limited (HTTP 429) requested %ld second delay, " - "exceeds http.maxRetryTime of %ld seconds"), - last_retry_after, http_max_retry_time); - trace2_data_string("http", the_repository, - "http/429-error", "exceeds-max-retry-time"); - trace2_data_intmax("http", the_repository, - "http/429-requested-delay", last_retry_after); - last_retry_after = -1; /* Reset after use */ - return HTTP_ERROR; - } - sleep_for_retry(last_retry_after); - last_retry_after = -1; /* Reset after use */ - } else { - /* No Retry-After header provided */ - if (http_retry_after < 0) { - /* Not configured - exit with error */ - error(_("rate limited (HTTP 429) and no Retry-After header provided. " - "Configure http.retryAfter or set GIT_HTTP_RETRY_AFTER.")); - trace2_data_string("http", the_repository, - "http/429-error", "no-retry-after-config"); - return HTTP_ERROR; - } - /* Check if configured default exceeds maximum allowed */ - if (http_retry_after > http_max_retry_time) { - error(_("configured http.retryAfter (%ld seconds) exceeds " - "http.maxRetryTime (%ld seconds)"), - http_retry_after, http_max_retry_time); - trace2_data_string("http", the_repository, - "http/429-error", "config-exceeds-max-retry-time"); - return HTTP_ERROR; - } - /* Use configured default retry-after value */ - trace2_data_string("http", the_repository, - "http/429-retry-source", "config-default"); - sleep_for_retry(http_retry_after); - } + int retry_result = handle_rate_limit_retry(&rate_limit_retries); + if (retry_result != 0) + return retry_result; } else if (ret == HTTP_REAUTH) { credential_fill(the_repository, &http_auth, 1); } @@ -2482,7 +2494,7 @@ int http_get_strbuf(const char *url, struct strbuf *result, struct http_get_options *options) { - return http_request_reauth(url, result, HTTP_REQUEST_STRBUF, options); + return http_request_recoverable(url, result, HTTP_REQUEST_STRBUF, options); } /* @@ -2506,7 +2518,7 @@ int http_get_file(const char *url, const char *filename, goto cleanup; } - ret = http_request_reauth(url, result, HTTP_REQUEST_FILE, options); + ret = http_request_recoverable(url, result, HTTP_REQUEST_FILE, options); fclose(result); if (ret == HTTP_OK && finalize_object_file(the_repository, tmpfile.buf, filename)) From fd7dd6b4c97df9d78c7f16520a4afc73f95e869f Mon Sep 17 00:00:00 2001 From: Vaidas Pilkauskas Date: Mon, 15 Dec 2025 16:16:36 +0200 Subject: [PATCH 11/15] Revert "remote-curl: fix memory leak in show_http_message()" This reverts commit 7639fa3ae895d7121d190a08a8223a9ecdc47703. --- remote-curl.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/remote-curl.c b/remote-curl.c index dd0680e5ae2461..5959461cd34220 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -371,7 +371,6 @@ static int show_http_message(struct strbuf *type, struct strbuf *charset, struct strbuf *msg) { const char *p, *eol; - struct strbuf msgbuf = STRBUF_INIT; /* * We only show text/plain parts, as other types are likely @@ -379,24 +378,19 @@ static int show_http_message(struct strbuf *type, struct strbuf *charset, */ if (strcmp(type->buf, "text/plain")) return -1; - - strbuf_addbuf(&msgbuf, msg); if (charset->len) - strbuf_reencode(&msgbuf, charset->buf, get_log_output_encoding()); + strbuf_reencode(msg, charset->buf, get_log_output_encoding()); - strbuf_trim(&msgbuf); - if (!msgbuf.len) { - strbuf_release(&msgbuf); + strbuf_trim(msg); + if (!msg->len) return -1; - } - p = msgbuf.buf; + p = msg->buf; do { eol = strchrnul(p, '\n'); fprintf(stderr, "remote: %.*s\n", (int)(eol - p), p); p = eol + 1; } while(*eol); - strbuf_release(&msgbuf); return 0; } From 5e7fe60a5a13920d223e8b132ec6063281cc6a25 Mon Sep 17 00:00:00 2001 From: Vaidas Pilkauskas Date: Mon, 15 Dec 2025 16:25:14 +0200 Subject: [PATCH 12/15] show_http_message_fatal --- remote-curl.c | 50 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/remote-curl.c b/remote-curl.c index 5959461cd34220..a64b997662f003 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -367,23 +367,25 @@ static void free_discovery(struct discovery *d) } } -static int show_http_message(struct strbuf *type, struct strbuf *charset, - struct strbuf *msg) +static NORETURN void show_http_message_fatal(struct strbuf *type, struct strbuf *charset, + struct strbuf *msg, const char *fmt, ...) { const char *p, *eol; + va_list ap; + report_fn die_message_routine = get_die_message_routine(); /* * We only show text/plain parts, as other types are likely * to be ugly to look at on the user's terminal. */ if (strcmp(type->buf, "text/plain")) - return -1; + goto out; if (charset->len) strbuf_reencode(msg, charset->buf, get_log_output_encoding()); strbuf_trim(msg); if (!msg->len) - return -1; + goto out; p = msg->buf; do { @@ -391,7 +393,15 @@ static int show_http_message(struct strbuf *type, struct strbuf *charset, fprintf(stderr, "remote: %.*s\n", (int)(eol - p), p); p = eol + 1; } while(*eol); - return 0; + +out: + strbuf_release(type); + strbuf_release(charset); + strbuf_release(msg); + + va_start(ap, fmt); + die_message_routine(fmt, ap); + va_end(ap); } static int get_protocol_http_header(enum protocol_version version, @@ -518,25 +528,25 @@ static struct discovery *discover_refs(const char *service, int for_push) case HTTP_OK: break; case HTTP_MISSING_TARGET: - show_http_message(&type, &charset, &buffer); - die(_("repository '%s' not found"), - transport_anonymize_url(url.buf)); + show_http_message_fatal(&type, &charset, &buffer, + _("repository '%s' not found"), + transport_anonymize_url(url.buf)); case HTTP_NOAUTH: - show_http_message(&type, &charset, &buffer); - die(_("Authentication failed for '%s'"), - transport_anonymize_url(url.buf)); + show_http_message_fatal(&type, &charset, &buffer, + _("Authentication failed for '%s'"), + transport_anonymize_url(url.buf)); case HTTP_NOMATCHPUBLICKEY: - show_http_message(&type, &charset, &buffer); - die(_("unable to access '%s' with http.pinnedPubkey configuration: %s"), - transport_anonymize_url(url.buf), curl_errorstr); + show_http_message_fatal(&type, &charset, &buffer, + _("unable to access '%s' with http.pinnedPubkey configuration: %s"), + transport_anonymize_url(url.buf), curl_errorstr); case HTTP_RATE_LIMITED: - show_http_message(&type, &charset, &buffer); - die(_("rate limited by '%s', please try again later"), - transport_anonymize_url(url.buf)); + show_http_message_fatal(&type, &charset, &buffer, + _("rate limited by '%s', please try again later"), + transport_anonymize_url(url.buf)); default: - show_http_message(&type, &charset, &buffer); - die(_("unable to access '%s': %s"), - transport_anonymize_url(url.buf), curl_errorstr); + show_http_message_fatal(&type, &charset, &buffer, + _("unable to access '%s': %s"), + transport_anonymize_url(url.buf), curl_errorstr); } if (options.verbosity && !starts_with(refs_url.buf, url.buf)) { From 9133f912ddacb07bb10e76a81e8d3008a371b70f Mon Sep 17 00:00:00 2001 From: Vaidas Pilkauskas Date: Mon, 15 Dec 2025 20:12:56 +0200 Subject: [PATCH 13/15] dien with 128 in fatal message --- remote-curl.c | 1 + 1 file changed, 1 insertion(+) diff --git a/remote-curl.c b/remote-curl.c index a64b997662f003..c122dcedaab86f 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -402,6 +402,7 @@ static NORETURN void show_http_message_fatal(struct strbuf *type, struct strbuf va_start(ap, fmt); die_message_routine(fmt, ap); va_end(ap); + exit(128); } static int get_protocol_http_header(enum protocol_version version, From 3527d87d9f2d8f1da7c03ee5d04ac20621b72eca Mon Sep 17 00:00:00 2001 From: Vaidas Pilkauskas Date: Tue, 16 Dec 2025 10:53:43 +0200 Subject: [PATCH 14/15] use test_grep --- t/t5584-http-429-retry.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t5584-http-429-retry.sh b/t/t5584-http-429-retry.sh index 095b4244e8175f..c0d30c5387034e 100755 --- a/t/t5584-http-429-retry.sh +++ b/t/t5584-http-429-retry.sh @@ -31,7 +31,7 @@ test_expect_success 'HTTP 429 with retries disabled (maxRetries=0) fails immedia test_must_fail git ls-remote "$HTTPD_URL/http_429/retries-disabled/1/repo.git" 2>err && # Verify no retry happened (no "waiting" message in stderr) - ! grep -i "waiting.*retry" err + test_grep ! -i "waiting.*retry" err ' test_expect_success 'HTTP 429 permanent should fail after max retries' ' @@ -224,7 +224,7 @@ test_expect_success 'HTTP 429 will not be retried without config' ' test_must_fail git ls-remote "$HTTPD_URL/http_429/no-retry-without-config/1/repo.git" 2>err && # Verify no retry happened (no "waiting" message) - ! grep -i "waiting.*retry" err && + test_grep ! -i "waiting.*retry" err && # Should get 429 error test_grep "429" err From 36bad7aad29d9685aea679e5ea7f6431215ba1f1 Mon Sep 17 00:00:00 2001 From: Vaidas Pilkauskas Date: Tue, 16 Dec 2025 15:33:02 +0200 Subject: [PATCH 15/15] Fix memleak --- strbuf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/strbuf.c b/strbuf.c index 6c3851a7f84d72..1d3860869e5e57 100644 --- a/strbuf.c +++ b/strbuf.c @@ -168,7 +168,7 @@ int strbuf_reencode(struct strbuf *sb, const char *from, const char *to) if (!out) return -1; - strbuf_attach(sb, out, len, len); + strbuf_attach(sb, out, len, len + 1); return 0; }