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
