Merge branch 'mm/lib-httpd-cgi-safe' into seen
CGI helper scripts used by HTTP-related test scripts have been updated to use atomic filesystem operations, preventing race conditions when Apache handles concurrent requests. * mm/lib-httpd-cgi-safe: t/lib-httpd: document writing concurrency-safe CGI helpers t/lib-httpd: make http-429 first-request check atomic t/lib-httpd: fix apply-one-time-script race under concurrent requests
Junio C Hamano
commit
b0812c6dd8
|
|
@ -159,6 +159,17 @@ prepare_httpd() {
|
|||
mkdir -p "$HTTPD_DOCUMENT_ROOT_PATH"
|
||||
cp "$TEST_PATH"/passwd "$HTTPD_ROOT_PATH"
|
||||
cp "$TEST_PATH"/proxy-passwd "$HTTPD_ROOT_PATH"
|
||||
# Apache can run the following scripts concurrently per request. Make
|
||||
# sure any state management logic is resilient to race conditions.
|
||||
#
|
||||
# For example:
|
||||
# - use "mkdir dir" to ensure only one request "succeeds" under some
|
||||
# condition (see http-429.sh).
|
||||
# - chain (&&) atomic operations like "rm marker" (no -f) with the
|
||||
# logic that "claims" the marker instead of relying on a separate
|
||||
# "test -f" and "rm marker" check (see apply-one-time-script.sh).
|
||||
# - use scratch file names that include the process ID ($$), so
|
||||
# concurrent requests do not overwrite each other's state.
|
||||
install_script incomplete-length-upload-pack-v2-http.sh
|
||||
install_script incomplete-body-upload-pack-v2-http.sh
|
||||
install_script error-no-report.sh
|
||||
|
|
|
|||
|
|
@ -6,21 +6,31 @@
|
|||
#
|
||||
# This can be used to simulate the effects of the repository changing in
|
||||
# between HTTP request-response pairs.
|
||||
if test -f one-time-script
|
||||
test -f one-time-script || exec "$GIT_EXEC_PATH/git-http-backend"
|
||||
|
||||
LC_ALL=C
|
||||
export LC_ALL
|
||||
|
||||
out=out.$$
|
||||
modified=out-modified.$$
|
||||
"$GIT_EXEC_PATH/git-http-backend" >"$out"
|
||||
|
||||
# Since Apache can execute this script for multiple requests
|
||||
# concurrently, we chain "rm one-time-script" with the logic
|
||||
# for generating a modified response. If the "rm" ran separately,
|
||||
# a concurrent request could pass the "test -f" above and
|
||||
# erroneously result in multiple modified responses or an empty
|
||||
# body depending on the race state.
|
||||
#
|
||||
# We discard stderr for ./one-time-script since it is possible
|
||||
# ./one-time-script has been removed already, which is expected
|
||||
# sometimes. In this case, the unmodified response will be returned.
|
||||
if ./one-time-script "$out" 2>/dev/null >"$modified" &&
|
||||
! cmp -s "$out" "$modified" &&
|
||||
rm one-time-script 2>/dev/null
|
||||
then
|
||||
LC_ALL=C
|
||||
export LC_ALL
|
||||
|
||||
"$GIT_EXEC_PATH/git-http-backend" >out
|
||||
./one-time-script out >out_modified
|
||||
|
||||
if cmp -s out out_modified
|
||||
then
|
||||
cat out
|
||||
else
|
||||
cat out_modified
|
||||
rm one-time-script
|
||||
fi
|
||||
cat "$modified"
|
||||
else
|
||||
"$GIT_EXEC_PATH/git-http-backend"
|
||||
cat "$out"
|
||||
fi
|
||||
rm -f "$out" "$modified"
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
|
|
@ -717,6 +717,7 @@ integration_tests = [
|
|||
't5564-http-proxy.sh',
|
||||
't5565-push-multiple.sh',
|
||||
't5566-push-group.sh',
|
||||
't5567-one-time-script.sh',
|
||||
't5570-git-daemon.sh',
|
||||
't5571-pre-push-hook.sh',
|
||||
't5572-pull-submodule.sh',
|
||||
|
|
|
|||
|
|
@ -0,0 +1,96 @@
|
|||
#!/bin/sh
|
||||
|
||||
test_description='apply-one-time-script CGI helper is safe under concurrent requests'
|
||||
|
||||
. ./test-lib.sh
|
||||
|
||||
HELPER="$TEST_DIRECTORY/lib-httpd/apply-one-time-script.sh"
|
||||
|
||||
test_expect_success PIPE 'helper only serves one rewritten response for concurrent requests' '
|
||||
mkdir workdir fakebin &&
|
||||
ENTERED="$PWD/entered" &&
|
||||
GATE="$PWD/gate" &&
|
||||
export ENTERED GATE &&
|
||||
mkfifo "$ENTERED" "$GATE" &&
|
||||
|
||||
# A stub git-http-backend that returns a response based on
|
||||
# $ROLE. For $ROLE = modify, return the response string
|
||||
# "packfile", which ends up being modified by the example
|
||||
# one-time-script below.
|
||||
#
|
||||
# Otherwise, run the branch returning a response that
|
||||
# should be passed through, and block until released
|
||||
# by "read -r $GATE".
|
||||
write_script fakebin/git-http-backend <<-\EOF &&
|
||||
printf "Status: 200 OK\r\n"
|
||||
printf "Content-Type: application/x-git-result\r\n"
|
||||
printf "\r\n"
|
||||
if test "$ROLE" = modify
|
||||
then
|
||||
printf "packfile\n"
|
||||
else
|
||||
echo entered >"$ENTERED"
|
||||
read -r released <"$GATE"
|
||||
printf "refs\n"
|
||||
fi
|
||||
EOF
|
||||
|
||||
# An example one-time-script for apply-one-time-script
|
||||
# to execute. Checks for "packfile" in the response
|
||||
# that will be returned, and replaces it with a
|
||||
# modified response. Passes through responses without
|
||||
# "packfile" in them.
|
||||
write_script workdir/one-time-script <<-\EOF &&
|
||||
if grep packfile "$1" >/dev/null
|
||||
then
|
||||
sed "/packfile/q" "$1" &&
|
||||
printf "REPLACED\n"
|
||||
else
|
||||
cat "$1"
|
||||
fi
|
||||
EOF
|
||||
|
||||
GIT_EXEC_PATH="$PWD/fakebin" &&
|
||||
export GIT_EXEC_PATH &&
|
||||
|
||||
# Ensure $GATE has a reader so the test does not block indefinitely if
|
||||
# the helper is buggy and "echo released >&9" below does not unblock
|
||||
# the unmodified response gate.
|
||||
exec 9<>"$GATE" &&
|
||||
|
||||
# Launch the passthrough request in the background. Record its pid
|
||||
# so it can be killed when the test finishes if, for some reason, the
|
||||
# request stays blocked and would stall a test runner.
|
||||
{ (
|
||||
cd workdir &&
|
||||
ROLE=passthrough sh "$HELPER" >../passthrough.out 2>../passthrough.err
|
||||
) & } &&
|
||||
passthrough_pid=$! &&
|
||||
test_when_finished "kill $passthrough_pid 2>/dev/null || :" &&
|
||||
|
||||
# Wait until the passthrough request is "in-flight" and paused
|
||||
# mid-response.
|
||||
read -r entered <"$ENTERED" &&
|
||||
|
||||
# Launch the request for a modified response while the passthrough
|
||||
# request is concurrently "in-flight" and paused.
|
||||
(
|
||||
cd workdir &&
|
||||
ROLE=modify sh "$HELPER" >../modify.out 2>../modify.err
|
||||
) &&
|
||||
|
||||
# Unblock the passthrough request, allowing git-http-backend to
|
||||
# complete its response.
|
||||
echo released >&9 &&
|
||||
{ wait "$passthrough_pid" || :; } &&
|
||||
|
||||
test_must_be_empty passthrough.err &&
|
||||
test_must_be_empty modify.err &&
|
||||
test_grep "Status: 200 OK" passthrough.out &&
|
||||
test_grep "Status: 200 OK" modify.out &&
|
||||
test_grep REPLACED modify.out &&
|
||||
test_grep ! REPLACED passthrough.out &&
|
||||
test_grep refs passthrough.out
|
||||
'
|
||||
|
||||
test_done
|
||||
Loading…
Reference in New Issue