Re: [PATCH] connect: handle putty/plink also in GIT_SSH_COMMAND

[Junio C Hamano <gitster@pobox.com>]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> +			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");

I suspect that this will break when GIT_SSH_COMMAND, which is meant
to be processed by the shell, hence the user can write anything,
begins with a one-shot environment variable assignment, e.g.

	[core] sshcommand = VAR1=VAL1 VAR2=VAL2 //path/to/tortoiseplink

One possible unintended side effect of this patch is when VAL1 ends
with /plink (or /tortoiseplink) and the command is not either of
these, in which case the "tortoiseplink" and "putty" variables will
tweak the command line for an SSH implementation that does not want
such a tweak to be made.  There may be other unintended fallouts.

Sorry, no concrete suggestion to get this to work comes to my mind
offhand. 

Perhaps give an explicit way to force "tortoiseplink" and "putty"
variables without looking at and guessing from the pathname, so that
the solution does not have to split and peek the command line?


 connect.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/connect.c b/connect.c
index 8cb93b0720..1768122456 100644
--- a/connect.c
+++ b/connect.c
@@ -691,6 +691,23 @@ static const char *get_ssh_command(void)
 	return NULL;
 }
 
+static void get_ssh_variant(int *tortoiseplink, int *putty)
+{
+	const char *variant;
+	if (!git_config_get_string_const("ssh.variant", &variant))
+		return;
+	if (!strcmp(variant, "tortoiseplink")) {
+		*tortoiseplink = 1;
+		*putty = 1;
+	} else if (!strcmp(variant, "putty")) {
+		*tortoiseplink = 0;
+		*putty = 1;
+	} else {
+		*tortoiseplink = 0;
+		*putty = 0;
+	}
+}
+
 /*
  * 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,
@@ -824,6 +841,9 @@ 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");
+
+			get_ssh_variant(&tortoiseplink, &putty);
+
 			if (tortoiseplink)
 				argv_array_push(&conn->args, "-batch");
 			if (port) {