From e895986727dfc4105c497132540dafa8ed51ec0a Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 10 Aug 2015 17:48:23 +0200 Subject: [PATCH 1/3] clone: do not include authentication data in guessed dir If the URI contains authentication data and the URI's path component is empty, we fail to guess a sensible directory name. E.g. cloning a repository 'ssh://user:password@example.com/' we guess a directory name 'password@example.com' where we would want the hostname only, e.g. 'example.com'. The naive way of just adding '@' as a path separator would break cloning repositories like 'foo/bar@baz.git' (which would currently become 'bar@baz' but would then become 'baz' only). Instead fix this by first dropping the scheme and then greedily scanning for an '@' sign until we find the first path separator. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/clone.c | 41 ++++++++++++++++++++++++++++++---------- t/t5603-clone-dirname.sh | 4 ++-- 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/builtin/clone.c b/builtin/clone.c index ed484cb6f4..6aa286fd7b 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -146,30 +146,51 @@ static char *get_repo_path(const char *repo, int *is_bundle) static char *guess_dir_name(const char *repo, int is_bundle, int is_bare) { - const char *end = repo + strlen(repo), *start; + const char *end = repo + strlen(repo), *start, *ptr; size_t len; char *dir; + /* + * Skip scheme. + */ + start = strstr(repo, "://"); + if (start == NULL) + start = repo; + else + start += 3; + + /* + * Skip authentication data. The stripping does happen + * greedily, such that we strip up to the last '@' inside + * the host part. + */ + for (ptr = start; ptr < end && !is_dir_sep(*ptr); ptr++) { + if (*ptr == '@') + start = ptr + 1; + } + /* * Strip trailing spaces, slashes and /.git */ - while (repo < end && (is_dir_sep(end[-1]) || isspace(end[-1]))) + while (start < end && (is_dir_sep(end[-1]) || isspace(end[-1]))) end--; - if (end - repo > 5 && is_dir_sep(end[-5]) && + if (end - start > 5 && is_dir_sep(end[-5]) && !strncmp(end - 4, ".git", 4)) { end -= 5; - while (repo < end && is_dir_sep(end[-1])) + while (start < end && is_dir_sep(end[-1])) end--; } /* - * Find last component, but be prepared that repo could have - * the form "remote.example.com:foo.git", i.e. no slash - * in the directory part. + * Find last component. To remain backwards compatible we + * also regard colons as path separators, such that + * cloning a repository 'foo:bar.git' would result in a + * directory 'bar' being guessed. */ - start = end; - while (repo < start && !is_dir_sep(start[-1]) && start[-1] != ':') - start--; + ptr = end; + while (start < ptr && !is_dir_sep(ptr[-1]) && ptr[-1] != ':') + ptr--; + start = ptr; /* * Strip .{bundle,git}. diff --git a/t/t5603-clone-dirname.sh b/t/t5603-clone-dirname.sh index 765cc434ef..0307462b03 100755 --- a/t/t5603-clone-dirname.sh +++ b/t/t5603-clone-dirname.sh @@ -77,11 +77,11 @@ test_clone_dir host:foo/.git/// foo # omitting the path should default to the hostname test_clone_dir ssh://host/ host test_clone_dir ssh://host:1234/ host fail -test_clone_dir ssh://user@host/ host fail +test_clone_dir ssh://user@host/ host test_clone_dir host:/ host fail # auth materials should be redacted -test_clone_dir ssh://user:password@host/ host fail +test_clone_dir ssh://user:password@host/ host test_clone_dir ssh://user:password@host:1234/ host fail test_clone_dir ssh://user:passw@rd@host:1234/ host fail test_clone_dir user@host:/ host fail From 92722efec01f67a54b68c83fcbc3cd65f9fbb7b8 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 10 Aug 2015 17:48:24 +0200 Subject: [PATCH 2/3] clone: do not use port number as dir name If the URI contains a port number and the URI's path component is empty we fail to guess a sensible directory name. E.g. cloning a repository 'ssh://example.com:2222/' we guess a directory name '2222' where we would want the hostname only, e.g. 'example.com'. We need to take care to not drop trailing port-like numbers in certain cases. E.g. when cloning a repository 'foo/bar:2222.git' we want to guess the directory name '2222' instead of 'bar'. Thus, we have to first check the stripped URI for path separators and only strip port numbers if there are path separators present. This heuristic breaks when cloning a repository 'bar:2222.git', though. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/clone.c | 17 +++++++++++++++++ t/t5603-clone-dirname.sh | 14 +++++++------- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/builtin/clone.c b/builtin/clone.c index 6aa286fd7b..c68eae13bf 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -181,6 +181,23 @@ static char *guess_dir_name(const char *repo, int is_bundle, int is_bare) end--; } + /* + * Strip trailing port number if we've got only a + * hostname (that is, there is no dir separator but a + * colon). This check is required such that we do not + * strip URI's like '/foo/bar:2222.git', which should + * result in a dir '2222' being guessed due to backwards + * compatibility. + */ + if (memchr(start, '/', end - start) == NULL + && memchr(start, ':', end - start) != NULL) { + ptr = end; + while (start < ptr && isdigit(ptr[-1]) && ptr[-1] != ':') + ptr--; + if (start < ptr && ptr[-1] == ':') + end = ptr - 1; + } + /* * Find last component. To remain backwards compatible we * also regard colons as path separators, such that diff --git a/t/t5603-clone-dirname.sh b/t/t5603-clone-dirname.sh index 0307462b03..d5af758129 100755 --- a/t/t5603-clone-dirname.sh +++ b/t/t5603-clone-dirname.sh @@ -76,17 +76,17 @@ test_clone_dir host:foo/.git/// foo # omitting the path should default to the hostname test_clone_dir ssh://host/ host -test_clone_dir ssh://host:1234/ host fail +test_clone_dir ssh://host:1234/ host test_clone_dir ssh://user@host/ host -test_clone_dir host:/ host fail +test_clone_dir host:/ host # auth materials should be redacted test_clone_dir ssh://user:password@host/ host -test_clone_dir ssh://user:password@host:1234/ host fail -test_clone_dir ssh://user:passw@rd@host:1234/ host fail -test_clone_dir user@host:/ host fail -test_clone_dir user:password@host:/ host fail -test_clone_dir user:passw@rd@host:/ host fail +test_clone_dir ssh://user:password@host:1234/ host +test_clone_dir ssh://user:passw@rd@host:1234/ host +test_clone_dir user@host:/ host +test_clone_dir user:password@host:/ host +test_clone_dir user:passw@rd@host:/ host # auth-like material should not be dropped test_clone_dir ssh://host/foo@bar foo@bar From adef9561f0c8cf2c974d78adac0ae236e159e49f Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 10 Aug 2015 17:48:25 +0200 Subject: [PATCH 3/3] clone: abort if no dir name could be guessed Due to various components of the URI being stripped off it may happen that we fail to guess a directory name. We currently error out with a message that it is impossible to create the working tree '' in such cases. Instead, error out early with a sensible error message hinting that a directory name should be specified manually on the command line. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/clone.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/builtin/clone.c b/builtin/clone.c index c68eae13bf..f60d3271ed 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -215,6 +215,10 @@ static char *guess_dir_name(const char *repo, int is_bundle, int is_bare) len = end - start; strip_suffix_mem(start, &len, is_bundle ? ".bundle" : ".git"); + if (!len || (len == 1 && *start == '/')) + die("No directory name could be guessed.\n" + "Please specify a directory on the command line"); + if (is_bare) dir = xstrfmt("%.*s.git", (int)len, start); else