@matheusbsbs / matheus-bagliano / commits / baaf233755

connect: improve check for plink to reduce false positives

The git_connect function has code to handle plink and tortoiseplink specially, as they require different command line arguments from OpenSSH (-P instead of -p for ports; tortoiseplink additionally requires -batch). However, the match was done by checking for "plink" anywhere in the string, which led to a GIT_SSH value containing "uplink" being treated as an invocation of putty's plink. Improve the check by looking for "plink" or "tortoiseplink" (or those names suffixed with ".exe") only in the final component of the path. This has the downside that a program such as "plink-0.63" would no longer be recognized, but the increased robustness is likely worth it. Add tests to cover these cases to avoid regressions. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>

brian m. carlson committed Apr 26, 2015 at 20:30 UTC baaf233755f71c057d28b9e8692e24d4fca7d22f
2 files changed +48 -3
connect.c
+15 -3
@@ -722,7 +722,7 @@ struct child_process *git_connect(int fd[2], const char *url,
722 conn->in = conn->out = -1;
723 if (protocol == PROTO_SSH) {
724 const char *ssh;
725 - int putty;
725 + int putty, tortoiseplink = 0;
726 char *ssh_host = hostandport;
727 const char *port = NULL;
728 get_host_and_port(&ssh_host, &port);
@@ -747,14 +747,26 @@ struct child_process *git_connect(int fd[2], const char *url,
747 conn->use_shell = 1;
748 putty = 0;
749 } else {
750 + const char *base;
751 + char *ssh_dup;
752 +
753 ssh = getenv("GIT_SSH");
754 if (!ssh)
755 ssh = "ssh";
753 - putty = !!strcasestr(ssh, "plink");
756 +
757 + ssh_dup = xstrdup(ssh);
758 + base = basename(ssh_dup);
759 +
760 + tortoiseplink = !strcasecmp(base, "tortoiseplink") ||
761 + !strcasecmp(base, "tortoiseplink.exe");
762 + putty = !strcasecmp(base, "plink") ||
763 + !strcasecmp(base, "plink.exe") || tortoiseplink;
764 +
765 + free(ssh_dup);
766 }
767
768 argv_array_push(&conn->args, ssh);
757 - if (putty && !strcasestr(ssh, "tortoiseplink"))
769 + if (tortoiseplink)
770 argv_array_push(&conn->args, "-batch");
771 if (port) {
772 /* P is for PuTTY, p is for OpenSSH */
t/t5601-clone.sh
+33
@@ -296,6 +296,12 @@ setup_ssh_wrapper () {
296 '
297 }
298
299 +copy_ssh_wrapper_as () {
300 + cp "$TRASH_DIRECTORY/ssh-wrapper" "$1" &&
301 + GIT_SSH="$1" &&
302 + export GIT_SSH
303 +}
304 +
305 expect_ssh () {
306 test_when_finished '
307 (cd "$TRASH_DIRECTORY" && rm -f ssh-expect && >ssh-output)
@@ -335,6 +341,33 @@ test_expect_success 'bracketed hostnames are still ssh' '
341 expect_ssh "-p 123" myhost src
342 '
343
344 +test_expect_success 'uplink is not treated as putty' '
345 + copy_ssh_wrapper_as "$TRASH_DIRECTORY/uplink" &&
346 + git clone "[myhost:123]:src" ssh-bracket-clone-uplink &&
347 + expect_ssh "-p 123" myhost src
348 +'
349 +
350 +test_expect_success 'plink is treated specially (as putty)' '
351 + copy_ssh_wrapper_as "$TRASH_DIRECTORY/plink" &&
352 + git clone "[myhost:123]:src" ssh-bracket-clone-plink-0 &&
353 + expect_ssh "-P 123" myhost src
354 +'
355 +
356 +test_expect_success 'plink.exe is treated specially (as putty)' '
357 + copy_ssh_wrapper_as "$TRASH_DIRECTORY/plink.exe" &&
358 + git clone "[myhost:123]:src" ssh-bracket-clone-plink-1 &&
359 + expect_ssh "-P 123" myhost src
360 +'
361 +
362 +test_expect_success 'tortoiseplink is like putty, with extra arguments' '
363 + copy_ssh_wrapper_as "$TRASH_DIRECTORY/tortoiseplink" &&
364 + git clone "[myhost:123]:src" ssh-bracket-clone-plink-2 &&
365 + expect_ssh "-batch -P 123" myhost src
366 +'
367 +
368 +# Reset the GIT_SSH environment variable for clone tests.
369 +setup_ssh_wrapper
370 +
371 counter=0
372 # $1 url
373 # $2 none|host