Re: [PATCH] push: anonymize URLs in error messages and warnings
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 24, 2020, 20:38 UTC
- Message-ID
- <xmqqtv18bov7.fsf@gitster.c.googlers.com>
- In-Reply-To
- <pull.618.git.1587738008248.gitgitgadget@gmail.com>
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:
> builtin/push.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-)
Is this something we can protect with a test? perhaps like 882d49ca (push: anonymize URL in status output, 2016-07-13) did?
Show 9 quoted lines
> diff --git a/builtin/push.c b/builtin/push.c
> index 6dbf0f0bb71..bd2a2cbfbd7 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -340,6 +340,7 @@ static int push_with_options(struct transport *transport, struct refspec *rs,
> {
> int err;
> unsigned int reject_reasons;
> + char *anon_url = transport_anonymize_url(transport->url);Do we need to anonymize this early, way before we know we'd fail the push? It wouldn't be that transport->url is going to be munged before we realize that we have to issue an error message---otherwise you'd not be writing a patch to replace transport->url in the error message. So this placement of anonymize call sounds like taking the error path as the main flow of control.
In other words, would it break to squash the following change in?
builtin/push.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/push.c b/builtin/push.c index bd2a2cbfbd..b88948b07e 100644 --- a/builtin/push.c +++ b/builtin/push.c @@ -340,7 +340,6 @@ static int push_with_options(struct transport *transport, struct refspec *rs, { int err; unsigned int reject_reasons; - char *anon_url = transport_anonymize_url(transport->url); transport_set_verbosity(transport, verbosity, progress); transport->family = family; @@ -364,13 +363,14 @@ static int push_with_options(struct transport *transport, struct refspec *rs, rs, flags, &reject_reasons); trace2_region_leave("push", "transport_push", the_repository); if (err != 0) { + char *anon_url = transport_anonymize_url(transport->url); fprintf(stderr, "%s", push_get_color(PUSH_COLOR_ERROR)); error(_("failed to push some refs to '%s'"), anon_url); fprintf(stderr, "%s", push_get_color(PUSH_COLOR_RESET)); + free(anon_url); } err |= transport_disconnect(transport); - free(anon_url); if (!err) return 0;