Re: [RFC PATCH v6 2/2] credential-store: warn for any incomplete credentials instead of using
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 29, 2020, 21:12 UTC
- Message-ID
- <xmqqk11yuhca.fsf@gitster.c.googlers.com>
- In-Reply-To
- <20200429203546.56753-3-carenas@gmail.com>
Carlo Marcelo Arenas Belón <carenas@gmail.com> writes:
> originally any credential found was tried for matching as far as it had > a username and password, but that resulted in fatal errors as the rules > were harden.
harden -> hardened
> now that we have a way to report malformed credentials, use it to notify > the user when username/password was missing, instead of just silently > skipping.
Sorry, but isn't that what happend already in the previous step? What are you ordering the codebase (after applying the previous stpe) do further? It already is "using it to notify the user when username and/or password is missing".
> do the same for credentials that are missing host (or had one that is > empty) or that are missing a path (for supporting cert://) as well.
While the intention may be a good one ...
Show 21 quoted lines
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
> ---
> credential-store.c | 7 ++++---
> t/t0302-credential-store.sh | 38 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 42 insertions(+), 3 deletions(-)
>
> diff --git a/credential-store.c b/credential-store.c
> index 1cc5ca081a..53f77ff6f5 100644
> --- a/credential-store.c
> +++ b/credential-store.c
> @@ -26,9 +26,10 @@ static int parse_credential_file(const char *fn,
>
> while (strbuf_getline_lf(&line, fh) != EOF) {
> lineno++;
> - if (!credential_from_url_gently(&entry, line.buf, 1)) {
> - if (entry.username && entry.password &&
> - credential_match(c, &entry)) {
> + if (!credential_from_url_gently(&entry, line.buf, 1) &&
> + ((entry.host && *entry.host) || entry.path) &&
> + entry.username && entry.password) {
> + if (credential_match(c, &entry)) {... this makes the code even harder to follow than it already was after the previous step. In the previous step, at least it was clear that we'd catch *all* the well-formed/parseable input will come into the first if () {...} block, but with the extra logic, that is no longer true. Even a line that is well formed may be bypassed from matching and will be fed to "else" side.