Re: [PATCH v3] git-credential-store: skip empty lines and comments from store
- From
Carlo Marcelo Arenas Belón <carenas@gmail.com>
- Date
- Apr 28, 2020, 07:18 UTC
- Message-ID
- <20200428071802.GF61348@Carlos-MBP>
- In-Reply-To
- <20200428054155.GB2376380@coredump.intra.peff.net>
On Tue, Apr 28, 2020 at 01:41:55AM -0400, Jeff King wrote:
Show 28 quoted lines
> 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