Anonymize URLs in error messages and warnings of git-remote-curl
cover Anonymize URLs in error messages and warnings of git-remote-curl
I have just noticed locally that certain errors (in my case, a stale entry in /etc/hosts) use the un-scrubbed URL in the error message when dying. Let's scrub them.
Johannes Schindelin (1):
curl: anonymize URLs in error messages and warnings
remote-curl.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)
base-commit: 8104ec994ea3849a968b4667d072fedd1e688642
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-156%2Fdscho%2Fmore-stripped-usernames-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-156/dscho/more-stripped-usernames-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/156
Signature · 1 line
gitgitgadget
PATCH 1/1 curl: anonymize URLs in error messages and warnings
From: Johannes Schindelin <[email protected]>
Just like 47abd85ba0 (fetch: Strip usernames from url's before storing them, 2009-04-17) and later 882d49ca5c (push: anonymize URL in status output, 2016-07-13), this change anonymizes URLs (read: strips them of user names and especially passwords) in user-facing error messages and warnings.
--- remote-curl.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-)
remote-curl.c +13 -6
@@ -16,6 +16,7 @@#include "send-pack.h" #include "protocol.h" #include "quote.h" +#include "transport.h" static struct remote *remote;/* always ends with a trailing slash */
@@ -251,7 +252,7 @@ static struct ref *parse_info_refs(struct discovery *heads)if (data[i] =='\n' ) {if (mid - start !=40 )die ("%sinfo/refs not valid: is this a git repository?" , - url.buf); +transport_anonymize_url (url.buf)); data[i] =0 ; ref_name = mid +1 ; ref =alloc_ref (ref_name);
@@ -442,17 +443,23 @@ static struct discovery *discover_refs(const char *service, int for_push)break ;case HTTP_MISSING_TARGET :show_http_message (&type, &charset, &buffer); -die ("repository '%s' not found" , url.buf); +die ("repository '%s' not found" , +transport_anonymize_url (url.buf));case HTTP_NOAUTH :show_http_message (&type, &charset, &buffer); -die ("Authentication failed for '%s'" , url.buf); +die ("Authentication failed for '%s'" , +transport_anonymize_url (url.buf));default :show_http_message (&type, &charset, &buffer); -die ("unable to access '%s': %s" , url.buf, curl_errorstr); +die ("unable to access '%s': %s" , +transport_anonymize_url (url.buf), curl_errorstr); } -if (options.verbosity && !starts_with (refs_url.buf, url.buf)) -warning (_ ("redirecting to %s" ), url.buf); +if (options.verbosity && !starts_with (refs_url.buf, url.buf)) { +char *u =transport_anonymize_url (url.buf); +warning (_ ("redirecting to %s" ), u); +free (u); + } last=xcalloc (1 ,sizeof (*last_discovery)); last->service =xstrdup (service);
Signature · 1 line
gitgitgadget
Re: [PATCH 1/1] curl: anonymize URLs in error messages and warnings
Quoted 7 lines · Johannes Schindelin via GitGitGadget
From: Johannes Schindelin <[email protected]>
Just like
47abd85ba0(fetch: Strip usernames from url's before storing them, 2009-04-17) and later882d49ca5c(push: anonymize URL in status output, 2016-07-13), this change anonymizes URLs (read: strips them of user names and especially passwords) in user-facing error messages and warnings.
Yeah, these all look obviously correct. I suspect most people haven't noticed them because it's pretty easy to use a real password helper these days instead of shoving it into the URL.
remote-curl.c static struct ref *parse_info_refs(struct discovery *heads) · line 255
Quoted 6 lines
remote-curl.c +1
-1@@ -251,7 +252,7 @@ static struct ref *parse_info_refs(struct discovery *heads)if (data[i] =='\n' ) {if (mid - start !=40 )die ("%sinfo/refs not valid: is this a git repository?" , - url.buf); +transport_anonymize_url (url.buf));
Looking at the code, I think we'd keep the trailing slash during the anonymization. Good.
remote-curl.c static struct discovery *discover_refs(const char *service, int for_push) · line 447
Quoted 7 lines
remote-curl.c +2
-1@@ -442,17 +443,23 @@ static struct discovery *discover_refs(const char *service, int for_push)break ;case HTTP_MISSING_TARGET :show_http_message (&type, &charset, &buffer); -die ("repository '%s' not found" , url.buf); +die ("repository '%s' not found" , +transport_anonymize_url (url.buf));
Sort of orthogonal to your patch, but these could probably stand to be marked for translation (I'm happy for that to come on top later, though).
remote-curl.c static struct discovery *discover_refs(const char *service, int for_push) · line 462
Quoted 7 lines
remote-curl.c +5
-2-if (options.verbosity && !starts_with (refs_url.buf, url.buf)) -warning (_ ("redirecting to %s" ), url.buf); +if (options.verbosity && !starts_with (refs_url.buf, url.buf)) { +char *u =transport_anonymize_url (url.buf); +warning (_ ("redirecting to %s" ), u); +free (u); + }
Good attention to detail here.
-Peff
Re: [PATCH 1/1] curl: anonymize URLs in error messages and warnings
Hi Peff,
Quoted 13 lines · Jeff King
remote-curl.c static struct discovery *discover_refs(const char *service, int for_push) · line 447
Quoted 7 lines · Johannes Schindelin via GitGitGadget
remote-curl.c +2
-1@@ -442,17 +443,23 @@ static struct discovery *discover_refs(const char *service, int for_push)break ;case HTTP_MISSING_TARGET :show_http_message (&type, &charset, &buffer); -die ("repository '%s' not found" , url.buf); +die ("repository '%s' not found" , +transport_anonymize_url (url.buf));Sort of orthogonal to your patch, but these could probably stand to be marked for translation (I'm happy for that to come on top later, though).
Good idea!
Ciao,
Dscho