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

On Thu, Sep 23, 2021 at 03:22:04AM +0200, Ævar Arnfjörð Bjarmason wrote:

quoted

quoted

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.

quoted

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

Sure. I was assuming we'd just have one server config (which _does_
work), but if we are spinning up two servers anyway for the separate
scripts, it would be easy enough to customize them. And I do think it
would make sense to do it in separate scripts.

And this dual-script thing might need to be repeated for others besides
t5551. I didn't look at which other ones might potentially benefit (or
if it's diminishing returns as we just add more basically-identical
tests that spend a bunch of CPU). This is why I say "it might not be
worth the effort".

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.

This does work oddly with GIT_TEST_HTTPD=Yes, which complains about
skipping (intentionally; it's how we notice when http setup code
breaks).  That might be acceptable, though, if the folks setting that
option (like me, or the linux CI jobs) are likely to have http2 support.

-Peff