git/t/lib-httpd/apache.conf, branch v2.40.2

Merge branch 'jk/http-test-fixes'

2023-03-01T00:38:47Z

Various fix-ups on HTTP tests. * jk/http-test-fixes: t5559: make SSL/TLS the default t5559: fix test failures with LIB_HTTPD_SSL t/lib-httpd: enable HTTP/2 "h2" protocol, not just h2c t/lib-httpd: respect $HTTPD_PROTO in expect_askpass() t5551: drop curl trace lines without headers t5551: handle v2 protocol in cookie test t5551: simplify expected cookie file t5551: handle v2 protocol in upload-pack service test t5551: handle v2 protocol when checking curl trace t5551: stop forcing clone to run with v0 protocol t5551: handle HTTP/2 when checking curl trace t5551: lower-case headers in expected curl trace t5551: drop redundant grep for Accept-Language t5541: simplify and move "no empty path components" test t5541: stop marking "used receive-pack service" test as v0 only t5541: run "used receive-pack service" test earlier

t/lib-httpd: enable HTTP/2 "h2" protocol, not just h2c

2023-02-23T21:01:17Z

Commit 73c49a4474 (t: run t5551 tests with both HTTP and HTTP/2, 2022-11-11) added Apache config to enable HTTP/2. However, it only enabled the "h2c" protocol, which allows cleartext HTTP/2 (generally based on an upgrade header during an HTTP/1.1 request). This is what t5559 is generally testing, since by default we don't set up SSL/TLS. However, it should be possible to run t5559 with LIB_HTTPD_SSL set. In that case, Apache will advertise support for HTTP/2 via ALPN during the TLS handshake. But we need to tell it support "h2" (the non-cleartext version) to do so. Without that, then curl does not even try to do the HTTP/1.1 upgrade (presumably because after seeing that we did TLS but didn't get the ALPN indicator, it assumes it would be fruitless). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

add basic http proxy tests

2023-02-17T00:24:23Z

We do not test our http proxy functionality at all in the test suite, so this is a pretty big blind spot. Let's at least add a basic check that we can go through an authenticating proxy to perform a clone. A few notes on the implementation: - I'm using a single apache instance to proxy to itself. This seems to work fine in practice, and we can check with a test that this rather unusual setup is doing what we expect. - I've put the proxy tests into their own script, and it's the only one which loads the apache proxy config. If any platform can't handle this (e.g., doesn't have the right modules), the start_httpd step should fail and gracefully skip the rest of the script (but all the other http tests in existing scripts will continue to run). - I used a separate passwd file to make sure we don't ever get confused between proxy and regular auth credentials. It's using the antiquated crypt() format. This is a terrible choice security-wise in the modern age, but it's what our existing passwd file uses, and should be portable. It would probably be reasonable to switch both of these to bcrypt, but we can do that in a separate patch. - On the client side, we test two situations with credentials: when they are present in the url, and when the username is present but we prompt for the password. I think we should be able to handle the case that _neither_ is present, but an HTTP 407 causes us to prompt for them. However, this doesn't seem to work. That's either a bug, or at the very least an opportunity for a feature, but I punted on it for now. The point of this patch is just getting basic coverage, and we can explore possible deficiencies later. - this doesn't work with LIB_HTTPD_SSL. This probably would be valuable to have, as https over an http proxy is totally different (it uses CONNECT to tunnel the session). But adding in mod_proxy_connect and some basic config didn't seem to work for me, so I punted for now. Much of the rest of the test suite does not currently work with LIB_HTTPD_SSL either, so we shouldn't be making anything much worse here. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

t/lib-httpd: drop SSLMutex config

2023-02-01T18:10:34Z

The SSL config enabled by setting LIB_HTTPD_SSL does not work with Apache versions greater than 2.2, as more recent versions complain about the SSLMutex directive. According to https://httpd.apache.org/docs/current/upgrading.html: Directives AcceptMutex, LockFile, RewriteLock, SSLMutex, SSLStaplingMutex, and WatchdogMutexPath have been replaced with a single Mutex directive. You will need to evaluate any use of these removed directives in your 2.2 configuration to determine if they can just be deleted or will need to be replaced using Mutex. Deleting this line will just use the system default, which seems sensible. The original came as part of faa4bc35a0 (http-push: add regression tests, 2008-02-27), but no specific reason is given there (or on the mailing list) for its presence. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

t/lib-httpd: bump required apache version to 2.4

2023-02-01T18:10:34Z

Apache 2.4 has been out since early 2012, almost 11 years. And its predecessor, 2.2, has been out of support since its last release in 2017, over 5 years ago. The last mention on the mailing list was from around the same time, in this thread: https://lore.kernel.org/git/20171231023234.21215-1-tmz@pobox.com/ We can probably assume that 2.4 is available everywhere. And the stakes are fairly low, as the worst case is that such a platform would skip the http tests. This lets us clean up a few minor version checks in the config file, but also revert f1f2b45be0 (tests: adjust the configuration for Apache 2.2, 2016-05-09). Its technique isn't _too_ bad, but certainly required a bit more explanation than the 2.4 version it replaced. I manually confirmed that the test in t5551 still behaves as expected (if you replace "cadabra" with "foo", the server correctly rejects the request). It will also help future patches which will no longer have to deal with conditional config for this old version. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

t/lib-httpd: bump required apache version to 2.2

2023-02-01T18:10:34Z

Apache 2.2 was released in 2005, almost 18 years ago. We can probably assume that people are running a version at least that old (and the stakes for removing it are fairly low, as the worst case is that they would not run the http tests against their ancient version). Dropping support for the older versions cleans up the config file a little, and will also enable us to bump the required version further (with more cleanups) in a future patch. Note that the file actually checks for version 2.1. In apache's versioning scheme, odd numbered versions are for development and even numbers are for stable releases. So 2.1 and 2.2 are effectively the same from our perspective. Older versions would just fail to start, which would generally cause us to skip the tests. However, we do have version detection code in lib-httpd.sh which produces a nicer error message, so let's update that, too. I didn't bother handling the case of "3.0", etc. Apache has been on 2.x for 21 years, with no signs of bumping the major version. And if they eventually do, I suspect there will be enough breaking changes that we'd need to update more than just the numeric version check. We can worry about that hypothetical when it happens. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

t: run t5551 tests with both HTTP and HTTP/2

2022-11-14T22:42:46Z

We have occasionally seen bugs that affect Git running only against an HTTP/2 web server, not an HTTP one. For instance, b66c77a64e (http: match headers case-insensitively when redacting, 2021-09-22). But since we have no test coverage using HTTP/2, we only uncover these bugs in the wild. That commit gives a recipe for converting our Apache setup to support HTTP/2, but: - it's not necessarily portable - we don't want to just test HTTP/2; we really want to do a variety of basic tests for _both_ protocols This patch handles both problems by running a duplicate of t5551 (labeled as t5559 here) with an alternate-universe setup that enables HTTP/2. So we'll continue to run t5551 as before, but run the same battery of tests again with HTTP/2. If HTTP/2 isn't supported on a given platform, then t5559 should bail during the webserver setup, and gracefully skip all tests (unless GIT_TEST_HTTPD has been changed from "auto" to "yes", where the point is to complain when webserver setup fails). In theory other http-related test scripts could benefit from the same duplication, but doing t5551 should give us a reasonable check of basic functionality, and would have caught both bugs we've seen in the wild with HTTP/2. A few notes on the implementation: - a script enables the server side config by calling enable_http2 before starting the webserver. This avoids even trying to load any HTTP/2 config for t5551 (which is what lets it keep working with regular HTTP even on systems that don't support it). This also sets a prereq which can be used by individual tests. - As discussed in b66c77a64e, the http2 module isn't compatible with the "prefork" mpm, so we need to pick something else. I chose "event" here, which works on my Debian system, but it's possible there are platforms which would prefer something else. We can adjust that later if somebody finds such a platform. - The test "large fetch-pack requests can be sent using chunked encoding" makes sure we use a chunked transfer-encoding by looking for that header in the trace. But since HTTP/2 has its own streaming mechanisms, we won't find such a header. We could skip the test entirely by marking it with !HTTP2. But there's some value in making sure that the fetch itself succeeded. So instead, we'll confirm that either we're using HTTP2 _or_ we saw the expected chunked header. - the redaction tests fail under HTTP/2 with recent versions of curl. This is a bug! I've marked them with !HTTP2 here to skip them under t5559 for the moment. Using test_expect_failure would be more appropriate, but would require a bunch of boilerplate. Since we'll be fixing them momentarily, let's just skip them for now to keep the test suite bisectable, and we can re-enable them in the commit that fixes the bug. - one alternative layout would be to push most of t5551 into a lib-t5551.sh script, then source it from both t5551 and t5559. Keeping t5551 intact seemed a little simpler, as its one less level of indirection for people fixing bugs/regressions in the non-HTTP/2 tests. Signed-off-by: Jeff King Signed-off-by: Taylor Blau

t/lib-httpd: pass LANG and LC_ALL to Apache

2022-10-06T16:16:26Z

t5411 starts a web server with no explicit language setting, so it uses the system default. Ten of its tests expect it to return error messages containing the prefix "fatal: ", emitted by die(). This prefix can be localized since a1fd2cf8cd (i18n: mark message helpers prefix for translation, 2022-06-21), however. As a result these ten tests break for me on a system with LANG="de_DE.UTF-8" because the web server sends localized messages with "Schwerwiegend: " instead of "fatal: ". Fix these tests by passing LANG and LC_ALL to the web server, which are set to "C" by t/test-lib.sh, to get untranslated messages on both sides. Helped-by: Junio C Hamano Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano

Merge branch 'jk/http-push-status-fix'

2021-10-29T22:43:12Z

"git push" client talking to an HTTP server did not diagnose the lack of the final status report from the other side correctly, which has been corrected. * jk/http-push-status-fix: transport-helper: recognize "expecting report" error from send-pack send-pack: complain about "expecting report" with --helper-status

send-pack: complain about "expecting report" with --helper-status

2021-10-18T20:26:52Z

When pushing to a server which erroneously omits the final ref-status report, the client side should complain about the refs for which we didn't receive the status (because we can't just assume they were updated). This works over most transports like ssh, but for http we'll print a very misleading "Everything up-to-date". It works for ssh because send-pack internally sets the status of each ref to REF_STATUS_EXPECTING_REPORT, and then if the server doesn't tell us about a particular ref, it will stay at that value. When we print the final status table, we'll see that we're still on EXPECTING_REPORT and complain then. But for http, we go through remote-curl, which invokes send-pack with "--stateless-rpc --helper-status". The latter option causes send-pack to return a machine-readable list of ref statuses to the remote helper. But ever since its inception in de1a2fdd38 (Smart push over HTTP: client side, 2009-10-30), the send-pack code has simply omitted mention of any ref which ended up in EXPECTING_REPORT. In the remote helper, we then take the absence of any status report from send-pack to mean that the ref was not even something we tried to send, and thus it prints "Everything up-to-date". Fortunately it does detect the eventual non-zero exit from send-pack, and propagates that in its own non-zero exit code. So at least a careful script invoking "git push" would notice the failure. But sending the misleading message on stderr is certainly confusing for humans (not to mention the machine-readable "push --porcelain" output, though again, any careful script should be checking the exit code from push, too). Nobody seems to have noticed because the server in this instance has to be misbehaving: it has promised to support the ref-status capability (otherwise the client will not set EXPECTING_REPORT at all), but didn't send us any. If the connection were simply cut, then send-pack would complain about getting EOF while trying to read the status. But if the server actually sends a flush packet (i.e., saying "now you have all of the ref statuses" without actually sending any), then the client ends up in this confused situation. The fix is simple: we should return an error message from "send-pack --helper-status", just like we would for any other error per-ref error condition (in the test I included, the server simply omits all ref status responses, but a more insidious version of this would skip only some of them). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano