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 ']'

Bug fix for msygit in t5601 : use $PWD insted of $(pwd)

Helped-by: Jeff King <p...@peff.net>
Signed-off-by: Torsten Bögershausen <tbo...@web.de>
---
 Changes since V3:
 - Integrated Peffs suggestions in t5601
   (Remove clear_ssh)
 - Decide early if it is ssl or local in connect.c
 - Use "git fetch" instead of "git clone" in t5601:
   clone use absolute_path() (adding a / before :), fetch does not
 - Add a test for dos_drive "C:temp" for msys
 - Make tests work for msys by using $PWD instead of $(pwd) (file://$PWD)

 connect.c        |  50 ++++++++++++++++--------
 connect.h        |   1 +
 t/t5601-clone.sh | 117 ++++++++++++++++++++++++++++++++++++++++++++-----------
 transport.c      |   8 ----
 4 files changed, 130 insertions(+), 46 deletions(-)

diff --git a/connect.c b/connect.c
index 06e88b0..022d122 100644
--- a/connect.c
+++ b/connect.c
@@ -547,6 +547,25 @@ 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, '/');
+       if (has_dos_drive_prefix(url))
+               return 1;
+       if (!colon)
+               return 1;
+       if (slash && slash < colon)
+               return 1;
+       if (url[0] == '[') {
+               const char *end = strchr(url + 1, ']');
+               if (!end)
+                       return 1;
+               return is_local(end);
+       }
+       return 0;
+}
+
 /*
  * 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,7 +583,7 @@ 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;
        int free_path = 0;
@@ -587,17 +606,19 @@ 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 = ':';
+               protocol = is_local(url) ? PROTO_LOCAL : PROTO_SSH;
+               separator = ':';
        }
 
        /*
         * Don't do destructive transforms with git:// as that
         * protocol code does '[]' unwrapping of its own.
+        * Don't change local URLs
         */
-       if (host[0] == '[') {
+       if (protocol != PROTO_LOCAL && host[0] == '[') {
                end = strchr(host + 1, ']');
                if (end) {
                        if (protocol != PROTO_GIT) {
@@ -610,17 +631,14 @@ 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_SSH) {
+                       *path++ = '\0';
+               } else {/* assume local path */
+                       path = end;
                }
-       } else
-               path = end;
+       }
 
        if (!path || !*path)
                die("No path specified. See 'man git-pull' for valid url 
syntax");
@@ -629,7 +647,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 +662,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..5f2b5a0 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -30,20 +30,20 @@ test_expect_success 'clone with excess parameters (1)' '
 test_expect_success 'clone with excess parameters (2)' '
 
        rm -fr dst &&
-       test_must_fail git clone -n "file://$(pwd)/src" dst junk
+       test_must_fail git clone -n "file://$PWD/src" dst junk
 
 '
 
 test_expect_success C_LOCALE_OUTPUT 'output from clone' '
        rm -fr dst &&
-       git clone -n "file://$(pwd)/src" dst >output 2>&1 &&
+       git clone -n "file://$PWD/src" dst >output 2>&1 &&
        test $(grep Clon output | wc -l) = 1
 '
 
 test_expect_success 'clone does not keep pack' '
 
        rm -fr dst &&
-       git clone -n "file://$(pwd)/src" dst &&
+       git clone -n "file://$PWD/src" dst &&
        ! test -f dst/file &&
        ! (echo dst/.git/objects/pack/pack-* | grep "\.keep")
 
@@ -172,12 +172,12 @@ test_expect_success 'clone a void' '
        (
                cd src-0 && git init
        ) &&
-       git clone "file://$(pwd)/src-0" target-6 2>err-6 &&
+       git clone "file://$PWD/src-0" target-6 2>err-6 &&
        ! grep "fatal:" err-6 &&
        (
                cd src-0 && test_commit A
        ) &&
-       git clone "file://$(pwd)/src-0" target-7 2>err-7 &&
+       git clone "file://$PWD/src-0" target-7 2>err-7 &&
        ! grep "fatal:" err-7 &&
        # There is no reason to insist they are bit-for-bit
        # identical, but this test should suffice for now.
@@ -291,14 +291,14 @@ test_expect_success 'setup ssh wrapper' '
 
        GIT_SSH="$TRASH_DIRECTORY/ssh-wrapper" &&
        export GIT_SSH &&
-       export TRASH_DIRECTORY
+       export TRASH_DIRECTORY &&
+       >"$TRASH_DIRECTORY"/ssh-output
 '
 
-clear_ssh () {
-       >"$TRASH_DIRECTORY/ssh-output"
-}
-
 expect_ssh () {
+       test_when_finished '
+         (cd "$TRASH_DIRECTORY" && rm -f ssh-expect && >ssh-output)
+       ' &&
        {
                case "$1" in
                none)
@@ -310,23 +310,96 @@ expect_ssh () {
        (cd "$TRASH_DIRECTORY" && test_cmp ssh-expect ssh-output)
 }
 
-test_expect_success 'cloning myhost:src uses ssh' '
-       clear_ssh &&
-       git clone myhost:src ssh-clone &&
-       expect_ssh myhost src
-'
-
 test_expect_success NOT_MINGW,NOT_CYGWIN 'clone local path foo:bar' '
-       clear_ssh &&
        cp -R src "foo:bar" &&
-       git clone "./foo:bar" foobar &&
+       git clone "foo:bar" foobar &&
+       expect_ssh none
+'
+
+test_expect_success 'clone local path foo2' '
+       cp -R src "foo2" &&
+       git clone "foo2" foobar2 &&
        expect_ssh none
 '
 
-test_expect_success 'bracketed hostnames are still ssh' '
-       clear_ssh &&
-       git clone "[myhost:123]:src" ssh-bracket-clone &&
-       expect_ssh myhost:123 src
+counter=0
+# $1 url
+# $2 none|host
+# $3 path
+test_fetch_url () {
+       counter=$(($counter + 1))
+       test_might_fail git fetch "$1" tmp$counter &&
+       expect_ssh "$2" "$3"
+}
+
+test_expect_success NOT_MINGW 'fetch c:temp is ssl' '
+       test_fetch_url c:temp c temp
+'
+
+test_expect_success MINGW 'fetch c:temp is dos drive' '
+       test_fetch_url c:temp none
+'
+
+#ip v4
+for repo in rep rep/home/project /~proj 123
+do
+       test_expect_success "fetchinghost:$repo" '
+               test_fetch_url host:$repo host $repo
+       '
+done
+
+#ipv6
+for repo in rep rep/home/project /~proj 123
+do
+       test_expect_success "fetching[::1]:$repo" '
+               test_fetch_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 "fetching$url is not ssh" '
+                       test_fetch_url $url none
+       '
+done
+
+#with ssh:// scheme
+test_expect_success 'ssh://host.xz/home/user/repo' '
+       test_fetch_url "ssh://host.xz/home/user/repo" host.xz "/home/user/repo"
+'
+
+# from home directory
+test_expect_success 'ssh://host.xz/~repo' '
+       test_fetch_url "ssh://host.xz/~repo" host.xz "~repo"
+'
+# with port number
+test_expect_success 'ssh://host.xz:22/home/user/repo' '
+       test_fetch_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_fetch_url "ssh://host.xz:22/~repo" "-p 22 host.xz" "~repo"
+'
+
+#IPv6
+test_expect_success 'ssh://[::1]/home/user/repo' '
+       test_fetch_url "ssh://[::1]/home/user/repo" "::1" "/home/user/repo"
+'
+
+#IPv6 from home directory
+test_expect_success 'ssh://[::1]/~repo' '
+       test_fetch_url "ssh://[::1]/~repo" "::1" "~repo"
+'
+
+#IPv6 with port number
+test_expect_success 'ssh://[::1]:22/home/user/repo' '
+       test_fetch_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_fetch_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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to