| Message ID | 20200426234750.40418-1-carenas@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | git-credential-store: skip empty lines and comments from store | expand |
[Cc:+Dirk -- so he knows his bug report[1] wasn't ignored] On Sun, Apr 26, 2020 at 7:48 PM Carlo Marcelo Arenas Belón <carenas@gmail.com> wrote: > with the added checks for invalid URLs in credentials, any locally > modified store files which might have empty lines or even comments > were reported as failing[1] to parse as valid credentials. > > instead of passing every line to the matcher as read, trim them > from spaces and skip the ones that will be otherwise empty or > start with "#" (assumed to be comments) > > Reported-by: Dirk <dirk@ed4u.de> > Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> > --- > diff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh > @@ -120,4 +120,21 @@ test_expect_success 'erase: erase matching credentials from both xdg and home fi > +test_expect_success 'get: allow for empty lines or comments in store file' ' > + echo "#this is a comment" >"$HOME/.git-credentials" && > + echo "" >>"$HOME/.git-credentials" && > + echo "https://user:pass@example.com" >>"$HOME/.git-credentials" && > + echo " " >>"$HOME/.git-credentials" && Is there a reason you don't use a here-doc for the above (which would be less noisy)? For instance: q_to_tab >"$HOME/.git-credentials" <<-\EOF && #this is a comment https://user:pass@example.com Q EOF [1]: https://lore.kernel.org/git/ad80aa0d-3a35-6d7e-7958-b3520e16c855@ed4u.de/
On Sun, Apr 26, 2020 at 08:19:35PM -0400, Eric Sunshine wrote: > [Cc:+Dirk -- so he knows his bug report[1] wasn't ignored] > On Sun, Apr 26, 2020 at 7:48 PM Carlo Marcelo Arenas Belón > <carenas@gmail.com> wrote: > > diff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh > > @@ -120,4 +120,21 @@ test_expect_success 'erase: erase matching credentials from both xdg and home fi > > +test_expect_success 'get: allow for empty lines or comments in store file' ' > > + echo "#this is a comment" >"$HOME/.git-credentials" && > > + echo "" >>"$HOME/.git-credentials" && > > + echo "https://user:pass@example.com" >>"$HOME/.git-credentials" && > > + echo " " >>"$HOME/.git-credentials" && > > Is there a reason you don't use a here-doc for the above (which would > be less noisy)? For instance: because I didn't knew it existed ;), thanks for your help and will include for next version. Carlo
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..7245d2f449 100755 --- a/t/t0302-credential-store.sh +++ b/t/t0302-credential-store.sh @@ -120,4 +120,21 @@ 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' ' + echo "#this is a comment" >"$HOME/.git-credentials" && + echo "" >>"$HOME/.git-credentials" && + echo "https://user:pass@example.com" >>"$HOME/.git-credentials" && + echo " " >>"$HOME/.git-credentials" && + check fill store <<-\EOF + protocol=https + host=example.com + -- + protocol=https + host=example.com + username=user + password=pass + -- + EOF +' + test_done
with the added checks for invalid URLs in credentials, any locally modified store files which might have empty lines or even comments were reported as failing[1] to parse as valid credentials. instead of passing every line to the matcher as read, trim them from spaces and skip the ones that will be otherwise empty or start with "#" (assumed to be comments) [1] https://stackoverflow.com/a/61420852/5005936 Reported-by: Dirk <dirk@ed4u.de> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> --- credential-store.c | 3 +++ t/t0302-credential-store.sh | 17 +++++++++++++++++ 2 files changed, 20 insertions(+)