git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 07/10] connect.c: Corner case for IPv6
@ 2013-11-21 20:41 Torsten Bögershausen
  2013-11-21 23:31 ` Junio C Hamano
  0 siblings, 1 reply; 2+ messages in thread
From: Torsten Bögershausen @ 2013-11-21 20:41 UTC (permalink / raw)
  To: git; +Cc: tboegi

Commit faea9ccbadf75128 introduced "user-relative paths":
"ssh://host.xz/~junio/repo" is the same as "host.xz:~junio/repo".

Commit 356bece0a27258181 "Support [address] in URLs" introduced a regression:
When an URL like "[::1]:/~repo" is used, the replacement of "/~"
with "~" was enabled for IPv6 host names, and "[::1]:/~repo" is
converted into "[::1]:~repo".

Solution:
Don't use "if (url != hostname)", but look at the separator instead.
Rename the variable "c" into separator.
---
 connect.c             | 14 +++++++-------
 t/t5500-fetch-pack.sh | 16 ++++------------
 t/t5601-clone.sh      | 12 ++----------
 3 files changed, 13 insertions(+), 29 deletions(-)

diff --git a/connect.c b/connect.c
index 1b93b4d..0cb88b8 100644
--- a/connect.c
+++ b/connect.c
@@ -566,7 +566,7 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host,
 	char *url;
 	char *host, *path;
 	char *end;
-	int c;
+	int separator;
 	enum protocol protocol = PROTO_LOCAL;
 	int free_path = 0;
 	char *port = NULL;
@@ -581,10 +581,10 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host,
 		*host = '\0';
 		protocol = get_protocol(url);
 		host += 3;
-		c = '/';
+		separator = '/';
 	} else {
 		host = url;
-		c = ':';
+		separator = ':';
 	}
 
 	/*
@@ -604,9 +604,9 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host,
 	} else
 		end = host;
 
-	path = strchr(end, c);
+	path = strchr(end, separator);
 	if (path && !has_dos_drive_prefix(end)) {
-		if (c == ':') {
+		if (separator == ':') {
 			if (host != url || path < strchrnul(host, '/')) {
 				protocol = PROTO_SSH;
 				*path++ = '\0';
@@ -623,7 +623,7 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host,
 	 * 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++;
@@ -638,7 +638,7 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host,
 	/*
 	 * Add support for ssh port: ssh://host.xy:<port>/...
 	 */
-	if (protocol == PROTO_SSH && host != url)
+	if (protocol == PROTO_SSH && separator == '/')
 		port = get_port(end);
 
 	*ret_host = xstrdup(host);
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 7f55b95..ac5b08b 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -604,19 +604,11 @@ do
 		test_expect_success "fetch-pack --diag-url $h:$r" '
 			check_prot_path $h:$r $p "$r"
 		'
+		# No "/~" -> "~" conversion
+		test_expect_success "fetch-pack --diag-url $h:/~$r" '
+			check_prot_host_path $h:/~$r $p "$hh" "/~$r"
+		'
 	done
-	h=host
-	hh=host
-	# "/~" -> "~" conversion
-	test_expect_failure "fetch-pack --diag-url $h:/~$r" '
-		check_prot_host_path $h:/~$r $p "$hh" "~$r"
-	'
-	h=[::1]
-	hh=$(echo $h | tr -d "[]")
-	# "/~" -> "~" conversion
-	test_expect_success "fetch-pack --diag-url $h:/~$r" '
-		check_prot_host_path $h:/~$r $p "$hh" "~$r"
-	'
 done
 
 test_expect_success MINGW 'fetch-pack --diag-url file://c:/repo' '
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index ba99972..57234c0 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -356,15 +356,7 @@ do
 done
 
 #ipv6
-# failing
-for repo in /~proj
-do
-	test_expect_failure "clone [::1]:$repo" '
-		test_clone_url [::1]:$repo ::1 $repo
-	'
-done
-
-for repo in rep rep/home/project 123
+for repo in rep rep/home/project 123 /~proj
 do
 	test_expect_success "clone [::1]:$repo" '
 		test_clone_url [::1]:$repo ::1 $repo
@@ -373,7 +365,7 @@ done
 
 # Corner cases
 # failing
-for repo in [foo]bar/baz:qux [foo/bar]:baz
+for url in [foo]bar/baz:qux [foo/bar]:baz
 do
 	test_expect_failure "clone $url is not ssh" '
 		test_clone_url $url none
-- 
1.8.4.457.g424cb08

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v6 07/10] connect.c: Corner case for IPv6
  2013-11-21 20:41 [PATCH v6 07/10] connect.c: Corner case for IPv6 Torsten Bögershausen
@ 2013-11-21 23:31 ` Junio C Hamano
  0 siblings, 0 replies; 2+ messages in thread
From: Junio C Hamano @ 2013-11-21 23:31 UTC (permalink / raw)
  To: Torsten Bögershausen; +Cc: git

Torsten Bögershausen <tboegi@web.de> writes:

> Commit faea9ccbadf75128 introduced "user-relative paths":
> "ssh://host.xz/~junio/repo" is the same as "host.xz:~junio/repo".
>
> Commit 356bece0a27258181 "Support [address] in URLs" introduced a regression:
> When an URL like "[::1]:/~repo" is used, the replacement of "/~"
> with "~" was enabled for IPv6 host names, and "[::1]:/~repo" is
> converted into "[::1]:~repo".
>
> Solution:
> Don't use "if (url != hostname)", but look at the separator instead.
> Rename the variable "c" into separator.
> ---

Sign-off?

The above explanation sounds sensible.

Thanks.

>  connect.c             | 14 +++++++-------
>  t/t5500-fetch-pack.sh | 16 ++++------------
>  t/t5601-clone.sh      | 12 ++----------
>  3 files changed, 13 insertions(+), 29 deletions(-)
>
> diff --git a/connect.c b/connect.c
> index 1b93b4d..0cb88b8 100644
> --- a/connect.c
> +++ b/connect.c
> @@ -566,7 +566,7 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host,
>  	char *url;
>  	char *host, *path;
>  	char *end;
> -	int c;
> +	int separator;
>  	enum protocol protocol = PROTO_LOCAL;
>  	int free_path = 0;
>  	char *port = NULL;
> @@ -581,10 +581,10 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host,
>  		*host = '\0';
>  		protocol = get_protocol(url);
>  		host += 3;
> -		c = '/';
> +		separator = '/';
>  	} else {
>  		host = url;
> -		c = ':';
> +		separator = ':';
>  	}
>  
>  	/*
> @@ -604,9 +604,9 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host,
>  	} else
>  		end = host;
>  
> -	path = strchr(end, c);
> +	path = strchr(end, separator);
>  	if (path && !has_dos_drive_prefix(end)) {
> -		if (c == ':') {
> +		if (separator == ':') {
>  			if (host != url || path < strchrnul(host, '/')) {
>  				protocol = PROTO_SSH;
>  				*path++ = '\0';
> @@ -623,7 +623,7 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host,
>  	 * 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++;
> @@ -638,7 +638,7 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host,
>  	/*
>  	 * Add support for ssh port: ssh://host.xy:<port>/...
>  	 */
> -	if (protocol == PROTO_SSH && host != url)
> +	if (protocol == PROTO_SSH && separator == '/')
>  		port = get_port(end);
>  
>  	*ret_host = xstrdup(host);
> diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
> index 7f55b95..ac5b08b 100755
> --- a/t/t5500-fetch-pack.sh
> +++ b/t/t5500-fetch-pack.sh
> @@ -604,19 +604,11 @@ do
>  		test_expect_success "fetch-pack --diag-url $h:$r" '
>  			check_prot_path $h:$r $p "$r"
>  		'
> +		# No "/~" -> "~" conversion
> +		test_expect_success "fetch-pack --diag-url $h:/~$r" '
> +			check_prot_host_path $h:/~$r $p "$hh" "/~$r"
> +		'
>  	done
> -	h=host
> -	hh=host
> -	# "/~" -> "~" conversion
> -	test_expect_failure "fetch-pack --diag-url $h:/~$r" '
> -		check_prot_host_path $h:/~$r $p "$hh" "~$r"
> -	'
> -	h=[::1]
> -	hh=$(echo $h | tr -d "[]")
> -	# "/~" -> "~" conversion
> -	test_expect_success "fetch-pack --diag-url $h:/~$r" '
> -		check_prot_host_path $h:/~$r $p "$hh" "~$r"
> -	'
>  done
>  
>  test_expect_success MINGW 'fetch-pack --diag-url file://c:/repo' '
> diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
> index ba99972..57234c0 100755
> --- a/t/t5601-clone.sh
> +++ b/t/t5601-clone.sh
> @@ -356,15 +356,7 @@ do
>  done
>  
>  #ipv6
> -# failing
> -for repo in /~proj
> -do
> -	test_expect_failure "clone [::1]:$repo" '
> -		test_clone_url [::1]:$repo ::1 $repo
> -	'
> -done
> -
> -for repo in rep rep/home/project 123
> +for repo in rep rep/home/project 123 /~proj
>  do
>  	test_expect_success "clone [::1]:$repo" '
>  		test_clone_url [::1]:$repo ::1 $repo
> @@ -373,7 +365,7 @@ done
>  
>  # Corner cases
>  # failing
> -for repo in [foo]bar/baz:qux [foo/bar]:baz
> +for url in [foo]bar/baz:qux [foo/bar]:baz
>  do
>  	test_expect_failure "clone $url is not ssh" '
>  		test_clone_url $url none

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2013-11-21 23:31 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-21 20:41 [PATCH v6 07/10] connect.c: Corner case for IPv6 Torsten Bögershausen
2013-11-21 23:31 ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).