Skip to content

Commit c03859a

Browse files
carenasgitster
authored andcommitted
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 <[email protected]> Helped-by: Eric Sunshine <[email protected]> Helped-by: Junio C Hamano <[email protected]> Based-on-patch-by: Jonathan Nieder <[email protected]> Signed-off-by: Carlo Marcelo Arenas Belón <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
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:[email protected]" >"$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://[email protected]'\'':
136+
--
137+
EOF
138+
'
139+
}
140+
141+
invalid_credential_test "scheme" ://user:[email protected]
142+
invalid_credential_test "valid host/path" https://user:pass@
143+
invalid_credential_test "username/password" https://[email protected]
144+
145+
test_expect_success 'get: credentials with DOS line endings are invalid' '
146+
printf "https://user:[email protected]\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://[email protected]'\'':
158+
--
159+
EOF
160+
'
161+
162+
test_expect_success 'get: credentials with path and DOS line endings are valid' '
163+
printf "https://user:[email protected]/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:[email protected]/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://[email protected]/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:[email protected]
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)