From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Todd Zullinger <tmz@pobox.com>,
git@vger.kernel.org,
Matthew John Cheetham <mjcheetham@outlook.com>
Subject: Re: [BUG 2.41.0] t/lib-httpd/apache.conf incompatible with RHEL/CentOS 7
Date: Thu, 18 May 2023 14:28:22 -0700 [thread overview]
Message-ID: <xmqqpm6xs51l.fsf@gitster.g> (raw)
In-Reply-To: <20230518192102.GA1514485@coredump.intra.peff.net> (Jeff King's message of "Thu, 18 May 2023 15:21:02 -0400")
Jeff King <peff@peff.net> writes:
> On Thu, May 18, 2023 at 02:45:32PM -0400, Jeff King wrote:
>
>> 2. We do some kind of version check in enable_cgipassauth(),
>> and skip tests manually if it doesn't pass.
>> [...]
>> Obviously (1) and (3) are the least work for us upstream, but I don't
>> think (2) would be too hard to do.
>
> It indeed was pretty easy. So here's a patch. I'm adding Junio to the cc
> before any review, as this is a change in the v2.41 cycle. So I think
> we'd want to address this before the release.
Thanks for being considerate, as always ;-) Very much appreciated.
> -- >8 --
> Subject: [PATCH] t/lib-httpd: make CGIPassAuth support conditional
>
> Commit 988aad99b4 (t5563: add tests for basic and anoymous HTTP access,
> 2023-02-27) added tests that require Apache to support the CGIPassAuth
> directive, which was added in Apache 2.4.13. This is fairly old (~8
> years), but recent enough that we still encounter it in the wild (e.g.,
> RHEL/CentOS 7, which is not EOL until June 2024).
nitpick: we are fine to encountering 2.4.13 in the wild---encountering
something a bit older than that is problematic. A quick internet
search tells me that CentOS 7 ships Apache 2.4.6, so if we trust that...
... fairly old (~8 years), but recent enough that we still
encounter versions older than that in the wild (e.g. CentOS 7,
which is not EOL until June 2024, comes with Apache 2.4.6 from
2014 plus security fixes).
or something?
> I tested this manually by mutilating the config directive to "FooBar",
> which would fail even on recent versions. And then tweaking the "13"
> in the version check up to "60" to make sure it properly skips even with
> recent Apache. But testing on real CentOS 7 would be very much
> appreciated.
Indeed, we'd love to see tests on a real thing, especially if we
were to fast-track this. FWIW, the manual test you outlined above
sounds like a very sensible way to "emulate" under the condition
that there is no access to the real thing.
> +enable_cgipassauth () {
> + # We are looking for 2.4.13 or more recent. Since we only support
> + # 2.4 and up, no need to check for older major/minor.
> + if test "$HTTPD_VERSION_MAJOR" = 2 &&
> + test "$HTTPD_VERSION_MINOR" = 4 &&
> + test "$(echo $HTTPD_VERSION | cut -d. -f3)" -lt 13
As HTTPD_VERSION comes from
$LIB_HTTPD_PATH -v | sed -n 's|^Server version: Apache/\([0-9.]*\).*|p'
and parses a line like "Server version: Apache/2.4.6 (CentOS)",
unless somebody ships 2.4 without any digit after it, the above
should be safe ;-)
> + then
> + echo >&4 "apache $HTTPD_VERSION too old for CGIPassAuth"
> + return
> + fi
> + HTTPD_PARA="$HTTPD_PARA -DUSE_CGIPASSAUTH"
> + test_set_prereq CGIPASSAUTH
> +}
OK.
> diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
> index 9e6892970d..a22d138605 100644
> --- a/t/lib-httpd/apache.conf
> +++ b/t/lib-httpd/apache.conf
> @@ -146,7 +146,9 @@ SetEnv PERL_PATH ${PERL_PATH}
> <LocationMatch /custom_auth/>
> SetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH}
> SetEnv GIT_HTTP_EXPORT_ALL
> + <IfDefine USE_CGIPASSAUTH>
> CGIPassAuth on
> + </IfDefine>
> </LocationMatch>
> ScriptAlias /smart/incomplete_length/git-upload-pack incomplete-length-upload-pack-v2-http.sh/
> ScriptAlias /smart/incomplete_body/git-upload-pack incomplete-body-upload-pack-v2-http.sh/
> diff --git a/t/t5563-simple-http-auth.sh b/t/t5563-simple-http-auth.sh
> index f45a43b4b5..ab8a721ccc 100755
> --- a/t/t5563-simple-http-auth.sh
> +++ b/t/t5563-simple-http-auth.sh
> @@ -5,6 +5,12 @@ test_description='test http auth header and credential helper interop'
> . ./test-lib.sh
> . "$TEST_DIRECTORY"/lib-httpd.sh
>
> +enable_cgipassauth
> +if ! test_have_prereq CGIPASSAUTH
> +then
> + skip_all="no CGIPassAuth support"
> + test_done
> +fi
> start_httpd
>
> test_expect_success 'setup_credential_helper' '
Yup, very cleanly done.
Thanks.
next prev parent reply other threads:[~2023-05-18 21:29 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-17 19:06 [BUG 2.41.0] t/lib-httpd/apache.conf incompatible with RHEL/CentOS 7 Todd Zullinger
2023-05-18 18:45 ` Jeff King
2023-05-18 19:14 ` Todd Zullinger
2023-05-18 19:22 ` Jeff King
2023-05-18 19:21 ` Jeff King
2023-05-18 19:50 ` Todd Zullinger
2023-05-18 20:11 ` Todd Zullinger
2023-05-18 21:31 ` Junio C Hamano
2023-05-18 21:28 ` Junio C Hamano [this message]
2023-05-18 23:10 ` Jeff King
2023-05-18 23:23 ` Junio C Hamano
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=xmqqpm6xs51l.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=mjcheetham@outlook.com \
--cc=peff@peff.net \
--cc=tmz@pobox.com \
/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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).