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

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> On Wed, Sep 22, 2021 at 12:19:26PM -0700, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>> 
>> > 	# Ensure that there is no "Basic" followed by a base64 string, but that
>> > 	# the auth details are redacted
>> > 	! grep "Authorization: Basic [0-9a-zA-Z+/]" trace &&
>> > 	grep "Authorization: Basic <redacted>" trace
>> >
>> > gets confused. It sees the "<redacted>" one from the pre-upgrade
>> > HTTP/1.1 request, but fails to see the unredacted HTTP/2 one, because it
>> > does not match the lower-case "authorization".
>> 
>> Neither pattern of the above two will not match the HTTP/2 one, so
>> the first one would report "there is no leakage of Auth with a
>> caplital letter"; the second one may see only one pre-upgrade Auth
>> with a capital letter, but as long as it does find one, it should be
>> happy, no?
>> 
>> I am a bit puzzled how the test gets confused.
>
> The first one matches nothing, because the HTTP/2 one which fails to
> redact has a lower-case "A". The second one _does_ match, because ...

I thought we were talking about the original case sensitive test
getting confused when testing the software that is fixed,
i.e. HTTP/2 lowercase "authorization" line properly redacted.

> I get (with some extraneous headers omitted):
> ...
>   => Send header: GET /auth/smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1
>   => Send header: Authorization: Basic <redacted>

So, this is what we see in HTTP/1.1 (with capitalization).  And then
...

> ...
>   => Send header: POST /auth/smart/repo.git/git-upload-pack HTTP/2
>   => Send header: authorization: Basic dXNlckBob3N0OnBhc3NAaG9zdA==

this one, once the redaction code is fixed by applying this patch,
would show that we redacted it, too, no?

With or without the fix in the code, I agree that neither of the two
"grep" patterns without "grep -i" change will match this line.  So
the end result is that the test finds no unredacted line, and one
redacted one (instead of two).

I agree that it is *not* testing what we want to test, and if you
said so, I wouldn't have been puzzled.  I just wanted to know if
there is something _else_ (other than "gee, we are not testing the
HTTP/2 case at all") going on that I failed to read in your
"... gets confused".

Thanks.