Merge branch 'bc/connect-plink'
authorJunio C Hamano <gitster@pobox.com>
Tue, 19 May 2015 20:17:54 +0000 (13:17 -0700)
committerJunio C Hamano <gitster@pobox.com>
Tue, 19 May 2015 20:17:55 +0000 (13:17 -0700)
The connection initiation code for "ssh" transport tried to absorb
differences between the stock "ssh" and Putty-supplied "plink" and
its derivatives, but the logic to tell that we are using "plink"
variants were too loose and falsely triggered when "plink" appeared
anywhere in the path (e.g. "/home/me/bin/uplink/ssh").

* bc/connect-plink:
connect: improve check for plink to reduce false positives
t5601: fix quotation error leading to skipped tests
connect: simplify SSH connection code path

1  2 
connect.c
t/t5601-clone.sh
diff --combined connect.c
index 391d21192f8d9593ce9b194488019eae3ec7d8d3,1f7b7694f66d11d2ac7cc45e03a9c69e96860b02..c0144d859ae4275860df464f73a688c649d092fe
+++ b/connect.c
@@@ -310,8 -310,6 +310,8 @@@ static void get_host_and_port(char **ho
                if (end != colon + 1 && *end == '\0' && 0 <= portnr && portnr < 65536) {
                        *colon = 0;
                        *port = colon + 1;
 +              } else if (!colon[1]) {
 +                      *colon = 0;
                }
        }
  }
@@@ -724,7 -722,7 +724,7 @@@ struct child_process *git_connect(int f
                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);
  
                                free(hostandport);
                                free(path);
 +                              free(conn);
                                return NULL;
+                       }
+                       ssh = getenv("GIT_SSH_COMMAND");
+                       if (ssh) {
+                               conn->use_shell = 1;
+                               putty = 0;
                        } else {
-                               ssh = getenv("GIT_SSH_COMMAND");
-                               if (ssh) {
-                                       conn->use_shell = 1;
-                                       putty = 0;
-                               } else {
-                                       ssh = getenv("GIT_SSH");
-                                       if (!ssh)
-                                               ssh = "ssh";
-                                       putty = !!strcasestr(ssh, "plink");
-                               }
-                               argv_array_push(&conn->args, ssh);
-                               if (putty && !strcasestr(ssh, "tortoiseplink"))
-                                       argv_array_push(&conn->args, "-batch");
-                               if (port) {
-                                       /* P is for PuTTY, p is for OpenSSH */
-                                       argv_array_push(&conn->args, putty ? "-P" : "-p");
-                                       argv_array_push(&conn->args, port);
-                               }
-                               argv_array_push(&conn->args, ssh_host);
+                               const char *base;
+                               char *ssh_dup;
+                               ssh = getenv("GIT_SSH");
+                               if (!ssh)
+                                       ssh = "ssh";
+                               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);
+                       if (tortoiseplink)
+                               argv_array_push(&conn->args, "-batch");
+                       if (port) {
+                               /* P is for PuTTY, p is for OpenSSH */
+                               argv_array_push(&conn->args, putty ? "-P" : "-p");
+                               argv_array_push(&conn->args, port);
                        }
+                       argv_array_push(&conn->args, ssh_host);
                } else {
                        /* remove repo-local variables from the environment */
                        conn->env = local_repo_env;
diff --combined t/t5601-clone.sh
index 1befc453a31e6ad67c3805eb94bfe00f6b7a5469,731c09c41801974531e12e7b070c235dcf1edc66..bfdaf75966f7b8bb057e4db0b8791306f6e6a44f
@@@ -296,6 -296,12 +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)
@@@ -332,9 -338,36 +338,36 @@@ test_expect_success !MINGW,!CYGWIN 'clo
  
  test_expect_success 'bracketed hostnames are still ssh' '
        git clone "[myhost:123]:src" ssh-bracket-clone &&
-       expect_ssh myhost '-p 123' 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
  done
  
  #with ssh:// scheme
 -test_expect_success 'clone ssh://host.xz/home/user/repo' '
 -      test_clone_url "ssh://host.xz/home/user/repo" host.xz "/home/user/repo"
 -'
 -
 -# from home directory
 -test_expect_success 'clone ssh://host.xz/~repo' '
 -      test_clone_url "ssh://host.xz/~repo" host.xz "~repo"
 +#ignore trailing colon
 +for tcol in "" :
 +do
 +      test_expect_success "clone ssh://host.xz$tcol/home/user/repo" '
 +              test_clone_url "ssh://host.xz$tcol/home/user/repo" host.xz /home/user/repo
 +      '
 +      # from home directory
 +      test_expect_success "clone ssh://host.xz$tcol/~repo" '
 +      test_clone_url "ssh://host.xz$tcol/~repo" host.xz "~repo"
  '
 +done
  
  # with port number
  test_expect_success 'clone ssh://host.xz:22/home/user/repo' '
@@@ -410,9 -440,9 +443,9 @@@ test_expect_success 'clone ssh://host.x
  '
  
  #IPv6
 -for tuah in ::1 [::1] user@::1 user@[::1] [user@::1]
 +for tuah in ::1 [::1] [::1]: user@::1 user@[::1] user@[::1]: [user@::1] [user@::1]:
  do
 -      ehost=$(echo $tuah | tr -d "[]")
 +      ehost=$(echo $tuah | sed -e "s/1]:/1]/ "| tr -d "[]")
        test_expect_success "clone ssh://$tuah/home/user/repo" "
          test_clone_url ssh://$tuah/home/user/repo $ehost /home/user/repo
        "