Re: [PATCH v2 1/3] connect.c: allow ssh://user@[2001:db8::1]/repo.git
Torsten Bögershausen tbo...@web.de writes: The ssh:// syntax was added in 2386d65822c91, it accepted ssh://user@2001:db8::1/repo.git, which is now legacy. Please do not quote just a raw object name, but annotate it so that we can more easily tell what the change was about and how far back in the history it was done without having to run git show -s on it. Over the years the parser was improved to support [] and port numbers, but the combination of ssh://user@[2001:db8::1]:222/repo.git did never work. The only only way to use a user name, a literall IPV6 address and a port number was ssh://[user@2001:db8::1]:222/repo.git Let me double check if I understand Brian and the above correctly: * The original 2386d658 (Add first cut at git protocol connect logic., 2005-07-13) did not even care that the IPv6 syntax was valid or not, and worse yet, it did not correctly handle literal addresses in brackets; * Later we started supporting literal addresses in brackets, but user@[literal]:port form did not work. Instead, a bug in the parser allowed [user@literal]:port (which is not a valid way to spell such things) to work as if it were user@[literal]:port, which is what users would expect. * This three-patch series is an attempt to allow that kosher syntax, user@[literal]:port, to work. * It does not terribly matter if we broke the invalid syntax user@literal:port without brackets, and it might even be beneficial if we declared such addresses invalid and deprecated for the sake of uniformity with other protocols. Making that judgement, however, is outside the scope of this series. user@literal:port will continue to behave as it did before. Is that what is going on? If that is the case, I think it addresses Brian's concern well. Thanks. (I haven't checked the patch text itself yet, though). -- 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
Re: [PATCH v2 1/3] connect.c: allow ssh://user@[2001:db8::1]/repo.git
(Sorry for the spam, a few things need correction already now, and forgot cc: Brian) On 2015-02-21 16.52, Torsten Bögershausen wrote: The ssh:// syntax was added in 2386d65822c91, it accepted ssh://user@2001:db8::1/repo.git, which is now legacy. Over the years the parser was improved to support [] and port numbers, but the combination of ssh://user@[2001:db8::1]:222/repo.git did never work. s/did never work/was never implemented/ The only only way to use a user name, a literall IPV6 address and a port s/literall/literal/ number was ssh://[user@2001:db8::1]:222/repo.git (Thanks to Christian Taube li...@hcf.yourweb.de for reporting this long standing issue) New users would use ssh://user@[2001:db8::1]:222/repo.git, so change the parser to handle it correctly. Support the old legacy URL's as well, to be backwards compatible, s/URL's/URLs/ -- 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
Re: [PATCH v2 1/3] connect.c: allow ssh://user@[2001:db8::1]/repo.git
On Sun, Feb 22, 2015 at 11:37:40AM -0800, Junio C Hamano wrote: Let me double check if I understand Brian and the above correctly: * The original 2386d658 (Add first cut at git protocol connect logic., 2005-07-13) did not even care that the IPv6 syntax was valid or not, and worse yet, it did not correctly handle literal addresses in brackets; * Later we started supporting literal addresses in brackets, but user@[literal]:port form did not work. Instead, a bug in the parser allowed [user@literal]:port (which is not a valid way to spell such things) to work as if it were user@[literal]:port, which is what users would expect. * This three-patch series is an attempt to allow that kosher syntax, user@[literal]:port, to work. Yes. My overwhelming concern is that this work, because this is the syntax we want people to use and that they will expect to work based on the standards, other software, and Git using HTTPS. * It does not terribly matter if we broke the invalid syntax user@literal:port without brackets, and it might even be beneficial if we declared such addresses invalid and deprecated for the sake of uniformity with other protocols. Making that judgement, however, is outside the scope of this series. user@literal:port will continue to behave as it did before. That's an accurate assessment of my position. Is that what is going on? If that is the case, I think it addresses Brian's concern well. Yes, if that's the case, it addresses my concern. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
[PATCH v2 1/3] connect.c: allow ssh://user@[2001:db8::1]/repo.git
The ssh:// syntax was added in 2386d65822c91, it accepted
ssh://user@2001:db8::1/repo.git, which is now legacy.
Over the years the parser was improved to support [] and port numbers,
but the combination of ssh://user@[2001:db8::1]:222/repo.git did
never work.
The only only way to use a user name, a literall IPV6 address and a port
number was ssh://[user@2001:db8::1]:222/repo.git
(Thanks to Christian Taube li...@hcf.yourweb.de for reporting this long
standing issue)
New users would use ssh://user@[2001:db8::1]:222/repo.git,
so change the parser to handle it correctly.
Support the old legacy URL's as well, to be backwards compatible,
and avoid regressions for users which upgrade an existing installation
to a later Git version.
Signed-off-by: Torsten Bögershausen tbo...@web.de
---
Thanks for the reviews
I hope the intention of being backward compatible is a little bit clearer now,
as well as the intention to accept URL's conforming to the RFC
connect.c| 63 ++--
t/t5601-clone.sh | 2 +-
2 files changed, 39 insertions(+), 26 deletions(-)
diff --git a/connect.c b/connect.c
index d47d0ec..b608976 100644
--- a/connect.c
+++ b/connect.c
@@ -274,28 +274,44 @@ static enum protocol get_protocol(const char *name)
die(I don't handle protocol '%s', name);
}
+static char *host_end(char **hoststart, int removebrackets)
+{
+ char *host = *hoststart;
+ char *end;
+ char *start = strstr(host, @[);
+ if (start)
+ start++; /* Jump over '@' */
+ else
+ start = host;
+ if (start[0] == '[') {
+ end = strchr(start + 1, ']');
+ if (end) {
+ if (removebrackets) {
+ *end = 0;
+ memmove(start, start + 1, end - start);
+ end++;
+ }
+ } else
+ end = host;
+ } else
+ end = host;
+ return end;
+}
+
#define STR_(s)# s
#define STR(s) STR_(s)
static void get_host_and_port(char **host, const char **port)
{
char *colon, *end;
-
- if (*host[0] == '[') {
- end = strchr(*host + 1, ']');
- if (end) {
- *end = 0;
- end++;
- (*host)++;
- } else
- end = *host;
- } else
- end = *host;
+ end = host_end(host, 1);
colon = strchr(end, ':');
-
if (colon) {
- *colon = 0;
- *port = colon + 1;
+ long portnr = strtol(colon + 1, end, 10);
+ if (end != colon + 1 *end == '\0' 0 = portnr portnr
65536) {
+ *colon = 0;
+ *port = colon + 1;
+ }
}
}
@@ -547,13 +563,16 @@ static struct child_process *git_proxy_connect(int fd[2],
char *host)
return proxy;
}
-static const char *get_port_numeric(const char *p)
+static char *get_port(char *host)
{
char *end;
+ char *p = strchr(host, ':');
+
if (p) {
long port = strtol(p + 1, end, 10);
if (end != p + 1 *end == '\0' 0 = port port 65536) {
- return p;
+ *p = '\0';
+ return p+1;
}
}
@@ -595,14 +614,7 @@ static enum protocol parse_connect_url(const char
*url_orig, char **ret_host,
* Don't do destructive transforms as protocol code does
* '[]' unwrapping in get_host_and_port()
*/
- if (host[0] == '[') {
- end = strchr(host + 1, ']');
- if (end) {
- end++;
- } else
- end = host;
- } else
- end = host;
+ end = host_end(host, 0);
if (protocol == PROTO_LOCAL)
path = end;
@@ -705,7 +717,8 @@ struct child_process *git_connect(int fd[2], const char
*url,
char *ssh_host = hostandport;
const char *port = NULL;
get_host_and_port(ssh_host, port);
- port = get_port_numeric(port);
+ if (!port)
+ port = get_port(ssh_host);
if (!ssh) ssh = ssh;
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index e4f10c0..f901b8a 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -326,7 +326,7 @@ test_expect_success !MINGW,!CYGWIN 'clone local path
foo:bar' '
test_expect_success 'bracketed hostnames are still ssh' '
git clone [myhost:123]:src ssh-bracket-clone
- expect_ssh myhost:123 src
+ expect_ssh myhost '-p 123' src
'
counter=0
--
2.2.0.rc1.790.ge19fcd2
--
To unsubscribe from this list: send the line unsubscribe git in
the body
