Jeff King <peff@xxxxxxxx> writes: > On Wed, Sep 22, 2021 at 12:19:26PM -0700, Junio C Hamano wrote: > >> Jeff King <peff@xxxxxxxx> 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.
