Re: [PATCH] connect: handle putty/plink also in GIT_SSH_COMMAND
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Jan 9, 2017, 11:13 UTC
- Message-ID
- <alpine.DEB.2.20.1701091207480.3469@virtualbox>
- In-Reply-To
- <xmqqh958y44c.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Mon, 9 Jan 2017, Junio C Hamano wrote:
Show 14 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > > If you feel strongly about your contrived examples possibly being > > affected by this patch, we could easily make this conditional on > > > > 1) no '&&' or '||' being found on the command-line, and > > 2) argv[0] not containing an '=' > > > > Another approach would be to verify that argv[i] starts with '-' for > > non-zero i. > > > > But do we really need to do that? > > No.
Exactly.
Show 9 quoted lines
> > That means that the user has to specify something like > > > > HAHAHA_IT_IS_NOT=/plink.exe ssh > > > > as GIT_SSH_COMMAND. > > My second message was to clarify that "VAR1=VAL2 command" is NOT a > contrived example, and this response indicates that I somehow failed > to convey that to you.
Indeed. The quite contrived example was about a script that chooses between plink and tortoiseplink (and fails to call anything else). And it failed to convince me.
But since you seem to convinced that a future bug report like this may happen (I am not, and it contradicts my conviction that one should cross a bridge only when reaching it, but whatever), how about this, on top:
-- snipsnap --
diff --git a/connect.c b/connect.c index c81f77001b..b990dd6190 100644 --- a/connect.c +++ b/connect.c @@ -797,7 +797,8 @@ struct child_process *git_connect(int fd[2], const char *url, char *split_ssh = xstrdup(ssh); const char **ssh_argv; - if (split_cmdline(split_ssh, &ssh_argv)) + if (split_cmdline(split_ssh, &ssh_argv) && + !strchr(ssh_argv[0], '=')) ssh_argv0 = xstrdup(ssh_argv[0]); free(split_ssh); free((void *)ssh_argv);