On Mon, May 11, 2020 at 10:43:10AM -0700, Jonathan Tan wrote:
> Whenever GIT_CURL_VERBOSE is set, teach Git to behave as if
> GIT_TRACE_CURL=1 and GIT_TRACE_CURL_NO_DATA=1 is set, instead of setting
> CURLOPT_VERBOSE.
>
> This is to prevent inadvertent revelation of sensitive data. In
> particular, GIT_CURL_VERBOSE redacts neither the "Authorization" header
> nor any cookies specified by GIT_REDACT_COOKIES.
>
> Unifying the tracing mechanism also has the future benefit that any
> improvements to the tracing mechanism will benefit both users of
> GIT_CURL_VERBOSE and GIT_TRACE_CURL, and we do not need to remember to
> implement any improvement twice.
Yeah, I think this is worth doing. The patch looks OK to me, though:
> +void http_trace_curl_no_data(void)
> +{
> + trace_override_envvar(&trace_curl, "1");
> + trace_curl_data = 0;
> +}
Would:
setenv("GIT_TRACE_CURL", "1", 0);
setenv("GIT_TRACE_CURL_NO_DATA", "0", 0);
be simpler? Perhaps it makes the flow more convoluted as we'd go on to
parse those variables, but it puts us on the same paths that we'd use if
the user specified those things (and avoids the need for the special
"override" function entirely).
Other than that nit, it seems very cleanly done.
-Peff
PS I sometimes find the normal trace a bit verbose, but I do still want
to see data. Do others feel the same? Particularly I find the "SSL"
lines totally worthless (I guess maybe you could be debugging ssl
stuff, but that would be the exception, I'd think). Ditto the split
of data into two lines: one with the size and one with the actual
data.
I dunno. I haven't been debugging any git-over-http stuff lately, so
it hasn't been bothering me. But I definitely have written perl
scripts to extract the data to a more readable format. Maybe it would
be easier if it had a few more knobs.
