Git development
 help / color / mirror / Atom feed
From: "Michael Montalbo via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Patrick Steinhardt <ps@pks.im>,
	Michael Montalbo <mmontalbo@gmail.com>,
	Michael Montalbo <mmontalbo@gmail.com>
Subject: [PATCH v5 2/3] t/lib-httpd: make http-429 first-request check atomic
Date: Tue, 01 Sep 2026 15:53:02 +0000	[thread overview]
Message-ID: <79396d491fe15c94a4e4c079d1109b425dcf966a.1788277983.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.2171.v5.git.1788277983.gitgitgadget@gmail.com>

From: Michael Montalbo <mmontalbo@gmail.com>

http-429.sh is a helper for testing retry logic. It uses "test -f" to
check for the existence of a state file and later uses "touch" or
"rm -f" on that file to determine if it should return a 429. This method
of managing state can fail if the helper script is invoked concurrently.
However, this failure does not currently manifest itself since the
helper is invoked sequentially.

As a preventive measure, fix the state management logic so it relies on
an atomic mkdir operation to mark that a 429 was returned. When
$retry_after is "permanent", always return 429 now that we do not rely
on a state file that is "touch"ed and "rm"ed to indicate when to respond
with a 429.

Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
 t/lib-httpd/http-429.sh | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/t/lib-httpd/http-429.sh b/t/lib-httpd/http-429.sh
index c97b16145b..1a5d7987db 100644
--- a/t/lib-httpd/http-429.sh
+++ b/t/lib-httpd/http-429.sh
@@ -3,7 +3,7 @@
 # Script to return HTTP 429 Too Many Requests responses for testing retry logic.
 # Usage: /http_429/<test-context>/<retry-after-value>/<repo-path>
 #
-# The test-context is a unique identifier for each test to isolate state files.
+# The test-context is a unique identifier for each test to isolate state directories.
 # 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
@@ -26,14 +26,16 @@ repo_path="${remaining#*/}"  # Get rest (repo path)
 # 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
+# Use current directory (HTTPD_ROOT_PATH) to hold state directory
+# Create a safe directory name from test_context, retry_after and repo_name
+# This ensures all requests for the same test context share the same state directory
 safe_name=$(echo "${test_context}-${retry_after}-${repo_name}" | tr '/' '_' | tr -cd 'a-zA-Z0-9_-')
-state_file="http-429-state-${safe_name}"
+state="http-429-state-${safe_name}"
 
-# Check if this is the first call (no state file exists)
-if test -f "$state_file"
+# Check if this is the first call (no state directory exists), or if
+# the retry-after-value is "permanent", which indicates a 429 must be
+# returned for every request (even if the state directory exists).
+if test "$retry_after" != permanent && ! mkdir "$state" 2>/dev/null
 then
 	# Already returned 429 once, forward to git-http-backend
 	# Set PATH_INFO to just the repo path (without retry-after value)
@@ -52,9 +54,6 @@ then
 	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"
 
@@ -67,8 +66,7 @@ case "$retry_after" in
 		printf "Retry-After: invalid-format-123abc\r\n"
 		;;
 	permanent)
-		# Always return 429, don't set state file for success
-		rm -f "$state_file"
+		# Always return 429
 		printf "Retry-After: 1\r\n"
 		printf "Content-Type: text/plain\r\n"
 		printf "\r\n"
-- 
gitgitgadget


  parent reply	other threads:[~2026-09-01 15:53 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-08  2:59 [PATCH 0/3] t/lib-httpd: make CGI test helpers concurrency-safe Michael Montalbo via GitGitGadget
2026-07-08  2:59 ` [PATCH 1/3] t/lib-httpd: fix apply-one-time-script race under concurrent requests Michael Montalbo via GitGitGadget
2026-07-08 19:54   ` Junio C Hamano
2026-07-09 17:26     ` Michael Montalbo
2026-07-08  2:59 ` [PATCH 2/3] t/lib-httpd: make http-429 first-request check atomic Michael Montalbo via GitGitGadget
2026-07-08 19:58   ` Junio C Hamano
2026-07-08 20:02   ` Junio C Hamano
2026-07-09 18:10     ` Michael Montalbo
2026-07-08  2:59 ` [PATCH 3/3] t/README: document writing concurrency-safe helpers Michael Montalbo via GitGitGadget
2026-07-08 19:59   ` Junio C Hamano
2026-07-10 17:30 ` [PATCH v2 0/3] t/lib-httpd: make CGI test helpers concurrency-safe Michael Montalbo via GitGitGadget
2026-07-10 17:30   ` [PATCH v2 1/3] t/lib-httpd: fix apply-one-time-script race under concurrent requests Michael Montalbo via GitGitGadget
2026-08-04  8:03     ` Patrick Steinhardt
2026-08-07 16:29       ` Michael Montalbo
2026-07-10 17:30   ` [PATCH v2 2/3] t/lib-httpd: make http-429 first-request check atomic Michael Montalbo via GitGitGadget
2026-07-10 17:30   ` [PATCH v2 3/3] t/README: document writing concurrency-safe helpers Michael Montalbo via GitGitGadget
2026-08-04  8:03     ` Patrick Steinhardt
2026-08-07 16:51       ` Michael Montalbo
2026-08-10  6:06         ` Patrick Steinhardt
2026-08-02  3:02   ` [PATCH v2 0/3] t/lib-httpd: make CGI test helpers concurrency-safe Michael Montalbo
2026-08-03 21:55   ` Junio C Hamano
2026-08-13  1:05 ` [PATCH v3 " Michael Montalbo via GitGitGadget
2026-08-13  1:05   ` [PATCH v3 1/3] t/lib-httpd: fix apply-one-time-script race under concurrent requests Michael Montalbo via GitGitGadget
2026-08-13  1:05   ` [PATCH v3 2/3] t/lib-httpd: make http-429 first-request check atomic Michael Montalbo via GitGitGadget
2026-08-31  7:18     ` Patrick Steinhardt
2026-08-31 14:50       ` Junio C Hamano
2026-08-31 17:31         ` Michael Montalbo
2026-08-31 17:09       ` Michael Montalbo
2026-08-13  1:05   ` [PATCH v3 3/3] t/lib-httpd: document writing concurrency-safe CGI helpers Michael Montalbo via GitGitGadget
2026-08-31  7:18     ` Patrick Steinhardt
2026-08-26 19:59   ` [PATCH v3 0/3] t/lib-httpd: make CGI test helpers concurrency-safe Junio C Hamano
2026-08-31  7:18     ` Patrick Steinhardt
2026-09-01  0:27 ` [PATCH v4 " Michael Montalbo via GitGitGadget
2026-09-01  0:27   ` [PATCH v4 1/3] t/lib-httpd: fix apply-one-time-script race under concurrent requests Michael Montalbo via GitGitGadget
2026-09-01  0:27   ` [PATCH v4 2/3] t/lib-httpd: make http-429 first-request check atomic Michael Montalbo via GitGitGadget
2026-09-01  0:27   ` [PATCH v4 3/3] t/lib-httpd: document writing concurrency-safe CGI helpers Michael Montalbo via GitGitGadget
2026-09-01 11:17     ` Patrick Steinhardt
2026-09-01 14:28       ` Michael Montalbo
2026-09-01 15:53 ` [PATCH v5 0/3] t/lib-httpd: make CGI test helpers concurrency-safe Michael Montalbo via GitGitGadget
2026-09-01 15:53   ` [PATCH v5 1/3] t/lib-httpd: fix apply-one-time-script race under concurrent requests Michael Montalbo via GitGitGadget
2026-09-01 15:53   ` Michael Montalbo via GitGitGadget [this message]
2026-09-01 15:53   ` [PATCH v5 3/3] t/lib-httpd: document writing concurrency-safe CGI helpers Michael Montalbo via GitGitGadget
2026-09-03  5:29   ` [PATCH v5 0/3] t/lib-httpd: make CGI test helpers concurrency-safe Patrick Steinhardt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=79396d491fe15c94a4e4c079d1109b425dcf966a.1788277983.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=mmontalbo@gmail.com \
    --cc=ps@pks.im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox