From db30130165bef1ceff04c0163db6676db23ba2fc Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 17 Jun 2023 01:15:59 -0400 Subject: [PATCH 1/3] http: handle both "h2" and "h2h3" in curl info lines When redacting auth tokens in trace output from curl, we look for http/2 headers of the form "h2h3 [header: value]". This comes from b637a41ebe (http: redact curl h2h3 headers in info, 2022-11-11). But the "h2h3" prefix changed to just "h2" in curl's fc2f1e547 (http2: support HTTP/2 to forward proxies, non-tunneling, 2023-04-14). That's in released version curl 8.1.0; linking against that version means we'll fail to correctly redact the trace. Our t5559.17 notices and fails. We can fix this by matching either prefix, which should handle both old and new versions. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- http.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/http.c b/http.c index 8a5ba3f477..f30bd0bca9 100644 --- a/http.c +++ b/http.c @@ -630,7 +630,8 @@ static void redact_sensitive_info_header(struct strbuf *header) * h2h3 [: ] */ if (trace_curl_redact && - skip_iprefix(header->buf, "h2h3 [", &sensitive_header)) { + (skip_iprefix(header->buf, "h2h3 [", &sensitive_header) || + skip_iprefix(header->buf, "h2 [", &sensitive_header))) { if (redact_sensitive_header(header, sensitive_header - header->buf)) { /* redaction ate our closing bracket */ strbuf_addch(header, ']'); From 39fa527c8976da84cf70a9ea6b6d92a1fd9bd772 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 15 Sep 2023 07:33:16 -0400 Subject: [PATCH 2/3] http: factor out matching of curl http/2 trace lines We have to parse out curl's http/2 trace lines so we can redact their headers. We already match two different types of lines from various vintages of curl. In preparation for adding another (which will be slightly more complex), let's pull the matching into its own function, rather than doing it in the middle of a conditional. While we're doing so, let's expand the comment a bit to describe the two matches. That probably should have been part of db30130165 (http: handle both "h2" and "h2h3" in curl info lines, 2023-06-17), but will become even more important as we add new types. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- http.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/http.c b/http.c index f30bd0bca9..7f7bc85dd0 100644 --- a/http.c +++ b/http.c @@ -620,18 +620,29 @@ static int redact_sensitive_header(struct strbuf *header, size_t offset) return ret; } +static int match_curl_h2_trace(const char *line, const char **out) +{ + /* + * curl prior to 8.1.0 gives us: + * + * h2h3 [: ] + * + * Starting in 8.1.0, the first token became just "h2". + */ + if (skip_iprefix(line, "h2h3 [", out) || + skip_iprefix(line, "h2 [", out)) + return 1; + + return 0; +} + /* Redact headers in info */ static void redact_sensitive_info_header(struct strbuf *header) { const char *sensitive_header; - /* - * curl's h2h3 prints headers in info, e.g.: - * h2h3 [: ] - */ if (trace_curl_redact && - (skip_iprefix(header->buf, "h2h3 [", &sensitive_header) || - skip_iprefix(header->buf, "h2 [", &sensitive_header))) { + match_curl_h2_trace(header->buf, &sensitive_header)) { if (redact_sensitive_header(header, sensitive_header - header->buf)) { /* redaction ate our closing bracket */ strbuf_addch(header, ']'); From 0763c3a2c4f21a9e81990cc5cbee4a66d4efefcb Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 15 Sep 2023 07:34:43 -0400 Subject: [PATCH 3/3] http: update curl http/2 info matching for curl 8.3.0 To redact header lines in http/2 curl traces, we have to parse past some prefix bytes that curl sticks in the info lines it passes to us. That changed once already, and we adapted in db30130165 (http: handle both "h2" and "h2h3" in curl info lines, 2023-06-17). Now it has changed again, in curl's fbacb14c4 (http2: cleanup trace messages, 2023-08-04), which was released in curl 8.3.0. Running a build of git linked against that version will fail to redact the trace (and as before, t5559 notices and complains). The format here is a little more complicated than the other ones, as it now includes a "stream id". This is not constant but is always numeric, so we can easily parse past it. We'll continue to match the old versions, of course, since we want to work with many different versions of curl. We can't even select one format at compile time, because the behavior depends on the runtime version of curl we use, not the version we build against. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- http.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/http.c b/http.c index 7f7bc85dd0..fb4ecf911f 100644 --- a/http.c +++ b/http.c @@ -622,6 +622,8 @@ static int redact_sensitive_header(struct strbuf *header, size_t offset) static int match_curl_h2_trace(const char *line, const char **out) { + const char *p; + /* * curl prior to 8.1.0 gives us: * @@ -633,6 +635,18 @@ static int match_curl_h2_trace(const char *line, const char **out) skip_iprefix(line, "h2 [", out)) return 1; + /* + * curl 8.3.0 uses: + * [HTTP/2] [] [: ] + * where is numeric. + */ + if (skip_iprefix(line, "[HTTP/2] [", &p)) { + while (isdigit(*p)) + p++; + if (skip_prefix(p, "] [", out)) + return 1; + } + return 0; }