On Fri, Nov 11 2022, Jeff King wrote:

On Thu, Nov 10, 2022 at 09:29:15PM -0500, Jeff King wrote:

quoted

We don't get the _whole_ test suite running with http2, but hopefully it
gives us a fairly representative sample. And it does find this bug.

I can try to work the above into patch form, but I may not get to it for
a day or two.

So here's a patch which finds the bug you're fixing. It's set up to be
prepared before your patch, which helps confirm the bug (and that we're
correctly using http/2 in the tests!). You'll want to unmark the !HTTP2
prereqs as part of your patch.

Alternatively, it could come after your patch to confirm the fix, in
which case it would omit those !HTTP2 prereqs from the get-go.

Let me know if you'd like to pick it up as part of your series.
Otherwise, I can submit it separately in the on-top form (but my
preference is for you to take it, since it makes testing the fix
easier).

-- >8 --
Subject: 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, 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.

Looking at the diff below, would it be much more work to just support a
GIT_TEST_HTTP_VERSION=2?

Then:

This would just skip_all if it was already set to 2, as we'd know we
were testing it in other tests, and lib-httpd.sh would do
GIT_TEST_HTTP_VERSION=1 by default.

There's the "!HTTP2" additions, which you mention you'll be "fixing
momentarily", but this is a stand-alone patch, so maybe I'm missing that
context...