Thank you very much. That's correct and ideal in my eyes. Regarding the comments, this is a new feature, of course. I think it's a worthless discussion about the question if the correct handling of empty lines are a bugfix or a new feature. In fact empty lines were handled correcty (ignored). So this might be considered a feature. But it wasn't documented, so it's a bugfix... Anyway. Thank you all. Dirk Am 27.04.20 um 14:59 schrieb Carlo Marcelo Arenas Belón: > with the added checks for invalid URLs in credentials, any locally > modified store files which might have empty lines or even comments > were reported[1] failing to parse as valid credentials. > > using the store file in this manner wasn't intended by the original > code and it had latent issues which the new code dutifully prevented > but since the strings used wouldn't had been valid credentials anyway > we could instead detect them and skip the matching logic and therefore > formalize this "feature". > > trim all lines as they are being read from the store file and skip the > ones that will be otherwise empty or that start with "#" (therefore > assuming them to be comments) > > [1] https://stackoverflow.com/a/61420852/5005936 > > Reported-by: Dirk <dirk@xxxxxxx> > Helped-by: Eric Sunshine <sunshine@xxxxxxxxxxxxxx> > Signed-off-by: Carlo Marcelo Arenas Belón <carenas@xxxxxxxxx> > --- > v3: > * avoid using q_to_cr as suggested by Peff > * a more verbose commit message and slightly more complete documentation > v2: > * use a here-doc for clarity as suggested by Eric > * improve commit message and include documentation > > Documentation/git-credential-store.txt | 7 +++++++ > credential-store.c | 3 +++ > t/t0302-credential-store.sh | 15 +++++++++++++++ > 3 files changed, 25 insertions(+) > > diff --git a/Documentation/git-credential-store.txt b/Documentation/git-credential-store.txt > index 693dd9d9d7..48ab4b13e5 100644 > --- a/Documentation/git-credential-store.txt > +++ b/Documentation/git-credential-store.txt > @@ -101,6 +101,13 @@ username (if we already have one) match, then the password is returned > to Git. See the discussion of configuration in linkgit:gitcredentials[7] > for more information. > > +Note that the file used is not a configuration file and should be ideally > +managed only through git, as any manually introduced typos will compromise > +the validation of credentials. > + > +The parser will ignore any lines starting with the '#' character during > +the processing of credentials for read, though. > + > GIT > --- > Part of the linkgit:git[1] suite > diff --git a/credential-store.c b/credential-store.c > index c010497cb2..b2f160890d 100644 > --- a/credential-store.c > +++ b/credential-store.c > @@ -24,6 +24,9 @@ static int parse_credential_file(const char *fn, > } > > while (strbuf_getline_lf(&line, fh) != EOF) { > + strbuf_trim(&line); > + if (line.len == 0 || *line.buf == '#') > + continue; > credential_from_url(&entry, line.buf); > if (entry.username && entry.password && > credential_match(c, &entry)) { > diff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh > index d6b54e8c65..5e6ace3a06 100755 > --- a/t/t0302-credential-store.sh > +++ b/t/t0302-credential-store.sh > @@ -120,4 +120,19 @@ test_expect_success 'erase: erase matching credentials from both xdg and home fi > test_must_be_empty "$HOME/.config/git/credentials" > ' > > +test_expect_success 'get: allow for empty lines or comments in store file' ' > + test_write_lines "#comment" " " "" \ > + https://user:pass@xxxxxxxxxxx >"$HOME/.git-credentials" && > + check fill store <<-\EOF > + protocol=https > + host=example.com > + -- > + protocol=https > + host=example.com > + username=user > + password=pass > + -- > + EOF > +' > + > test_done
