Commit 8d3d28f5 added test cases for URLs which should be ssh.
Add more tests testing all the combinations:
-IPv4 or IPv6
-path starting with "/" or with "/~"
-with and without the ssh:// scheme
Add tests for ssh:// with port number.
When a git repository "foo:bar" exist, git clone will call
absolute_path() and git_connect() will be called with
something like "/home/user/projects/foo:bar"
Tighten the test and use "foo:bar" instead of "./foo:bar",
refactor clear_ssh() and expect_ssh() into test_clone_url().
"git clone foo/bar:baz" should not be ssh:
Make the rule
"if there is a slash before the first colon, it is not ssh"
more strict by using the same is_local() in both connect.c
and transport.c. Add a test case.
Bug fixes for corner cases:
- Handle clone of [::1]:/~repo correctly (2e7766655):
Git should call "ssh ::1 ~repo", not ssh ::1 /~repo
This was caused by looking at (host != url), which never
worked for host names with literal IPv6 adresses, embedded by []
Support for the [] URLs was introduced in 356bece0a.
- Do not tamper local URLs starting with '[' which have a ']'
Signed-off-by: Torsten Bögershausen <tboegi@xxxxxx>
---
(This does apply on pu, not on master.
I'm almost sure there are more corner cases, but the
most important things should be covered)
Changes since V1: Comments from Eric Sunshine (thanks)
connect.c | 47 +++++++++++++++++----------
connect.h | 1 +
t/t5601-clone.sh | 98 ++++++++++++++++++++++++++++++++++++++++++++------------
transport.c | 8 -----
4 files changed, 108 insertions(+), 46 deletions(-)
diff --git a/connect.c b/connect.c
index 06e88b0..d61adc9 100644
--- a/connect.c
+++ b/connect.c
@@ -231,6 +231,7 @@ int server_supports(const char *feature)
}
enum protocol {
+ PROTO_LOCAL_OR_SSH = 0,
PROTO_LOCAL = 1,
PROTO_SSH,
PROTO_GIT
@@ -547,6 +548,15 @@ static char *get_port(char *host)
static struct child_process no_fork;
+int is_local(const char *url)
+{
+ const char *colon = strchr(url, ':');
+ const char *slash = strchr(url, '/');
+ return !colon || (slash && slash < colon) ||
+ has_dos_drive_prefix(url);
+}
+
+
/*
* This returns a dummy child_process if the transport protocol does not
* need fork(2), or a struct child_process object if it does. Once done,
@@ -564,9 +574,9 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
char *url;
char *host, *path;
char *end;
- int c;
+ int separator;
struct child_process *conn = &no_fork;
- enum protocol protocol = PROTO_LOCAL;
+ enum protocol protocol = PROTO_LOCAL_OR_SSH;
int free_path = 0;
char *port = NULL;
const char **arg;
@@ -587,20 +597,23 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
*host = '\0';
protocol = get_protocol(url);
host += 3;
- c = '/';
+ separator = '/';
} else {
host = url;
- c = ':';
+ separator = ':';
+ if (is_local(url))
+ protocol = PROTO_LOCAL;
}
/*
* Don't do destructive transforms with git:// as that
* protocol code does '[]' unwrapping of its own.
+ * Don't change local URLs
*/
if (host[0] == '[') {
end = strchr(host + 1, ']');
if (end) {
- if (protocol != PROTO_GIT) {
+ if (protocol != PROTO_GIT && protocol != PROTO_LOCAL) {
*end = 0;
host++;
}
@@ -610,17 +623,17 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
} else
end = host;
- path = strchr(end, c);
- if (path && !has_dos_drive_prefix(end)) {
- if (c == ':') {
- if (host != url || path < strchrnul(host, '/')) {
- protocol = PROTO_SSH;
- *path++ = '\0';
- } else /* '/' in the host part, assume local path */
- path = end;
+ path = strchr(end, separator);
+ if (separator == ':') {
+ if (path && protocol == PROTO_LOCAL_OR_SSH) {
+ /* We have a ':' */
+ protocol = PROTO_SSH;
+ *path++ = '\0';
+ } else {/* assume local path */
+ protocol = PROTO_LOCAL;
+ path = end;
}
- } else
- path = end;
+ }
if (!path || !*path)
die("No path specified. See 'man git-pull' for valid url syntax");
@@ -629,7 +642,7 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
* null-terminate hostname and point path to ~ for URL's like this:
* ssh://host.xz/~user/repo
*/
- if (protocol != PROTO_LOCAL && host != url) {
+ if (protocol != PROTO_LOCAL && separator == '/') {
char *ptr = path;
if (path[1] == '~')
path++;
@@ -644,7 +657,7 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
/*
* Add support for ssh port: ssh://host.xy:<port>/...
*/
- if (protocol == PROTO_SSH && host != url)
+ if (protocol == PROTO_SSH && separator == '/')
port = get_port(end);
if (protocol == PROTO_GIT) {
diff --git a/connect.h b/connect.h
index 64fb7db..fb2de9b 100644
--- a/connect.h
+++ b/connect.h
@@ -5,6 +5,7 @@
extern struct child_process *git_connect(int fd[2], const char *url, const char *prog, int flags);
extern int finish_connect(struct child_process *conn);
extern int git_connection_is_socket(struct child_process *conn);
+extern int is_local(const char *url);
extern int server_supports(const char *feature);
extern int parse_feature_request(const char *features, const char *feature);
extern const char *server_feature_value(const char *feature, int *len_ret);
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 1d1c875..a126f08 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -294,39 +294,95 @@ test_expect_success 'setup ssh wrapper' '
export TRASH_DIRECTORY
'
-clear_ssh () {
- >"$TRASH_DIRECTORY/ssh-output"
-}
-
-expect_ssh () {
+i5601=0
+# $1 url
+# $2 none|host
+# $3 path
+test_clone_url () {
+ i5601=$(($i5601 + 1))
+ >"$TRASH_DIRECTORY/ssh-output" &&
+ test_might_fail git clone "$1" tmp$i5601 &&
{
- case "$1" in
+ case "$2" in
none)
;;
*)
- echo "ssh: $1 git-upload-pack '$2'"
+ echo "ssh: $2 git-upload-pack '$3'"
esac
} >"$TRASH_DIRECTORY/ssh-expect" &&
- (cd "$TRASH_DIRECTORY" && test_cmp ssh-expect ssh-output)
+ (
+ cd "$TRASH_DIRECTORY" &&
+ test_cmp ssh-expect ssh-output &&
+ rm -rf ssh-expect ssh-output
+ )
}
-test_expect_success 'cloning myhost:src uses ssh' '
- clear_ssh &&
- git clone myhost:src ssh-clone &&
- expect_ssh myhost src
-'
-
+# url looks ssh like, and is on disc: should be local
test_expect_success NOT_MINGW,NOT_CYGWIN 'clone local path foo:bar' '
- clear_ssh &&
cp -R src "foo:bar" &&
- git clone "./foo:bar" foobar &&
- expect_ssh none
+ test_clone_url "foo:bar" none &&
+ ( cd tmp$i5601 && git log)
+'
+#ip v4
+for repo in rep rep/home/project /~proj 123
+do
+ test_expect_success "cloning host:$repo" '
+ test_clone_url host:$repo host $repo
+ '
+done
+
+#ipv6
+for repo in rep rep/home/project /~proj 123
+do
+ test_expect_success "cloning [::1]:$repo" '
+ test_clone_url [::1]:$repo ::1 $repo
+ '
+done
+
+# Corner cases
+for url in [foo]bar/baz:qux [foo/bar]:baz foo/bar:baz
+do
+ test_expect_success "cloning $url is not ssh" '
+ test_clone_url $url none
+ '
+done
+
+#with ssh:// scheme
+test_expect_success 'ssh://host.xz/home/user/repo' '
+ test_clone_url "ssh://host.xz/home/user/repo" host.xz "/home/user/repo"
'
-test_expect_success 'bracketed hostnames are still ssh' '
- clear_ssh &&
- git clone "[myhost:123]:src" ssh-bracket-clone &&
- expect_ssh myhost:123 src
+# from home directory
+test_expect_success 'ssh://host.xz/~repo' '
+ test_clone_url "ssh://host.xz/~repo" host.xz "~repo"
+'
+# with port number
+test_expect_success 'ssh://host.xz:22/home/user/repo' '
+ test_clone_url "ssh://host.xz:22/home/user/repo" "-p 22 host.xz" "/home/user/repo"
+'
+
+# from home directory with port number
+test_expect_success 'ssh://host.xz:22/~repo' '
+ test_clone_url "ssh://host.xz:22/~repo" "-p 22 host.xz" "~repo"
+'
+
+#IPv6
+test_expect_success 'ssh://[::1]/home/user/repo' '
+ test_clone_url "ssh://[::1]/home/user/repo" "::1" "/home/user/repo"
+'
+
+#IPv6 from home directory
+test_expect_success 'ssh://[::1]/~repo' '
+ test_clone_url "ssh://[::1]/~repo" "::1" "~repo"
+'
+
+#IPv6 with port number
+test_expect_success 'ssh://[::1]:22/home/user/repo' '
+ test_clone_url "ssh://[::1]:22/home/user/repo" "-p 22 ::1" "/home/user/repo"
+'
+#IPv6 from home directory with port number
+test_expect_success 'ssh://[::1]:22/~repo' '
+ test_clone_url "ssh://[::1]:22/~repo" "-p 22 ::1" "~repo"
'
test_expect_success 'clone from a repository with two identical branches' '
diff --git a/transport.c b/transport.c
index 7202b77..a09ba95 100644
--- a/transport.c
+++ b/transport.c
@@ -885,14 +885,6 @@ void transport_take_over(struct transport *transport,
transport->cannot_reuse = 1;
}
-static int is_local(const char *url)
-{
- const char *colon = strchr(url, ':');
- const char *slash = strchr(url, '/');
- return !colon || (slash && slash < colon) ||
- has_dos_drive_prefix(url);
-}
-
static int is_file(const char *url)
{
struct stat buf;
--
1.8.4.457.g424cb08
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html