credential-store: ignore bogus lines from store file

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.

Instead of doing a hard check for credentials, do a soft one and
therefore avoid the reported fatal error.

While at it add tests for all known corruptions that are currently
ignored to keep track of them and avoid the risk of regressions.

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

Reported-by: Dirk <dirk@ed4u.de>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Based-on-patch-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/credential-store.c b/credential-store.c
index ac295420dd0d03d1b31922f9b7c16a83e987e6a3..68c02fb5d9b90c15a442c65c2df18c9a7407a93f 100644 (file)
--- a/credential-store.c
+++ b/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) {

diff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh
index d6b54e8c65a3ec4408fe07ee76a6d8bed957207a..716bf1af9fd04e64b7d5c03570b5ba74a8529d46 100755 (executable)
--- a/t/t0302-credential-store.sh
+++ b/t/t0302-credential-store.sh
@@ -107,7 +107,6 @@ test_expect_success 'store: if both xdg and home files exist, only store in home
        test_must_be_empty "$HOME/.config/git/credentials"
 '
 
-
 test_expect_success 'erase: erase matching credentials from both xdg and home files' '
        echo "https://home-user:home-pass@example.com" >"$HOME/.git-credentials" &&
        mkdir -p "$HOME/.config/git" &&
@@ -120,4 +119,94 @@ test_expect_success 'erase: erase matching credentials from both xdg and home fi
        test_must_be_empty "$HOME/.config/git/credentials"
 '
 
+invalid_credential_test() {
+       test_expect_success "get: ignore credentials without $1 as invalid" '
+               echo "$2" >"$HOME/.git-credentials" &&
+               check fill store <<-\EOF
+               protocol=https
+               host=example.com
+               --
+               protocol=https
+               host=example.com
+               username=askpass-username
+               password=askpass-password
+               --
+               askpass: Username for '\''https://example.com'\'':
+               askpass: Password for '\''https://askpass-username@example.com'\'':
+               --
+               EOF
+       '
+}
+
+invalid_credential_test "scheme" ://user:pass@example.com
+invalid_credential_test "valid host/path" https://user:pass@
+invalid_credential_test "username/password" https://pass@example.com
+
+test_expect_success 'get: credentials with DOS line endings are invalid' '
+       printf "https://user:pass@example.com\r\n" >"$HOME/.git-credentials" &&
+       check fill store <<-\EOF
+       protocol=https
+       host=example.com
+       --
+       protocol=https
+       host=example.com
+       username=askpass-username
+       password=askpass-password
+       --
+       askpass: Username for '\''https://example.com'\'':
+       askpass: Password for '\''https://askpass-username@example.com'\'':
+       --
+       EOF
+'
+
+test_expect_success 'get: credentials with path and DOS line endings are valid' '
+       printf "https://user:pass@example.com/repo.git\r\n" >"$HOME/.git-credentials" &&
+       check fill store <<-\EOF
+       url=https://example.com/repo.git
+       --
+       protocol=https
+       host=example.com
+       username=user
+       password=pass
+       --
+       EOF
+'
+
+test_expect_success 'get: credentials with DOS line endings are invalid if path is relevant' '
+       printf "https://user:pass@example.com/repo.git\r\n" >"$HOME/.git-credentials" &&
+       test_config credential.useHttpPath true &&
+       check fill store <<-\EOF
+       url=https://example.com/repo.git
+       --
+       protocol=https
+       host=example.com
+       path=repo.git
+       username=askpass-username
+       password=askpass-password
+       --
+       askpass: Username for '\''https://example.com/repo.git'\'':
+       askpass: Password for '\''https://askpass-username@example.com/repo.git'\'':
+       --
+       EOF
+'
+
+test_expect_success 'get: store file can contain empty/bogus lines' '
+       echo "" >"$HOME/.git-credentials" &&
+       q_to_tab <<-\CREDENTIAL >>"$HOME/.git-credentials" &&
+       #comment
+       Q
+       https://user:pass@example.com
+       CREDENTIAL
+       check fill store <<-\EOF
+       protocol=https
+       host=example.com
+       --
+       protocol=https
+       host=example.com
+       username=user
+       password=pass
+       --
+       EOF
+'
+
 test_done