Re: [PATCH] http: match headers case-insensitively when redacting

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Wed, Sep 22 2021, Jeff King wrote:

> On Wed, Sep 22, 2021 at 09:32:41AM +0700, Bagas Sanjaya wrote:
>
>> > but this has a few issues:
>> > 
>> >    - it's not necessarily portable. The http2 apache module might not be
>> >      available on all systems. Further, the http2 module isn't compatible
>> >      with the prefork mpm, so we have to switch to something else. But we
>> >      don't necessarily know what's available. It would be nice if we
>> >      could have conditional config, but IfModule only tells us if a
>> >      module is already loaded, not whether it is available at all.
>> > 
>> >      This might be a non-issue. The http tests are already optional, and
>> >      modern-enough systems may just have both of these. But...
>> > 
>> >    - if we do this, then we'd no longer be testing HTTP/1.1 at all. I'm
>> >      not sure how much that matters since it's all handled by curl under
>> >      the hood, but I'd worry that some detail leaks through. We'd
>> >      probably want two scripts running similar tests, one with HTTP/2 and
>> >      one with HTTP/1.1.
>> 
>> Maybe for httpd config we can say that if mpm_prefork isn't loaded, load
>> mpm_event and mod_http2.
>
> That doesn't work. We can say "is mpm_prefork" loaded, and indeed we
> already do, in order to load mpm_prefork! That's because the module may
> or may not be built-in, and if not, we have to load it (or some mpm
> module). See 296f0b3ea9 (t/lib-httpd/apache.conf: configure an MPM
> module for apache 2.4, 2013-06-09).
>
> But we have no way of knowing _which_ modules are available. It may just
> be that "event" or "worker" (both of which support mod_http2) are
> available close enough to everywhere that we can just guess.
>
>> And for testing both HTTP/2 and HTTP/1.1 did you mean sharing the same test
>> code (with adjustments for each protocol)?
>
> Yes. I'd literally run the same battery of tests against both protocols
> (see my other response to Taylor with a sketched-out example). I'm still
> not sure it's entirely worth the effort, though. The underlying
> transport should be pretty transparent to Git, with the exception of
> things like debugging output.

Maybe I'm missing something, but it seems to me that trying to figure
out if we support http v2 or not beforehand is the wrong thing to do in
this case. Why don't we simply try to start the server, and fail and
skip_all="sorry, no httpv2" if it fails?

Then have 2 test files:

t1234-http-v1.sh
t1235-http-v2.sh

Where the latter includes the former (or is a symlink with a $0 check),
or both include a library. Doing it this way also means you'll get a
message you notice via "prove", since you won't run all v1 tests in one
file, then skip some v2.

It also means we could add "ssl" in that mix and have 4x files, and
unlike a GIT_TEST_* mode or shoving it all in one test we can run these
in parallel and test all combinations in one test run.