diff mbox series

[v4,2/4] git-credential-store: skip empty lines and comments from store

Message ID 20200428105254.28658-2-carenas@gmail.com (mailing list archive)
State New, archived
Headers show
Series credential-store: prevent fatal errors | expand

Commit Message

Carlo Marcelo Arenas Belón April 28, 2020, 10:52 a.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[1] failing to parse as valid credentials.

add corresponding failing cases

[1] https://stackoverflow.com/a/61420852/5005936

Reported-by: Dirk <dirk@ed4u.de>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
 t/t0302-credential-store.sh | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

Eric Sunshine April 28, 2020, 4:09 p.m. UTC | #1
On Tue, Apr 28, 2020 at 6:53 AM Carlo Marcelo Arenas Belón
<carenas@gmail.com> wrote:
> git-credential-store: skip empty lines and comments from store

I don't see anything in this patch which makes it skip anything at
all; it only introduces a new test.

> 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.
>
> add corresponding failing cases
>
> [1] https://stackoverflow.com/a/61420852/5005936
>
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
Carlo Marcelo Arenas Belón April 28, 2020, 4:42 p.m. UTC | #2
On Tue, Apr 28, 2020 at 12:09:50PM -0400, Eric Sunshine wrote:
> On Tue, Apr 28, 2020 at 6:53 AM Carlo Marcelo Arenas Belón
> <carenas@gmail.com> wrote:
> > git-credential-store: skip empty lines and comments from store
> 
> I don't see anything in this patch which makes it skip anything at
> all; it only introduces a new test.

my bad, forgot to fix the subject when rerolling; does something like:
credential-store: show "regression" when bogus credentials in file
make sense for the reroll?

Carlo
diff mbox series

Patch

diff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh
index d6b54e8c65..94cdcb9e56 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_failure 'get: store file can contain empty/bogus lines' '
+	test_write_lines "#comment" " " "" \
+		 https://user:pass@example.com >"$HOME/.git-credentials" &&
+	check fill store <<-\EOF
+	protocol=https
+	host=example.com
+	--
+	protocol=https
+	host=example.com
+	username=user
+	password=pass
+	--
+	EOF
+'
+
 test_done