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