From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 06 Oct 2026 12:25:18 GMT
Subject: Re: [PATCH] t5551: fix quoting in curl version bug prereq
Message-ID: <xmqqik3fhv81.fsf@gitster.g>
In-Reply-To: <20261006034331.GA1325722@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Thu, Sep 24, 2026 at 08:53:49PM +0000, Johannes Schindelin via GitGitGadget wrote:
>
>> +# The cURL version which Debian 12 ships (v7.88.1) can fail to retry
>> +# authentication after an early HTTP/2 response. This bug was introduced
>> +# in cURL v7.88.0 (8c762f5998 (http2: minor buffer and error path fixes,
>> +# 2023-02-08)) and fixed in v8.3.0 (https://github.com/curl/curl/pull/11756).
>> +test_lazy_prereq HAVE_CURL_HTTP2_BUG "
>> +	test_have_prereq HTTP2 &&
>> +	build_option libcurl |
>> +	awk -F. '
>> +		($1 == 7 && $2 >= 88) || ($1 == 8 && $2 < 3) { broken = 1 }
>> +		END { exit !broken }
>> +	'
>> +"
>
> Doh, this is totally broken. The prereq snippet is in double-quotes, so
> the $1, etc in the awk invocation are interpolated before we even eval
> it. Fix is below.

Ah, I missed that "double-quote outside, single-quote inside"
anti-pattern.

I also like your "HERE-doc solves many such issues" approach in the
other message.

Thanks.



> -- >8 --
> Subject: [PATCH] t5551: fix quoting in curl version bug prereq
>
> We have a prereq snippet that invokes awk. The awk script's $1, etc,
> variables need to be quoted to avoid shell interpolation. We correctly
> use a single-quote inside the prereq snippet, but the snippet itself is
> contained in double-quotes. So we interpolate "$1" into whatever value
> that happens to have in the outer shell, and eval nonsense like:
>
>   awk '(--some-garbage == 7 && --other-garbage >= 88) ...'
>
> As a result, we don't think we have a buggy curl version even when we
> do, and run the test anyway. But of course it's easy not to notice,
> since this prereq was protecting us from a racy bug. It only breaks
> sometimes.
>
> There are a few options for fixing the quoting:
>
>   1. Backslash-escaping the dollar signs. This is perhaps the least-ugly
>      version, but it's a minor hassle to remember if somebody touches
>      the code later.
>
>   2. Single-quote the snippet, then quote interior single-quotes as
>      '\''. Reasonably obvious, but ugly.
>
>   3. Use the '<<\EOT' here-doc trick to specify the snippet. This would
>      look nice, but we don't yet support it for prereqs. ;)
>
> This patch uses (2), and we can circle back to (3) to make it look nicer
> later.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> This should go on top of js/ci-debian-12-http2-workaround.
>
> Since I know we both used GPT to work on this, I was curious if this
> slipped past it. Doesn't look like it from what I sent (which used
> option 2 above). I wonder if your agent flipped it, or if you saw how
> ugly it was and flipped it yourself. Not blaming, but it's just a funny
> and interesting data point if a human second-guessing the AI output
> introduced a bug.
>
>  t/t5551-http-fetch-smart.sh | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
> index f66d7ce7ac..cb681e644f 100755
> --- a/t/t5551-http-fetch-smart.sh
> +++ b/t/t5551-http-fetch-smart.sh
> @@ -21,14 +21,14 @@ start_httpd
>  # authentication after an early HTTP/2 response. This bug was introduced
>  # in cURL v7.88.0 (8c762f5998 (http2: minor buffer and error path fixes,
>  # 2023-02-08)) and fixed in v8.3.0 (https://github.com/curl/curl/pull/11756).
> -test_lazy_prereq HAVE_CURL_HTTP2_BUG "
> +test_lazy_prereq HAVE_CURL_HTTP2_BUG '
>  	test_have_prereq HTTP2 &&
>  	build_option libcurl |
> -	awk -F. '
> +	awk -F. '\''
>  		($1 == 7 && $2 >= 88) || ($1 == 8 && $2 < 3) { broken = 1 }
>  		END { exit !broken }
> -	'
> -"
> +	'\''
> +'
>  
>  test_expect_success HTTP2 'enable client-side http/2' '
>  	git config --global http.version HTTP/2