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>

diff --git a/connect.c b/connect.c
index 170e391221cfcef7f8a55d635dfcd7cb4c719f92..1f7b7694f66d11d2ac7cc45e03a9c69e96860b02 100644 (file)
--- a/connect.c
+++ b/connect.c
@@ -722,7 +722,7 @@ struct child_process *git_connect(int fd[2], const char *url,
                conn->in = conn->out = -1;
                if (protocol == PROTO_SSH) {
                        const char *ssh;
                conn->in = conn->out = -1;
                if (protocol == PROTO_SSH) {
                        const char *ssh;

-                       int putty;
+                       int putty, tortoiseplink = 0;

                        char *ssh_host = hostandport;
                        const char *port = NULL;
                        get_host_and_port(&ssh_host, &port);
                        char *ssh_host = hostandport;
                        const char *port = NULL;
                        get_host_and_port(&ssh_host, &port);


@@ -747,14 +747,26 @@ struct child_process *git_connect(int fd[2], const char *url,
                                conn->use_shell = 1;
                                putty = 0;
                        } else {
                                conn->use_shell = 1;
                                putty = 0;
                        } else {

+                               const char *base;
+                               char *ssh_dup;
+

                                ssh = getenv("GIT_SSH");
                                if (!ssh)
                                        ssh = "ssh";
                                ssh = getenv("GIT_SSH");
                                if (!ssh)
                                        ssh = "ssh";

-                               putty = !!strcasestr(ssh, "plink");
+
+                               ssh_dup = xstrdup(ssh);
+                               base = basename(ssh_dup);
+
+                               tortoiseplink = !strcasecmp(base, "tortoiseplink") ||
+                                       !strcasecmp(base, "tortoiseplink.exe");
+                               putty = !strcasecmp(base, "plink") ||
+                                       !strcasecmp(base, "plink.exe") || tortoiseplink;
+
+                               free(ssh_dup);

                        }
 
                        argv_array_push(&conn->args, ssh);
                        }
 
                        argv_array_push(&conn->args, ssh);

-                       if (putty && !strcasestr(ssh, "tortoiseplink"))
+                       if (tortoiseplink)

                                argv_array_push(&conn->args, "-batch");
                        if (port) {
                                /* P is for PuTTY, p is for OpenSSH */
                                argv_array_push(&conn->args, "-batch");
                        if (port) {
                                /* P is for PuTTY, p is for OpenSSH */

diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 5efc1779177a53027c5dca2da94fa45caae383ca..731c09c41801974531e12e7b070c235dcf1edc66 100755 (executable)
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -296,6 +296,12 @@ setup_ssh_wrapper () {
        '
 }
 
        '
 }
 

+copy_ssh_wrapper_as () {
+       cp "$TRASH_DIRECTORY/ssh-wrapper" "$1" &&
+       GIT_SSH="$1" &&
+       export GIT_SSH
+}
+

 expect_ssh () {
        test_when_finished '
                (cd "$TRASH_DIRECTORY" && rm -f ssh-expect && >ssh-output)
 expect_ssh () {
        test_when_finished '
                (cd "$TRASH_DIRECTORY" && rm -f ssh-expect && >ssh-output)


@@ -335,6 +341,33 @@ test_expect_success 'bracketed hostnames are still ssh' '
        expect_ssh "-p 123" myhost src
 '
 
        expect_ssh "-p 123" myhost src
 '
 

+test_expect_success 'uplink is not treated as putty' '
+       copy_ssh_wrapper_as "$TRASH_DIRECTORY/uplink" &&
+       git clone "[myhost:123]:src" ssh-bracket-clone-uplink &&
+       expect_ssh "-p 123" myhost src
+'
+
+test_expect_success 'plink is treated specially (as putty)' '
+       copy_ssh_wrapper_as "$TRASH_DIRECTORY/plink" &&
+       git clone "[myhost:123]:src" ssh-bracket-clone-plink-0 &&
+       expect_ssh "-P 123" myhost src
+'
+
+test_expect_success 'plink.exe is treated specially (as putty)' '
+       copy_ssh_wrapper_as "$TRASH_DIRECTORY/plink.exe" &&
+       git clone "[myhost:123]:src" ssh-bracket-clone-plink-1 &&
+       expect_ssh "-P 123" myhost src
+'
+
+test_expect_success 'tortoiseplink is like putty, with extra arguments' '
+       copy_ssh_wrapper_as "$TRASH_DIRECTORY/tortoiseplink" &&
+       git clone "[myhost:123]:src" ssh-bracket-clone-plink-2 &&
+       expect_ssh "-batch -P 123" myhost src
+'
+
+# Reset the GIT_SSH environment variable for clone tests.
+setup_ssh_wrapper
+

 counter=0
 # $1 url
 # $2 none|host
 counter=0
 # $1 url
 # $2 none|host