Subject: Re: [PATCH] clone: local URLs are not for ssh

From: Torsten Bögershausen <[email-masked]>
Date: Sun, 13 Oct 2013 22:00:12 +0200

On 05.10.13 21:48, Torsten Bögershausen wrote:
> On 2013-10-03 03.31, Jeff King wrote:
>>
>>   http://article.gmane.org/gmane.comp.version-control.git/235473
What do we think about extending the test a little bit:

git diff 771cf1dab9303bab3c8198b8b6 -- t5602-clone-remote-exec.sh 

diff --git a/t/t5602-clone-remote-exec.sh b/t/t5602-clone-remote-exec.sh
index 3f353d9..790896f 100755
--- a/t/t5602-clone-remote-exec.sh
+++ b/t/t5602-clone-remote-exec.sh
@@ -30,5 +30,124 @@ test_expect_success 'clone calls specified git upload-pack with -u option' '
 	echo "localhost ./something/bin/git-upload-pack '\''/path/to/repo'\''" >expected &&
 	test_cmp expected not_ssh_output
 '
+test_expect_success 'setup ssh wrapper' '
+	write_script "$TRASH_DIRECTORY/ssh-wrapper" <<-\EOF &&
+	echo >>"$TRASH_DIRECTORY/ssh-output" "ssh: $*" &&
+	# throw away all but the last argument, which should be the
+	# command
+	while test $# -gt 1; do shift; done
+	eval "$1"
+	EOF
+
+	GIT_SSH="$TRASH_DIRECTORY/ssh-wrapper" &&
+	export GIT_SSH &&
+	export TRASH_DIRECTORY
+'
+
+clear_ssh () {
+	>"$TRASH_DIRECTORY/ssh-output"
+}
+
+expect_ssh () {
+	{
+		case "$1" in
+		none)
+			;;
+		*)
+			echo "ssh: $1 git-upload-pack '$2'"
+		esac
+	} >"$TRASH_DIRECTORY/ssh-expect" &&
+	(cd "$TRASH_DIRECTORY" && test_cmp ssh-expect ssh-output)
+}
+
+test_expect_success 'create src.git' '
+	mkdir src.git &&
+	( 
+		cd src.git &&
+		git init &&
+		>file &&
+		git add file &&
+		git commit -m "add file"
+	)
+'
+
+# git clone could fail, so break the && chain and ignore the exit code
+# clone local
+test_expect_success './foo:bar is not ssh' '
+	clear_ssh &&
+	git clone "./foo:bar" foobar
+	expect_ssh none
+'
+
+test_expect_success './[nohost:123]:src is not ssh' '
+	clear_ssh &&
+	git clone "./[nohost:123]:src" 1_2_3
+	expect_ssh none
+'
+
+test_expect_success '[nohost:234] is not ssh' '
+	clear_ssh &&
+	git clone "[nohost:234]" 2_3_4
+	expect_ssh none
+'
+
+test_expect_success ':345 is not ssh' '
+	clear_ssh &&
+	git clone ":345" 3_4_5 
+	expect_ssh none
+'
+
+test_expect_success '456: is not ssh' '
+	clear_ssh &&
+	git clone "456:" 4_5_6 
+	expect_ssh none
+'
+
+# clone via ssh
+# the expect_ssh checks that git clone tried to use ssh
+test_expect_success 'myhost:567 is ssh' '
+	clear_ssh &&
+	git clone myhost:567 myhost_567
+	expect_ssh myhost 567
+'
+
+test_expect_success '[myhost:678]:src is ssh' '
+	clear_ssh &&
+	git clone "[myhost:678]:src" myhost_678
+	expect_ssh myhost:678 src
+'
+
+#clone url looks like ssh, but is on disk
+test_expect_success SYMLINKS 'dir:123 on disk' '
+	clear_ssh &&
+	ln -s src.git dir:123 &&
+	git clone dir:123 dir_123 &&
+	expect_ssh none
+'
+
+test_expect_success SYMLINKS '[dir:234]:src on disk' '
+	clear_ssh &&
+	ln -s src.git [dir:234]:src &&
+	git clone [dir:234]:src dir_234_src &&
+	expect_ssh none
+'
+
+test_expect_success 'ssh://host.xz/~user/repo' '
+	clear_ssh &&
+	git clone "ssh://host.xz/~user/repo" user-repo
+	expect_ssh host.xz "~user/repo"
+'
+
+test_expect_success 'ssh://host.xz:22/~user/repo' '
+	clear_ssh &&
+	git clone "ssh://host.xz:22/~user/repo" user-repo
+	expect_ssh "-p 22 host.xz" "~user/repo"
+'
+
+test_expect_success 'ssh://[::1]:22/~user/repo' '
+	clear_ssh &&
+	git clone "ssh://[::1]:22/~user/repo" user-repo6
+	expect_ssh "-p 22 ::1" "~user/repo"
+'
 
 test_done

==============
And we need this on top of Duys patch:

diff --git a/connect.c b/connect.c
index e8473f3..09be2b3 100644
--- a/connect.c
+++ b/connect.c
@@ -611,7 +611,7 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
                end = host;
 
        path = strchr(end, c);
-       if (path && !has_dos_drive_prefix(end)) {
+       if (path && host != path && !has_dos_drive_prefix(end)) {
                if (c == ':') {
       if (host != url || path < strchrnul(host, '/')) {
                                protocol = PROTO_SSH;