Re: [PATCH v5] credential-store: warn instead of fatal for bogus lines from store

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 29, 2020, 04:36 UTC
Message-ID: <xmqqd07qzz67.fsf@gitster.c.googlers.com>
In-Reply-To: <20200429003303.93583-1-carenas@gmail.com>

Carlo Marcelo Arenas Belón  <carenas@gmail.com> writes:

Show 11 quoted lines

> +static char *redact_credential(const struct strbuf *line)
> +{
> +	struct strbuf redacted_line = STRBUF_INIT;
> +	char *at = strchr(line->buf, '@');
> +	char *colon;
> +	int redacted = 0;
> +
> +	if (at) {
> +		strbuf_addf(&redacted_line, "%.*s",
> +			(int)(at - line->buf), line->buf);
> +		colon = strrchr(redacted_line.buf, ':');

Just showing my ignorance, but ...

 - Is the above strrchr() that forbids a colon in the password
   intended, or should it be strchr() that only forbids a colon in
   the username instead?

 - Would it hurt to redact both username and password as sensitive?
   If not, it would certainly make it simpler to unconditionally:

                int i;
                for (i = 0; i < redacted_line.len; i++) {
                        if (redacted_line.buf[i] != ':')
                                redacted_line.buf[i] = 'x';
                }