Re: [PATCHv5 0/2] Implement the GIT_TRACE_CURL environment variable

[Jeff King <peff@peff.net>]

On Mon, May 02, 2016 at 02:28:11PM +0000, Elia Pinto wrote:

> -  redo the authorization header skip with a replace of possible sensitive data. 
>    We prefer to print only:
>        09:00:53.238330 http.c:534              => Send header: Authorization:  <redacted>
>    intested of 
>        09:00:53.238330 http.c:534              => Send header: Authorization:  Basic(o other scheme) <redacted>
>    as it was done in the original proposed suggestion by Jeff King. 
>    This is because i think it's better not to print even the authorization scheme.

I'm not sure I agree. If you're debugging curl's auth selection, that's
omitting an important piece of data. And unlike the actual credential, I
don't think it's particularly secret (and in many cases can be deduced
from the "WWW-Authenticate" header the server sends, coupled with curl's
code).

>    We add also the (previously missing) proxy-authorization case

Good catch.

> In this series i keep the original curl_dump parsing code, even though it is 
> objectively difficult to read. This is because the same code is used internally by curl 
> to do "ascii-trace" and is also reported in the libcurl code examples and test. 
> I think this may make maintenance of code easier in the future (libcurl 
> new dev, new features and so on) 

I don't agree with this line of reasoning. The code in question is
purely about how we format the output buffer, not about parsing what
curl gives us. We _should_ be diverging if we prefer a different output
format. And I don't think it's a question just of readability (though I
do agree the existing one is hard to read); it also foils the redaction
of the authorization header.

-Peff