{"thread":{"id":"35276","subject":"[PATCH V4] git clone: is an URL local or ssh","startedAt":"2013-11-04T21:18:08Z","lastAt":"2013-11-04T21:18:08Z","messageCount":1,"participants":["Torsten Bögershausen"],"isPatch":true,"patchVersion":4,"patchTotal":null},"messages":[{"id":"230092","messageId":"201311042218.08444.tboegi@web.de","threadId":"35276","inReplyTo":null,"subject":"[PATCH V4] git clone: is an URL local or ssh","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2013-11-04T21:18:08Z","receivedAt":"2013-11-04T21:18:08Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"Commit 8d3d28f5 added test cases for URLs which should be ssh.\n\nAdd more tests testing all the combinations:\n -IPv4 or IPv6\n -path starting with \"/\" or with \"/~\"\n -with and without the ssh:// scheme\n\nAdd tests for ssh:// with port number.\n\nWhen a git repository \"foo:bar\" exist, git clone will call\nabsolute_path() and git_connect() will be called with\nsomething like \"/home/user/projects/foo:bar\"\n\nTighten the test and use \"foo:bar\" instead of \"./foo:bar\",\nrefactor clear_ssh() and expect_ssh() into test_clone_url().\n\n\"git clone foo/bar:baz\" should not be ssh:\n  Make the rule\n  \"if there is a slash before the first colon, it is not ssh\"\n  more strict by using the same is_local() in both connect.c\n  and transport.c. Add a test case.\n\nBug fixes for corner cases:\n- Handle clone of [::1]:/~repo correctly (2e7766655):\n  Git should call \"ssh ::1 ~repo\", not ssh ::1 /~repo\n  This was caused by looking at (host != url), which never\n  worked for host names with literal IPv6 adresses, embedded by []\n  Support for the [] URLs was introduced in 356bece0a.\n\n- Do not tamper local URLs starting with '[' which have a ']'\n\nBug fix for msygit in t5601 : use $PWD insted of $(pwd)\n\nHelped-by: Jeff King <peff@peff.net>\nSigned-off-by: Torsten Bögershausen <tboegi@web.de>\n---\n Changes since V3:\n - Integrated Peffs suggestions in t5601\n   (Remove clear_ssh)\n - Decide early if it is ssl or local in connect.c\n - Use \"git fetch\" instead of \"git clone\" in t5601:\n   clone use absolute_path() (adding a / before :), fetch does not\n - Add a test for dos_drive \"C:temp\" for msys\n - Make tests work for msys by using $PWD instead of $(pwd) (file://$PWD)\n\n connect.c        |  50 ++++++++++++++++--------\n connect.h        |   1 +\n t/t5601-clone.sh | 117 ++++++++++++++++++++++++++++++++++++++++++++-----------\n transport.c      |   8 ----\n 4 files changed, 130 insertions(+), 46 deletions(-)\n\ndiff --git a/connect.c b/connect.c\nindex 06e88b0..022d122 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -547,6 +547,25 @@ static char *get_port(char *host)\n \n static struct child_process no_fork;\n \n+int is_local(const char *url)\n+{\n+\tconst char *colon = strchr(url, ':');\n+\tconst char *slash = strchr(url, '/');\n+\tif (has_dos_drive_prefix(url))\n+\t\treturn 1;\n+\tif (!colon)\n+\t\treturn 1;\n+\tif (slash && slash < colon)\n+\t\treturn 1;\n+\tif (url[0] == '[') {\n+\t\tconst char *end = strchr(url + 1, ']');\n+\t\tif (!end)\n+\t\t\treturn 1;\n+\t\treturn is_local(end);\n+\t}\n+\treturn 0;\n+}\n+\n /*\n  * This returns a dummy child_process if the transport protocol does not\n  * need fork(2), or a struct child_process object if it does.  Once done,\n@@ -564,7 +583,7 @@ struct child_process *git_connect(int fd[2], const char *url_orig,\n \tchar *url;\n \tchar *host, *path;\n \tchar *end;\n-\tint c;\n+\tint separator;\n \tstruct child_process *conn = &no_fork;\n \tenum protocol protocol = PROTO_LOCAL;\n \tint free_path = 0;\n@@ -587,17 +606,19 @@ struct child_process *git_connect(int fd[2], const char *url_orig,\n \t\t*host = '\\0';\n \t\tprotocol = get_protocol(url);\n \t\thost += 3;\n-\t\tc = '/';\n+\t\tseparator = '/';\n \t} else {\n \t\thost = url;\n-\t\tc = ':';\n+\t\tprotocol = is_local(url) ? PROTO_LOCAL : PROTO_SSH;\n+\t\tseparator = ':';\n \t}\n \n \t/*\n \t * Don't do destructive transforms with git:// as that\n \t * protocol code does '[]' unwrapping of its own.\n+\t * Don't change local URLs\n \t */\n-\tif (host[0] == '[') {\n+\tif (protocol != PROTO_LOCAL && host[0] == '[') {\n \t\tend = strchr(host + 1, ']');\n \t\tif (end) {\n \t\t\tif (protocol != PROTO_GIT) {\n@@ -610,17 +631,14 @@ struct child_process *git_connect(int fd[2], const char *url_orig,\n \t} else\n \t\tend = host;\n \n-\tpath = strchr(end, c);\n-\tif (path && !has_dos_drive_prefix(end)) {\n-\t\tif (c == ':') {\n-\t\t\tif (host != url || path < strchrnul(host, '/')) {\n-\t\t\t\tprotocol = PROTO_SSH;\n-\t\t\t\t*path++ = '\\0';\n-\t\t\t} else /* '/' in the host part, assume local path */\n-\t\t\t\tpath = end;\n+\tpath = strchr(end, separator);\n+\tif (separator == ':') {\n+\t\tif (path && protocol == PROTO_SSH) {\n+\t\t\t*path++ = '\\0';\n+\t\t} else {/* assume local path */\n+\t\t\tpath = end;\n \t\t}\n-\t} else\n-\t\tpath = end;\n+\t}\n \n \tif (!path || !*path)\n \t\tdie(\"No path specified. See 'man git-pull' for valid url syntax\");\n@@ -629,7 +647,7 @@ struct child_process *git_connect(int fd[2], const char *url_orig,\n \t * null-terminate hostname and point path to ~ for URL's like this:\n \t *    ssh://host.xz/~user/repo\n \t */\n-\tif (protocol != PROTO_LOCAL && host != url) {\n+\tif (protocol != PROTO_LOCAL && separator == '/') {\n \t\tchar *ptr = path;\n \t\tif (path[1] == '~')\n \t\t\tpath++;\n@@ -644,7 +662,7 @@ struct child_process *git_connect(int fd[2], const char *url_orig,\n \t/*\n \t * Add support for ssh port: ssh://host.xy:<port>/...\n \t */\n-\tif (protocol == PROTO_SSH && host != url)\n+\tif (protocol == PROTO_SSH && separator == '/')\n \t\tport = get_port(end);\n \n \tif (protocol == PROTO_GIT) {\ndiff --git a/connect.h b/connect.h\nindex 64fb7db..fb2de9b 100644\n--- a/connect.h\n+++ b/connect.h\n@@ -5,6 +5,7 @@\n extern struct child_process *git_connect(int fd[2], const char *url, const char *prog, int flags);\n extern int finish_connect(struct child_process *conn);\n extern int git_connection_is_socket(struct child_process *conn);\n+extern int is_local(const char *url);\n extern int server_supports(const char *feature);\n extern int parse_feature_request(const char *features, const char *feature);\n extern const char *server_feature_value(const char *feature, int *len_ret);\ndiff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\nindex 1d1c875..5f2b5a0 100755\n--- a/t/t5601-clone.sh\n+++ b/t/t5601-clone.sh\n@@ -30,20 +30,20 @@ test_expect_success 'clone with excess parameters (1)' '\n test_expect_success 'clone with excess parameters (2)' '\n \n \trm -fr dst &&\n-\ttest_must_fail git clone -n \"file://$(pwd)/src\" dst junk\n+\ttest_must_fail git clone -n \"file://$PWD/src\" dst junk\n \n '\n \n test_expect_success C_LOCALE_OUTPUT 'output from clone' '\n \trm -fr dst &&\n-\tgit clone -n \"file://$(pwd)/src\" dst >output 2>&1 &&\n+\tgit clone -n \"file://$PWD/src\" dst >output 2>&1 &&\n \ttest $(grep Clon output | wc -l) = 1\n '\n \n test_expect_success 'clone does not keep pack' '\n \n \trm -fr dst &&\n-\tgit clone -n \"file://$(pwd)/src\" dst &&\n+\tgit clone -n \"file://$PWD/src\" dst &&\n \t! test -f dst/file &&\n \t! (echo dst/.git/objects/pack/pack-* | grep \"\\.keep\")\n \n@@ -172,12 +172,12 @@ test_expect_success 'clone a void' '\n \t(\n \t\tcd src-0 && git init\n \t) &&\n-\tgit clone \"file://$(pwd)/src-0\" target-6 2>err-6 &&\n+\tgit clone \"file://$PWD/src-0\" target-6 2>err-6 &&\n \t! grep \"fatal:\" err-6 &&\n \t(\n \t\tcd src-0 && test_commit A\n \t) &&\n-\tgit clone \"file://$(pwd)/src-0\" target-7 2>err-7 &&\n+\tgit clone \"file://$PWD/src-0\" target-7 2>err-7 &&\n \t! grep \"fatal:\" err-7 &&\n \t# There is no reason to insist they are bit-for-bit\n \t# identical, but this test should suffice for now.\n@@ -291,14 +291,14 @@ test_expect_success 'setup ssh wrapper' '\n \n \tGIT_SSH=\"$TRASH_DIRECTORY/ssh-wrapper\" &&\n \texport GIT_SSH &&\n-\texport TRASH_DIRECTORY\n+\texport TRASH_DIRECTORY &&\n+\t>\"$TRASH_DIRECTORY\"/ssh-output\n '\n \n-clear_ssh () {\n-\t>\"$TRASH_DIRECTORY/ssh-output\"\n-}\n-\n expect_ssh () {\n+\ttest_when_finished '\n+\t  (cd \"$TRASH_DIRECTORY\" && rm -f ssh-expect && >ssh-output)\n+\t' &&\n \t{\n \t\tcase \"$1\" in\n \t\tnone)\n@@ -310,23 +310,96 @@ expect_ssh () {\n \t(cd \"$TRASH_DIRECTORY\" && test_cmp ssh-expect ssh-output)\n }\n \n-test_expect_success 'cloning myhost:src uses ssh' '\n-\tclear_ssh &&\n-\tgit clone myhost:src ssh-clone &&\n-\texpect_ssh myhost src\n-'\n-\n test_expect_success NOT_MINGW,NOT_CYGWIN 'clone local path foo:bar' '\n-\tclear_ssh &&\n \tcp -R src \"foo:bar\" &&\n-\tgit clone \"./foo:bar\" foobar &&\n+\tgit clone \"foo:bar\" foobar &&\n+\texpect_ssh none\n+'\n+\n+test_expect_success 'clone local path foo2' '\n+\tcp -R src \"foo2\" &&\n+\tgit clone \"foo2\" foobar2 &&\n \texpect_ssh none\n '\n \n-test_expect_success 'bracketed hostnames are still ssh' '\n-\tclear_ssh &&\n-\tgit clone \"[myhost:123]:src\" ssh-bracket-clone &&\n-\texpect_ssh myhost:123 src\n+counter=0\n+# $1 url\n+# $2 none|host\n+# $3 path\n+test_fetch_url () {\n+\tcounter=$(($counter + 1))\n+\ttest_might_fail git fetch \"$1\" tmp$counter &&\n+\texpect_ssh \"$2\" \"$3\"\n+}\n+\n+test_expect_success NOT_MINGW 'fetch c:temp is ssl' '\n+\ttest_fetch_url c:temp c temp\n+'\n+\n+test_expect_success MINGW 'fetch c:temp is dos drive' '\n+\ttest_fetch_url c:temp none\n+'\n+\n+#ip v4\n+for repo in rep rep/home/project /~proj 123\n+do\n+\ttest_expect_success \"fetchinghost:$repo\" '\n+\t\ttest_fetch_url host:$repo host $repo\n+\t'\n+done\n+\n+#ipv6\n+for repo in rep rep/home/project /~proj 123\n+do\n+\ttest_expect_success \"fetching[::1]:$repo\" '\n+\t\ttest_fetch_url [::1]:$repo ::1 $repo\n+\t'\n+done\n+\n+# Corner cases\n+for url in [foo]bar/baz:qux [foo/bar]:baz foo/bar:baz\n+do\n+\ttest_expect_success \"fetching$url is not ssh\" '\n+\t\t\ttest_fetch_url $url none\n+\t'\n+done\n+\n+#with ssh:// scheme\n+test_expect_success 'ssh://host.xz/home/user/repo' '\n+\ttest_fetch_url \"ssh://host.xz/home/user/repo\" host.xz \"/home/user/repo\"\n+'\n+\n+# from home directory\n+test_expect_success 'ssh://host.xz/~repo' '\n+\ttest_fetch_url \"ssh://host.xz/~repo\" host.xz \"~repo\"\n+'\n+# with port number\n+test_expect_success 'ssh://host.xz:22/home/user/repo' '\n+\ttest_fetch_url \"ssh://host.xz:22/home/user/repo\" \"-p 22 host.xz\" \"/home/user/repo\"\n+'\n+\n+# from home directory with port number\n+test_expect_success 'ssh://host.xz:22/~repo' '\n+\ttest_fetch_url \"ssh://host.xz:22/~repo\" \"-p 22 host.xz\" \"~repo\"\n+'\n+\n+#IPv6\n+test_expect_success 'ssh://[::1]/home/user/repo' '\n+\ttest_fetch_url \"ssh://[::1]/home/user/repo\" \"::1\" \"/home/user/repo\"\n+'\n+\n+#IPv6 from home directory\n+test_expect_success 'ssh://[::1]/~repo' '\n+\ttest_fetch_url \"ssh://[::1]/~repo\" \"::1\" \"~repo\"\n+'\n+\n+#IPv6 with port number\n+test_expect_success 'ssh://[::1]:22/home/user/repo' '\n+\ttest_fetch_url \"ssh://[::1]:22/home/user/repo\" \"-p 22 ::1\" \"/home/user/repo\"\n+'\n+#IPv6 from home directory with port number\n+test_expect_success 'ssh://[::1]:22/~repo' '\n+\ttest_fetch_url \"ssh://[::1]:22/~repo\" \"-p 22 ::1\" \"~repo\"\n '\n \n test_expect_success 'clone from a repository with two identical branches' '\ndiff --git a/transport.c b/transport.c\nindex 7202b77..a09ba95 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -885,14 +885,6 @@ void transport_take_over(struct transport *transport,\n \ttransport->cannot_reuse = 1;\n }\n \n-static int is_local(const char *url)\n-{\n-\tconst char *colon = strchr(url, ':');\n-\tconst char *slash = strchr(url, '/');\n-\treturn !colon || (slash && slash < colon) ||\n-\t\thas_dos_drive_prefix(url);\n-}\n-\n static int is_file(const char *url)\n {\n \tstruct stat buf;\n-- \n1.8.4.457.g424cb08\n"}]}