Skip to content

Commit 73c49a4

Browse files
peffttaylorr
authored andcommitted
t: run t5551 tests with both HTTP and HTTP/2
We have occasionally seen bugs that affect Git running only against an HTTP/2 web server, not an HTTP one. For instance, b66c77a (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 b66c77a, 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 <[email protected]> Signed-off-by: Taylor Blau <[email protected]>
1 parent 319605f commit 73c49a4

File tree

4 files changed

+39
-8
lines changed

4 files changed

+39
-8
lines changed

t/lib-httpd.sh

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,11 @@ prepare_httpd() {
174174
fi
175175
}
176176

177+
enable_http2 () {
178+
HTTPD_PARA="$HTTPD_PARA -DHTTP2"
179+
test_set_prereq HTTP2
180+
}
181+
177182
start_httpd() {
178183
prepare_httpd >&3 2>&4
179184

t/lib-httpd/apache.conf

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,11 @@ ErrorLog error.log
2929
LoadModule setenvif_module modules/mod_setenvif.so
3030
</IfModule>
3131

32+
<IfDefine HTTP2>
33+
LoadModule http2_module modules/mod_http2.so
34+
Protocols h2c
35+
</IfDefine>
36+
3237
<IfVersion < 2.4>
3338
LockFile accept.lock
3439
</IfVersion>
@@ -64,12 +69,20 @@ LockFile accept.lock
6469
<IfModule !mod_access_compat.c>
6570
LoadModule access_compat_module modules/mod_access_compat.so
6671
</IfModule>
67-
<IfModule !mod_mpm_prefork.c>
68-
LoadModule mpm_prefork_module modules/mod_mpm_prefork.so
69-
</IfModule>
7072
<IfModule !mod_unixd.c>
7173
LoadModule unixd_module modules/mod_unixd.so
7274
</IfModule>
75+
76+
<IfDefine HTTP2>
77+
<IfModule !mod_mpm_event.c>
78+
LoadModule mpm_event_module modules/mod_mpm_event.so
79+
</IfModule>
80+
</IfDefine>
81+
<IfDefine !HTTP2>
82+
<IfModule !mod_mpm_prefork.c>
83+
LoadModule mpm_prefork_module modules/mod_mpm_prefork.so
84+
</IfModule>
85+
</IfDefine>
7386
</IfVersion>
7487

7588
PassEnv GIT_VALGRIND

t/t5551-http-fetch-smart.sh

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,19 @@
11
#!/bin/sh
22

3-
test_description='test smart fetching over http via http-backend'
3+
: ${HTTP_PROTO:=HTTP}
4+
test_description="test smart fetching over http via http-backend ($HTTP_PROTO)"
45
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
56
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
67

78
. ./test-lib.sh
89
. "$TEST_DIRECTORY"/lib-httpd.sh
10+
test "$HTTP_PROTO" = "HTTP/2" && enable_http2
911
start_httpd
1012

13+
test_expect_success HTTP2 'enable client-side http/2' '
14+
git config --global http.version HTTP/2
15+
'
16+
1117
test_expect_success 'setup repository' '
1218
git config push.default matching &&
1319
echo content >file &&
@@ -194,7 +200,7 @@ test_expect_success 'redirects send auth to new location' '
194200
expect_askpass both user@host auth/smart/repo.git
195201
'
196202

197-
test_expect_success 'GIT_TRACE_CURL redacts auth details' '
203+
test_expect_success !HTTP2 'GIT_TRACE_CURL redacts auth details' '
198204
rm -rf redact-auth trace &&
199205
set_askpass user@host pass@host &&
200206
GIT_TRACE_CURL="$(pwd)/trace" git clone --bare "$HTTPD_URL/auth/smart/repo.git" redact-auth &&
@@ -206,7 +212,7 @@ test_expect_success 'GIT_TRACE_CURL redacts auth details' '
206212
grep -i "Authorization: Basic <redacted>" trace
207213
'
208214

209-
test_expect_success 'GIT_CURL_VERBOSE redacts auth details' '
215+
test_expect_success !HTTP2 'GIT_CURL_VERBOSE redacts auth details' '
210216
rm -rf redact-auth trace &&
211217
set_askpass user@host pass@host &&
212218
GIT_CURL_VERBOSE=1 git clone --bare "$HTTPD_URL/auth/smart/repo.git" redact-auth 2>trace &&
@@ -347,7 +353,10 @@ test_expect_success CMDLINE_LIMIT \
347353
test_expect_success 'large fetch-pack requests can be sent using chunked encoding' '
348354
GIT_TRACE_CURL=true git -c http.postbuffer=65536 \
349355
clone --bare "$HTTPD_URL/smart/repo.git" split.git 2>err &&
350-
grep "^=> Send header: Transfer-Encoding: chunked" err
356+
{
357+
test_have_prereq HTTP2 ||
358+
grep "^=> Send header: Transfer-Encoding: chunked" err
359+
}
351360
'
352361

353362
test_expect_success 'test allowreachablesha1inwant' '
@@ -473,7 +482,7 @@ test_expect_success 'fetch by SHA-1 without tag following' '
473482
--no-tags origin $(cat bar_hash)
474483
'
475484

476-
test_expect_success 'cookies are redacted by default' '
485+
test_expect_success !HTTP2 'cookies are redacted by default' '
477486
rm -rf clone &&
478487
echo "Set-Cookie: Foo=1" >cookies &&
479488
echo "Set-Cookie: Bar=2" >>cookies &&

t/t5559-http-fetch-smart-http2.sh

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
#!/bin/sh
2+
3+
HTTP_PROTO=HTTP/2
4+
. ./t5551-http-fetch-smart.sh

0 commit comments

Comments
 (0)