{"thread":{"id":"59174","subject":"[PATCH 0/4] t/lib-httpd ssl fixes","startedAt":"2023-02-01T11:35:57Z","lastAt":"2023-02-03T17:19:35Z","messageCount":9,"participants":["Jeff King","Todd Zullinger"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"471227","messageId":"Y9pOmR5fOfCHwYpF@coredump.intra.peff.net","threadId":"59174","inReplyTo":null,"subject":"[PATCH 0/4] t/lib-httpd ssl fixes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-02-01T11:35:53Z","receivedAt":"2023-02-01T11:35:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"While chasing down a possible HTTP/2 problem in our test suite (which\nturns out not to be a Git bug, I think), I tried running the tests with\nLIB_HTTPD_SSL=1, as TLS affects HTTP/2 upgrade.\n\nSadly, apache would not start at all with our ssl config. It looks like\nthis has probably been broken for many years, depending on your apache\nand openssl versions.\n\nThe final two patches here fix ssl problems I found. The first two\npatches drop support for older apache. This yields some minor cleanups,\nand makes the ssl fixes slightly easier. I've cc'd Todd as the last\nperson to express support for Apache 2.2, in 2017. I'm hoping even\nCentOS has moved on by now, but we'll see. :)\n\n [1/4]: t/lib-httpd: bump required apache version to 2.2\n [2/4]: t/lib-httpd: bump required apache version to 2.4\n [3/4]: t/lib-httpd: drop SSLMutex config\n [4/4]: t/lib-httpd: increase ssl key size to 2048 bits\n\n t/lib-httpd.sh | 11 +++++++----\n t/lib-httpd/apache.conf | 31 ++++---------------------------\n t/lib-httpd/ssl.cnf | 2 +-\n 3 files changed, 12 insertions(+), 32 deletions(-)\n\n-Peff\n"},{"id":"471228","messageId":"Y9pO8bBZjXjEZhR/@coredump.intra.peff.net","threadId":"59174","inReplyTo":"Y9pOmR5fOfCHwYpF@coredump.intra.peff.net","subject":"[PATCH 1/4] t/lib-httpd: bump required apache version to 2.2","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-02-01T11:37:21Z","receivedAt":"2023-02-01T11:37:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Apache 2.2 was released in 2005, almost 18 years ago. We can probably\nassume that people are running a version at least that old (and the\nstakes for removing it are fairly low, as the worst case is that they\nwould not run the http tests against their ancient version).\n\nDropping support for the older versions cleans up the config file a\nlittle, and will also enable us to bump the required version further\n(with more cleanups) in a future patch.\n\nNote that the file actually checks for version 2.1. In apache's\nversioning scheme, odd numbered versions are for development and even\nnumbers are for stable releases. So 2.1 and 2.2 are effectively the same\nfrom our perspective.\n\nOlder versions would just fail to start, which would generally cause us\nto skip the tests. However, we do have version detection code in\nlib-httpd.sh which produces a nicer error message, so let's update that,\ntoo. I didn't bother handling the case of \"3.0\", etc. Apache has been on\n2.x for 21 years, with no signs of bumping the major version. And if\nthey eventually do, I suspect there will be enough breaking changes that\nwe'd need to update more than just the numeric version check. We can\nworry about that hypothetical when it happens.\n\nSigned-off-by: Jeff King \n---\n t/lib-httpd.sh | 11 +++++++----\n t/lib-httpd/apache.conf | 8 --------\n 2 files changed, 7 insertions(+), 12 deletions(-)\n\ndiff --git a/t/lib-httpd.sh b/t/lib-httpd.sh\nindex 608949ea80..8fc411ff41 100644\n--- a/t/lib-httpd.sh\n+++ b/t/lib-httpd.sh\n@@ -99,16 +99,19 @@ then\n fi\n \n HTTPD_VERSION=$($LIB_HTTPD_PATH -v | \\\n-\tsed -n 's/^Server version: Apache\\/\\([0-9]*\\)\\..*$/\\1/p; q')\n+\tsed -n 's/^Server version: Apache\\/\\([0-9.]*\\).*$/\\1/p; q')\n+HTTPD_VERSION_MAJOR=$(echo $HTTPD_VERSION | cut -d. -f1)\n+HTTPD_VERSION_MINOR=$(echo $HTTPD_VERSION | cut -d. -f2)\n \n-if test -n \"$HTTPD_VERSION\"\n+if test -n \"$HTTPD_VERSION_MAJOR\"\n then\n \tif test -z \"$LIB_HTTPD_MODULE_PATH\"\n \tthen\n-\t\tif ! test $HTTPD_VERSION -ge 2\n+\t\tif ! test \"$HTTPD_VERSION_MAJOR\" -eq 2 ||\n+\t\t ! test \"$HTTPD_VERSION_MINOR\" -ge 2\n \t\tthen\n \t\t\ttest_skip_or_die GIT_TEST_HTTPD \\\n-\t\t\t\t\"at least Apache version 2 is required\"\n+\t\t\t\t\"at least Apache version 2.2 is required\"\n \t\tfi\n \t\tif ! test -d \"$DEFAULT_HTTPD_MODULE_PATH\"\n \t\tthen\ndiff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf\nindex 0294739a77..35f5e28507 100644\n--- a/t/lib-httpd/apache.conf\n+++ b/t/lib-httpd/apache.conf\n@@ -38,13 +38,6 @@ Protocols h2c\n LockFile accept.lock\n \n \n-\n-\n-\tLoadModule auth_module modules/mod_auth.so\n-\n-\n-\n-= 2.1>\n \n \tLoadModule auth_basic_module modules/mod_auth_basic.so\n \n@@ -57,7 +50,6 @@ LockFile accept.lock\n \n \tLoadModule authz_host_module modules/mod_authz_host.so\n \n-\n \n = 2.4>\n \n-- \n2.39.1.767.gb4615b3bd3\n\n"},{"id":"471229","messageId":"Y9pPML4wqui0BtXg@coredump.intra.peff.net","threadId":"59174","inReplyTo":"Y9pOmR5fOfCHwYpF@coredump.intra.peff.net","subject":"[PATCH 2/4] t/lib-httpd: bump required apache version to 2.4","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-02-01T11:38:24Z","receivedAt":"2023-02-01T11:38:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Apache 2.4 has been out since early 2012, almost 11 years. And its\npredecessor, 2.2, has been out of support since its last release in\n2017, over 5 years ago. The last mention on the mailing list was from\naround the same time, in this thread:\n\n https://lore.kernel.org/git/20171231023234.21215-1-tmz@pobox.com/\n\nWe can probably assume that 2.4 is available everywhere. And the stakes\nare fairly low, as the worst case is that such a platform would skip the\nhttp tests.\n\nThis lets us clean up a few minor version checks in the config file, but\nalso revert f1f2b45be0 (tests: adjust the configuration for Apache 2.2,\n2016-05-09). Its technique isn't _too_ bad, but certainly required a bit\nmore explanation than the 2.4 version it replaced. I manually confirmed\nthat the test in t5551 still behaves as expected (if you replace\n\"cadabra\" with \"foo\", the server correctly rejects the request).\n\nIt will also help future patches which will no longer have to deal with\nconditional config for this old version.\n\nSigned-off-by: Jeff King \n---\n t/lib-httpd.sh | 4 ++--\n t/lib-httpd/apache.conf | 22 ++++------------------\n 2 files changed, 6 insertions(+), 20 deletions(-)\n\ndiff --git a/t/lib-httpd.sh b/t/lib-httpd.sh\nindex 8fc411ff41..5d2d56c445 100644\n--- a/t/lib-httpd.sh\n+++ b/t/lib-httpd.sh\n@@ -108,10 +108,10 @@ then\n \tif test -z \"$LIB_HTTPD_MODULE_PATH\"\n \tthen\n \t\tif ! test \"$HTTPD_VERSION_MAJOR\" -eq 2 ||\n-\t\t ! test \"$HTTPD_VERSION_MINOR\" -ge 2\n+\t\t ! test \"$HTTPD_VERSION_MINOR\" -ge 4\n \t\tthen\n \t\t\ttest_skip_or_die GIT_TEST_HTTPD \\\n-\t\t\t\t\"at least Apache version 2.2 is required\"\n+\t\t\t\t\"at least Apache version 2.4 is required\"\n \t\tfi\n \t\tif ! test -d \"$DEFAULT_HTTPD_MODULE_PATH\"\n \t\tthen\ndiff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf\nindex 35f5e28507..332617f10d 100644\n--- a/t/lib-httpd/apache.conf\n+++ b/t/lib-httpd/apache.conf\n@@ -34,10 +34,6 @@ LoadModule http2_module modules/mod_http2.so\n Protocols h2c\n \n \n-\n-LockFile accept.lock\n-\n-\n \n \tLoadModule auth_basic_module modules/mod_auth_basic.so\n \n@@ -51,7 +47,6 @@ LockFile accept.lock\n \tLoadModule authz_host_module modules/mod_authz_host.so\n \n \n-= 2.4>\n \n \tLoadModule authn_core_module modules/mod_authn_core.so\n \n@@ -75,7 +70,6 @@ LockFile accept.lock\n \tLoadModule mpm_prefork_module modules/mod_mpm_prefork.so\n \n \n-\n \n PassEnv GIT_VALGRIND\n PassEnv GIT_VALGRIND_OPTIONS\n@@ -115,6 +109,10 @@ Alias /auth/dumb/ www/auth/dumb/\n \tHeader set Set-Cookie name=value\n \n \n+\t\n+\t\tRequire expr %{HTTP:x-magic-one} == 'abra'\n+\t\tRequire expr %{HTTP:x-magic-two} == 'cadabra'\n+\t\n \tSetEnv GIT_EXEC_PATH ${GIT_EXEC_PATH}\n \tSetEnv GIT_HTTP_EXPORT_ALL\n \n@@ -197,18 +195,6 @@ RewriteRule ^/intern-redir/(.*)/foo$ /smart/$1 [PT]\n RewriteRule ^/redir-objects/(.*/info/refs)$ /dumb/$1 [PT]\n RewriteRule ^/redir-objects/(.*/objects/.*)$ /dumb/$1 [R=301]\n \n-# Apache 2.2 does not understand , so we use RewriteCond.\n-# And as RewriteCond does not allow testing for non-matches, we match\n-# the desired case first (one has abra, two has cadabra), and let it\n-# pass by marking the RewriteRule as [L], \"last rule, do not process\n-# any other matching RewriteRules after this\"), and then have another\n-# RewriteRule that matches all other cases and lets them fail via '[F]',\n-# \"fail the request\".\n-RewriteCond %{HTTP:x-magic-one} =abra\n-RewriteCond %{HTTP:x-magic-two} =cadabra\n-RewriteRule ^/smart_headers/.* - [L]\n-RewriteRule ^/smart_headers/.* - [F]\n-\n \n LoadModule ssl_module modules/mod_ssl.so\n \n-- \n2.39.1.767.gb4615b3bd3\n\n"},{"id":"471230","messageId":"Y9pPWroQIBh9NR35@coredump.intra.peff.net","threadId":"59174","inReplyTo":"Y9pOmR5fOfCHwYpF@coredump.intra.peff.net","subject":"[PATCH 3/4] t/lib-httpd: drop SSLMutex config","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-02-01T11:39:06Z","receivedAt":"2023-02-01T11:39:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The SSL config enabled by setting LIB_HTTPD_SSL does not work with\nApache versions greater than 2.2, as more recent versions complain about\nthe SSLMutex directive. According to\nhttps://httpd.apache.org/docs/current/upgrading.html:\n\n Directives AcceptMutex, LockFile, RewriteLock, SSLMutex,\n SSLStaplingMutex, and WatchdogMutexPath have been replaced with a\n single Mutex directive. You will need to evaluate any use of these\n removed directives in your 2.2 configuration to determine if they can\n just be deleted or will need to be replaced using Mutex.\n\nDeleting this line will just use the system default, which seems\nsensible. The original came as part of faa4bc35a0 (http-push: add\nregression tests, 2008-02-27), but no specific reason is given there (or\non the mailing list) for its presence.\n\nSigned-off-by: Jeff King \n---\nIf we can't drop support for 2.2 in patch 2, then this would need to be\nadjusted with an IfVersion.\n\n t/lib-httpd/apache.conf | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf\nindex 332617f10d..51a4fbcf62 100644\n--- a/t/lib-httpd/apache.conf\n+++ b/t/lib-httpd/apache.conf\n@@ -203,7 +203,6 @@ SSLCertificateKeyFile httpd.pem\n SSLRandomSeed startup file:/dev/urandom 512\n SSLRandomSeed connect file:/dev/urandom 512\n SSLSessionCache none\n-SSLMutex file:ssl_mutex\n SSLEngine On\n \n \n-- \n2.39.1.767.gb4615b3bd3\n\n"},{"id":"471231","messageId":"Y9pPbqT8wkysK5Ko@coredump.intra.peff.net","threadId":"59174","inReplyTo":"Y9pOmR5fOfCHwYpF@coredump.intra.peff.net","subject":"[PATCH 4/4] t/lib-httpd: increase ssl key size to 2048 bits","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-02-01T11:39:26Z","receivedAt":"2023-02-01T11:39:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Recent versions of openssl will refuse to work with 1024-bit RSA keys,\nas they are considered insecure. I didn't track down the exact version\nin which the defaults were tightened, but the Debian-package openssl 3.0\non my system yields:\n\n $ LIB_HTTPD_SSL=1 ./t5551-http-fetch-smart.sh -v -i\n [...]\n SSL Library Error: error:0A00018F:SSL routines::ee key too small\n 1..0 # SKIP web server setup failed\n\nThis could probably be overcome with configuration, but that's likely\nto be a headache (especially if it requires touching /etc/openssl).\nLet's just pick a key size that's less outrageously out of date.\n\nSigned-off-by: Jeff King \n---\n t/lib-httpd/ssl.cnf | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/lib-httpd/ssl.cnf b/t/lib-httpd/ssl.cnf\nindex 6dab2579cb..812e8253f0 100644\n--- a/t/lib-httpd/ssl.cnf\n+++ b/t/lib-httpd/ssl.cnf\n@@ -1,7 +1,7 @@\n RANDFILE = $ENV::RANDFILE_PATH\n \n [ req ]\n-default_bits = 1024\n+default_bits = 2048\n distinguished_name = req_distinguished_name\n prompt = no\n [ req_distinguished_name ]\n-- \n2.39.1.767.gb4615b3bd3\n"},{"id":"471305","messageId":"Y9s7vyHKXP+TQPRm@pobox.com","threadId":"59174","inReplyTo":"Y9pOmR5fOfCHwYpF@coredump.intra.peff.net","subject":"Re: [PATCH 0/4] t/lib-httpd ssl fixes","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2023-02-02T04:27:43Z","receivedAt":"2023-02-02T04:29:48Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Jeff King wrote:\n> While chasing down a possible HTTP/2 problem in our test suite (which\n> turns out not to be a Git bug, I think), I tried running the tests with\n> LIB_HTTPD_SSL=1, as TLS affects HTTP/2 upgrade.\n> \n> Sadly, apache would not start at all with our ssl config. It looks like\n> this has probably been broken for many years, depending on your apache\n> and openssl versions.\n> \n> The final two patches here fix ssl problems I found. The first two\n> patches drop support for older apache. This yields some minor cleanups,\n> and makes the ssl fixes slightly easier. I've cc'd Todd as the last\n> person to express support for Apache 2.2, in 2017. I'm hoping even\n> CentOS has moved on by now, but we'll see. :)\n\nHeh. Fortunately, CentOS 6 has been EOL for a few years.\nCentOS 7 has httpd-2.4.6.\n\nI applied these patches and ran builds for CentOS/RHEL 7-9\nand Fedora 36-38. I had not previously run the test suite\nwith LIB_HTTPD_SSL=1 and I ran into many, many failures.\n(154 failures across 12 tests, to be precise.)\n\nNone of the failures were due to the httpd config, so the\nchanges here seem fine. :)\n\nThe below diff is what I applied to resolve all but 21\ngit-svn failures:\n\nTest Summary Report\n-------------------\nt9118-git-svn-funky-branch-names.sh (Wstat: 256 (exited 1) Tests: 5 Failed: 4)\n Failed tests: 2-5\n Non-zero exit status: 1\nt9115-git-svn-dcommit-funky-renames.sh (Wstat: 256 (exited 1) Tests: 12 Failed: 11)\n Failed tests: 2-12\n Non-zero exit status: 1\nt9120-git-svn-clone-with-percent-escapes.sh (Wstat: 256 (exited 1) Tests: 8 Failed: 5)\n Failed tests: 2, 5-8\n Non-zero exit status: 1\nt9142-git-svn-shallow-clone.sh (Wstat: 256 (exited 1) Tests: 2 Failed: 1)\n Failed test: 2\n Non-zero exit status: 1\nFiles=983, Tests=27522, 353 wallclock secs (11.78 usr 2.61 sys + 935.44 cusr 1358.38 csys = 2308.21 CPU)\n\nI didn't feel like digging into the git-svn code and the\nsubversion bindings to try and fix those tests. It's likely\na relatively simple matter to someone who knows them better\nthan I do, hopefully.\n\nI can clean up this diff if you think it's worthwhile. It\nsounds like it may be quite useful for the http/2 tests, but\nmaybe LIB_HTTPD_SSL=1 in t5559-http-fetch-smart-http2 is\nsimpler for now?\n\n-- 8< --\ndiff --git i/t/lib-git-svn.sh w/t/lib-git-svn.sh\nindex ea28971e8e..f636bcafe4 100644\n--- i/t/lib-git-svn.sh\n+++ w/t/lib-git-svn.sh\n@@ -67,7 +67,7 @@ svn_cmd () {\n \t\tsvn\n \t\treturn\n \tfi\n-\tsvn \"$orig_svncmd\" --config-dir \"$svnconf\" \"$@\"\n+\tsvn --non-interactive --trust-server-cert \"$orig_svncmd\" --config-dir \"$svnconf\" \"$@\"\n }\n \n maybe_start_httpd () {\ndiff --git i/t/lib-httpd.sh w/t/lib-httpd.sh\nindex 608949ea80..a4f787f580 100644\n--- i/t/lib-httpd.sh\n+++ w/t/lib-httpd.sh\n@@ -168,7 +168,7 @@ prepare_httpd() {\n \t\tthen\n \t\t\tHTTPD_PARA=\"$HTTPD_PARA -DSVN\"\n \t\t\tLIB_HTTPD_SVNPATH=\"$rawsvnrepo\"\n-\t\t\tsvnrepo=\"http://127.0.0.1:$LIB_HTTPD_PORT/\"\n+\t\t\tsvnrepo=\"$HTTPD_PROTO://127.0.0.1:$LIB_HTTPD_PORT/\"\n \t\t\tsvnrepo=\"$svnrepo$LIB_HTTPD_SVN\"\n \t\t\texport LIB_HTTPD_SVN LIB_HTTPD_SVNPATH\n \t\tfi\n@@ -280,11 +280,11 @@ expect_askpass() {\n \t\tnone)\n \t\t\t;;\n \t\tpass)\n-\t\t\techo \"askpass: Password for 'http://$2@$dest': \"\n+\t\t\techo \"askpass: Password for '$HTTPD_PROTO://$2@$dest': \"\n \t\t\t;;\n \t\tboth)\n-\t\t\techo \"askpass: Username for 'http://$dest': \"\n-\t\t\techo \"askpass: Password for 'http://$2@$dest': \"\n+\t\t\techo \"askpass: Username for '$HTTPD_PROTO://$dest': \"\n+\t\t\techo \"askpass: Password for '$HTTPD_PROTO://$2@$dest': \"\n \t\t\t;;\n \t\t*)\n \t\t\tfalse\ndiff --git i/t/t5411-proc-receive-hook.sh w/t/t5411-proc-receive-hook.sh\nindex 92cf52c6d4..c85f4668c4 100755\n--- i/t/t5411-proc-receive-hook.sh\n+++ w/t/t5411-proc-receive-hook.sh\n@@ -63,7 +63,7 @@ run_proc_receive_hook_test() {\n \tcase $1 in\n \thttp)\n \t\tPROTOCOL=\"HTTP protocol\"\n-\t\tURL_PREFIX=\"http://.*\"\n+\t\tURL_PREFIX=\"$HTTPD_PROTO://.*\"\n \t\t;;\n \tlocal)\n \t\tPROTOCOL=\"builtin protocol\"\ndiff --git i/t/t5541-http-push-smart.sh w/t/t5541-http-push-smart.sh\nindex fbad2d5ff5..b1f414dfe0 100755\n--- i/t/t5541-http-push-smart.sh\n+++ w/t/t5541-http-push-smart.sh\n@@ -122,9 +122,9 @@ test_expect_success 'setup rejected update hook' '\n \n \tcat >exp <<-EOF\n \tremote: error: hook declined to update refs/heads/dev2\n-\tTo http://127.0.0.1:$LIB_HTTPD_PORT/smart/test_repo.git\n+\tTo '$HTTPD_PROTO'://127.0.0.1:$LIB_HTTPD_PORT/smart/test_repo.git\n \t ! [remote rejected] dev2 -> dev2 (hook declined)\n-\terror: failed to push some refs to '\\''http://127.0.0.1:$LIB_HTTPD_PORT/smart/test_repo.git'\\''\n+\terror: failed to push some refs to '\\'$HTTPD_PROTO'://127.0.0.1:$LIB_HTTPD_PORT/smart/test_repo.git'\\''\n \tEOF\n '\n \ndiff --git i/t/t5548-push-porcelain.sh w/t/t5548-push-porcelain.sh\nindex 6282728eaf..18e47c8a30 100755\n--- i/t/t5548-push-porcelain.sh\n+++ w/t/t5548-push-porcelain.sh\n@@ -84,7 +84,7 @@ run_git_push_porcelain_output_test() {\n \tcase $1 in\n \thttp)\n \t\tPROTOCOL=\"HTTP protocol\"\n-\t\tURL_PREFIX=\"http://.*\"\n+\t\tURL_PREFIX=\"$HTTPD_PROTO://.*\"\n \t\t;;\n \tfile)\n \t\tPROTOCOL=\"builtin protocol\"\ndiff --git i/t/t5550-http-fetch-dumb.sh w/t/t5550-http-fetch-dumb.sh\nindex 8f182a3cbf..070d04cdce 100755\n--- i/t/t5550-http-fetch-dumb.sh\n+++ w/t/t5550-http-fetch-dumb.sh\n@@ -384,7 +384,7 @@ test_expect_success 'remote-http complains cleanly about malformed urls' '\n # learned to handle early remote helper failures more cleanly.\n test_expect_success 'remote-http complains cleanly about empty scheme' '\n \ttest_must_fail ok=sigpipe git ls-remote \\\n-\t\thttp::${HTTPD_URL#http}/dumb/repo.git 2>stderr &&\n+\t\thttp::${HTTPD_URL#$HTTPD_PROTO}/dumb/repo.git 2>stderr &&\n \ttest_i18ngrep \"url has no scheme\" stderr\n '\n \n@@ -454,9 +454,9 @@ test_expect_success 'http-alternates triggers not-from-user protocol check' '\n \techo \"$HTTPD_URL/dumb/victim.git/objects\" \\\n \t\t>\"$evil/objects/info/http-alternates\" &&\n \ttest_config_global http.followRedirects true &&\n-\ttest_must_fail git -c protocol.http.allow=user \\\n+\ttest_must_fail git -c protocol.'$HTTPD_PROTO'.allow=user \\\n \t\tclone $HTTPD_URL/dumb/evil.git evil-user &&\n-\tgit -c protocol.http.allow=always \\\n+\tgit -c protocol.'$HTTPD_PROTO'.allow=always \\\n \t\tclone $HTTPD_URL/dumb/evil.git evil-user\n '\n \ndiff --git i/t/t5561-http-backend.sh w/t/t5561-http-backend.sh\nindex 9c57d84315..b3f5759972 100755\n--- i/t/t5561-http-backend.sh\n+++ w/t/t5561-http-backend.sh\n@@ -15,7 +15,7 @@ fi\n start_httpd\n \n GET() {\n-\tcurl --include \"$HTTPD_URL/$SMART/repo.git/$1\" >out &&\n+\tcurl --include --insecure \"$HTTPD_URL/$SMART/repo.git/$1\" >out &&\n \ttr '\\015' Q out &&\n \ttr '\\015' Q Apache 2.2 was released in 2005, almost 18 years ago. We can probably\n> assume that people are running a version at least that old (and the\n> stakes for removing it are fairly low, as the worst case is that they\n> would not run the http tests against their ancient version).\n> \n> Dropping support for the older versions cleans up the config file a\n> little, and will also enable us to bump the required version further\n> (with more cleanups) in a future patch.\n> \n> Note that the file actually checks for version 2.1. In apache's\n> versioning scheme, odd numbered versions are for development and even\n> numbers are for stable releases. So 2.1 and 2.2 are effectively the same\n> from our perspective.\n> \n> Older versions would just fail to start, which would generally cause us\n> to skip the tests. However, we do have version detection code in\n> lib-httpd.sh which produces a nicer error message, so let's update that,\n> too. I didn't bother handling the case of \"3.0\", etc. Apache has been on\n> 2.x for 21 years, with no signs of bumping the major version. And if\n> they eventually do, I suspect there will be enough breaking changes that\n> we'd need to update more than just the numeric version check. We can\n> worry about that hypothetical when it happens.\n> \n> Signed-off-by: Jeff King \n> ---\n> t/lib-httpd.sh | 11 +++++++----\n> t/lib-httpd/apache.conf | 8 --------\n> 2 files changed, 7 insertions(+), 12 deletions(-)\n> \n> diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh\n> index 608949ea80..8fc411ff41 100644\n> --- a/t/lib-httpd.sh\n> +++ b/t/lib-httpd.sh\n> @@ -99,16 +99,19 @@ then\n> fi\n> \n> HTTPD_VERSION=$($LIB_HTTPD_PATH -v | \\\n\nTrivial, but is it worth getting rid of the unnecessary\nbackslash, while you're improving things here? Maybe that's\na mild tangent for code that's otherwise not adjusted very\noften?\n\nThe backslash was present when lib-httpd.sh was added in\nfaa4bc35a0 (http-push: add regression tests, 2008-02-27).\nThe line was last touched in e429dfd5e4 (t/lib-httpd.sh: use\nthe $( ... ) construct for command substitution,\n2015-12-22).\n\n> -\tsed -n 's/^Server version: Apache\\/\\([0-9]*\\)\\..*$/\\1/p; q')\n> +\tsed -n 's/^Server version: Apache\\/\\([0-9.]*\\).*$/\\1/p; q')\n> +HTTPD_VERSION_MAJOR=$(echo $HTTPD_VERSION | cut -d. -f1)\n> +HTTPD_VERSION_MINOR=$(echo $HTTPD_VERSION | cut -d. -f2)\n\n-- \nTodd\n"},{"id":"471446","messageId":"Y91Bq+xYUhDnPHfS@coredump.intra.peff.net","threadId":"59174","inReplyTo":"Y9s7vyHKXP+TQPRm@pobox.com","subject":"Re: [PATCH 0/4] t/lib-httpd ssl fixes","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-02-03T17:17:31Z","receivedAt":"2023-02-03T17:17:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 01, 2023 at 11:27:43PM -0500, Todd Zullinger wrote:\n\n> > The final two patches here fix ssl problems I found. The first two\n> > patches drop support for older apache. This yields some minor cleanups,\n> > and makes the ssl fixes slightly easier. I've cc'd Todd as the last\n> > person to express support for Apache 2.2, in 2017. I'm hoping even\n> > CentOS has moved on by now, but we'll see. :)\n> \n> Heh. Fortunately, CentOS 6 has been EOL for a few years.\n> CentOS 7 has httpd-2.4.6.\n\nOh, good. So I think we are OK to take these patches, then.\n\n> I applied these patches and ran builds for CentOS/RHEL 7-9\n> and Fedora 36-38. I had not previously run the test suite\n> with LIB_HTTPD_SSL=1 and I ran into many, many failures.\n> (154 failures across 12 tests, to be precise.)\n\nOof. I was just focusing on getting a handful of interesting tests to\nrun, and didn't think to try the whole suite.\n\n> I can clean up this diff if you think it's worthwhile.\n\nYes, please. Your fixes all look to be along the lines I'd expect. I\nthink my patches can stand on their own as an incremental improvement,\nand then yours can come on top as a separate series.\n\n> It sounds like it may be quite useful for the http/2 tests, but maybe\n> LIB_HTTPD_SSL=1 in t5559-http-fetch-smart-http2 is simpler for now?\n\nI'd prefer to avoid requiring ssl support for those tests if we can.\nVersion 2.0.12 of mod_http2 does fix the test failure. There may be some\nother issues (there's more details in the GitHub issue I linked\nearlier), but I think it would be enough to stop the immediate pain. I\ndon't know how long it will take before that version makes it into an\napache release.\n\n> -- 8< --\n\nA few bits I noticed on your test fixes:\n\n> diff --git i/t/lib-httpd.sh w/t/lib-httpd.sh\n> index 608949ea80..a4f787f580 100644\n> --- i/t/lib-httpd.sh\n> +++ w/t/lib-httpd.sh\n> @@ -168,7 +168,7 @@ prepare_httpd() {\n> \t\tthen\n> \t\t\tHTTPD_PARA=\"$HTTPD_PARA -DSVN\"\n> \t\t\tLIB_HTTPD_SVNPATH=\"$rawsvnrepo\"\n> -\t\t\tsvnrepo=\"http://127.0.0.1:$LIB_HTTPD_PORT/\"\n> +\t\t\tsvnrepo=\"$HTTPD_PROTO://127.0.0.1:$LIB_HTTPD_PORT/\"\n\nThis probably should have been $HTTPD_URL all along, which fixes the\nprotocol and avoids hard-coding the loopback address.\n\n> diff --git i/t/t5541-http-push-smart.sh w/t/t5541-http-push-smart.sh\n> index fbad2d5ff5..b1f414dfe0 100755\n> --- i/t/t5541-http-push-smart.sh\n> +++ w/t/t5541-http-push-smart.sh\n> @@ -122,9 +122,9 @@ test_expect_success 'setup rejected update hook' '\n> \n> \tcat >exp <<-EOF\n> \tremote: error: hook declined to update refs/heads/dev2\n> -\tTo http://127.0.0.1:$LIB_HTTPD_PORT/smart/test_repo.git\n> +\tTo '$HTTPD_PROTO'://127.0.0.1:$LIB_HTTPD_PORT/smart/test_repo.git\n\nDitto here.\n\n> diff --git i/t/t5550-http-fetch-dumb.sh w/t/t5550-http-fetch-dumb.sh\n> index 8f182a3cbf..070d04cdce 100755\n> --- i/t/t5550-http-fetch-dumb.sh\n> +++ w/t/t5550-http-fetch-dumb.sh\n> @@ -384,7 +384,7 @@ test_expect_success 'remote-http complains cleanly about malformed urls' '\n> # learned to handle early remote helper failures more cleanly.\n> test_expect_success 'remote-http complains cleanly about empty scheme' '\n> \ttest_must_fail ok=sigpipe git ls-remote \\\n> -\t\thttp::${HTTPD_URL#http}/dumb/repo.git 2>stderr &&\n> +\t\thttp::${HTTPD_URL#$HTTPD_PROTO}/dumb/repo.git 2>stderr &&\n> \ttest_i18ngrep \"url has no scheme\" stderr\n> '\n\nI wondered if this should also be adjusting \"http::\" to match the\nprotocol. It doesn't really matter, though. The point of the test is use\n\"http::\" to route it to git-remote-curl, and then \"://anything\" should\nfail. The fact that it is using $HTTPD_URL in the first place is only so\nthat we might detect an accidental success. We never expect to actually\nhit the endpoint.\n\n> @@ -454,9 +454,9 @@ test_expect_success 'http-alternates triggers not-from-user protocol check' '\n> \techo \"$HTTPD_URL/dumb/victim.git/objects\" \\\n> \t\t>\"$evil/objects/info/http-alternates\" &&\n> \ttest_config_global http.followRedirects true &&\n> -\ttest_must_fail git -c protocol.http.allow=user \\\n> +\ttest_must_fail git -c protocol.'$HTTPD_PROTO'.allow=user \\\n> \t\tclone $HTTPD_URL/dumb/evil.git evil-user &&\n> -\tgit -c protocol.http.allow=always \\\n> +\tgit -c protocol.'$HTTPD_PROTO'.allow=always \\\n> \t\tclone $HTTPD_URL/dumb/evil.git evil-user\n> '\n\nThese single quotes seem unnecessary. We're in a single-quoted string,\nbut it's a test-body that will be eval'd. So the variable can just be\nreferenced in the usual way, just like $HTTPD_URL in the existing\ncontext.\n\n> diff --git i/t/t5703-upload-pack-ref-in-want.sh w/t/t5703-upload-pack-ref-in-want.sh\n> index df74f80061..b365e30eda 100755\n> --- i/t/t5703-upload-pack-ref-in-want.sh\n> +++ w/t/t5703-upload-pack-ref-in-want.sh\n> @@ -450,7 +450,7 @@ test_expect_success 'setup repos for change-while-negotiating test' '\n> \t\t# Local repo with many commits (so that negotiation will take\n> \t\t# more than 1 request/response pair)\n> \t\trm -rf \"$LOCAL_PRISTINE\" &&\n> -\t\tgit clone \"http://127.0.0.1:$LIB_HTTPD_PORT/smart/repo\" \"$LOCAL_PRISTINE\" &&\n> +\t\tgit clone \"$HTTPD_PROTO://127.0.0.1:$LIB_HTTPD_PORT/smart/repo\" \"$LOCAL_PRISTINE\" &&\n\nAnother one that could be $HTTPD_URL.\n\n> @@ -462,7 +462,7 @@ test_expect_success 'setup repos for change-while-negotiating test' '\n> \t\ttest_commit m3 &&\n> \t\tgit tag -d m2 m3\n> \t) &&\n> -\tgit -C \"$LOCAL_PRISTINE\" remote set-url origin \"http://127.0.0.1:$LIB_HTTPD_PORT/one_time_perl/repo\" &&\n> +\tgit -C \"$LOCAL_PRISTINE\" remote set-url origin \"$HTTPD_PROTO://127.0.0.1:$LIB_HTTPD_PORT/one_time_perl/repo\" &&\n\nLikewise.\n\n> diff --git i/t/t5812-proto-disable-http.sh w/t/t5812-proto-disable-http.sh\n> index d8da5f58d1..9ee5132276 100755\n> --- i/t/t5812-proto-disable-http.sh\n> +++ w/t/t5812-proto-disable-http.sh\n> [...]\n> @@ -28,9 +28,9 @@ test_expect_success 'curl limits redirects' '\n> '\n> \n> test_expect_success 'http can be limited to from-user' '\n> -\tgit -c protocol.http.allow=user \\\n> +\tgit -c protocol.'$HTTPD_PROTO'.allow=user \\\n> \t\tclone \"$HTTPD_URL/smart/repo.git\" plain.git &&\n> -\ttest_must_fail git -c protocol.http.allow=user \\\n> +\ttest_must_fail git -c protocol.'$HTTPD_PROTO'.allow=user \\\n> \t\tclone \"$HTTPD_URL/smart-redir-perm/repo.git\" redir.git\n> '\n\nAnd another one with funny quotes.\n\n-Peff\n"},{"id":"471447","messageId":"Y91CI1NKcy4pEzjn@coredump.intra.peff.net","threadId":"59174","inReplyTo":"Y9s+ifALGYniOVHK@pobox.com","subject":"Re: [PATCH 1/4] t/lib-httpd: bump required apache version to 2.2","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-02-03T17:19:31Z","receivedAt":"2023-02-03T17:19:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 01, 2023 at 11:39:37PM -0500, Todd Zullinger wrote:\n\n> > diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh\n> > index 608949ea80..8fc411ff41 100644\n> > --- a/t/lib-httpd.sh\n> > +++ b/t/lib-httpd.sh\n> > @@ -99,16 +99,19 @@ then\n> > fi\n> > \n> > HTTPD_VERSION=$($LIB_HTTPD_PATH -v | \\\n> \n> Trivial, but is it worth getting rid of the unnecessary\n> backslash, while you're improving things here? Maybe that's\n> a mild tangent for code that's otherwise not adjusted very\n> often?\n\nHeh, I didn't even notice. I don't think it's a big deal either way. It\ndefinitely doesn't match our style, and I don't mind if we fix it on\ntop, but it could just be left alone.\n\nIt might make more sense as part of a broader cleanup. Running:\n\n git grep '| \\\\'\n\nturns up a number of hits.\n\n-Peff\n"}]}