From: Glen Choo <email@example.com>
To: Jeff King <firstname.lastname@example.org>
Cc: Taylor Blau <email@example.com>,
Glen Choo via GitGitGadget <firstname.lastname@example.org>,
Subject: Re: [PATCH] http: redact curl h2h3 headers in info
Date: Thu, 10 Nov 2022 14:53:54 -0800 [thread overview]
Message-ID: <email@example.com> (raw)
Jeff King <firstname.lastname@example.org> writes:
> On Thu, Nov 10, 2022 at 09:48:38AM -0800, Glen Choo wrote:
>> Taylor Blau <email@example.com> writes:
>> >> * How could we set up end-to-end tests to ensure that we're testing
>> >> this against affected versions of curl? To avoid regressions, I'd
>> >> also prefer to test against future versions of curl too.
>> > Does that necessarily matter? We want to make sure that we don't see
>> > sensitive headers from the h2h3 module with any version of cURL, no?
>> It would help, but it might not be worth setting up infrastructure for
>> just this use case alone. Given the various platforms running tests
>> against the Git codebase, we probably get close to a representative
>> sample of the population with enough time.
>> I think it would be more important to have tests against HTTP/2.0. If we
>> did, we probably would have already caught this, e.g.
>> t/t5551-http-fetch-smart.sh:'GIT_TRACE_CURL redacts auth details' and
> 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
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.
- Write our own HTTP/2 server for redaction tests. I assume this won't
be trivial, but maybe not prohibitive, e.g.  implements its own
http server for credential helper tests.
> I agree with you that trying various curl versions isn't worth doing. If
> enough people/platforms run Git's suite, one of them will eventually
> see the problem.
next prev parent reply other threads:[~2022-11-10 22:54 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-09 0:52 [PATCH] http: redact curl h2h3 headers in info Glen Choo via GitGitGadget
2022-11-10 2:52 ` Taylor Blau
2022-11-10 17:48 ` Glen Choo
2022-11-10 21:50 ` Jeff King
2022-11-10 22:53 ` Glen Choo [this message]
2022-11-11 2:29 ` Jeff King
2022-11-11 2:31 ` Taylor Blau
2022-11-11 14:49 ` [PATCH] t: run t5551 tests with both HTTP and HTTP/2 Jeff King
2022-11-11 15:06 ` Ævar Arnfjörð Bjarmason
2022-11-11 15:19 ` Jeff King
2022-11-11 15:20 ` Jeff King
2022-11-10 21:57 ` [PATCH] http: redact curl h2h3 headers in info Emily Shaffer
2022-11-10 22:14 ` Glen Choo
2022-11-11 2:35 ` Taylor Blau
2022-11-10 22:57 ` [PATCH v2] " Glen Choo via GitGitGadget
2022-11-11 2:36 ` Taylor Blau
2022-11-11 2:38 ` Jeff King
2022-11-11 2:39 ` Taylor Blau
2022-11-11 17:55 ` Glen Choo
2022-11-11 22:35 ` [PATCH v3 0/2] " Glen Choo via GitGitGadget
2022-11-11 22:35 ` [PATCH v3 1/2] t: run t5551 tests with both HTTP and HTTP/2 Jeff King via GitGitGadget
2022-11-11 22:35 ` [PATCH v3 2/2] http: redact curl h2h3 headers in info Glen Choo via GitGitGadget
2022-11-14 22:33 ` [PATCH v3 0/2] " Jeff King
2022-11-14 22:43 ` Taylor Blau
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).