Thread (43 messages) flat view 43 messages, 4 authors, 6d ago

Re: [PATCH v2 1/3] t/lib-httpd: fix apply-one-time-script race under concurrent requests

From: Patrick Steinhardt <hidden>
Date: 2026-08-04 08:03:21

On Fri, Jul 10, 2026 at 05:30:55PM +0000, Michael Montalbo via GitGitGadget wrote:
quoted hunk ↗ jump to hunk
diff --git a/t/lib-httpd/apply-one-time-script.sh b/t/lib-httpd/apply-one-time-script.sh
index b1682944e2..adb9cec528 100644
--- a/t/lib-httpd/apply-one-time-script.sh
+++ b/t/lib-httpd/apply-one-time-script.sh
@@ -6,21 +6,37 @@
 #
 # This can be used to simulate the effects of the repository changing in
 # between HTTP request-response pairs.
-if test -f one-time-script
-then
-	LC_ALL=C
-	export LC_ALL
+#
+# Apache can run this CGI for concurrent requests (for example a partial fetch
+# that lazily fetches a missing object while the first response is still in
+# flight), so the helper claims the marker atomically with a rename, and only
+# once it has decided to modify the response. A request that loses the race
+# finds the marker already gone and serves its response unchanged; no request
+# is left emitting an empty body, which the server would report as HTTP 500.
+# Scratch files are per-request ($$) so concurrent requests do not clobber each
+# other.
+#
+# The script may run more than once: the marker is consumed when the response
+# actually changes (the rename after "cmp"), not when the script runs, so a
+# request whose response is not the targeted one runs the script, sees no
+# change, and leaves the marker for a later request. That is safe because the
+# scripts are stateless filters over the captured response.
 
-	"$GIT_EXEC_PATH/git-http-backend" >out
-	./one-time-script out >out_modified
+test -f one-time-script || exec "$GIT_EXEC_PATH/git-http-backend"
 
-	if cmp -s out out_modified
-	then
-		cat out
-	else
-		cat out_modified
-		rm one-time-script
-	fi
+LC_ALL=C
+export LC_ALL
+
+out=out.$$
+modified=out-modified.$$
+"$GIT_EXEC_PATH/git-http-backend" >"$out"
+
+if ./one-time-script "$out" 2>/dev/null >"$modified" &&
Is it intentional that we swallow stderr of this script now? We didn't
before. I assume that this is to swallow the error in case the script
got removed by the concurrent request?

Patrick
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help