On Thu, Nov 10, 2022 at 02:53:54PM -0800, Glen Choo wrote:

quoted

There's some discussion in b66c77a64e (http: match headers
case-insensitively when redacting, 2021-09-22) about testing with
HTTP/2. Which ironically is basically this exact same bug in a different
form. ;)

The short answer is that it's do-able, but probably there are some
headaches to make it work portably.

Argh, what a shame :( Okay, maybe it's not worth trying to use httpd
then.

Some other ideas I had were:

- Create a test-tool that calls the redaction logic directly (without
  involving about curl), and we pass the strings we want to redacted to
  it. Way less than ideal, since we'd never be able to proactively catch
  failures, but better than nothing I suppose.

I don't think this is worth the effort. It's nice to exercise the code a
bit, but it wouldn't have actually found this regression, since the
unexpected thing here was curl changing.

- Write our own HTTP/2 server for redaction tests. I assume this won't
  be trivial, but maybe not prohibitive, e.g. [1] implements its own
  http server for credential helper tests.

These seems like a lot more work than just setting up HTTP/2 support for
Apache. I checked the recipe from b66c77a64e, and it still works. It
does indeed find the bug (my curl is 7.86.0) and confirms your fix.

I think a simple path forward could be something like:

  - teach lib-httpd to conditionally use the current set up versus the
    http2 one outlined in b66c77a64e

  - push most of t5551 into a lib-http-fetch.sh; the client-side set up
    from b66c77a64e checks for an HTTP2 prereq. The test that looks for
    chunked encoding (and only works on HTTP1) checks for !HTTP2.

  - t5551 tells lib-httpd to use the usual setup, and then sources
    lib-http-fetch; it behaves as before

  - t5559 (sadly, not contiguous without renumbering intermediate tests)
    tells lib-httpd to use http2, and sets the HTTP2 prereq. It runs the
    same tests but via http2.

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.

-Peff