{"thread":{"id":"38404","subject":"[PATCH 1/3] connect.c: Improve parsing of literal IPV6 addresses","startedAt":"2015-01-19T17:21:24Z","lastAt":"2015-02-20T22:11:17Z","messageCount":9,"participants":["Torsten Bögershausen","brian m. carlson","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"254903","messageId":"54BD3D14.90309@web.de","threadId":"38404","inReplyTo":null,"subject":"[PATCH 1/3] connect.c: Improve parsing of literal IPV6 addresses","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2015-01-19T17:21:24Z","receivedAt":"2015-01-19T17:21:24Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"When parsing an URL, older Git versions did handle\nURLs like ssh://2001:db8::1/repo.git the same way as\nssh://[2001:db8::1]/repo.git\n\nCommit 83b058 broke the parsing of IPV6 adresses without \"[]\":\nIt was written in mind that the fist ':' in a URL was the beginning of a\nport number, while the old code was more clever:\nLiteral IPV6 addresses have always at least two ':'.\nWhen the \"hostandport\" had a ':' and the rest of the hostandport string\ncould be parsed into an integer between 0 and 65536, then it was used\nas a port number, like \"host:22\".\nOtherwise the first ':' was assumed to be part of a literal IPV6 address,\nlike \"ssh://[2001:db8::1]/repo.git\" or \"ssh://[::1]/repo.git\".\n\nRe-introduce the old parsing in get_host_and_port().\n\nImprove the parsing to handle URLs which have a user name and a literal\nIPV6 like \"ssh://user@[2001:db8::1]/repo.git\".\n(Thanks to Christian Taube for reporting this long\nstanding issue)\n\nAnother regression was introduced in 83b058:\nA non-RFC conform URL like \"[localhost:222]\" could be used in older versions\nof Git to connect to run \"ssh -p 222 localhost\".\nThe new stricter parsing did not allow this any more.\nSee even 8d3d28f5dba why \"[localhost:222]\" should be declared a feature.\n\nSigned-off-by: Torsten Bögershausen \n---\nUnfortunatly my attemps to improve connect.c introduced some\nregressions:\n- \"git clone ssh://::1/repo\" did not work anymore (for some reason I assumed\n that literall IPV6 addresses always should have brackets, but that was wrong)\n- \"ssh://host:2222/repo\" written in the \"unofficial short form\"\n \"[host:2222]:repo did not work anymore. I think that should be\n an \"unoffical feature\", because it worked in older Git versions\n\nOn top of that, the combination of ssh://username@[host] had never\nbe handled correctly, so fix that as well.\nThanks to Christian Taube for reporting it.\n\n\nComments and an extra pair of critical eyes are welcome.\n\n\n connect.c | 63 ++++++++++++++++++++++++++++++++++----------------------\n t/t5601-clone.sh | 2 +-\n 2 files changed, 39 insertions(+), 26 deletions(-)\n\ndiff --git a/connect.c b/connect.c\nindex d47d0ec..b608976 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -274,28 +274,44 @@ static enum protocol get_protocol(const char *name)\n \tdie(\"I don't handle protocol '%s'\", name);\n }\n \n+static char *host_end(char **hoststart, int removebrackets)\n+{\n+\tchar *host = *hoststart;\n+\tchar *end;\n+\tchar *start = strstr(host, \"@[\");\n+\tif (start)\n+\t\tstart++; /* Jump over '@' */\n+\telse\n+\t\tstart = host;\n+\tif (start[0] == '[') {\n+\t\tend = strchr(start + 1, ']');\n+\t\tif (end) {\n+\t\t\tif (removebrackets) {\n+\t\t\t\t*end = 0;\n+\t\t\t\tmemmove(start, start + 1, end - start);\n+\t\t\t\tend++;\n+\t\t\t}\n+\t\t} else\n+\t\t\tend = host;\n+\t} else\n+\t\tend = host;\n+\treturn end;\n+}\n+\n #define STR_(s)\t# s\n #define STR(s)\tSTR_(s)\n \n static void get_host_and_port(char **host, const char **port)\n {\n \tchar *colon, *end;\n-\n-\tif (*host[0] == '[') {\n-\t\tend = strchr(*host + 1, ']');\n-\t\tif (end) {\n-\t\t\t*end = 0;\n-\t\t\tend++;\n-\t\t\t(*host)++;\n-\t\t} else\n-\t\t\tend = *host;\n-\t} else\n-\t\tend = *host;\n+\tend = host_end(host, 1);\n \tcolon = strchr(end, ':');\n-\n \tif (colon) {\n-\t\t*colon = 0;\n-\t\t*port = colon + 1;\n+\t\tlong portnr = strtol(colon + 1, &end, 10);\n+\t\tif (end != colon + 1 && *end == '\\0' && 0 <= portnr && portnr < 65536) {\n+\t\t\t*colon = 0;\n+\t\t\t*port = colon + 1;\n+\t\t}\n \t}\n }\n \n@@ -547,13 +563,16 @@ static struct child_process *git_proxy_connect(int fd[2], char *host)\n \treturn proxy;\n }\n \n-static const char *get_port_numeric(const char *p)\n+static char *get_port(char *host)\n {\n \tchar *end;\n+\tchar *p = strchr(host, ':');\n+\n \tif (p) {\n \t\tlong port = strtol(p + 1, &end, 10);\n \t\tif (end != p + 1 && *end == '\\0' && 0 <= port && port < 65536) {\n-\t\t\treturn p;\n+\t\t\t*p = '\\0';\n+\t\t\treturn p+1;\n \t\t}\n \t}\n \n@@ -595,14 +614,7 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host,\n \t * Don't do destructive transforms as protocol code does\n \t * '[]' unwrapping in get_host_and_port()\n \t */\n-\tif (host[0] == '[') {\n-\t\tend = strchr(host + 1, ']');\n-\t\tif (end) {\n-\t\t\tend++;\n-\t\t} else\n-\t\t\tend = host;\n-\t} else\n-\t\tend = host;\n+\tend = host_end(&host, 0);\n \n \tif (protocol == PROTO_LOCAL)\n \t\tpath = end;\n@@ -705,7 +717,8 @@ struct child_process *git_connect(int fd[2], const char *url,\n \t\t\tchar *ssh_host = hostandport;\n \t\t\tconst char *port = NULL;\n \t\t\tget_host_and_port(&ssh_host, &port);\n-\t\t\tport = get_port_numeric(port);\n+\t\t\tif (!port)\n+\t\t\t\tport = get_port(ssh_host);\n \n \t\t\tif (!ssh) ssh = \"ssh\";\n \ndiff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\nindex e4f10c0..f901b8a 100755\n--- a/t/t5601-clone.sh\n+++ b/t/t5601-clone.sh\n@@ -326,7 +326,7 @@ test_expect_success !MINGW,!CYGWIN 'clone local path foo:bar' '\n \n test_expect_success 'bracketed hostnames are still ssh' '\n \tgit clone \"[myhost:123]:src\" ssh-bracket-clone &&\n-\texpect_ssh myhost:123 src\n+\texpect_ssh myhost '-p 123' src\n '\n \n counter=0\n-- \n2.2.0.rc1.26.g3e3a70d\n"},{"id":"255125","messageId":"20150122200702.GA96498@vauxhall.crustytoothpaste.net","threadId":"38404","inReplyTo":"54BD3D14.90309@web.de","subject":"Re: [PATCH 1/3] connect.c: Improve parsing of literal IPV6 addresses","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2015-01-22T20:07:03Z","receivedAt":"2015-01-22T20:07:03Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Mon, Jan 19, 2015 at 06:21:24PM +0100, Torsten Bögershausen wrote:\n>When parsing an URL, older Git versions did handle\n>URLs like ssh://2001:db8::1/repo.git the same way as\n>ssh://[2001:db8::1]/repo.git\n>\n>Commit 83b058 broke the parsing of IPV6 adresses without \"[]\":\n>It was written in mind that the fist ':' in a URL was the beginning of a\n>port number, while the old code was more clever:\n>Literal IPV6 addresses have always at least two ':'.\n>When the \"hostandport\" had a ':' and the rest of the hostandport string\n>could be parsed into an integer between 0 and 65536, then it was used\n>as a port number, like \"host:22\".\n>Otherwise the first ':' was assumed to be part of a literal IPV6 address,\n>like \"ssh://[2001:db8::1]/repo.git\" or \"ssh://[::1]/repo.git\".\n>\n>Re-introduce the old parsing in get_host_and_port().\n>\n>Improve the parsing to handle URLs which have a user name and a literal\n>IPV6 like \"ssh://user@[2001:db8::1]/repo.git\".\n>(Thanks to Christian Taube for reporting this long\n>standing issue)\n>\n>Another regression was introduced in 83b058:\n>A non-RFC conform URL like \"[localhost:222]\" could be used in older versions\n>of Git to connect to run \"ssh -p 222 localhost\".\n>The new stricter parsing did not allow this any more.\n>See even 8d3d28f5dba why \"[localhost:222]\" should be declared a feature.\n\nI'm not sure this is a very good idea. While this may have worked in \nthe past, it's also completely inconsistent with the way all non-SSH \nURLs work in Git:\n\n vauxhall ok % git push https://bmc@2001:470:1f05:79::1/git/bmc/homedir.git master\n fatal: unable to access 'https://bmc@2001:470:1f05:79::1/git/bmc/homedir.git/': IPv6 numerical address used in URL without brackets\n\n vauxhall no % git push https://bmc@[castro.crustytoothpaste.net]/git/bmc/homedir.git master\n fatal: unable to access 'https://bmc@[castro.crustytoothpaste.net]/git/bmc/homedir.git/': Could not resolve host: [castro.crustytoothpaste.net]\n\n vauxhall no % git push https://bmc@[castro.crustytoothpaste.net:443]/git/bmc/homedir.git master\n fatal: unable to access 'https://bmc@[castro.crustytoothpaste.net:443]/git/bmc/homedir.git/': Could not resolve host: [castro.crustytoothpaste.net\n\nI would argue that people using IPv6 literals in URLs should already \nknow how to do it correctly, and the consistency between SSH and HTTPS \nURLs is a feature, not a bug. In the non-URL SSH form, you still have \nto use the bracketed form to deal with the case in which somebody \ncreates a directory called \"1\" in their home directory and writes:\n\n git push 2001:470:1f05:79::1:1 master \n\nwhen they mean\n\n git push [2001:470:1f05:79::1]:1 master\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187\n"},{"id":"255138","messageId":"54C17429.1090403@web.de","threadId":"38404","inReplyTo":"20150122200702.GA96498@vauxhall.crustytoothpaste.net","subject":"Re: [PATCH 1/3] connect.c: Improve parsing of literal IPV6 addresses","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2015-01-22T22:05:29Z","receivedAt":"2015-01-22T22:05:29Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 2015-01-22 21.07, brian m. carlson wrote:\n> On Mon, Jan 19, 2015 at 06:21:24PM +0100, Torsten Bögershausen wrote:\n>> When parsing an URL, older Git versions did handle\n>> URLs like ssh://2001:db8::1/repo.git the same way as\n>> ssh://[2001:db8::1]/repo.git\n>>\n>> Commit 83b058 broke the parsing of IPV6 adresses without \"[]\":\n>> It was written in mind that the fist ':' in a URL was the beginning of a\n>> port number, while the old code was more clever:\n>> Literal IPV6 addresses have always at least two ':'.\n>> When the \"hostandport\" had a ':' and the rest of the hostandport string\n>> could be parsed into an integer between 0 and 65536, then it was used\n>> as a port number, like \"host:22\".\n>> Otherwise the first ':' was assumed to be part of a literal IPV6 address,\n>> like \"ssh://[2001:db8::1]/repo.git\" or \"ssh://[::1]/repo.git\".\n>>\n>> Re-introduce the old parsing in get_host_and_port().\n>>\n>> Improve the parsing to handle URLs which have a user name and a literal\n>> IPV6 like \"ssh://user@[2001:db8::1]/repo.git\".\n>> (Thanks to Christian Taube for reporting this long\n>> standing issue)\n>>\n>> Another regression was introduced in 83b058:\n>> A non-RFC conform URL like \"[localhost:222]\" could be used in older versions\n>> of Git to connect to run \"ssh -p 222 localhost\".\n>> The new stricter parsing did not allow this any more.\n>> See even 8d3d28f5dba why \"[localhost:222]\" should be declared a feature.\n> \n> I'm not sure this is a very good idea. While this may have worked in the past, it's also completely inconsistent with the way all non-SSH URLs work in Git:\n> \n> vauxhall ok % git push https://bmc@2001:470:1f05:79::1/git/bmc/homedir.git master\n> fatal: unable to access 'https://bmc@2001:470:1f05:79::1/git/bmc/homedir.git/': IPv6 numerical address used in URL without brackets\n> \n> vauxhall no % git push https://bmc@[castro.crustytoothpaste.net]/git/bmc/homedir.git master\n> fatal: unable to access 'https://bmc@[castro.crustytoothpaste.net]/git/bmc/homedir.git/': Could not resolve host: [castro.crustytoothpaste.net]\n> \n> vauxhall no % git push https://bmc@[castro.crustytoothpaste.net:443]/git/bmc/homedir.git master\n> fatal: unable to access 'https://bmc@[castro.crustytoothpaste.net:443]/git/bmc/homedir.git/': Could not resolve host: [castro.crustytoothpaste.net\n> \n> I would argue that people using IPv6 literals in URLs should already know how to do it correctly, and the consistency between SSH and HTTPS URLs is a feature, not a bug. In the non-URL SSH form, you still have to use the bracketed form to deal with the case in which somebody creates a directory called \"1\" in their home directory and writes:\n> \nWe want to support ssh://bmc@2001:470:1f05:79::1/git/bmc/homedir.git/\n because e.g. the Git shipped with Debian (1.7.10.4) (and a lot of other installations) supports it.\n\nWe want to support ssh://bmc@[2001:470:1f05:79::1]/git/bmc/homedir.git/\n because that is what other people may expect to work as well:\n ssh://bmc@[2001:470:1f05:79::1]:4444/git/bmc/homedir.git/\n \n> git push 2001:470:1f05:79::1:1 master\n> when they mean\n> \n> git push [2001:470:1f05:79::1]:1 master\nThat I don't understand this, where is the path name in your example ?\n\nEverything after the first ':' is the path in the short form:\nbmc@hostxx:/git/bmc/homedir.git/\n\nIf you really want to use a literal IPV6 with the short form, you must use the brackets:\nbmc@[2001:470:1f05:79::1]:/git/bmc/homedir.git/\n(And you can not have a port number here)\n\n\nNobody forces somebody to use any specific form.\n"},{"id":"255155","messageId":"20150122234117.GD96498@vauxhall.crustytoothpaste.net","threadId":"38404","inReplyTo":"54C17429.1090403@web.de","subject":"Re: [PATCH 1/3] connect.c: Improve parsing of literal IPV6 addresses","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2015-01-22T23:41:17Z","receivedAt":"2015-01-22T23:41:17Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Thu, Jan 22, 2015 at 11:05:29PM +0100, Torsten Bögershausen wrote:\n>We want to support ssh://bmc@2001:470:1f05:79::1/git/bmc/homedir.git/\n> because e.g. the Git shipped with Debian (1.7.10.4) (and a lot of other installations) supports it.\n\nI understand that this used to work, but it probably shouldn't have ever \nbeen accepted. It's nonstandard, and if we accept it for ssh, people \nwill want it to work for https, and due to libcurl, it simply won't.\n\nI prefer to see our past acceptance of this format as a bug. This is \nthe first that I've heard of anyone noticing this (since 2013), so it \ncan't be in common usage.\n\nIf we accept it, we should explicitly document it as being deprecated and \nnote that it's inconsistent with the way everything else works.\n\n>We want to support ssh://bmc@[2001:470:1f05:79::1]/git/bmc/homedir.git/\n> because that is what other people may expect to work as well:\n> ssh://bmc@[2001:470:1f05:79::1]:4444/git/bmc/homedir.git/\n\nEveryone expects this to work properly, because it's the standard URL \nform (RFC 2732). I agree we should support it.\n\n>> git push 2001:470:1f05:79::1:1 master\n>> when they mean\n>>\n>> git push [2001:470:1f05:79::1]:1 master\n>That I don't understand this, where is the path name in your example ?\n\nThe path in question is $HOME/1. That's why the bracket notation is \nobligatory in the short form. I agree it's a bit bizarre.\n\n>Everything after the first ':' is the path in the short form:\n>bmc@hostxx:/git/bmc/homedir.git/\n>\n>If you really want to use a literal IPV6 with the short form, you must use the brackets:\n>bmc@[2001:470:1f05:79::1]:/git/bmc/homedir.git/\n>(And you can not have a port number here)\n\nRight. In my experience, nobody uses the ssh:// form unless they have \nto (i.e. they need to use a port number); it's extremely uncommon. So \nthey've already become used to using the bracketed notation, because \nit's already required for the usual form and it's required in the IPv6 \nURL standard.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187\n"},{"id":"256290","messageId":"xmqqtwyj2hdk.fsf@gitster.dls.corp.google.com","threadId":"38404","inReplyTo":"20150122234117.GD96498@vauxhall.crustytoothpaste.net","subject":"Re: [PATCH 1/3] connect.c: Improve parsing of literal IPV6 addresses","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-18T18:40:07Z","receivedAt":"2015-02-18T18:40:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> On Thu, Jan 22, 2015 at 11:05:29PM +0100, Torsten Bögershausen wrote:\n>>We want to support ssh://bmc@2001:470:1f05:79::1/git/bmc/homedir.git/\n>> because e.g. the Git shipped with Debian (1.7.10.4) (and a lot of\n>> other installations) supports it.\n>\n> I understand that this used to work, but it probably shouldn't have\n> ever been accepted. It's nonstandard, and if we accept it for ssh,\n> people will want it to work for https, and due to libcurl, it simply\n> won't.\n>\n> I prefer to see our past acceptance of this format as a bug. This is\n> the first that I've heard of anyone noticing this (since 2013), so it\n> can't be in common usage.\n>\n> If we accept it, we should explicitly document it as being deprecated\n> and note that it's inconsistent with the way everything else works.\n\nI was reviewing my Undecided pile today, and I think your objection\nmakes sense.\n\nEither of you care to update documentation, please, before I drop\nthis series and forget about it?\n\nThanks.\n"},{"id":"256354","messageId":"54E61285.5070600@web.de","threadId":"38404","inReplyTo":"xmqqtwyj2hdk.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 1/3] connect.c: Improve parsing of literal IPV6 addresses","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2015-02-19T16:42:45Z","receivedAt":"2015-02-19T16:42:45Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 02/18/2015 07:40 PM, Junio C Hamano wrote:\n> \"brian m. carlson\" writes:\n>\n>> On Thu, Jan 22, 2015 at 11:05:29PM +0100, Torsten Bögershausen wrote:\n>>> We want to support ssh://bmc@2001:470:1f05:79::1/git/bmc/homedir.git/\n>>> because e.g. the Git shipped with Debian (1.7.10.4) (and a lot of\n>>> other installations) supports it.\n>> I understand that this used to work, but it probably shouldn't have\n>> ever been accepted. It's nonstandard, and if we accept it for ssh,\n>> people will want it to work for https, and due to libcurl, it simply\n>> won't.\n>>\n>> I prefer to see our past acceptance of this format as a bug. This is\n>> the first that I've heard of anyone noticing this (since 2013), so it\n>> can't be in common usage.\n>>\n>> If we accept it, we should explicitly document it as being deprecated\n>> and note that it's inconsistent with the way everything else works.\n> I was reviewing my Undecided pile today, and I think your objection\n> makes sense.\n>\n> Either of you care to update documentation, please, before I drop\n> this series and forget about it?\nThe URL RFC is much stricter regarding which characters that are allowed\nin which part of the URL, as least as I read it.\n\nThe \"problem\" started when /usr/bin/ssh excepted things like\n/usr/bin/ssh fe80:x:y:z%eth0 and Git simply passed the hostname\nto ssh.\n\nAnd when the [] was there, it was stripped because ssh doesn't like them.\nURLs like\n\nssh://bmc@2001:470:1f05:79::1/git/bmc/homedir.git/\n\nsimply worked, and nobody ever complained about this,\n(until now), Git never rejected IPV6 URLs without [], please correct me if\nI'm wrong.\n\nGit never cared about the exact URL, so that IPV6 URL's without [] where allowed\nfrom \"day one\".\n\nOn top of that, we support the short form,\nuser@host:~ or other variants.\nBut we never claimed to be compatible to RFC 1738, even if it makes sense to do so.\n\nWhat exactly should we write in the documentation ?\n\nGit supports RFC1738 (but is not as strict in parsing the URL, because\nwe assume that the host name resolver will do some checking for us.\n\nGit currently does not support user@[fe80::x:y:z], even if RFC suggests it\n\nGit never claimed to be 100% compatible to RFC 1738, and will\nprobably never be, (as we have old code that is as it is).\n\nWe (at least I) don't want to break existing repos, rejecting URL's that had been\nworking before and stopped working because the Git version is updated or so)\n\nThis patch series is attempting to be backwards compatible to what\nold, older. and oldest versions of Git accepted.\n\nAt the price that we accept URL's which do not conform to the RFC are accepted.\nIt fixes the long standing issue that user@[fe80:] did not work.\n\nI'm somewhat unsure what to write in the documentation, I must admit.\n\nUnfortunately URL parsing is a tricky thing, this patch tries to do improvements.\nEspecially it adds test cases, which are good to prevent further breakage.\n \nUpdating the documentation was never part of the patch series,\nand if the documentation is updated, this is done in a separate commit anyway.\n\nHow much does this series qualify for the \"we didn't update the docs\",\nbut fixed the code, let's drop it ?\n \n\n\n \n"},{"id":"256357","messageId":"xmqq386123df.fsf@gitster.dls.corp.google.com","threadId":"38404","inReplyTo":"54E61285.5070600@web.de","subject":"Re: [PATCH 1/3] connect.c: Improve parsing of literal IPV6 addresses","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-19T17:54:52Z","receivedAt":"2015-02-19T17:54:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen writes:\n\n> On 02/18/2015 07:40 PM, Junio C Hamano wrote:\n>> \"brian m. carlson\" writes:\n>>\n>>> I understand that this used to work, but it probably shouldn't have\n>>> ever been accepted. It's nonstandard, and if we accept it for ssh,\n>>> people will want it to work for https, and due to libcurl, it simply\n>>> won't.\n>>>\n>>> I prefer to see our past acceptance of this format as a bug. This is\n>>> the first that I've heard of anyone noticing this (since 2013), so it\n>>> can't be in common usage.\n>>>\n>>> If we accept it, we should explicitly document it as being deprecated\n>>> and note that it's inconsistent with the way everything else works.\n>> I was reviewing my Undecided pile today, and I think your objection\n>> makes sense.\n>>\n>> Either of you care to update documentation, please, before I drop\n>> this series and forget about it?\n>\n> The URL RFC is much stricter regarding which characters that are allowed\n> in which part of the URL, as least as I read it.\n> ...\n> I'm somewhat unsure what to write in the documentation, I must admit.\n\nI can see that you do not agree with the \"If we accept it\" part\n(where \"it\" refers to \"allowing [...] was a bug.\")---past acceptance\nwas not a bug for you.\n\nBrian is for that \"If we accept it\", and sees it as a bug.\n\nSo let's see what he comes up with as a follow-up to the \"we should\nexplicitly document it\" part.\n"},{"id":"256362","messageId":"20150219194050.GA5021@vauxhall.crustytoothpaste.net","threadId":"38404","inReplyTo":"xmqq386123df.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 1/3] connect.c: Improve parsing of literal IPV6 addresses","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2015-02-19T19:40:50Z","receivedAt":"2015-02-19T19:40:50Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Thu, Feb 19, 2015 at 09:54:52AM -0800, Junio C Hamano wrote:\n>I can see that you do not agree with the \"If we accept it\" part\n>(where \"it\" refers to \"allowing [...] was a bug.\")---past acceptance\n>was not a bug for you.\n>\n>Brian is for that \"If we accept it\", and sees it as a bug.\n>\n>So let's see what he comes up with as a follow-up to the \"we should\n>explicitly document it\" part.\n\nHere's what I propose:\n\n-- >8 --\nSubject: [PATCH] Documentation: note deprecated syntax for IPv6 SSH URLs\n\nWe have historically accepted some invalid syntax for SSH URLs\ncontaining IPv6 literals. Older versions of Git accepted URLs missing\nthe brackets required by RFC 2732. Note that this behavior is\ndeprecated and that other protocol handlers will not accept this syntax.\n\nSigned-off-by: brian m. carlson \n---\n Documentation/urls.txt | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/Documentation/urls.txt b/Documentation/urls.txt\nindex 9ccb246..2c1a84f 100644\n--- a/Documentation/urls.txt\n+++ b/Documentation/urls.txt\n@@ -38,6 +38,10 @@ The ssh and git protocols additionally support ~username expansion:\n - git://host.xz{startsb}:port{endsb}/~{startsb}user{endsb}/path/to/repo.git/\n - {startsb}user@{endsb}host.xz:/~{startsb}user{endsb}/path/to/repo.git/\n \n+For backwards compatibility reasons, Git, when using ssh URLs, accepts\n+some URLs containing IPv6 literals that are missing the brackets. This\n+syntax is deprecated, and other protocol handlers do not permit this.\n+\n For local repositories, also supported by Git natively, the following\n syntaxes may be used:\n \n-- \n2.2.1.209.g41e5f3a\n"},{"id":"256441","messageId":"54E7B105.8020100@web.de","threadId":"38404","inReplyTo":"20150219194050.GA5021@vauxhall.crustytoothpaste.net","subject":"Re: [PATCH 1/3] connect.c: Improve parsing of literal IPV6 addresses","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2015-02-20T22:11:17Z","receivedAt":"2015-02-20T22:11:17Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 2015-02-19 20.40, brian m. carlson wrote:\n> On Thu, Feb 19, 2015 at 09:54:52AM -0800, Junio C Hamano wrote:\n>> I can see that you do not agree with the \"If we accept it\" part\n>> (where \"it\" refers to \"allowing [...] was a bug.\")---past acceptance\n>> was not a bug for you.\nDo we talk about the same thing here ?\n\nThe support for the ssh://host/path was introduced 2005, I think here:\ncommit 2386d65822c912f0889ac600b1698b0659190133\nAuthor: Linus Torvalds \nDate: Wed Jul 13 18:46:20 2005 -0700\n\n Add first cut at \"git protocol\" connect logic.\n------------\n\nIt happily accepted everything for host, including \n ssh://bmc@2001:470:1f05:79::1/git/bmc/homedir.git/\n\nAnd this was the only way to connect to a server using a literal IPV6\naddress, the support for [] came in later.\n\nToday, in 2015, we can declare this syntax as deprecated, no problem.\n\nThe parser we have in git.master does not handle URLs like\nssh://bmc@[2001:470:1f05:79::1]/git/bmc/homedir.git/ correctly.\n\nInstead of this,\nssh://[bmc@2001:470:1f05:79::1]/git/bmc/homedir.git/\nneeds to be used, and this is the main purpose of the series.\n(If we ignore updates of the test cases, which I think are good\nto prevent regressions)\n\n\nI could probably shorten the commit message of [1/3] to read like this: \n\n Improve the parsing to handle URLs which have a user name and a literal\n IPV6 like \"ssh://user@[2001:db8::1]/repo.git\".\n (Thanks to Christian Taube for reporting this long\n standing issue)\n\n\n>>\n>> Brian is for that \"If we accept it\", and sees it as a bug.\n>>\n>> So let's see what he comes up with as a follow-up to the \"we should\n>> explicitly document it\" part.\n> \n> Here's what I propose:\n> \n> -- >8 --\n> Subject: [PATCH] Documentation: note deprecated syntax for IPv6 SSH URLs\n> \n> We have historically accepted some invalid syntax for SSH URLs\n> containing IPv6 literals. Older versions of Git accepted URLs missing\n> the brackets required by RFC 2732. Note that this behavior is\n> deprecated and that other protocol handlers will not accept this syntax.\n> \n> Signed-off-by: brian m. carlson \n> ---\n> Documentation/urls.txt | 4 ++++\n> 1 file changed, 4 insertions(+)\n> \n> diff --git a/Documentation/urls.txt b/Documentation/urls.txt\n> index 9ccb246..2c1a84f 100644\n> --- a/Documentation/urls.txt\n> +++ b/Documentation/urls.txt\n> @@ -38,6 +38,10 @@ The ssh and git protocols additionally support ~username expansion:\n> - git://host.xz{startsb}:port{endsb}/~{startsb}user{endsb}/path/to/repo.git/\n> - {startsb}user@{endsb}host.xz:/~{startsb}user{endsb}/path/to/repo.git/\n> \n> +For backwards compatibility reasons, Git, when using ssh URLs, accepts\n> +some URLs containing IPv6 literals that are missing the brackets. This\n> +syntax is deprecated, and other protocol handlers do not permit this.\n> +\n> For local repositories, also supported by Git natively, the following\n> syntaxes may be used:\n> \n"}]}