Subject: Re: [PATCH v3] git-credential-store: skip empty lines and comments from store

From: Carlo Marcelo Arenas Belón <[email-masked]>
Date: Tue, 28 Apr 2020 00:18:02 -0700

On Tue, Apr 28, 2020 at 01:41:55AM -0400, Jeff King wrote:
> On Mon, Apr 27, 2020 at 10:25:10PM -0700, Jonathan Nieder wrote:
> 
> > I wonder if in addition to the above documentation change we may want
> > something guaranteed to catch all cases where people would have
> > experienced a regression, like
> > 
> > diff --git i/credential-store.c w/credential-store.c
> > index c010497cb21..294e7716815 100644
> > --- i/credential-store.c
> > +++ w/credential-store.c
> > @@ -24,8 +24,8 @@ static int parse_credential_file(const char *fn,
> >  	}
> >  
> >  	while (strbuf_getline_lf(&line, fh) != EOF) {
> > -		credential_from_url(&entry, line.buf);
> > -		if (entry.username && entry.password &&
> > +		if (!credential_from_url_gently(&entry, line.buf, 1) &&
> > +		    entry.username && entry.password &&
> >  		    credential_match(c, &entry)) {
> >  			found_credential = 1;
> >  			if (match_cb) {
> > 
> > And then we can tighten the handling of unrecognized lines to first
> > warn and then error out, as a controlled change that doesn't lead
> > people to regret updating git.
> 
> I like that solution, as it mostly brings us back to the original
> behavior, as weird or unexpected as it was.

I like this version better as well, and we could even reuse my test
case.

it wouldn't cover cases where there were leading spaces/tabs around
the credential which I have to admit I liked just because it is
more robust to bad input, and there is no sane way now to tell the
user that there is invalid data anyway, but I am ok eitherway.

Carlo