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

Re: [BUG] having 'plink' anywhere in the GIT_SSH environment variables sets putty = true

From
Jeff King <peff@peff.net>
Date
Apr 22, 2015, 20:29 UTC
Message-ID
<20150422202910.GB26917@peff.net>
In-Reply-To
<F1AB6016-DA88-446C-84E9-1BD42436843D@gmail.com>
On Wed, Apr 22, 2015 at 02:12:53PM -0500, Patrick Sharp wrote:
Show 17 quoted lines
> 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
Previous: Patrick SharpNext: brian m. carlson
Message 4 of 41 in “[BUG] having 'plink' anywhere in the GIT_SSH environment variables sets putty = true”
  1. Patrick SharpApr 22, 2015
  2. Johannes SchindelinApr 22, 2015
  3. Patrick SharpApr 22, 2015
  4. Jeff KingApr 22, 2015
  5. brian m. carlsonApr 22, 2015
  6. Jeff KingApr 22, 2015
  7. brian m. carlsonApr 22, 2015
  8. Jeff KingApr 22, 2015
  9. brian m. carlsonApr 22, 2015
  10. Jeff KingApr 22, 2015
  11. 1/2 connect: simplify SSH connection code pathbrian m. carlson, Apr 23, 2015
  12. 2/2 connect: improve check for plink to reduce false positivesbrian m. carlson, Apr 23, 2015
  13. Johannes SchindelinApr 23, 2015
  14. Jeff KingApr 23, 2015
  15. brian m. carlsonApr 23, 2015
  16. Johannes SchindelinApr 24, 2015
  17. 1/2 connect: simplify SSH connection code pathbrian m. carlson, Apr 24, 2015
  18. 2/2 connect: improve check for plink to reduce false positivesbrian m. carlson, Apr 24, 2015
  19. Pete HarlanApr 24, 2015
  20. brian m. carlsonApr 24, 2015
  21. Torsten BögershausenApr 25, 2015
  22. brian m. carlsonApr 26, 2015
  23. 0/3 Improve robustness of putty detectionbrian m. carlson, Apr 26, 2015
  24. 1/3 connect: simplify SSH connection code pathbrian m. carlson, Apr 26, 2015
  25. 2/3 t5601: fix quotation error leading to skipped testsbrian m. carlson, Apr 26, 2015
  26. 3/3 connect: improve check for plink to reduce false positivesbrian m. carlson, Apr 26, 2015
  27. Johannes SchindelinApr 27, 2015
  28. Jeff KingApr 28, 2015
  29. Jeff KingJun 26, 2015
  30. Junio C HamanoJun 26, 2015
  31. Jeff KingJun 26, 2015
  32. Johannes SchindelinJun 26, 2015
  33. Jeff KingJun 26, 2015
  34. brian m. carlsonJun 26, 2015
  35. Junio C HamanoApr 26, 2015
  36. Torsten BögershausenApr 27, 2015
  37. Jeff KingApr 28, 2015
  38. brian m. carlsonApr 29, 2015
  39. Johannes SchindelinApr 24, 2015
  40. Torsten BögershausenApr 23, 2015
  41. Patrick SharpApr 23, 2015

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.