{"thread":{"id":"50643","subject":"[PATCH 0/1] Anonymize URLs in error messages and warnings of git-remote-curl","startedAt":"2019-03-04T15:33:49Z","lastAt":"2019-03-05T22:55:43Z","messageCount":4,"participants":["Johannes Schindelin via GitGitGadget","Jeff King","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"370611","messageId":"pull.156.git.gitgitgadget@gmail.com","threadId":"50643","inReplyTo":null,"subject":"[PATCH 0/1] Anonymize URLs in error messages and warnings of git-remote-curl","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-03-04T15:33:45Z","receivedAt":"2019-03-04T15:33:49Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"I have just noticed locally that certain errors (in my case, a stale entry\nin /etc/hosts) use the un-scrubbed URL in the error message when dying.\nLet's scrub them.\n\nJohannes Schindelin (1):\n curl: anonymize URLs in error messages and warnings\n\n remote-curl.c | 19 +++++++++++++------\n 1 file changed, 13 insertions(+), 6 deletions(-)\n\n\nbase-commit: 8104ec994ea3849a968b4667d072fedd1e688642\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-156%2Fdscho%2Fmore-stripped-usernames-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-156/dscho/more-stripped-usernames-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/156\n-- \ngitgitgadget\n"},{"id":"370612","messageId":"ab6f4032cb8cbbe77dbfad4660c30acd7ba688bb.1551713624.git.gitgitgadget@gmail.com","threadId":"50643","inReplyTo":"pull.156.git.gitgitgadget@gmail.com","subject":"[PATCH 1/1] curl: anonymize URLs in error messages and warnings","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-03-04T15:33:46Z","receivedAt":"2019-03-04T15:33:50Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nJust like 47abd85ba0 (fetch: Strip usernames from url's before storing\nthem, 2009-04-17) and later 882d49ca5c (push: anonymize URL in status\noutput, 2016-07-13), this change anonymizes URLs (read: strips them of\nuser names and especially passwords) in user-facing error messages and\nwarnings.\n\nSigned-off-by: Johannes Schindelin \n---\n remote-curl.c | 19 +++++++++++++------\n 1 file changed, 13 insertions(+), 6 deletions(-)\n\ndiff --git a/remote-curl.c b/remote-curl.c\nindex bb7421023b..e32359034d 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -16,6 +16,7 @@\n #include \"send-pack.h\"\n #include \"protocol.h\"\n #include \"quote.h\"\n+#include \"transport.h\"\n \n static struct remote *remote;\n /* always ends with a trailing slash */\n@@ -251,7 +252,7 @@ static struct ref *parse_info_refs(struct discovery *heads)\n \t\tif (data[i] == '\\n') {\n \t\t\tif (mid - start != 40)\n \t\t\t\tdie(\"%sinfo/refs not valid: is this a git repository?\",\n-\t\t\t\t url.buf);\n+\t\t\t\t transport_anonymize_url(url.buf));\n \t\t\tdata[i] = 0;\n \t\t\tref_name = mid + 1;\n \t\t\tref = alloc_ref(ref_name);\n@@ -442,17 +443,23 @@ static struct discovery *discover_refs(const char *service, int for_push)\n \t\tbreak;\n \tcase HTTP_MISSING_TARGET:\n \t\tshow_http_message(&type, &charset, &buffer);\n-\t\tdie(\"repository '%s' not found\", url.buf);\n+\t\tdie(\"repository '%s' not found\",\n+\t\t transport_anonymize_url(url.buf));\n \tcase HTTP_NOAUTH:\n \t\tshow_http_message(&type, &charset, &buffer);\n-\t\tdie(\"Authentication failed for '%s'\", url.buf);\n+\t\tdie(\"Authentication failed for '%s'\",\n+\t\t transport_anonymize_url(url.buf));\n \tdefault:\n \t\tshow_http_message(&type, &charset, &buffer);\n-\t\tdie(\"unable to access '%s': %s\", url.buf, curl_errorstr);\n+\t\tdie(\"unable to access '%s': %s\",\n+\t\t transport_anonymize_url(url.buf), curl_errorstr);\n \t}\n \n-\tif (options.verbosity && !starts_with(refs_url.buf, url.buf))\n-\t\twarning(_(\"redirecting to %s\"), url.buf);\n+\tif (options.verbosity && !starts_with(refs_url.buf, url.buf)) {\n+\t\tchar *u = transport_anonymize_url(url.buf);\n+\t\twarning(_(\"redirecting to %s\"), u);\n+\t\tfree(u);\n+\t}\n \n \tlast= xcalloc(1, sizeof(*last_discovery));\n \tlast->service = xstrdup(service);\n-- \ngitgitgadget\n"},{"id":"370668","messageId":"20190305051126.GL19800@sigill.intra.peff.net","threadId":"50643","inReplyTo":"ab6f4032cb8cbbe77dbfad4660c30acd7ba688bb.1551713624.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] curl: anonymize URLs in error messages and warnings","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-03-05T05:11:27Z","receivedAt":"2019-03-05T05:11:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 04, 2019 at 07:33:46AM -0800, Johannes Schindelin via GitGitGadget wrote:\n\n> From: Johannes Schindelin \n> \n> Just like 47abd85ba0 (fetch: Strip usernames from url's before storing\n> them, 2009-04-17) and later 882d49ca5c (push: anonymize URL in status\n> output, 2016-07-13), this change anonymizes URLs (read: strips them of\n> user names and especially passwords) in user-facing error messages and\n> warnings.\n\nYeah, these all look obviously correct. I suspect most people haven't\nnoticed them because it's pretty easy to use a real password helper\nthese days instead of shoving it into the URL.\n\n> @@ -251,7 +252,7 @@ static struct ref *parse_info_refs(struct discovery *heads)\n> \t\tif (data[i] == '\\n') {\n> \t\t\tif (mid - start != 40)\n> \t\t\t\tdie(\"%sinfo/refs not valid: is this a git repository?\",\n> -\t\t\t\t url.buf);\n> +\t\t\t\t transport_anonymize_url(url.buf));\n\nLooking at the code, I think we'd keep the trailing slash during the\nanonymization. Good.\n\n> @@ -442,17 +443,23 @@ static struct discovery *discover_refs(const char *service, int for_push)\n> \t\tbreak;\n> \tcase HTTP_MISSING_TARGET:\n> \t\tshow_http_message(&type, &charset, &buffer);\n> -\t\tdie(\"repository '%s' not found\", url.buf);\n> +\t\tdie(\"repository '%s' not found\",\n> +\t\t transport_anonymize_url(url.buf));\n\nSort of orthogonal to your patch, but these could probably stand to be\nmarked for translation (I'm happy for that to come on top later,\nthough).\n\n> -\tif (options.verbosity && !starts_with(refs_url.buf, url.buf))\n> -\t\twarning(_(\"redirecting to %s\"), url.buf);\n> +\tif (options.verbosity && !starts_with(refs_url.buf, url.buf)) {\n> +\t\tchar *u = transport_anonymize_url(url.buf);\n> +\t\twarning(_(\"redirecting to %s\"), u);\n> +\t\tfree(u);\n> +\t}\n\nGood attention to detail here.\n\n-Peff\n"},{"id":"370758","messageId":"nycvar.QRO.7.76.6.1903051709330.41@tvgsbejvaqbjf.bet","threadId":"50643","inReplyTo":"20190305051126.GL19800@sigill.intra.peff.net","subject":"Re: [PATCH 1/1] curl: anonymize URLs in error messages and warnings","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-03-05T22:55:02Z","receivedAt":"2019-03-05T22:55:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Tue, 5 Mar 2019, Jeff King wrote:\n\n> On Mon, Mar 04, 2019 at 07:33:46AM -0800, Johannes Schindelin via GitGitGadget wrote:\n> \n> > @@ -442,17 +443,23 @@ static struct discovery *discover_refs(const char *service, int for_push)\n> > \t\tbreak;\n> > \tcase HTTP_MISSING_TARGET:\n> > \t\tshow_http_message(&type, &charset, &buffer);\n> > -\t\tdie(\"repository '%s' not found\", url.buf);\n> > +\t\tdie(\"repository '%s' not found\",\n> > +\t\t transport_anonymize_url(url.buf));\n> \n> Sort of orthogonal to your patch, but these could probably stand to be\n> marked for translation (I'm happy for that to come on top later,\n> though).\n\nGood idea!\n\nCiao,\nDscho\n"}]}