git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Elia Pinto <gitter.spiros@gmail.com>
To: git@vger.kernel.org, tboegi@web.de, ramsay@ramsayjones.plus.com,
	gitster@pobox.com, sunshine@sunshineco.com, peff@peff.net
Cc: Elia Pinto <gitter.spiros@gmail.com>
Subject: [PATCH v7 0/2] Implement the GIT_TRACE_CURL environment variable
Date: Mon, 23 May 2016 13:44:01 +0000	[thread overview]
Message-ID: <20160523134403.43880-1-gitter.spiros@gmail.com> (raw)

This is the  seventh version but in reality is the complete rewriting of the patches discussed here
(here called V1)

$gmane/290520
$gmane/290521

*Changes from V6
($gmane/295158)
- updated with junio suggestions ($gmane/295207)
  * http.c: add redact_sensitive_header() helper
  * http.c: renamed  header_list and ptr_list to
    headers and header
  * http.c: reduce width in curl_dump_data
  * http.c: curl_dump_data. cut strbuf_addch at a sensible place
  * http.c: drop the nopriv_header variable: add an enum instead


*Changes from V5
($gmane/293236)

- don't export curl_trace anymore. Define it static
- fix a minor cleanup (style) in setup_curl_trace
- and, finally, i rewrote completely curl_dump, separating it
into two functions (one for http header and one for the http data),
hoping for a coherent implementation with curl --ascii-trace output
but easier to read than the previous implementation that
used a hard-to-read double-loop.

 
*Changes from V4
($gmane/292867)

-  add a better abstraction with the routine setup_curl_trace
-  curl_dump : drop the noex parameter, define nopriv boolean as int
-  use decimal constant where appropiate
-  fix multi-line comment
-  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.
   We add also the (previously missing) proxy-authorization case
-  curl_dump: fix strbuf memory leak

as suggested by Jeff King
($gmane/292891)
($gmane/292892)

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)

Of course if the maintainer (or other) believes it is really necessary
to rewrite the above code to accept the patches i will do.

*Changes from V3
($gmane/292040)

- add missing static to curl_dump
- reorder the patch order
- tried to fix all (but i am not sure) the problems reported by Julio ($gmane/292055)
- * squash the documentation with the http.c commit.
  * in the trace prefix each line to indicate it is about sending a header, and drop the
    initial hex count
  * auto-censor Authorization headers by default

    as suggested by Jeff King ($gmane/292074)

*Changes from V2
($gmane/291868)

- fix garbage comment in http.c (i am very sorry for that)
- add final '.' to the commit message for imap-send.c and to other commit messages
- typofix double ; in http.c
- merge the nice cleanup and code refactoring of Ramsay Jones (Thank you very much !!)
- squash the previous commit 2/4

*Changes from V1

- introduced GIT_TRACE_CURL variable with its documentation
- changed the name of the temporary variable "i" in "w" in the helper routine
- used the c escape sequences instead of the hex equivalent
- dropped the previous GIT_DEBUG_CURL env var
- curl_dump and curl_trace factored out to a shared implementation
in http.c



Elia Pinto (2):
  http.c: implement the GIT_TRACE_CURL environment variable
  imap-send.c: introduce the GIT_TRACE_CURL enviroment variable

 Documentation/git.txt |   8 ++++
 http.c                | 125 +++++++++++++++++++++++++++++++++++++++++++++++++-
 http.h                |   2 +
 imap-send.c           |   1 +
 4 files changed, 134 insertions(+), 2 deletions(-)

-- 
2.8.3.614.g2da74fb.dirty

             reply	other threads:[~2016-05-23 13:44 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-23 13:44 Elia Pinto [this message]
2016-05-23 13:44 ` [PATCH v7 1/2] http.c: implement the GIT_TRACE_CURL environment variable Elia Pinto
2016-05-25  0:50   ` Junio C Hamano
2016-05-23 13:44 ` [PATCH v7 2/2] imap-send.c: introduce the GIT_TRACE_CURL enviroment variable Elia Pinto

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20160523134403.43880-1-gitter.spiros@gmail.com \
    --to=gitter.spiros@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=ramsay@ramsayjones.plus.com \
    --cc=sunshine@sunshineco.com \
    --cc=tboegi@web.de \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).