summaryrefslogtreecommitdiff
path: root/credential.c
diff options
context:
space:
mode:
authorJeff King <peff@peff.net>2020-04-18 20:53:09 -0700
committerJonathan Nieder <jrnieder@gmail.com>2020-04-19 16:10:58 -0700
commitfe29a9b7b0236d3d45c254965580d6aff7fa8504 (patch)
tree9d8568f2f438723a5b8c842c3c8233abfa68b94a /credential.c
parenta2b26ffb1a81aa23dd14453f4db05d8fe24ee7cc (diff)
downloadgit-fe29a9b7b0236d3d45c254965580d6aff7fa8504.tar.gz
credential: die() when parsing invalid urls
When we try to initialize credential loading by URL and find that the URL is invalid, we set all fields to NULL in order to avoid acting on malicious input. Later when we request credentials, we diagonse the erroneous input: fatal: refusing to work with credential missing host field This is problematic in two ways: - The message doesn't tell the user *why* we are missing the host field, so they can't tell from this message alone how to recover. There can be intervening messages after the original warning of bad input, so the user may not have the context to put two and two together. - The error only occurs when we actually need to get a credential. If the URL permits anonymous access, the only encouragement the user gets to correct their bogus URL is a quiet warning. This is inconsistent with the check we perform in fsck, where any use of such a URL as a submodule is an error. When we see such a bogus URL, let's not try to be nice and continue without helpers. Instead, die() immediately. This is simpler and obviously safe. And there's very little chance of disrupting a normal workflow. It's _possible_ that somebody has a legitimate URL with a raw newline in it. It already wouldn't work with credential helpers, so this patch steps that up from an inconvenience to "we will refuse to work with it at all". If such a case does exist, we should figure out a way to work with it (especially if the newline is only in the path component, which we normally don't even pass to helpers). But until we see a real report, we're better off being defensive. Reported-by: Carlo Arenas <carenas@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Diffstat (limited to 'credential.c')
-rw-r--r--credential.c6
1 files changed, 2 insertions, 4 deletions
diff --git a/credential.c b/credential.c
index 39b7cb0c0b..7d43359591 100644
--- a/credential.c
+++ b/credential.c
@@ -405,8 +405,6 @@ int credential_from_url_gently(struct credential *c, const char *url,
void credential_from_url(struct credential *c, const char *url)
{
- if (credential_from_url_gently(c, url, 0) < 0) {
- warning(_("skipping credential lookup for url: %s"), url);
- credential_clear(c);
- }
+ if (credential_from_url_gently(c, url, 0) < 0)
+ die(_("credential url cannot be parsed: %s"), url);
}