Skip to content

Commit c03859a

Browse files
carenasgitster
carenas
authored and
gitster
committed
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>
1 parent 20b4964 commit c03859a

File tree

2 files changed

+92
-3
lines changed

2 files changed

+92
-3
lines changed

‎credential-store.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ static int parse_credential_file(const char *fn,
2424
}
2525

2626
while (strbuf_getline_lf(&line, fh) != EOF) {
27-
credential_from_url(&entry, line.buf);
28-
if (entry.username && entry.password &&
27+
if (!credential_from_url_gently(&entry, line.buf, 1) &&
28+
entry.username && entry.password &&
2929
credential_match(c, &entry)) {
3030
found_credential = 1;
3131
if (match_cb) {

‎t/t0302-credential-store.sh‎

Lines changed: 90 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,6 @@ test_expect_success 'store: if both xdg and home files exist, only store in home
107107
test_must_be_empty "$HOME/.config/git/credentials"
108108
'
109109

110-
111110
test_expect_success 'erase: erase matching credentials from both xdg and home files' '
112111
echo "https://home-user:home-pass@example.com" >"$HOME/.git-credentials" &&
113112
mkdir -p "$HOME/.config/git" &&
@@ -120,4 +119,94 @@ test_expect_success 'erase: erase matching credentials from both xdg and home fi
120119
test_must_be_empty "$HOME/.config/git/credentials"
121120
'
122121

122+
invalid_credential_test() {
123+
test_expect_success "get: ignore credentials without $1 as invalid" '
124+
echo "$2" >"$HOME/.git-credentials" &&
125+
check fill store <<-\EOF
126+
protocol=https
127+
host=example.com
128+
--
129+
protocol=https
130+
host=example.com
131+
username=askpass-username
132+
password=askpass-password
133+
--
134+
askpass: Username for '\''https://example.com'\'':
135+
askpass: Password for '\''https://askpass-username@example.com'\'':
136+
--
137+
EOF
138+
'
139+
}
140+
141+
invalid_credential_test "scheme" ://user:pass@example.com
142+
invalid_credential_test "valid host/path" https://user:pass@
143+
invalid_credential_test "username/password" https://pass@example.com
144+
145+
test_expect_success 'get: credentials with DOS line endings are invalid' '
146+
printf "https://user:pass@example.com\r\n" >"$HOME/.git-credentials" &&
147+
check fill store <<-\EOF
148+
protocol=https
149+
host=example.com
150+
--
151+
protocol=https
152+
host=example.com
153+
username=askpass-username
154+
password=askpass-password
155+
--
156+
askpass: Username for '\''https://example.com'\'':
157+
askpass: Password for '\''https://askpass-username@example.com'\'':
158+
--
159+
EOF
160+
'
161+
162+
test_expect_success 'get: credentials with path and DOS line endings are valid' '
163+
printf "https://user:pass@example.com/repo.git\r\n" >"$HOME/.git-credentials" &&
164+
check fill store <<-\EOF
165+
url=https://example.com/repo.git
166+
--
167+
protocol=https
168+
host=example.com
169+
username=user
170+
password=pass
171+
--
172+
EOF
173+
'
174+
175+
test_expect_success 'get: credentials with DOS line endings are invalid if path is relevant' '
176+
printf "https://user:pass@example.com/repo.git\r\n" >"$HOME/.git-credentials" &&
177+
test_config credential.useHttpPath true &&
178+
check fill store <<-\EOF
179+
url=https://example.com/repo.git
180+
--
181+
protocol=https
182+
host=example.com
183+
path=repo.git
184+
username=askpass-username
185+
password=askpass-password
186+
--
187+
askpass: Username for '\''https://example.com/repo.git'\'':
188+
askpass: Password for '\''https://askpass-username@example.com/repo.git'\'':
189+
--
190+
EOF
191+
'
192+
193+
test_expect_success 'get: store file can contain empty/bogus lines' '
194+
echo "" >"$HOME/.git-credentials" &&
195+
q_to_tab <<-\CREDENTIAL >>"$HOME/.git-credentials" &&
196+
#comment
197+
Q
198+
https://user:pass@example.com
199+
CREDENTIAL
200+
check fill store <<-\EOF
201+
protocol=https
202+
host=example.com
203+
--
204+
protocol=https
205+
host=example.com
206+
username=user
207+
password=pass
208+
--
209+
EOF
210+
'
211+
123212
test_done

0 commit comments

Comments
 (0)