git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 8/8] ssh: 'simple' variant does not support --port

From
Jonathan Nieder <jrnieder@gmail.com>
Date
Nov 20, 2017, 21:31 UTC
Message-ID
<20171120213101.yquv2ywyjajjtr5x@aiede.mtv.corp.google.com>
In-Reply-To
<20171120212134.lh2l4drdzu6fh5g2@aiede.mtv.corp.google.com>

When trying to connect to an ssh:// URL with port explicitly specified and the ssh command configured with GIT_SSH does not support such a setting, it is less confusing to error out than to silently suppress the port setting and continue.

This requires updating the GIT_SSH setting in t5603-clone-dirname.sh. That test is about the directory name produced when cloning various URLs. It uses an ssh wrapper that ignores all its arguments but does not declare that it supports a port argument; update it to set GIT_SSH_VARIANT=ssh to do so. (Real-life ssh wrappers that pass a port argument to OpenSSH would also support -G and would not require such an update.)

Reported-by: William Yan <wyan@google.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Acked-by: Stefan Beller <sbeller@google.com>
---
As before.
That's the end of the series.  Thanks for reading.
 connect.c                | 15 ++++++++++++---
 t/t5601-clone.sh         | 10 ++++++++--
 t/t5603-clone-dirname.sh |  2 ++
 3 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/connect.c b/connect.c
index be106d1868..20ed1d9574 100644
--- a/connect.c
+++ b/connect.c
@@ -967,11 +967,20 @@ static void push_ssh_options(struct argv_array *args, struct argv_array *env,
 	if (variant == VARIANT_TORTOISEPLINK)
 		argv_array_push(args, "-batch");
 
-	if (port && variant != VARIANT_SIMPLE) {
-		if (variant == VARIANT_SSH)
+	if (port) {
+		switch (variant) {
+		case VARIANT_AUTO:
+			BUG("VARIANT_AUTO passed to push_ssh_options");
+		case VARIANT_SIMPLE:
+			die("ssh variant 'simple' does not support setting port");
+		case VARIANT_SSH:
 			argv_array_push(args, "-p");
-		else
+			break;
+		case VARIANT_PLINK:
+		case VARIANT_PUTTY:
+		case VARIANT_TORTOISEPLINK:
 			argv_array_push(args, "-P");
+		}
 
 		argv_array_push(args, port);
 	}
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index ad910ae9fa..78dab81d87 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -378,12 +378,18 @@ test_expect_success 'variant=auto picks based on basename' '
 
 test_expect_success 'simple does not support -4/-6' '
 	copy_ssh_wrapper_as "$TRASH_DIRECTORY/simple" &&
-	test_must_fail git clone -4 "[myhost:123]:src" ssh-bracket-clone-simple
+	test_must_fail git clone -4 "myhost:src" ssh-4-clone-simple
+'
+
+test_expect_success 'simple does not support port' '
+	copy_ssh_wrapper_as "$TRASH_DIRECTORY/simple" &&
+	test_must_fail git clone "[myhost:123]:src" ssh-bracket-clone-simple
 '
 
 test_expect_success 'uplink is treated as simple' '
 	copy_ssh_wrapper_as "$TRASH_DIRECTORY/uplink" &&
-	git clone "[myhost:123]:src" ssh-bracket-clone-uplink &&
+	test_must_fail git clone "[myhost:123]:src" ssh-bracket-clone-uplink &&
+	git clone "myhost:src" ssh-clone-uplink &&
 	expect_ssh myhost src
 '
 
diff --git a/t/t5603-clone-dirname.sh b/t/t5603-clone-dirname.sh
index d5af758129..13b5e5eb9b 100755
--- a/t/t5603-clone-dirname.sh
+++ b/t/t5603-clone-dirname.sh
@@ -11,7 +11,9 @@ test_expect_success 'setup ssh wrapper' '
 	git upload-pack "$TRASH_DIRECTORY"
 	EOF
 	GIT_SSH="$TRASH_DIRECTORY/ssh-wrapper" &&
+	GIT_SSH_VARIANT=ssh &&
 	export GIT_SSH &&
+	export GIT_SSH_VARIANT &&
 	export TRASH_DIRECTORY
 '
 
-- 
2.15.0.448.gf294e3d99a
Previous: Jonathan NiederNext: Brandon Williams
Message 24 of 27 in “Coping with unrecognized ssh wrapper scripts in GIT_SSH”
  1. 0/8 Coping with unrecognized ssh wrapper scripts in GIT_SSHJonathan Nieder, Nov 20, 2017
  2. 1/8 ssh test: make copy_ssh_wrapper_as clean up after itselfJonathan Nieder, Nov 20, 2017
  3. Brandon WilliamsNov 20, 2017
  4. Junio C HamanoNov 21, 2017
  5. 1/8 ssh test: make copy_ssh_wrapper_as clean up after itselfJonathan Nieder, Nov 21, 2017
  6. Stefan BellerNov 21, 2017
  7. 2/8 connect: move no_fork fallback to git_tcp_connectJonathan Nieder, Nov 20, 2017
  8. 3/8 connect: split git:// setup into a separate functionJonathan Nieder, Nov 20, 2017
  9. Brandon WilliamsNov 20, 2017
  10. Jonathan NiederNov 20, 2017
  11. Brandon WilliamsNov 20, 2017
  12. 4/8 connect: split ssh command line options into separate functionJonathan Nieder, Nov 20, 2017
  13. Brandon WilliamsNov 20, 2017
  14. Jonathan NiederNov 20, 2017
  15. Brandon WilliamsNov 20, 2017
  16. 4/8 connect: split ssh command line options into separate functionJonathan Nieder, Nov 20, 2017
  17. 5/8 connect: split ssh option computation to its own functionJonathan Nieder, Nov 20, 2017
  18. Junio C HamanoNov 21, 2017
  19. 6/8 ssh: 'auto' variant to select between 'ssh' and 'simple'Jonathan Nieder, Nov 20, 2017
  20. Brandon WilliamsNov 20, 2017
  21. Junio C HamanoNov 21, 2017
  22. Jonathan NiederNov 21, 2017
  23. 7/8 ssh: 'simple' variant does not support -4/-6Jonathan Nieder, Nov 20, 2017
  24. 8/8 ssh: 'simple' variant does not support --portJonathan Nieder, Nov 20, 2017
  25. Brandon WilliamsNov 20, 2017
  26. Stefan BellerNov 22, 2017
  27. Junio C HamanoNov 22, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.