All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Patrick Sharp <jakanapes@gmail.com>
Cc: Johannes Schindelin <johannes.schindelin@gmx.de>, git@vger.kernel.org
Subject: Re: [BUG] having 'plink' anywhere in the GIT_SSH environment variables sets putty = true
Date: Wed, 22 Apr 2015 16:29:10 -0400	[thread overview]
Message-ID: <20150422202910.GB26917@peff.net> (raw)
In-Reply-To: <F1AB6016-DA88-446C-84E9-1BD42436843D@gmail.com>

On Wed, Apr 22, 2015 at 02:12:53PM -0500, Patrick Sharp wrote:

> Johannes,
> 
> You’re correct, looking back over it, I was pretty vague.
> 
> In truth, we are not using Windows OR putty at all.  Running git on an Ubuntu system, but we are setting the GIT_SSH environment variable to point to a shell script to use.
> 
> Upon attempting to run git ls-remote, the system was spitting out getaddrinfo errors for ‘atch’ .
> 
> Setting GIT_TRACE=2 showed that -batch was being added to the git command.
> 
> This was seen on several different servers with git versions 1.8.5.2, 1.9.1 and 2.3.5
> 
> After a bit we realized that it was the string ‘uplink’ in the GIT_SSH variable that was linked to the extra -batch flag.
> 
> Finally, after searching the git source, we narrowed it down to the ‘plink’ portion of the string.
> 
> https://github.com/git/git/blob/7c597ef345aed345576de616c51f27e6f4b342b3/connect.c#L747-L756

I think you want something like:

diff --git a/connect.c b/connect.c
index 9ae991a..58aad56 100644
--- a/connect.c
+++ b/connect.c
@@ -568,7 +568,8 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
 	conn->argv = arg = xcalloc(7, sizeof(*arg));
 	if (protocol == PROTO_SSH) {
 		const char *ssh = getenv("GIT_SSH");
-		int putty = ssh && strcasestr(ssh, "plink");
+		int putty = ssh && (ends_with(ssh, "plink") ||
+				    ends_with("plink.exe"));
 		if (!ssh) ssh = "ssh";
 
 		*arg++ = ssh;

though that is not quite enough (we do not have a case-insensitive
version of "ends_with"). I'm also not sure if matching just "plink" and
"plink.exe" at the end of the string is enough (I'm just guessing that
was the original reason for using strstr in the first place).

Note that I don't think just switching the strcasestr to look for
"plink.exe" is right. For one thing, it just punts on the problem (it
can still happen, it's just less likely to trigger). But for another,
you can have plink (without ".exe") on Linux systems.

-Peff

  reply	other threads:[~2015-04-22 20:29 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-22 14:36 [BUG] having 'plink' anywhere in the GIT_SSH environment variables sets putty = true Patrick Sharp
2015-04-22 17:46 ` Johannes Schindelin
2015-04-22 19:12   ` Patrick Sharp
2015-04-22 20:29     ` Jeff King [this message]
2015-04-22 21:19       ` brian m. carlson
2015-04-22 21:29         ` Jeff King
2015-04-22 21:44           ` brian m. carlson
2015-04-22 22:00             ` Jeff King
2015-04-22 22:24               ` brian m. carlson
2015-04-22 23:23                 ` Jeff King
2015-04-23  0:06                   ` [PATCH 1/2] connect: simplify SSH connection code path brian m. carlson
2015-04-23  0:06                     ` [PATCH 2/2] connect: improve check for plink to reduce false positives brian m. carlson
2015-04-23  6:50                       ` Johannes Schindelin
2015-04-23 15:53                         ` Jeff King
2015-04-23 23:14                           ` brian m. carlson
2015-04-24  6:41                             ` Johannes Schindelin
2015-04-24 22:28                             ` [PATCH v2 1/2] connect: simplify SSH connection code path brian m. carlson
2015-04-24 22:28                               ` [PATCH v2 2/2] connect: improve check for plink to reduce false positives brian m. carlson
2015-04-24 22:46                                 ` Pete Harlan
2015-04-24 22:48                                   ` brian m. carlson
2015-04-25 16:03                                 ` Torsten Bögershausen
2015-04-26 18:52                                   ` brian m. carlson
2015-04-26 20:30                               ` [PATCH v3 0/3] Improve robustness of putty detection brian m. carlson
2015-04-26 20:30                                 ` [PATCH v3 1/3] connect: simplify SSH connection code path brian m. carlson
2015-04-26 20:30                                 ` [PATCH v3 2/3] t5601: fix quotation error leading to skipped tests brian m. carlson
2015-04-26 20:30                                 ` [PATCH v3 3/3] connect: improve check for plink to reduce false positives brian m. carlson
2015-04-27  7:57                                   ` Johannes Schindelin
2015-04-28  3:53                                   ` Jeff King
2015-06-26 13:15                                   ` Jeff King
2015-06-26 16:16                                     ` Junio C Hamano
2015-06-26 16:27                                       ` Jeff King
2015-06-26 17:13                                         ` Johannes Schindelin
2015-06-26 17:23                                           ` Jeff King
2015-06-26 20:43                                     ` brian m. carlson
2015-04-26 22:04                                 ` [PATCH v3 0/3] Improve robustness of putty detection Junio C Hamano
2015-04-27 15:46                                   ` Torsten Bögershausen
2015-04-28  4:15                                   ` Jeff King
2015-04-29  1:38                                   ` brian m. carlson
2015-04-24  6:37                           ` [PATCH 2/2] connect: improve check for plink to reduce false positives Johannes Schindelin
2015-04-23  5:08     ` [BUG] having 'plink' anywhere in the GIT_SSH environment variables sets putty = true Torsten Bögershausen
2015-04-23 13:15       ` Patrick Sharp

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150422202910.GB26917@peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=jakanapes@gmail.com \
    --cc=johannes.schindelin@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.