diff --git a/Documentation/RelNotes/2.54.0.adoc b/Documentation/RelNotes/2.54.0.adoc index f7b2db616d0413..85b15284f3b958 100644 --- a/Documentation/RelNotes/2.54.0.adoc +++ b/Documentation/RelNotes/2.54.0.adoc @@ -104,6 +104,11 @@ UI, Workflows & Features line number when it encounters a corrupt patch, and correctly resets the line counter when processing multiple patch files. + * The HTTP transport learned to react to "429 Too Many Requests". + + * "git repo info -h" and "git repo structure -h" limit their help output + to the part that is specific to the subcommand. + Performance, Internal Implementation, Development Support etc. -------------------------------------------------------------- @@ -253,6 +258,15 @@ Performance, Internal Implementation, Development Support etc. of branches interpret_branch_name() function has been changed to use a dedicated enum type. + * Various updates to contrib/diff-highlight, including documentation + updates, test improvements, and color configuration handling. + + * Code paths that loop over another array to push each element into a + strvec have been rewritten to use strvec_pushv() instead. + + * In case homebrew breaks REG_ENHANCED again, leave a in-code comment + to suggest use of our replacement regex as a workaround. + Fixes since v2.53 ----------------- @@ -420,6 +434,17 @@ Fixes since v2.53 * "git apply -p" parses more carefully now. (merge d05d84c5f5 mf/apply-p-no-atoi later to maint). + * A test to run a .bat file with whitespaces in the name with arguments + with whitespaces in them was flaky in that sometimes it got killed + before it produced expected side effects, which has been rewritten to + make it more robust. + (merge 3ad4921838 jk/t0061-bat-test-update later to maint). + + * "git ls-remote '+refs/tags/*:refs/tags/*' https://..." run outside a + repository would dereference a NULL while trying to see if the given + refspec is a single-object refspec, which has been corrected. + (merge 4e5dc601dd kj/refspec-parsing-outside-repository later to maint). + * Other code cleanup, docfix, build fix, etc. (merge d79fff4a11 jk/remote-tracking-ref-leakfix later to maint). (merge 7a747f972d dd/t5403-modernise later to maint). diff --git a/Documentation/config/http.adoc b/Documentation/config/http.adoc index 9da5c298cc1d5e..849c89f36c5ad8 100644 --- a/Documentation/config/http.adoc +++ b/Documentation/config/http.adoc @@ -315,6 +315,32 @@ 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. + Defaults to 0 (retry immediately). When a Retry-After header is + present, its value takes precedence over this setting; however, + automatic use of the server-provided `Retry-After` header requires + libcurl 7.66.0 or later. On older versions, configure this setting + manually to control the retry delay. 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/builtin/am.c b/builtin/am.c index 9d0b51c651924a..fe6e087eee9ff5 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1188,7 +1188,7 @@ static void am_append_signoff(struct am_state *state) { struct strbuf sb = STRBUF_INIT; - strbuf_attach(&sb, state->msg, state->msg_len, state->msg_len); + strbuf_attach(&sb, state->msg, state->msg_len, state->msg_len + 1); append_signoff(&sb, 0, 0); state->msg = strbuf_detach(&sb, &state->msg_len); } diff --git a/builtin/fast-import.c b/builtin/fast-import.c index 9fc6c35b742871..570fd048d753f2 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -3307,7 +3307,7 @@ static void cat_blob(struct object_entry *oe, struct object_id *oid) cat_blob_write("\n", 1); if (oe && oe->pack_id == pack_id) { last_blob.offset = oe->idx.offset; - strbuf_attach(&last_blob.data, buf, size, size); + strbuf_attach(&last_blob.data, buf, size, size + 1); last_blob.depth = oe->depth; } else free(buf); diff --git a/builtin/rebase.c b/builtin/rebase.c index a1c7d78196750e..fa4f5d9306b856 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -182,8 +182,7 @@ static struct replay_opts get_replay_opts(const struct rebase_options *opts) replay.signoff = opts->signoff; - for (size_t i = 0; i < opts->trailer_args.nr; i++) - strvec_push(&replay.trailer_args, opts->trailer_args.v[i]); + strvec_pushv(&replay.trailer_args, opts->trailer_args.v); replay.allow_ff = !(opts->flags & REBASE_FORCE); if (opts->allow_rerere_autoupdate) diff --git a/builtin/repo.c b/builtin/repo.c index 55f9b9095c1f92..71a5c1c29c05fe 100644 --- a/builtin/repo.c +++ b/builtin/repo.c @@ -20,11 +20,27 @@ #include "tree-walk.h" #include "utf8.h" +#define REPO_INFO_USAGE \ + "git repo info [--format=(lines|nul) | -z] [--all | ...]", \ + "git repo info --keys [--format=(lines|nul) | -z]" + +#define REPO_STRUCTURE_USAGE \ + "git repo structure [--format=(table|lines|nul) | -z]" + static const char *const repo_usage[] = { - "git repo info [--format=(lines|nul) | -z] [--all | ...]", - "git repo info --keys [--format=(lines|nul) | -z]", - "git repo structure [--format=(table|lines|nul) | -z]", - NULL + REPO_INFO_USAGE, + REPO_STRUCTURE_USAGE, + NULL, +}; + +static const char *const repo_info_usage[] = { + REPO_INFO_USAGE, + NULL, +}; + +static const char *const repo_structure_usage[] = { + REPO_STRUCTURE_USAGE, + NULL, }; typedef int get_value_fn(struct repository *repo, struct strbuf *buf); @@ -214,7 +230,7 @@ static int cmd_repo_info(int argc, const char **argv, const char *prefix, OPT_END() }; - argc = parse_options(argc, argv, prefix, options, repo_usage, 0); + argc = parse_options(argc, argv, prefix, options, repo_info_usage, 0); if (show_keys && (all_keys || argc)) die(_("--keys cannot be used with a or --all")); @@ -879,7 +895,7 @@ static int cmd_repo_structure(int argc, const char **argv, const char *prefix, OPT_END() }; - argc = parse_options(argc, argv, prefix, options, repo_usage, 0); + argc = parse_options(argc, argv, prefix, options, repo_structure_usage, 0); if (argc) usage(_("too many arguments")); diff --git a/compat/regcomp_enhanced.c b/compat/regcomp_enhanced.c index 84193ce53b64f5..29c74eee995043 100644 --- a/compat/regcomp_enhanced.c +++ b/compat/regcomp_enhanced.c @@ -3,6 +3,11 @@ int git_regcomp(regex_t *preg, const char *pattern, int cflags) { + /* + * If you are on macOS with clang and fail to compile this line, + * https://lore.kernel.org/git/458ad3c1-96df-4575-ee42-e6eb754f25f6@gmx.de/ + * might be relevant. + */ if (!(cflags & REG_EXTENDED)) cflags |= REG_ENHANCED; return regcomp(preg, pattern, cflags); diff --git a/contrib/diff-highlight/DiffHighlight.pm b/contrib/diff-highlight/DiffHighlight.pm index f0607a4b687cf4..abe457882e5117 100644 --- a/contrib/diff-highlight/DiffHighlight.pm +++ b/contrib/diff-highlight/DiffHighlight.pm @@ -1,6 +1,6 @@ package DiffHighlight; -require v5.26; +require v5.008; use warnings FATAL => 'all'; use strict; @@ -9,18 +9,11 @@ use File::Spec; my $NULL = File::Spec->devnull(); -# Highlight by reversing foreground and background. You could do -# other things like bold or underline if you prefer. -my @OLD_HIGHLIGHT = ( - color_config('color.diff-highlight.oldnormal'), - color_config('color.diff-highlight.oldhighlight', "\x1b[7m"), - color_config('color.diff-highlight.oldreset', "\x1b[27m") -); -my @NEW_HIGHLIGHT = ( - color_config('color.diff-highlight.newnormal', $OLD_HIGHLIGHT[0]), - color_config('color.diff-highlight.newhighlight', $OLD_HIGHLIGHT[1]), - color_config('color.diff-highlight.newreset', $OLD_HIGHLIGHT[2]) -); +# The color theme is initially set to nothing here to allow outside callers +# to set the colors for their application. If nothing is sent in we use +# colors from git config in load_color_config(). +our @OLD_HIGHLIGHT = (); +our @NEW_HIGHLIGHT = (); my $RESET = "\x1b[m"; my $COLOR = qr/\x1b\[[0-9;]*m/; @@ -138,9 +131,21 @@ sub highlight_stdin { # of it being used in other settings. Let's handle our own # fallback, which means we will work even if git can't be run. sub color_config { + our $cached_config; my ($key, $default) = @_; - my $s = `git config --get-color $key 2>$NULL`; - return length($s) ? $s : $default; + + if (!defined $cached_config) { + $cached_config = {}; + my $data = `git config --type=color --get-regexp '^color\.diff-highlight\.' 2>$NULL`; + for my $line (split /\n/, $data) { + my ($key, $color) = split ' ', $line, 2; + $key =~ s/^color\.diff-highlight\.// or next; + $cached_config->{$key} = $color; + } + } + + my $s = $cached_config->{$key}; + return defined($s) ? $s : $default; } sub show_hunk { @@ -170,6 +175,29 @@ sub show_hunk { $line_cb->(@queue); } +sub load_color_config { + # If the colors were NOT set from outside this module we load them on-demand + # from the git config. Note that only one of elements 0 and 2 in each + # array is used (depending on whether you are doing set/unset on an + # attribute, or specifying normal vs highlighted coloring). So we use + # element 1 as our check for whether colors were passed in; it should + # always be set if you want highlighting to do anything. + if (!defined $OLD_HIGHLIGHT[1]) { + @OLD_HIGHLIGHT = ( + color_config('oldnormal'), + color_config('oldhighlight', "\x1b[7m"), + color_config('oldreset', "\x1b[27m") + ); + } + if (!defined $NEW_HIGHLIGHT[1]) { + @NEW_HIGHLIGHT = ( + color_config('newnormal', $OLD_HIGHLIGHT[0]), + color_config('newhighlight', $OLD_HIGHLIGHT[1]), + color_config('newreset', $OLD_HIGHLIGHT[2]) + ); + }; +} + sub highlight_pair { my @a = split_line(shift); my @b = split_line(shift); @@ -218,6 +246,7 @@ sub highlight_pair { } if (is_pair_interesting(\@a, $pa, $sa, \@b, $pb, $sb)) { + load_color_config(); return highlight_line(\@a, $pa, $sa, \@OLD_HIGHLIGHT), highlight_line(\@b, $pb, $sb, \@NEW_HIGHLIGHT); } diff --git a/contrib/diff-highlight/README b/contrib/diff-highlight/README index 1db4440e68324d..ed8d876a18af2a 100644 --- a/contrib/diff-highlight/README +++ b/contrib/diff-highlight/README @@ -39,10 +39,21 @@ visually distracting. Non-diff lines and existing diff coloration is preserved; the intent is that the output should look exactly the same as the input, except for the occasional highlight. +Build/Install +------------- + +You can build the `diff-highlight` script by running `make` from within +the diff-highlight directory. There is no `make install` target; you can +copy the built script to your $PATH. + +You can run diff-highlight's internal tests by running `make test`. Note +that you must also build Git itself first (by running `make` from the +top-level of the project). + Use --- -You can try out the diff-highlight program with: +You can try out the built diff-highlight program with: --------------------------------------------- git log -p --color | /path/to/diff-highlight @@ -127,6 +138,12 @@ Your script may set up one or more of the following variables: processing a logical chunk of input). The default function flushes stdout. + - @DiffHighlight::OLD_HIGHLIGHT and @DiffHighlight::NEW_HIGHLIGHT - these + arrays specify the normal, highlighted, and reset colors (in that order) + for old/new lines. If unset, values will be retrieved by calling `git + config` (see "Color Config" above). Note that these should be the literal + color bytes (starting with an ANSI escape code), not color names. + The script may then feed lines, one at a time, to DiffHighlight::handle_line(). When lines are done processing, they will be fed to $line_cb. Note that DiffHighlight may queue up many input lines (to analyze a whole hunk) diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh b/contrib/diff-highlight/t/t9400-diff-highlight.sh index 2a9b68cf3bb8cd..b38fe2196a9cbe 100755 --- a/contrib/diff-highlight/t/t9400-diff-highlight.sh +++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh @@ -7,9 +7,6 @@ TEST_OUTPUT_DIRECTORY=$(pwd) TEST_DIRECTORY="$CURR_DIR"/../../../t DIFF_HIGHLIGHT="$CURR_DIR"/../diff-highlight -CW="$(printf "\033[7m")" # white -CR="$(printf "\033[27m")" # reset - GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME . "$TEST_DIRECTORY"/test-lib.sh @@ -41,8 +38,10 @@ dh_test () { git show >commit.raw } >/dev/null && - "$DIFF_HIGHLIGHT" diff.act && - "$DIFF_HIGHLIGHT" commit.act && + "$DIFF_HIGHLIGHT" diff.hi && + test_strip_patch_header diff.act && + "$DIFF_HIGHLIGHT" commit.hi && + test_strip_patch_header commit.act && test_cmp patch.exp diff.act && test_cmp patch.exp commit.act } @@ -124,8 +123,8 @@ test_expect_success 'diff-highlight highlights the beginning of a line' ' dh_test a b <<-EOF @@ -1,3 +1,3 @@ aaa - -${CW}b${CR}bb - +${CW}0${CR}bb + -bbb + +0bb ccc EOF ' @@ -146,8 +145,8 @@ test_expect_success 'diff-highlight highlights the end of a line' ' dh_test a b <<-EOF @@ -1,3 +1,3 @@ aaa - -bb${CW}b${CR} - +bb${CW}0${CR} + -bbb + +bb0 ccc EOF ' @@ -168,8 +167,8 @@ test_expect_success 'diff-highlight highlights the middle of a line' ' dh_test a b <<-EOF @@ -1,3 +1,3 @@ aaa - -b${CW}b${CR}b - +b${CW}0${CR}b + -bbb + +b0b ccc EOF ' @@ -211,8 +210,8 @@ test_expect_failure 'diff-highlight highlights mismatched hunk size' ' dh_test a b <<-EOF @@ -1,3 +1,3 @@ aaa - -b${CW}b${CR}b - +b${CW}0${CR}b + -bbb + +b0b +ccc EOF ' @@ -230,8 +229,8 @@ test_expect_success 'diff-highlight treats multibyte utf-8 as a unit' ' echo "unic${o_stroke}de" >b && dh_test a b <<-EOF @@ -1 +1 @@ - -unic${CW}${o_accent}${CR}de - +unic${CW}${o_stroke}${CR}de + -unic${o_accent}de + +unic${o_stroke}de EOF ' @@ -248,8 +247,8 @@ test_expect_failure 'diff-highlight treats combining code points as a unit' ' echo "unico${combine_circum}de" >b && dh_test a b <<-EOF @@ -1 +1 @@ - -unic${CW}o${combine_accent}${CR}de - +unic${CW}o${combine_circum}${CR}de + -unico${combine_accent}de + +unico${combine_circum}de EOF ' @@ -331,12 +330,12 @@ test_expect_success 'diff-highlight handles --graph with leading dash' ' +++ b/file @@ -1,3 +1,3 @@ before - -the ${CW}old${CR} line - +the ${CW}new${CR} line + -the old line + +the new line -leading dash EOF git log --graph -p -1 | "$DIFF_HIGHLIGHT" >actual.raw && - trim_graph actual && + trim_graph actual && test_cmp expect actual ' @@ -351,4 +350,32 @@ test_expect_success 'highlight diff that removes final newline' ' EOF ' +test_expect_success 'configure set/reset colors' ' + test_config color.diff-highlight.oldhighlight bold && + test_config color.diff-highlight.oldreset nobold && + test_config color.diff-highlight.newhighlight italic && + test_config color.diff-highlight.newreset noitalic && + echo "prefix a suffix" >a && + echo "prefix b suffix" >b && + dh_test a b <<-\EOF + @@ -1 +1 @@ + -prefix a suffix + +prefix b suffix + EOF +' + +test_expect_success 'configure normal/highlight colors' ' + test_config color.diff-highlight.oldnormal red && + test_config color.diff-highlight.oldhighlight magenta && + test_config color.diff-highlight.newnormal green && + test_config color.diff-highlight.newhighlight yellow && + echo "prefix a suffix" >a && + echo "prefix b suffix" >b && + dh_test a b <<-\EOF + @@ -1 +1 @@ + -prefix a suffix + +prefix b suffix + EOF +' + test_done diff --git a/fetch-pack.c b/fetch-pack.c index 6ecd468ef766a8..a32224ed026592 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -1024,12 +1024,8 @@ static int get_pack(struct fetch_pack_args *args, fsck_msg_types.buf); } - if (index_pack_args) { - int i; - - for (i = 0; i < cmd.args.nr; i++) - strvec_push(index_pack_args, cmd.args.v[i]); - } + if (index_pack_args) + strvec_pushv(index_pack_args, cmd.args.v); sigchain_push(SIGPIPE, SIG_IGN); diff --git a/git-curl-compat.h b/git-curl-compat.h index 659e5a3875e3d6..dccdd4d6e54158 100644 --- a/git-curl-compat.h +++ b/git-curl-compat.h @@ -37,6 +37,14 @@ #define GIT_CURL_NEED_TRANSFER_ENCODING_HEADER #endif +/** + * CURLINFO_RETRY_AFTER was added in 7.66.0, released in September 2019. + * It allows curl to automatically parse Retry-After headers. + */ +#if LIBCURL_VERSION_NUM >= 0x074200 +#define GIT_CURL_HAVE_CURLINFO_RETRY_AFTER 1 +#endif + /** * CURLOPT_PROTOCOLS_STR and CURLOPT_REDIR_PROTOCOLS_STR were added in 7.85.0, * released in August 2022. diff --git a/git.c b/git.c index 2b212e6675d926..5a40eab8a26a66 100644 --- a/git.c +++ b/git.c @@ -877,8 +877,7 @@ static int run_argv(struct strvec *args) commit_pager_choice(); strvec_push(&cmd.args, "git"); - for (size_t i = 0; i < args->nr; i++) - strvec_push(&cmd.args, args->v[i]); + strvec_pushv(&cmd.args, args->v); trace_argv_printf(cmd.args.v, "trace: exec:"); diff --git a/http.c b/http.c index 8ea1b9d1f68c16..d8d016891b7974 100644 --- a/http.c +++ b/http.c @@ -22,6 +22,8 @@ #include "object-file.h" #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; @@ -149,6 +151,11 @@ static char *cached_accept_language; static char *http_ssl_backend; static int http_schannel_check_revoke = 1; + +static long http_retry_after = 0; +static long http_max_retries = 0; +static long http_max_retry_time = 300; + /* * 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,7 +216,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 UNUSED) +static size_t fwrite_wwwauth(char *ptr, size_t eltsize, size_t nmemb, void *p MAYBE_UNUSED) { size_t size = eltsize * nmemb; struct strvec *values = &http_auth.wwwauth_headers; @@ -575,6 +582,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 +1444,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 +1897,10 @@ static int handle_curl_result(struct slot_results *results) } return HTTP_REAUTH; } + } else if (results->http_code == 429) { + trace2_data_intmax("http", the_repository, "http/429-retry-after", + results->retry_after); + return HTTP_RATE_LIMITED; } else { if (results->http_connectcode == 407) credential_reject(the_repository, &proxy_auth); @@ -1886,6 +1916,7 @@ int run_one_slot(struct active_request_slot *slot, struct slot_results *results) { slot->results = results; + if (!start_active_slot(slot)) { xsnprintf(curl_errorstr, sizeof(curl_errorstr), "failed to start HTTP request"); @@ -2119,10 +2150,10 @@ static void http_opt_request_remainder(CURL *curl, off_t pos) static int http_request(const char *url, void *result, int target, - const struct http_get_options *options) + struct http_get_options *options) { struct active_request_slot *slot; - struct slot_results results; + struct slot_results results = { .retry_after = -1 }; struct curl_slist *headers = http_copy_default_headers(); struct strbuf buf = STRBUF_INIT; const char *accept_language; @@ -2156,22 +2187,19 @@ static int http_request(const char *url, headers = curl_slist_append(headers, accept_language); strbuf_addstr(&buf, "Pragma:"); - if (options && options->no_cache) + if (options->no_cache) strbuf_addstr(&buf, " no-cache"); - if (options && options->initial_request && + if (options->initial_request && http_follow_config == HTTP_FOLLOW_INITIAL) curl_easy_setopt(slot->curl, CURLOPT_FOLLOWLOCATION, 1L); headers = curl_slist_append(headers, buf.buf); /* Add additional headers here */ - if (options && options->extra_headers) { + if (options->extra_headers) { const struct string_list_item *item; - if (options && options->extra_headers) { - for_each_string_list_item(item, options->extra_headers) { - headers = curl_slist_append(headers, item->string); - } - } + for_each_string_list_item(item, options->extra_headers) + headers = curl_slist_append(headers, item->string); } headers = http_append_auth_header(&http_auth, headers); @@ -2183,7 +2211,18 @@ static int http_request(const char *url, ret = run_one_slot(slot, &results); - if (options && options->content_type) { +#ifdef GIT_CURL_HAVE_CURLINFO_RETRY_AFTER + if (ret == HTTP_RATE_LIMITED) { + curl_off_t retry_after; + if (curl_easy_getinfo(slot->curl, CURLINFO_RETRY_AFTER, + &retry_after) == CURLE_OK && retry_after > 0) + results.retry_after = (long)retry_after; + } +#endif + + options->retry_after = results.retry_after; + + if (options->content_type) { struct strbuf raw = STRBUF_INIT; curlinfo_strbuf(slot->curl, CURLINFO_CONTENT_TYPE, &raw); extract_content_type(&raw, options->content_type, @@ -2191,7 +2230,7 @@ static int http_request(const char *url, strbuf_release(&raw); } - if (options && options->effective_url) + if (options->effective_url) curlinfo_strbuf(slot->curl, CURLINFO_EFFECTIVE_URL, options->effective_url); @@ -2253,22 +2292,66 @@ static int update_url_from_redirect(struct strbuf *base, return 1; } -static int http_request_reauth(const char *url, +/* + * Compute the retry delay for an HTTP 429 response. + * Returns a negative value if configuration is invalid (delay exceeds + * http.maxRetryTime), otherwise returns the delay in seconds (>= 0). + */ +static long handle_rate_limit_retry(long slot_retry_after) +{ + /* Use the slot-specific retry_after value or configured default */ + if (slot_retry_after >= 0) { + /* Check if retry delay exceeds maximum allowed */ + if (slot_retry_after > http_max_retry_time) { + error(_("response requested a delay greater than http.maxRetryTime (%ld > %ld seconds)"), + slot_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", slot_retry_after); + return -1; + } + return slot_retry_after; + } else { + /* No Retry-After header provided, use configured default */ + if (http_retry_after > http_max_retry_time) { + error(_("configured http.retryAfter exceeds http.maxRetryTime (%ld > %ld seconds)"), + http_retry_after, http_max_retry_time); + trace2_data_string("http", the_repository, + "http/429-error", "config-exceeds-max-retry-time"); + return -1; + } + trace2_data_string("http", the_repository, + "http/429-retry-source", "config-default"); + return http_retry_after; + } +} + +static int http_request_recoverable(const char *url, void *result, int target, struct http_get_options *options) { + static struct http_get_options empty_opts; int i = 3; int ret; + int rate_limit_retries = http_max_retries; + + if (!options) + options = &empty_opts; 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) { + /* If retries are disabled and we got a 429, fail immediately */ + if (ret == HTTP_RATE_LIMITED && !http_max_retries) + return HTTP_ERROR; + + if (options->effective_url && options->base_url) { if (update_url_from_redirect(options->base_url, url, options->effective_url)) { credential_from_url(&http_auth, options->base_url->buf); @@ -2276,7 +2359,9 @@ static int http_request_reauth(const char *url, } } - while (ret == HTTP_REAUTH && --i) { + while ((ret == HTTP_REAUTH && --i) || + (ret == HTTP_RATE_LIMITED && --rate_limit_retries)) { + long retry_delay = -1; /* * The previous request may have put cruft into our output stream; we * should clear it out before making our next request. @@ -2301,11 +2386,28 @@ static int http_request_reauth(const char *url, default: BUG("Unknown http_request target"); } - - credential_fill(the_repository, &http_auth, 1); + if (ret == HTTP_RATE_LIMITED) { + retry_delay = handle_rate_limit_retry(options->retry_after); + if (retry_delay < 0) + return HTTP_ERROR; + + if (retry_delay > 0) { + warning(_("rate limited, waiting %ld seconds before retry"), retry_delay); + trace2_data_intmax("http", the_repository, + "http/retry-sleep-seconds", retry_delay); + sleep(retry_delay); + } + } else if (ret == HTTP_REAUTH) { + credential_fill(the_repository, &http_auth, 1); + } ret = http_request(url, result, target, options); } + if (ret == HTTP_RATE_LIMITED) { + trace2_data_string("http", the_repository, + "http/429-error", "retries-exhausted"); + return HTTP_RATE_LIMITED; + } return ret; } @@ -2313,7 +2415,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); } /* @@ -2337,7 +2439,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)) diff --git a/http.h b/http.h index f9d459340476e4..f9ee888c3ed67e 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 { @@ -157,6 +158,13 @@ struct http_get_options { * request has completed. */ struct string_list *extra_headers; + + /* + * After a request completes, contains the Retry-After delay in seconds + * if the server returned HTTP 429 with a Retry-After header (requires + * libcurl 7.66.0 or later), or -1 if no such header was present. + */ + long retry_after; }; /* Return values for http_get_*() */ @@ -167,6 +175,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/mailinfo.c b/mailinfo.c index a2f06dbd96ff6f..13949ff31e1769 100644 --- a/mailinfo.c +++ b/mailinfo.c @@ -470,7 +470,7 @@ static int convert_to_utf8(struct mailinfo *mi, return error("cannot convert from %s to %s", charset, mi->metainfo_charset); } - strbuf_attach(line, out, out_len, out_len); + strbuf_attach(line, out, out_len, out_len + 1); return 0; } diff --git a/refs/files-backend.c b/refs/files-backend.c index 7ce0d574781ffd..0537a72b2af9e0 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1813,7 +1813,7 @@ static int commit_ref(struct ref_lock *lock) size_t len = strlen(path); struct strbuf sb_path = STRBUF_INIT; - strbuf_attach(&sb_path, path, len, len); + strbuf_attach(&sb_path, path, len, len + 1); /* * If this fails, commit_lock_file() will also fail diff --git a/refspec.c b/refspec.c index 0775358d96cacd..fb89bce1db23fb 100644 --- a/refspec.c +++ b/refspec.c @@ -85,7 +85,7 @@ static int parse_refspec(struct refspec_item *item, const char *refspec, int fet if (!*item->src) return 0; /* negative refspecs must not be empty */ else if (llen == the_hash_algo->hexsz && !get_oid_hex(item->src, &unused)) - return 0; /* negative refpsecs cannot be exact sha1 */ + return 0; /* negative refspecs cannot be exact sha1 */ else if (!check_refname_format(item->src, flags)) ; /* valid looking ref is ok */ else diff --git a/remote-curl.c b/remote-curl.c index 92e40bb682d34d..aba60d571282d3 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -529,6 +529,17 @@ 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: + if (http_options.retry_after > 0) { + show_http_message(&type, &charset, &buffer); + die(_("rate limited by '%s', please try again in %ld seconds"), + transport_anonymize_url(url.buf), + http_options.retry_after); + } else { + 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"), @@ -1552,6 +1563,13 @@ int cmd_main(int argc, const char **argv) goto cleanup; } + /* + * yuck, see 9e89dcb66a (builtin/ls-remote: fall back to SHA1 outside + * of a repo, 2024-08-02) + */ + if (nongit) + repo_set_hash_algo(the_repository, GIT_HASH_DEFAULT); + options.verbosity = 1; options.progress = !!isatty(2); options.thin = 1; diff --git a/strbuf.c b/strbuf.c index 3939863cf31ffd..3e04addc22febb 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; } diff --git a/submodule.c b/submodule.c index e20537ba8dc66b..b1a0363f9d2a96 100644 --- a/submodule.c +++ b/submodule.c @@ -1828,7 +1828,6 @@ int fetch_submodules(struct repository *r, int default_option, int quiet, int max_parallel_jobs) { - int i; struct submodule_parallel_fetch spf = SPF_INIT; const struct run_process_parallel_opts opts = { .tr2_category = "submodule", @@ -1855,8 +1854,7 @@ int fetch_submodules(struct repository *r, die(_("index file corrupt")); strvec_push(&spf.args, "fetch"); - for (i = 0; i < options->nr; i++) - strvec_push(&spf.args, options->v[i]); + strvec_pushv(&spf.args, options->v); strvec_push(&spf.args, "--recurse-submodules-default"); /* default value, "--submodule-prefix" and its value are added later */ diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh index 5f42c311c2f2f5..4c76e813e396bf 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 6b8c50a51a3b72..40a690b0bb7c9b 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/meson.build b/t/meson.build index 919a0ae4cbb38e..7528e5cda5fef0 100644 --- a/t/meson.build +++ b/t/meson.build @@ -713,6 +713,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/t0061-run-command.sh b/t/t0061-run-command.sh index 2f77fde0d964c8..60cfe65979e215 100755 --- a/t/t0061-run-command.sh +++ b/t/t0061-run-command.sh @@ -287,16 +287,8 @@ test_expect_success MINGW 'can spawn .bat with argv[0] containing spaces' ' rm -f out && echo "echo %* >>out" >"$bat" && - # Ask git to invoke .bat; clone will fail due to fake SSH helper - test_must_fail env GIT_SSH="$bat" git clone myhost:src ssh-clone && - - # Spawning .bat can fail if there are two quoted cmd.exe arguments. - # .bat itself is first (due to spaces in name), so just one more is - # needed to verify. GIT_SSH will invoke .bat multiple times: - # 1) -G myhost - # 2) myhost "git-upload-pack src" - # First invocation will always succeed. Test the second one. - grep "git-upload-pack" out + test-tool run-command run-command "$bat" "arg with spaces" && + test_grep "arg with spaces" out ' test_done diff --git a/t/t1900-repo-info.sh b/t/t1900-repo-info.sh index a9eb07abe8aaea..39bb77dda0c327 100755 --- a/t/t1900-repo-info.sh +++ b/t/t1900-repo-info.sh @@ -149,4 +149,10 @@ test_expect_success 'git repo info --keys uses lines as its default output forma test_cmp expect actual ' +test_expect_success 'git repo info -h shows only repo info usage' ' + test_must_fail git repo info -h >actual && + test_grep "git repo info" actual && + test_grep ! "git repo structure" actual +' + test_done diff --git a/t/t1901-repo-structure.sh b/t/t1901-repo-structure.sh index 98921ce1cbb66f..10050abd70fd67 100755 --- a/t/t1901-repo-structure.sh +++ b/t/t1901-repo-structure.sh @@ -224,4 +224,10 @@ test_expect_success 'progress meter option' ' ) ' +test_expect_success 'git repo structure -h shows only repo structure usage' ' + test_must_fail git repo structure -h >actual && + test_grep "git repo structure" actual && + test_grep ! "git repo info" actual +' + test_done diff --git a/t/t5315-pack-objects-compression.sh b/t/t5315-pack-objects-compression.sh index 8bacd96275b0ac..d0feab17b4cc54 100755 --- a/t/t5315-pack-objects-compression.sh +++ b/t/t5315-pack-objects-compression.sh @@ -10,7 +10,7 @@ test_expect_success setup ' # make sure it resulted in a loose object ob=$(sed -e "s/\(..\).*/\1/" object-name) && ject=$(sed -e "s/..\(.*\)/\1/" object-name) && - test -f .git/objects/$ob/$ject + test_path_is_file .git/objects/$ob/$ject ' while read expect config diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh index 73cf5315800fa4..a26b6c284401a2 100755 --- a/t/t5551-http-fetch-smart.sh +++ b/t/t5551-http-fetch-smart.sh @@ -782,4 +782,11 @@ test_expect_success 'tag following always works over v0 http' ' test_cmp expect actual ' +test_expect_success 'ls-remote outside repo does not segfault with fetch refspec' ' + nongit git \ + -c remote.origin.url="$HTTPD_URL/smart/repo.git" \ + -c remote.origin.fetch=anything \ + ls-remote origin +' + test_done diff --git a/t/t5584-http-429-retry.sh b/t/t5584-http-429-retry.sh new file mode 100755 index 00000000000000..a22007b2cff416 --- /dev/null +++ b/t/t5584-http-429-retry.sh @@ -0,0 +1,266 @@ +#!/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 +' + +# 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/http_429/retries-disabled/1/repo.git" 2>err && + + # Verify no retry happened (no "waiting" message in stderr) + test_grep ! -i "waiting.*retry" err +' + +test_expect_success 'HTTP 429 permanent should fail after max retries' ' + # 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/http_429/permanent-fail/permanent/repo.git" 2>err +' + +test_expect_success 'HTTP 429 with Retry-After is retried and succeeds' ' + # Enable retries + test_config http.maxRetries 3 && + + # Git should retry after receiving 429 and eventually succeed + 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' ' + # 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/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' ' + # Enable retries + test_config http.maxRetries 3 && + + # Time the operation - it should take at least 2 seconds due to retry delay + start=$(test-tool date getnanos) && + git ls-remote "$HTTPD_URL/http_429/retry-delays-respected/2/repo.git" >output 2>err && + duration=$(test-tool date getnanos $start) && + + # Verify it took at least 2 seconds (allowing some tolerance) + duration_int=${duration%.*} && + test "$duration_int" -ge 1 && + test_grep "refs/heads/" output +' + +test_expect_success 'HTTP 429 fails immediately if Retry-After exceeds http.maxRetryTime' ' + # 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=$(test-tool date getnanos) && + test_must_fail git ls-remote "$HTTPD_URL/http_429/retry-after-exceeds-max-time/100/repo.git" 2>err && + duration=$(test-tool date getnanos $start) && + + # Should fail quickly (no 100 second wait) + duration_int=${duration%.*} && + test "$duration_int" -lt 99 && + test_grep "greater than http.maxRetryTime" err +' + +test_expect_success 'HTTP 429 fails if configured http.retryAfter exceeds http.maxRetryTime' ' + # Test misconfiguration: retryAfter > maxRetryTime + # 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=$(test-tool date getnanos) && + test_must_fail git ls-remote "$HTTPD_URL/http_429/config-retry-after-exceeds-max-time/none/repo.git" 2>err && + duration=$(test-tool date getnanos $start) && + + # Should fail quickly (no 100 second wait) + duration_int=${duration%.*} && + test "$duration_int" -lt 99 && + 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 + raw=$(test-tool date timestamp now) && + now="${raw#* -> }" && + future_time=$((now + 2)) && + raw=$(test-tool date show:rfc2822 $future_time) && + future_date="${raw#* -> }" && + 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=$(test-tool date getnanos) && + git ls-remote "$HTTPD_URL/http_429/http-date-format/$future_date_encoded/repo.git" >output 2>err && + duration=$(test-tool date getnanos $start) && + + # Should take at least 1 second (allowing tolerance for processing time) + duration_int=${duration%.*} && + test "$duration_int" -ge 1 && + test_grep "refs/heads/" output +' + +test_expect_success 'HTTP 429 with HTTP-date exceeding maxRetryTime fails immediately' ' + raw=$(test-tool date timestamp now) && + now="${raw#* -> }" && + future_time=$((now + 200)) && + raw=$(test-tool date show:rfc2822 $future_time) && + future_date="${raw#* -> }" && + 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 && + test_config http.maxRetryTime 10 && + + # Should fail immediately without waiting 200 seconds + start=$(test-tool date getnanos) && + test_must_fail git ls-remote "$HTTPD_URL/http_429/http-date-exceeds-max-time/$future_date_encoded/repo.git" 2>err && + duration=$(test-tool date getnanos $start) && + + # Should fail quickly (not wait 200 seconds) + duration_int=${duration%.*} && + test "$duration_int" -lt 199 && + test_grep "http.maxRetryTime" err +' + +test_expect_success 'HTTP 429 with past HTTP-date should not wait' ' + raw=$(test-tool date timestamp now) && + now="${raw#* -> }" && + past_time=$((now - 10)) && + raw=$(test-tool date show:rfc2822 $past_time) && + past_date="${raw#* -> }" && + past_date_encoded=$(echo "$past_date" | sed "s/ /%20/g") && + + # Enable retries + test_config http.maxRetries 3 && + + # Git should retry immediately without waiting + start=$(test-tool date getnanos) && + git ls-remote "$HTTPD_URL/http_429/past-http-date/$past_date_encoded/repo.git" >output 2>err && + duration=$(test-tool date getnanos $start) && + + # Should complete quickly (no wait for a past-date Retry-After) + duration_int=${duration%.*} && + test "$duration_int" -lt 5 && + test_grep "refs/heads/" output +' + +test_expect_success 'HTTP 429 with invalid Retry-After format uses configured default' ' + # 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=$(test-tool date getnanos) && + git ls-remote "$HTTPD_URL/http_429/invalid-retry-after-format/invalid/repo.git" >output 2>err && + duration=$(test-tool date getnanos $start) && + + # Should take at least 1 second (the configured default) + duration_int=${duration%.*} && + test "$duration_int" -ge 1 && + test_grep "refs/heads/" output && + 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 + # Do NOT configure anything - use defaults (http.maxRetries defaults to 0) + + # Should fail immediately without retry + 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) + test_grep ! -i "waiting.*retry" err && + + # Should get 429 error + test_grep "429" err +' + +test_expect_success 'GIT_HTTP_RETRY_AFTER overrides http.retryAfter config' ' + # Configure retryAfter to 10 seconds + test_config http.maxRetries 3 && + test_config http.retryAfter 10 && + + # Override with environment variable to 1 second + start=$(test-tool date getnanos) && + GIT_HTTP_RETRY_AFTER=1 git ls-remote "$HTTPD_URL/http_429/env-retry-after-override/none/repo.git" >output 2>err && + duration=$(test-tool date getnanos $start) && + + # Should use env var (1 second), not config (10 seconds) + duration_int=${duration%.*} && + test "$duration_int" -ge 1 && + test "$duration_int" -lt 5 && + test_grep "refs/heads/" output && + test_grep "waiting.*retry" err +' + +test_expect_success 'GIT_HTTP_MAX_RETRIES overrides http.maxRetries config' ' + # 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/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_expect_success 'GIT_HTTP_MAX_RETRY_TIME overrides http.maxRetryTime config' ' + # 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=$(test-tool date getnanos) && + test_must_fail env GIT_HTTP_MAX_RETRY_TIME=10 \ + git ls-remote "$HTTPD_URL/http_429/env-max-retry-time-override/50/repo.git" 2>err && + duration=$(test-tool date getnanos $start) && + + # Should fail quickly (not wait 50 seconds) because env var limits to 10 + duration_int=${duration%.*} && + test "$duration_int" -lt 49 && + test_grep "greater than http.maxRetryTime" err +' + +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 diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 14e238d24da9ad..f3af10fb7e0205 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -48,6 +48,9 @@ test_decode_color () { if (n == 2) return "FAINT"; if (n == 3) return "ITALIC"; if (n == 7) return "REVERSE"; + if (n == 22) return "NORMAL_INTENSITY"; + if (n == 23) return "NOITALIC"; + if (n == 27) return "NOREVERSE"; if (n == 30) return "BLACK"; if (n == 31) return "RED"; if (n == 32) return "GREEN"; diff --git a/tools/coccinelle/strvec.cocci b/tools/coccinelle/strvec.cocci new file mode 100644 index 00000000000000..64edb09f1c76fd --- /dev/null +++ b/tools/coccinelle/strvec.cocci @@ -0,0 +1,46 @@ +@@ +type T; +identifier i; +expression dst; +struct strvec *src_ptr; +struct strvec src_arr; +@@ +( +- for (T i = 0; i < src_ptr->nr; i++) { strvec_push(dst, src_ptr->v[i]); } ++ strvec_pushv(dst, src_ptr->v); +| +- for (T i = 0; i < src_arr.nr; i++) { strvec_push(dst, src_arr.v[i]); } ++ strvec_pushv(dst, src_arr.v); +) + +@ separate_loop_index @ +type T; +identifier i; +expression dst; +struct strvec *src_ptr; +struct strvec src_arr; +@@ + T i; + ... +( +- for (i = 0; i < src_ptr->nr; i++) { strvec_push(dst, src_ptr->v[i]); } ++ strvec_pushv(dst, src_ptr->v); +| +- for (i = 0; i < src_arr.nr; i++) { strvec_push(dst, src_arr.v[i]); } ++ strvec_pushv(dst, src_arr.v); +) + +@ unused_loop_index extends separate_loop_index @ +@@ + { + ... +- T i; + ... when != i + } + +@ depends on unused_loop_index @ +@@ + if (...) +- { + strvec_pushv(...); +- } diff --git a/trailer.c b/trailer.c index ca8abd18826bf2..470f86a4a2f83e 100644 --- a/trailer.c +++ b/trailer.c @@ -1041,7 +1041,7 @@ static struct trailer_block *trailer_block_get(const struct process_trailer_opti for (ptr = trailer_lines; *ptr; ptr++) { if (last && isspace((*ptr)->buf[0])) { struct strbuf sb = STRBUF_INIT; - strbuf_attach(&sb, *last, strlen(*last), strlen(*last)); + strbuf_attach(&sb, *last, strlen(*last), strlen(*last) + 1); strbuf_addbuf(&sb, *ptr); *last = strbuf_detach(&sb, NULL); continue;