All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Jonathan Tan <jonathantanmy@google.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 2/2] http, imap-send: stop using CURLOPT_VERBOSE
Date: Tue, 12 May 2020 15:16:10 -0400	[thread overview]
Message-ID: <20200512191610.GB54565@coredump.intra.peff.net> (raw)
In-Reply-To: <1df9e9deb7831b32694ea453759bf5d21952e165.1589218693.git.jonathantanmy@google.com>

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.

  reply	other threads:[~2020-05-12 19:16 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-11 17:43 [PATCH 0/2] Safer GIT_CURL_VERBOSE Jonathan Tan
2020-05-11 17:43 ` [PATCH 1/2] t5551: test that GIT_TRACE_CURL redacts password Jonathan Tan
2020-05-12 19:08   ` Jeff King
2020-05-11 17:43 ` [PATCH 2/2] http, imap-send: stop using CURLOPT_VERBOSE Jonathan Tan
2020-05-12 19:16   ` Jeff King [this message]
2020-05-12 19:23     ` Jonathan Tan
2020-05-12 19:27       ` Jeff King
2020-05-12 23:13   ` brian m. carlson
2020-05-13  0:10     ` Junio C Hamano
2020-05-13  4:50       ` Jeff King
2020-05-13  5:05         ` Junio C Hamano
2020-05-13  6:16     ` Daniel Stenberg
2020-05-13 14:45       ` Jeff King
2020-05-13 19:12 ` [PATCH v2 0/3] Safer GIT_CURL_VERBOSE Jonathan Tan
2020-05-13 19:12   ` [PATCH v2 1/3] t5551: test that GIT_TRACE_CURL redacts password Jonathan Tan
2020-05-13 19:12   ` [PATCH v2 2/3] http: make GIT_TRACE_CURL auth redaction optional Jonathan Tan
2020-05-13 19:29     ` Junio C Hamano
2020-05-13 19:12   ` [PATCH v2 3/3] http, imap-send: stop using CURLOPT_VERBOSE Jonathan Tan
2020-05-13 19:27   ` [PATCH v2 0/3] Safer GIT_CURL_VERBOSE Junio C Hamano
2020-05-13 19:33     ` Junio C Hamano
2020-05-15 20:47       ` Jeff King

Reply instructions:

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200512191610.GB54565@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=jonathantanmy@google.com \
    --subject='Re: [PATCH 2/2] http, imap-send: stop using CURLOPT_VERBOSE' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.