connect.c: Ignore extra colon after hostname

PATCH/RFC Torsten Bögershausen 2 participants 2 messages Jun 15, 2016

Share & tools

Torsten Bögershausen

author

2016-06-15 23:04 UTC #0 lore raw

Ignore an extra ':' at the end of the hostname in URL's like "ssh://example.com:/path/to/repo"

The colon is ment to separate a port number from the hostname. If the port is empty, the colon should be ignored, see RFC 3986.

It had been working for URLs with ssh:// scheme, but was unintentionally broken in 86ceb3, "allow ssh://user@[2001:db8::1]/repo.git"

Reported-by Reid Woodbury Jr. <[email protected]>

Signed-off-by Torsten Bögershausen <[email protected]>

---
Thanks everybody else for comments,
I was especially not aware about the very details in RFC 3986, and now we should have some TC covering the "now unbroken".

 connect.c             |  2 ++
 t/t5500-fetch-pack.sh | 17 ++++++++++-------
 t/t5601-clone.sh      | 22 +++++++++++++---------
 3 files changed, 25 insertions(+), 16 deletions(-)

connect.c +2 -0

index ce0e121..14c924b 100644
@@ -310,6 +310,8 @@ static void get_host_and_port(char **host, const char **port)
 		if (end != colon + 1 && *end == '\0' && 0 <= portnr && portnr < 65536) {
 			*colon = 0;
 			*port = colon + 1;
+		} else if (!colon[1]) {
+			*colon = 0;
 		}
 	}
 }

t/t5500-fetch-pack.sh +10 -7

index 692d717..3a9b775 100755
@@ -576,13 +576,16 @@ do
 	do
 		for h in host user@host user@[::1] user@::1
 		do
-			test_expect_success "fetch-pack --diag-url $p://$h/$r" '
-				check_prot_host_port_path $p://$h/$r $p "$h" NONE "/$r"
-			'
-			# "/~" -> "~" conversion
-			test_expect_success "fetch-pack --diag-url $p://$h/~$r" '
-				check_prot_host_port_path $p://$h/~$r $p "$h" NONE "~$r"
-			'
+			for c in "" :
+			do
+				test_expect_success "fetch-pack --diag-url $p://$h$c/$r" '
+					check_prot_host_port_path $p://$h/$r $p "$h" NONE "/$r"
+				'
+				# "/~" -> "~" conversion
+				test_expect_success "fetch-pack --diag-url $p://$h$c/~$r" '
+					check_prot_host_port_path $p://$h/~$r $p "$h" NONE "~$r"
+				'
+			done
 		done
 		for h in host User@host User@[::1]
 		do

t/t5601-clone.sh +13 -9

index 02b40b1..1f67bdd 100755
@@ -387,14 +387,18 @@ do
 done
 
 #with ssh:// scheme
-test_expect_success 'clone ssh://host.xz/home/user/repo' '
-	test_clone_url "ssh://host.xz/home/user/repo" host.xz "/home/user/repo"
-'
-
-# from home directory
-test_expect_success 'clone ssh://host.xz/~repo' '
-	test_clone_url "ssh://host.xz/~repo" host.xz "~repo"
+#ignore trailing colon
+for tcol in "" :
+do
+	test_expect_success "clone ssh://host.xz$tcol/home/user/repo" '
+		test_clone_url "ssh://host.xz$tcol/home/user/repo" host.xz /home/user/repo
+	'
+	# from home directory
+	test_expect_success "clone ssh://host.xz$tcol/~repo" '
+	test_clone_url "ssh://host.xz$tcol/~repo" host.xz "~repo"
 '
+done
+test_done
 
 # with port number
 test_expect_success 'clone ssh://host.xz:22/home/user/repo' '

@@ -407,9 +411,9 @@ test_expect_success 'clone ssh://host.xz:22/~repo' '
 '
 
 #IPv6
-for tuah in ::1 [::1] user@::1 user@[::1] [user@::1]
+for tuah in ::1 [::1] [::1]: user@::1 user@[::1] user@[::1]: [user@::1] [user@::1]:
 do
-	ehost=$(echo $tuah | tr -d "[]")
+	ehost=$(echo $tuah | sed -e "s/1]:/1]/ "| tr -d "[]")
 	test_expect_success "clone ssh://$tuah/home/user/repo" "
 	  test_clone_url ssh://$tuah/home/user/repo $ehost /home/user/repo
 	"

Signature · 1 line

2.2.0.rc1.790.ge19fcd2

Eric Sunshine

2016-06-15 23:04 UTC #1 lore raw

Quoted 4 lines · Torsten Bögershausen

Ignore an extra ':' at the end of the hostname in URL's like "ssh://example.com:/path/to/repo"
The colon is ment to separate a port number from the hostname.

s/ment/meant/

More below.

Quoted 25 lines

If the port is empty, the colon should be ignored, see RFC 3986.
It had been working for URLs with ssh:// scheme, but was unintentionally broken in 86ceb3, "allow ssh://user@[2001:db8::1]/repo.git"
Reported-by Reid Woodbury Jr. <[email protected]>
Signed-off-by Torsten Bögershausen <[email protected]>
---
t/t5601-clone.sh +2 -7
index 02b40b1..1f67bdd 100755
@@ -387,14 +387,18 @@ do
 done
 
 #with ssh:// scheme
-test_expect_success 'clone ssh://host.xz/home/user/repo' '
-       test_clone_url "ssh://host.xz/home/user/repo" host.xz "/home/user/repo"
-'
-
-# from home directory
-test_expect_success 'clone ssh://host.xz/~repo' '
-       test_clone_url "ssh://host.xz/~repo" host.xz "~repo"
+#ignore trailing colon
+for tcol in "" :

t/t5601-clone.sh do · line 401

Quoted 10 lines

t/t5601-clone.sh +9 -0

+do
+       test_expect_success "clone ssh://host.xz$tcol/home/user/repo" '
+               test_clone_url "ssh://host.xz$tcol/home/user/repo" host.xz /home/user/repo
+       '
+       # from home directory
+       test_expect_success "clone ssh://host.xz$tcol/~repo" '
+       test_clone_url "ssh://host.xz$tcol/~repo" host.xz "~repo"
 '
+done
+test_done

Unwanted test_done crept in?

Quoted 2 lines

t/t5601-clone.sh +0 -0

 # with port number
 test_expect_success 'clone ssh://host.xz:22/home/user/repo' '