git-credential-store: skip empty lines and comments from store
diff mbox series

Message ID 20200426234750.40418-1-carenas@gmail.com
State New
Headers show
Series
  • git-credential-store: skip empty lines and comments from store
Related show

Commit Message

Carlo Marcelo Arenas Belón April 26, 2020, 11:47 p.m. UTC
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(+)

Comments

Eric Sunshine April 27, 2020, 12:19 a.m. UTC | #1
[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/
Carlo Marcelo Arenas Belón April 27, 2020, 12:46 a.m. UTC | #2
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

Patch
diff mbox series

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