git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Sep 23, 2021, 01:22 UTC
Message-ID
<87lf3o5bdz.fsf@evledraar.gmail.com>
In-Reply-To
<YUuN+KguN0WetC49@coredump.intra.peff.net>
On Wed, Sep 22 2021, Jeff King wrote:
Show 41 quoted lines
> 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.

Previous: Jeff KingNext: Jeff King
Message 14 of 22 in “http: match headers case-insensitively when redacting”
  1. http: match headers case-insensitively when redactingJeff King, Sep 21, 2021
  2. Jeff KingSep 21, 2021
  3. Carlo ArenasSep 21, 2021
  4. Jeff KingSep 21, 2021
  5. Daniel StenbergSep 21, 2021
  6. Jeff KingSep 22, 2021
  7. Eric SunshineSep 21, 2021
  8. Jeff KingSep 21, 2021
  9. Junio C HamanoSep 22, 2021
  10. Taylor BlauSep 21, 2021
  11. Jeff KingSep 22, 2021
  12. Bagas SanjayaSep 22, 2021
  13. Jeff KingSep 22, 2021
  14. Ævar Arnfjörð BjarmasonSep 23, 2021
  15. Jeff KingSep 23, 2021
  16. Junio C HamanoSep 22, 2021
  17. Jeff KingSep 22, 2021
  18. Junio C HamanoSep 22, 2021
  19. Jeff KingSep 22, 2021
  20. Junio C HamanoSep 22, 2021
  21. http: match headers case-insensitively when redactingJeff King, Sep 22, 2021
  22. Jeff KingSep 22, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.