From 99abb157438b9f8b1fc1259dc13a84a98a168a3f Mon Sep 17 00:00:00 2001 From: Segev Finer Date: Fri, 23 Dec 2016 23:49:02 +0200 Subject: [PATCH 1/4] connect: handle putty/plink also in GIT_SSH_COMMAND Git for Windows has special support for the popular SSH client PuTTY: when using PuTTY's non-interactive version ("plink.exe"), we use the -P option to specify the port rather than OpenSSH's -p option. TortoiseGit ships with its own, forked version of plink.exe, that adds support for the -batch option, and for good measure we special-case that, too. However, this special-casing of PuTTY only covers the case where the user overrides the SSH command via the environment variable GIT_SSH (which allows specifying the name of the executable), not GIT_SSH_COMMAND (which allows specifying a full command, including additional command-line options). When users want to pass any additional arguments to (Tortoise-)Plink, such as setting a private key, they are required to either use a shell script named plink or tortoiseplink or duplicate the logic that is already in Git for passing the correct style of command line arguments, which can be difficult, error prone and annoying to get right. This patch simply reuses the existing logic and expands it to cover GIT_SSH_COMMAND, too. Note: it may look a little heavy-handed to duplicate the entire command-line and then split it, only to extract the name of the executable. However, this is not a performance-critical code path, and the code is much more readable this way. Signed-off-by: Segev Finer Signed-off-by: Johannes Schindelin --- connect.c | 23 ++++++++++++++++------- t/t5601-clone.sh | 15 +++++++++++++++ 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/connect.c b/connect.c index 8cb93b0720d9a3..c81f77001b452e 100644 --- a/connect.c +++ b/connect.c @@ -772,6 +772,7 @@ struct child_process *git_connect(int fd[2], const char *url, int putty = 0, tortoiseplink = 0; char *ssh_host = hostandport; const char *port = NULL; + char *ssh_argv0 = NULL; transport_check_allowed("ssh"); get_host_and_port(&ssh_host, &port); @@ -792,10 +793,15 @@ struct child_process *git_connect(int fd[2], const char *url, } ssh = get_ssh_command(); - if (!ssh) { - const char *base; - char *ssh_dup; - + if (ssh) { + char *split_ssh = xstrdup(ssh); + const char **ssh_argv; + + if (split_cmdline(split_ssh, &ssh_argv)) + ssh_argv0 = xstrdup(ssh_argv[0]); + free(split_ssh); + free((void *)ssh_argv); + } else { /* * GIT_SSH is the no-shell version of * GIT_SSH_COMMAND (and must remain so for @@ -807,8 +813,11 @@ struct child_process *git_connect(int fd[2], const char *url, if (!ssh) ssh = "ssh"; - ssh_dup = xstrdup(ssh); - base = basename(ssh_dup); + ssh_argv0 = xstrdup(ssh); + } + + if (ssh_argv0) { + const char *base = basename(ssh_argv0); tortoiseplink = !strcasecmp(base, "tortoiseplink") || !strcasecmp(base, "tortoiseplink.exe"); @@ -816,7 +825,7 @@ struct child_process *git_connect(int fd[2], const char *url, !strcasecmp(base, "plink") || !strcasecmp(base, "plink.exe"); - free(ssh_dup); + free(ssh_argv0); } argv_array_push(&conn->args, ssh); diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh index 4241ea5b32db4a..9335e10c2ad7c1 100755 --- a/t/t5601-clone.sh +++ b/t/t5601-clone.sh @@ -386,6 +386,21 @@ test_expect_success 'tortoiseplink is like putty, with extra arguments' ' expect_ssh "-batch -P 123" myhost src ' +test_expect_success 'double quoted plink.exe in GIT_SSH_COMMAND' ' + copy_ssh_wrapper_as "$TRASH_DIRECTORY/plink.exe" && + GIT_SSH_COMMAND="\"$TRASH_DIRECTORY/plink.exe\" -v" \ + git clone "[myhost:123]:src" ssh-bracket-clone-plink-3 && + expect_ssh "-v -P 123" myhost src +' + +SQ="'" +test_expect_success 'single quoted plink.exe in GIT_SSH_COMMAND' ' + copy_ssh_wrapper_as "$TRASH_DIRECTORY/plink.exe" && + GIT_SSH_COMMAND="$SQ$TRASH_DIRECTORY/plink.exe$SQ -v" \ + git clone "[myhost:123]:src" ssh-bracket-clone-plink-4 && + expect_ssh "-v -P 123" myhost src +' + # Reset the GIT_SSH environment variable for clone tests. setup_ssh_wrapper From db1362a189cc81f91d473cf5fc422bff1850678f Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 25 Jan 2017 14:37:32 -0800 Subject: [PATCH 2/4] connect: rename tortoiseplink and putty variables One of these two may have originally been named after "what exact SSH implementation do we have" so that we can tweak the command line options, but these days "putty=1" no longer means "We are using the plink SSH implementation that comes with PuTTY". It is set when we guess that either PuTTY plink or Tortoiseplink is in use. Rename them after what effect is desired. The current "putty" option is about using "-P " when OpenSSH would use "-p ", so rename it to port_option whose value is either 'p' or 'P". The other one is about passing an extra command line option "-batch", so rename it needs_batch. [jes: wrapped overly-long line] Signed-off-by: Junio C Hamano Signed-off-by: Johannes Schindelin --- connect.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/connect.c b/connect.c index c81f77001b452e..9f750eacb6d5b7 100644 --- a/connect.c +++ b/connect.c @@ -769,7 +769,8 @@ struct child_process *git_connect(int fd[2], const char *url, conn->in = conn->out = -1; if (protocol == PROTO_SSH) { const char *ssh; - int putty = 0, tortoiseplink = 0; + int needs_batch = 0; + int port_option = 'p'; char *ssh_host = hostandport; const char *port = NULL; char *ssh_argv0 = NULL; @@ -819,12 +820,14 @@ struct child_process *git_connect(int fd[2], const char *url, if (ssh_argv0) { const char *base = basename(ssh_argv0); - tortoiseplink = !strcasecmp(base, "tortoiseplink") || - !strcasecmp(base, "tortoiseplink.exe"); - putty = tortoiseplink || - !strcasecmp(base, "plink") || - !strcasecmp(base, "plink.exe"); - + if (!strcasecmp(base, "tortoiseplink") || + !strcasecmp(base, "tortoiseplink.exe")) { + port_option = 'P'; + needs_batch = 1; + } else if (!strcasecmp(base, "plink") || + !strcasecmp(base, "plink.exe")) { + port_option = 'P'; + } free(ssh_argv0); } @@ -833,11 +836,11 @@ struct child_process *git_connect(int fd[2], const char *url, argv_array_push(&conn->args, "-4"); else if (flags & CONNECT_IPV6) argv_array_push(&conn->args, "-6"); - if (tortoiseplink) + if (needs_batch) argv_array_push(&conn->args, "-batch"); if (port) { - /* P is for PuTTY, p is for OpenSSH */ - argv_array_push(&conn->args, putty ? "-P" : "-p"); + argv_array_pushf(&conn->args, + "-%c", port_option); argv_array_push(&conn->args, port); } argv_array_push(&conn->args, ssh_host); From c5f5f3bbf866ea002496b0a7a0e55dfb9043691d Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 1 Feb 2017 11:35:49 +0100 Subject: [PATCH 3/4] git_connect(): factor out SSH variant handling We handle plink and tortoiseplink as OpenSSH replacements, by passing the correct command-line options when detecting that they are used. To let users override that auto-detection (in case Git gets it wrong), we need to introduce new code to that end. In preparation for this code, let's factor out the SSH variant handling into its own function, handle_ssh_variant(). Signed-off-by: Johannes Schindelin --- connect.c | 72 +++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 46 insertions(+), 26 deletions(-) diff --git a/connect.c b/connect.c index 9f750eacb6d5b7..2734b9a1ca5a3b 100644 --- a/connect.c +++ b/connect.c @@ -691,6 +691,44 @@ static const char *get_ssh_command(void) return NULL; } +static int handle_ssh_variant(const char *ssh_command, int is_cmdline, + int *port_option, int *needs_batch) +{ + const char *variant; + char *p = NULL; + + if (!is_cmdline) { + p = xstrdup(ssh_command); + variant = basename(p); + } else { + const char **ssh_argv; + + p = xstrdup(ssh_command); + if (split_cmdline(p, &ssh_argv)) { + variant = basename((char *)ssh_argv[0]); + /* + * At this point, variant points into the buffer + * referenced by p, hence we do not need ssh_argv + * any longer. + */ + free(ssh_argv); + } else + return 0; + } + + if (!strcasecmp(variant, "plink") || + !strcasecmp(variant, "plink.exe")) + *port_option = 'P'; + else if (!strcasecmp(variant, "tortoiseplink") || + !strcasecmp(variant, "tortoiseplink.exe")) { + *port_option = 'P'; + *needs_batch = 1; + } + free(p); + + return 1; +} + /* * This returns a dummy child_process if the transport protocol does not * need fork(2), or a struct child_process object if it does. Once done, @@ -773,7 +811,6 @@ struct child_process *git_connect(int fd[2], const char *url, int port_option = 'p'; char *ssh_host = hostandport; const char *port = NULL; - char *ssh_argv0 = NULL; transport_check_allowed("ssh"); get_host_and_port(&ssh_host, &port); @@ -794,15 +831,10 @@ struct child_process *git_connect(int fd[2], const char *url, } ssh = get_ssh_command(); - if (ssh) { - char *split_ssh = xstrdup(ssh); - const char **ssh_argv; - - if (split_cmdline(split_ssh, &ssh_argv)) - ssh_argv0 = xstrdup(ssh_argv[0]); - free(split_ssh); - free((void *)ssh_argv); - } else { + if (ssh) + handle_ssh_variant(ssh, 1, &port_option, + &needs_batch); + else { /* * GIT_SSH is the no-shell version of * GIT_SSH_COMMAND (and must remain so for @@ -813,22 +845,10 @@ struct child_process *git_connect(int fd[2], const char *url, ssh = getenv("GIT_SSH"); if (!ssh) ssh = "ssh"; - - ssh_argv0 = xstrdup(ssh); - } - - if (ssh_argv0) { - const char *base = basename(ssh_argv0); - - if (!strcasecmp(base, "tortoiseplink") || - !strcasecmp(base, "tortoiseplink.exe")) { - port_option = 'P'; - needs_batch = 1; - } else if (!strcasecmp(base, "plink") || - !strcasecmp(base, "plink.exe")) { - port_option = 'P'; - } - free(ssh_argv0); + else + handle_ssh_variant(ssh, 0, + &port_option, + &needs_batch); } argv_array_push(&conn->args, ssh); From 3e2aa50f33a80b8d5f26cd4495f14649588a0610 Mon Sep 17 00:00:00 2001 From: Segev Finer Date: Sat, 14 Jan 2017 17:19:04 +0200 Subject: [PATCH 4/4] connect: Add the envvar GIT_SSH_VARIANT and ssh.variant config This environment variable and configuration value allow to override the autodetection of plink/tortoiseplink in case that Git gets it wrong. [jes: wrapped overly-long lines, factored out and changed get_ssh_variant() to handle_ssh_variant() to accomodate the change from the putty/tortoiseplink variables to port_option/needs_batch, adjusted the documentation, free()d value obtained from the config.] Signed-off-by: Segev Finer Signed-off-by: Johannes Schindelin --- Documentation/config.txt | 11 +++++++++++ Documentation/git.txt | 6 ++++++ connect.c | 11 ++++++++--- t/t5601-clone.sh | 26 ++++++++++++++++++++++++++ 4 files changed, 51 insertions(+), 3 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 01dbaee83e55df..79905895e6be7c 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1981,6 +1981,17 @@ Environment variable settings always override any matches. The URLs that are matched against are those given directly to Git commands. This means any URLs visited as a result of a redirection do not participate in matching. +ssh.variant:: + Depending on the value of the environment variables `GIT_SSH` or + `GIT_SSH_COMMAND`, or the config setting `core.sshCommand`, Git + auto-detects whether to adjust its command-line parameters for use + with plink or tortoiseplink, as opposed to the default (OpenSSH). ++ +The config variable `ssh.variant` can be set to override this auto-detection; +valid values are `ssh`, `plink`, `putty` or `tortoiseplink`. Any other value +will be treated as normal ssh. This setting can be overridden via the +environment variable `GIT_SSH_VARIANT`. + i18n.commitEncoding:: Character encoding the commit messages are stored in; Git itself does not care per se, but this information is necessary e.g. when diff --git a/Documentation/git.txt b/Documentation/git.txt index 80bc6e40eb7b92..d6fe0797ac28cd 100644 --- a/Documentation/git.txt +++ b/Documentation/git.txt @@ -1027,6 +1027,12 @@ Usually it is easier to configure any desired options through your personal `.ssh/config` file. Please consult your ssh documentation for further details. +`GIT_SSH_VARIANT`:: + If this environment variable is set, it overrides Git's autodetection + whether `GIT_SSH`/`GIT_SSH_COMMAND`/`core.sshCommand` refer to OpenSSH, + plink or tortoiseplink. This variable overrides the config setting + `ssh.variant` that serves the same purpose. + `GIT_ASKPASS`:: If this environment variable is set, then Git commands which need to acquire passwords or passphrases (e.g. for HTTP or IMAP authentication) diff --git a/connect.c b/connect.c index 2734b9a1ca5a3b..7f1f80239646cb 100644 --- a/connect.c +++ b/connect.c @@ -694,10 +694,14 @@ static const char *get_ssh_command(void) static int handle_ssh_variant(const char *ssh_command, int is_cmdline, int *port_option, int *needs_batch) { - const char *variant; + const char *variant = getenv("GIT_SSH_VARIANT"); char *p = NULL; - if (!is_cmdline) { + if (variant) + ; /* okay, fall through */ + else if (!git_config_get_string("ssh.variant", &p)) + variant = p; + else if (!is_cmdline) { p = xstrdup(ssh_command); variant = basename(p); } else { @@ -717,7 +721,8 @@ static int handle_ssh_variant(const char *ssh_command, int is_cmdline, } if (!strcasecmp(variant, "plink") || - !strcasecmp(variant, "plink.exe")) + !strcasecmp(variant, "plink.exe") || + !strcasecmp(variant, "putty")) *port_option = 'P'; else if (!strcasecmp(variant, "tortoiseplink") || !strcasecmp(variant, "tortoiseplink.exe")) { diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh index 9335e10c2ad7c1..b52b8acf9859b0 100755 --- a/t/t5601-clone.sh +++ b/t/t5601-clone.sh @@ -401,6 +401,32 @@ test_expect_success 'single quoted plink.exe in GIT_SSH_COMMAND' ' expect_ssh "-v -P 123" myhost src ' +test_expect_success 'GIT_SSH_VARIANT overrides plink detection' ' + copy_ssh_wrapper_as "$TRASH_DIRECTORY/plink" && + GIT_SSH_VARIANT=ssh \ + git clone "[myhost:123]:src" ssh-bracket-clone-variant-1 && + expect_ssh "-p 123" myhost src +' + +test_expect_success 'ssh.variant overrides plink detection' ' + copy_ssh_wrapper_as "$TRASH_DIRECTORY/plink" && + git -c ssh.variant=ssh \ + clone "[myhost:123]:src" ssh-bracket-clone-variant-2 && + expect_ssh "-p 123" myhost src +' + +test_expect_success 'GIT_SSH_VARIANT overrides plink detection to plink' ' + GIT_SSH_VARIANT=plink \ + git clone "[myhost:123]:src" ssh-bracket-clone-variant-3 && + expect_ssh "-P 123" myhost src +' + +test_expect_success 'GIT_SSH_VARIANT overrides plink to tortoiseplink' ' + GIT_SSH_VARIANT=tortoiseplink \ + git clone "[myhost:123]:src" ssh-bracket-clone-variant-4 && + expect_ssh "-batch -P 123" myhost src +' + # Reset the GIT_SSH environment variable for clone tests. setup_ssh_wrapper