Skip to content

Commit

Permalink
credential-store: ignore bogus lines from store file
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
carenas authored and gitster committed May 3, 2020
1 parent 20b4964 commit c03859a
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 3 deletions.
4 changes: 2 additions & 2 deletions credential-store.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
91 changes: 90 additions & 1 deletion t/t0302-credential-store.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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" &&
Expand All @@ -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

0 comments on commit c03859a

Please sign in to comment.