Re: [PATCH 2/2] http: update curl http/2 info matching for curl 8.3.0
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 15, 2023, 18:21 UTC
- Message-ID
- <xmqqsf7fe1q4.fsf@gitster.g>
- In-Reply-To
- <20230915113443.GB3531587@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 15 quoted lines
> @@ -751,6 +753,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] [<stream-id>] [<header-name>: <header-val>]
> + * where <stream-id> is numeric.
> + */
> + if (skip_iprefix(line, "[HTTP/2] [", &p)) {
> + while (isdigit(*p))
> + p++;
> + if (skip_prefix(p, "] [", out))
> + return 1;
> + }Looking good assuming that <stream-id> part will never be updated to allow spaces around the ID, or allow non-digits in the ID, in the future. Is there much harm if this code allowed false positives and sent something that is *not* a curl trace, like "foo]" parsed out of "[HTTP/2] [PATCH] [foo]", to redact_sensitive_header() function?
By the way, would this patch make sense? Everybody in the function that try to notice a sensitive header seems to check the sentting independently, which seems error prone for those who want to add a new header to redact.
http.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git c/http.c w/http.c index 8f71bf00d8..3dfa34fe65 100644 --- c/http.c +++ w/http.c @@ -684,8 +684,10 @@ static int redact_sensitive_header(struct strbuf *header, size_t offset) int ret = 0; const char *sensitive_header; - if (trace_curl_redact && - (skip_iprefix(header->buf + offset, "Authorization:", &sensitive_header) || + if (!trace_curl_redact) + return ret; + + if ((skip_iprefix(header->buf + offset, "Authorization:", &sensitive_header) || skip_iprefix(header->buf + offset, "Proxy-Authorization:", &sensitive_header))) { /* The first token is the type, which is OK to log */ while (isspace(*sensitive_header)) @@ -696,8 +698,7 @@ static int redact_sensitive_header(struct strbuf *header, size_t offset) strbuf_setlen(header, sensitive_header - header->buf); strbuf_addstr(header, " <redacted>"); ret = 1; - } else if (trace_curl_redact && - skip_iprefix(header->buf + offset, "Cookie:", &sensitive_header)) { + } else if (skip_iprefix(header->buf + offset, "Cookie:", &sensitive_header)) { struct strbuf redacted_header = STRBUF_INIT; const char *cookie;