Repository navigation
Conversation
git-credential store treats only LF as an EOL character, but reads the CR in a CR terminated line as part of the entry. Since the entries are URLs, CR can never be a valid character in an entry. [1] While we don't encourage users to manually edit the credential file, the format is simple enough and documented well enough that users do sometimes do that. Most editors on Windows produce CRLF as EOL characters. Users currently need to be careful when manually editing the credential file on Windows to not add stray CR characters into the file. We have tests around when we consider entries with CRLF valid and not valid, but they only seem to test the known quirks of the existing implementation, not some expectations founded in some systemic design decisions. Since the entries can never validly contain a CR character we should just treat CRLF as an EOL character. Start treating it that way and adjust the old tests. [1] https://www.rfc-editor.org/info/rfc1738/#section-2.2 Signed-off-by: Matthias Aßhauer <mha1993@live.de>
| invalid_credential_test "username/password" https://pass@example.com | ||
|
|
||
| test_expect_success 'get: credentials with DOS line endings are invalid' ' | ||
| test_expect_success 'get: credentials with DOS line endings are valid' ' |
There was a problem hiding this comment.
Hmm. This reverts c03859a, and I wonder whether we're not actually making things worse that way: We're essentially walking back a promise that we've upheld for 6 years...
There was a problem hiding this comment.
From my perspective as a user, it seems both normal and reasonable for "Git for Windows" to support a credentials file with CRLF line endings, since the program is natively intended for Windows.
Also, CRLF handling seems currently to be inconsistent: CRLF credentials are considered invalid at line 146 (test_expect_success 'get: credentials with DOS line endings are invalid'), but valid at line 163 (test_expect_success 'get: credentials with path and DOS line endings are valid') ?!
There was a problem hiding this comment.
@i80487 what you missed, though, is that we're not even talking about user-facing files, but about files that are written by Git. Some users may feel free to muck with those, and change the line endings. That's what this PR tries to accommodate. I don't think it would set a good precedent, though. Soon we'd have to add patches left and right to accommodate for all sorts of subtle inconsistencies users would introduce into Git's internal files, when it would be better to draw a hard line: Iff you want to mess with those files, you're completely on your own, we won't cover up your mistakes.
git-credential store treats only LF as an EOL character, but reads the CR in a CR terminated line as part of the entry. Since the entries are URLs, CR can never be a valid character in an entry.
While we don't encourage users to manually edit the credential file, the format is simple enough and documented well enough that users do sometimes do that. Most editors on Windows produce CRLF as EOL characters. Users currently need to be careful when manually editing the credential file on Windows to not add stray CR characters into the file.
We have tests around when we consider entries with CRLF valid and not valid, but they only seem to test the known quirks of the existing implementation, not some expectations founded in some systemic design decisions.
Since the entries can never validly contain a CR character we should just treat CRLF as an EOL character. Start treating it that way and adjust the old tests.