{"thread":{"id":"35384","subject":"[PATCH v6 07/10] connect.c: Corner case for IPv6","startedAt":"2013-11-21T20:41:10Z","lastAt":"2013-11-21T23:31:01Z","messageCount":2,"participants":["Torsten Bögershausen","Junio C Hamano"],"isPatch":true,"patchVersion":6,"patchTotal":10},"messages":[{"id":"230909","messageId":"201311212141.10690.tboegi@web.de","threadId":"35384","inReplyTo":null,"subject":"[PATCH v6 07/10] connect.c: Corner case for IPv6","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2013-11-21T20:41:10Z","receivedAt":"2013-11-21T20:41:10Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"Commit faea9ccbadf75128 introduced \"user-relative paths\":\n\"ssh://host.xz/~junio/repo\" is the same as \"host.xz:~junio/repo\".\n\nCommit 356bece0a27258181 \"Support [address] in URLs\" introduced a regression:\nWhen an URL like \"[::1]:/~repo\" is used, the replacement of \"/~\"\nwith \"~\" was enabled for IPv6 host names, and \"[::1]:/~repo\" is\nconverted into \"[::1]:~repo\".\n\nSolution:\nDon't use \"if (url != hostname)\", but look at the separator instead.\nRename the variable \"c\" into separator.\n---\n connect.c             | 14 +++++++-------\n t/t5500-fetch-pack.sh | 16 ++++------------\n t/t5601-clone.sh      | 12 ++----------\n 3 files changed, 13 insertions(+), 29 deletions(-)\n\ndiff --git a/connect.c b/connect.c\nindex 1b93b4d..0cb88b8 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -566,7 +566,7 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host,\n \tchar *url;\n \tchar *host, *path;\n \tchar *end;\n-\tint c;\n+\tint separator;\n \tenum protocol protocol = PROTO_LOCAL;\n \tint free_path = 0;\n \tchar *port = NULL;\n@@ -581,10 +581,10 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host,\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\tseparator = ':';\n \t}\n \n \t/*\n@@ -604,9 +604,9 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host,\n \t} else\n \t\tend = host;\n \n-\tpath = strchr(end, c);\n+\tpath = strchr(end, separator);\n \tif (path && !has_dos_drive_prefix(end)) {\n-\t\tif (c == ':') {\n+\t\tif (separator == ':') {\n \t\t\tif (host != url || path < strchrnul(host, '/')) {\n \t\t\t\tprotocol = PROTO_SSH;\n \t\t\t\t*path++ = '\\0';\n@@ -623,7 +623,7 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host,\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@@ -638,7 +638,7 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host,\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 \t*ret_host = xstrdup(host);\ndiff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh\nindex 7f55b95..ac5b08b 100755\n--- a/t/t5500-fetch-pack.sh\n+++ b/t/t5500-fetch-pack.sh\n@@ -604,19 +604,11 @@ do\n \t\ttest_expect_success \"fetch-pack --diag-url $h:$r\" '\n \t\t\tcheck_prot_path $h:$r $p \"$r\"\n \t\t'\n+\t\t# No \"/~\" -> \"~\" conversion\n+\t\ttest_expect_success \"fetch-pack --diag-url $h:/~$r\" '\n+\t\t\tcheck_prot_host_path $h:/~$r $p \"$hh\" \"/~$r\"\n+\t\t'\n \tdone\n-\th=host\n-\thh=host\n-\t# \"/~\" -> \"~\" conversion\n-\ttest_expect_failure \"fetch-pack --diag-url $h:/~$r\" '\n-\t\tcheck_prot_host_path $h:/~$r $p \"$hh\" \"~$r\"\n-\t'\n-\th=[::1]\n-\thh=$(echo $h | tr -d \"[]\")\n-\t# \"/~\" -> \"~\" conversion\n-\ttest_expect_success \"fetch-pack --diag-url $h:/~$r\" '\n-\t\tcheck_prot_host_path $h:/~$r $p \"$hh\" \"~$r\"\n-\t'\n done\n \n test_expect_success MINGW 'fetch-pack --diag-url file://c:/repo' '\ndiff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\nindex ba99972..57234c0 100755\n--- a/t/t5601-clone.sh\n+++ b/t/t5601-clone.sh\n@@ -356,15 +356,7 @@ do\n done\n \n #ipv6\n-# failing\n-for repo in /~proj\n-do\n-\ttest_expect_failure \"clone [::1]:$repo\" '\n-\t\ttest_clone_url [::1]:$repo ::1 $repo\n-\t'\n-done\n-\n-for repo in rep rep/home/project 123\n+for repo in rep rep/home/project 123 /~proj\n do\n \ttest_expect_success \"clone [::1]:$repo\" '\n \t\ttest_clone_url [::1]:$repo ::1 $repo\n@@ -373,7 +365,7 @@ done\n \n # Corner cases\n # failing\n-for repo in [foo]bar/baz:qux [foo/bar]:baz\n+for url in [foo]bar/baz:qux [foo/bar]:baz\n do\n \ttest_expect_failure \"clone $url is not ssh\" '\n \t\ttest_clone_url $url none\n-- \n1.8.4.457.g424cb08\n"},{"id":"230924","messageId":"xmqqbo1d2uyy.fsf@gitster.dls.corp.google.com","threadId":"35384","inReplyTo":"201311212141.10690.tboegi@web.de","subject":"Re: [PATCH v6 07/10] connect.c: Corner case for IPv6","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-21T23:31:01Z","receivedAt":"2013-11-21T23:31:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen <tboegi@web.de> writes:\n\n> Commit faea9ccbadf75128 introduced \"user-relative paths\":\n> \"ssh://host.xz/~junio/repo\" is the same as \"host.xz:~junio/repo\".\n>\n> Commit 356bece0a27258181 \"Support [address] in URLs\" introduced a regression:\n> When an URL like \"[::1]:/~repo\" is used, the replacement of \"/~\"\n> with \"~\" was enabled for IPv6 host names, and \"[::1]:/~repo\" is\n> converted into \"[::1]:~repo\".\n>\n> Solution:\n> Don't use \"if (url != hostname)\", but look at the separator instead.\n> Rename the variable \"c\" into separator.\n> ---\n\nSign-off?\n\nThe above explanation sounds sensible.\n\nThanks.\n\n>  connect.c             | 14 +++++++-------\n>  t/t5500-fetch-pack.sh | 16 ++++------------\n>  t/t5601-clone.sh      | 12 ++----------\n>  3 files changed, 13 insertions(+), 29 deletions(-)\n>\n> diff --git a/connect.c b/connect.c\n> index 1b93b4d..0cb88b8 100644\n> --- a/connect.c\n> +++ b/connect.c\n> @@ -566,7 +566,7 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host,\n>  \tchar *url;\n>  \tchar *host, *path;\n>  \tchar *end;\n> -\tint c;\n> +\tint separator;\n>  \tenum protocol protocol = PROTO_LOCAL;\n>  \tint free_path = 0;\n>  \tchar *port = NULL;\n> @@ -581,10 +581,10 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host,\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\tseparator = ':';\n>  \t}\n>  \n>  \t/*\n> @@ -604,9 +604,9 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host,\n>  \t} else\n>  \t\tend = host;\n>  \n> -\tpath = strchr(end, c);\n> +\tpath = strchr(end, separator);\n>  \tif (path && !has_dos_drive_prefix(end)) {\n> -\t\tif (c == ':') {\n> +\t\tif (separator == ':') {\n>  \t\t\tif (host != url || path < strchrnul(host, '/')) {\n>  \t\t\t\tprotocol = PROTO_SSH;\n>  \t\t\t\t*path++ = '\\0';\n> @@ -623,7 +623,7 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host,\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> @@ -638,7 +638,7 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host,\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>  \t*ret_host = xstrdup(host);\n> diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh\n> index 7f55b95..ac5b08b 100755\n> --- a/t/t5500-fetch-pack.sh\n> +++ b/t/t5500-fetch-pack.sh\n> @@ -604,19 +604,11 @@ do\n>  \t\ttest_expect_success \"fetch-pack --diag-url $h:$r\" '\n>  \t\t\tcheck_prot_path $h:$r $p \"$r\"\n>  \t\t'\n> +\t\t# No \"/~\" -> \"~\" conversion\n> +\t\ttest_expect_success \"fetch-pack --diag-url $h:/~$r\" '\n> +\t\t\tcheck_prot_host_path $h:/~$r $p \"$hh\" \"/~$r\"\n> +\t\t'\n>  \tdone\n> -\th=host\n> -\thh=host\n> -\t# \"/~\" -> \"~\" conversion\n> -\ttest_expect_failure \"fetch-pack --diag-url $h:/~$r\" '\n> -\t\tcheck_prot_host_path $h:/~$r $p \"$hh\" \"~$r\"\n> -\t'\n> -\th=[::1]\n> -\thh=$(echo $h | tr -d \"[]\")\n> -\t# \"/~\" -> \"~\" conversion\n> -\ttest_expect_success \"fetch-pack --diag-url $h:/~$r\" '\n> -\t\tcheck_prot_host_path $h:/~$r $p \"$hh\" \"~$r\"\n> -\t'\n>  done\n>  \n>  test_expect_success MINGW 'fetch-pack --diag-url file://c:/repo' '\n> diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\n> index ba99972..57234c0 100755\n> --- a/t/t5601-clone.sh\n> +++ b/t/t5601-clone.sh\n> @@ -356,15 +356,7 @@ do\n>  done\n>  \n>  #ipv6\n> -# failing\n> -for repo in /~proj\n> -do\n> -\ttest_expect_failure \"clone [::1]:$repo\" '\n> -\t\ttest_clone_url [::1]:$repo ::1 $repo\n> -\t'\n> -done\n> -\n> -for repo in rep rep/home/project 123\n> +for repo in rep rep/home/project 123 /~proj\n>  do\n>  \ttest_expect_success \"clone [::1]:$repo\" '\n>  \t\ttest_clone_url [::1]:$repo ::1 $repo\n> @@ -373,7 +365,7 @@ done\n>  \n>  # Corner cases\n>  # failing\n> -for repo in [foo]bar/baz:qux [foo/bar]:baz\n> +for url in [foo]bar/baz:qux [foo/bar]:baz\n>  do\n>  \ttest_expect_failure \"clone $url is not ssh\" '\n>  \t\ttest_clone_url $url none\n"}]}