Re: [RFC PATCH v6 2/2] credential-store: warn for any incomplete credentials instead of using

[Junio C Hamano <gitster@pobox.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 ...


> 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.