ssh: introduce a 'simple' ssh variant

When using the 'ssh' transport, the '-o' option is used to specify an
environment variable which should be set on the remote end.  This allows
git to send additional information when contacting the server,
requesting the use of a different protocol version via the
'GIT_PROTOCOL' environment variable like so: "-o SendEnv=GIT_PROTOCOL".

Unfortunately not all ssh variants support the sending of environment
variables to the remote end.  To account for this, only use the '-o'
option for ssh variants which are OpenSSH compliant.  This is done by
checking that the basename of the ssh command is 'ssh' or the ssh
variant is overridden to be 'ssh' (via the ssh.variant config).

Other options like '-p' and '-P', which are used to specify a specific
port to use, or '-4' and '-6', which are used to indicate that IPV4 or
IPV6 addresses should be used, may also not be supported by all ssh
variants.

Currently if an ssh command's basename wasn't 'plink' or
'tortoiseplink' git assumes that the command is an OpenSSH variant.
Since user configured ssh commands may not be OpenSSH compliant, tighten
this constraint and assume a variant of 'simple' if the basename of the
command doesn't match the variants known to git.  The new ssh variant
'simple' will only have the host and command to execute ([username@]host
command) passed as parameters to the ssh command.

Update the Documentation to better reflect the command-line options sent
to ssh commands based on their variant.

Reported-by: Jeffrey Yasskin <jyasskin@google.com>
Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/config.txt b/Documentation/config.txt
index b78747abc66c325fadb2a69db95ce0bbdf262663..0460af37e2617d0ab7ab8f33c3d9d745237da606 100644 (file)
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2084,12 +2084,31 @@ ssh.variant::
        Depending on the value of the environment variables `GIT_SSH` or
        `GIT_SSH_COMMAND`, or the config setting `core.sshCommand`, Git
        auto-detects whether to adjust its command-line parameters for use
        Depending on the value of the environment variables `GIT_SSH` or
        `GIT_SSH_COMMAND`, or the config setting `core.sshCommand`, Git
        auto-detects whether to adjust its command-line parameters for use

-       with plink or tortoiseplink, as opposed to the default (OpenSSH).
+       with ssh (OpenSSH), plink or tortoiseplink, as opposed to the default
+       (simple).

 +
 The config variable `ssh.variant` can be set to override this auto-detection;
 +
 The config variable `ssh.variant` can be set to override this auto-detection;

-valid values are `ssh`, `plink`, `putty` or `tortoiseplink`. Any other value
-will be treated as normal ssh. This setting can be overridden via the
-environment variable `GIT_SSH_VARIANT`.
+valid values are `ssh`, `simple`, `plink`, `putty` or `tortoiseplink`. Any
+other value will be treated as normal ssh. This setting can be overridden via
+the environment variable `GIT_SSH_VARIANT`.
++
+The current command-line parameters used for each variant are as
+follows:
++
+--
+
+* `ssh` - [-p port] [-4] [-6] [-o option] [username@]host command
+
+* `simple` - [username@]host command
+
+* `plink` or `putty` - [-P port] [-4] [-6] [username@]host command
+
+* `tortoiseplink` - [-P port] [-4] [-6] -batch [username@]host command
+
+--
++
+Except for the `simple` variant, command-line parameters are likely to
+change as git gains new features.

 
 i18n.commitEncoding::
        Character encoding the commit messages are stored in; Git itself
 
 i18n.commitEncoding::
        Character encoding the commit messages are stored in; Git itself

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 7518ea3af2d94b035c188e267869a3a2ee0db657..8bc3f21473b339983c6b4456a83a8393e7b15616 100644 (file)
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -518,11 +518,10 @@ other
        If either of these environment variables is set then 'git fetch'
        and 'git push' will use the specified command instead of 'ssh'
        when they need to connect to a remote system.
        If either of these environment variables is set then 'git fetch'
        and 'git push' will use the specified command instead of 'ssh'
        when they need to connect to a remote system.

-       The command will be given exactly two or four arguments: the
-       'username@host' (or just 'host') from the URL and the shell
-       command to execute on that remote system, optionally preceded by
-       `-p` (literally) and the 'port' from the URL when it specifies
-       something other than the default SSH port.
+       The command-line parameters passed to the configured command are
+       determined by the ssh variant.  See `ssh.variant` option in
+       linkgit:git-config[1] for details.
+

 +
 `$GIT_SSH_COMMAND` takes precedence over `$GIT_SSH`, and is interpreted
 by the shell, which allows additional arguments to be included.
 +
 `$GIT_SSH_COMMAND` takes precedence over `$GIT_SSH`, and is interpreted
 by the shell, which allows additional arguments to be included.

diff --git a/connect.c b/connect.c
index b8695a2fa28b5fd9eb9dc4b3e0291b481c57bd6a..7fbd396b35ee8f84350d45fc74d9fee07fec86ba 100644 (file)
--- a/connect.c
+++ b/connect.c
@@ -776,37 +776,44 @@ static const char *get_ssh_command(void)
        return NULL;
 }
 
        return NULL;
 }
 

-static int override_ssh_variant(int *port_option, int *needs_batch)
+enum ssh_variant {
+       VARIANT_SIMPLE,
+       VARIANT_SSH,
+       VARIANT_PLINK,
+       VARIANT_PUTTY,
+       VARIANT_TORTOISEPLINK,
+};
+
+static int override_ssh_variant(enum ssh_variant *ssh_variant)

 {
 {

-       char *variant;
+       const char *variant = getenv("GIT_SSH_VARIANT");

 
 

-       variant = xstrdup_or_null(getenv("GIT_SSH_VARIANT"));
-       if (!variant &&
-           git_config_get_string("ssh.variant", &variant))
+       if (!variant && git_config_get_string_const("ssh.variant", &variant))

                return 0;
 
                return 0;
 

-       if (!strcmp(variant, "plink") || !strcmp(variant, "putty")) {
-               *port_option = 'P';
-               *needs_batch = 0;
-       } else if (!strcmp(variant, "tortoiseplink")) {
-               *port_option = 'P';
-               *needs_batch = 1;
-       } else {
-               *port_option = 'p';
-               *needs_batch = 0;
-       }
-       free(variant);
+       if (!strcmp(variant, "plink"))
+               *ssh_variant = VARIANT_PLINK;
+       else if (!strcmp(variant, "putty"))
+               *ssh_variant = VARIANT_PUTTY;
+       else if (!strcmp(variant, "tortoiseplink"))
+               *ssh_variant = VARIANT_TORTOISEPLINK;
+       else if (!strcmp(variant, "simple"))
+               *ssh_variant = VARIANT_SIMPLE;
+       else
+               *ssh_variant = VARIANT_SSH;
+

        return 1;
 }
 
        return 1;
 }
 

-static void handle_ssh_variant(const char *ssh_command, int is_cmdline,
-                              int *port_option, int *needs_batch)
+static enum ssh_variant determine_ssh_variant(const char *ssh_command,
+                                             int is_cmdline)

 {
 {

+       enum ssh_variant ssh_variant = VARIANT_SIMPLE;

        const char *variant;
        char *p = NULL;
 
        const char *variant;
        char *p = NULL;
 

-       if (override_ssh_variant(port_option, needs_batch))
-               return;
+       if (override_ssh_variant(&ssh_variant))
+               return ssh_variant;

 
        if (!is_cmdline) {
                p = xstrdup(ssh_command);
 
        if (!is_cmdline) {
                p = xstrdup(ssh_command);


@@ -825,19 +832,22 @@ static void handle_ssh_variant(const char *ssh_command, int is_cmdline,
                        free(ssh_argv);
                } else {
                        free(p);
                        free(ssh_argv);
                } else {
                        free(p);

-                       return;
+                       return ssh_variant;

                }
        }
 
                }
        }
 

-       if (!strcasecmp(variant, "plink") ||
-           !strcasecmp(variant, "plink.exe"))
-               *port_option = 'P';
+       if (!strcasecmp(variant, "ssh") ||
+           !strcasecmp(variant, "ssh.exe"))
+               ssh_variant = VARIANT_SSH;
+       else if (!strcasecmp(variant, "plink") ||
+                !strcasecmp(variant, "plink.exe"))
+               ssh_variant = VARIANT_PLINK;

        else if (!strcasecmp(variant, "tortoiseplink") ||
        else if (!strcasecmp(variant, "tortoiseplink") ||

-                !strcasecmp(variant, "tortoiseplink.exe")) {
-               *port_option = 'P';
-               *needs_batch = 1;
-       }
+                !strcasecmp(variant, "tortoiseplink.exe"))
+               ssh_variant = VARIANT_TORTOISEPLINK;
+

        free(p);
        free(p);

+       return ssh_variant;

 }
 
 /*
 }
 
 /*


@@ -937,8 +947,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 needs_batch = 0;
-                       int port_option = 'p';
+                       enum ssh_variant variant;

                        char *ssh_host = hostandport;
                        const char *port = NULL;
                        transport_check_allowed("ssh");
                        char *ssh_host = hostandport;
                        const char *port = NULL;
                        transport_check_allowed("ssh");


@@ -965,10 +974,9 @@ struct child_process *git_connect(int fd[2], const char *url,
                                die("strange hostname '%s' blocked", ssh_host);
 
                        ssh = get_ssh_command();
                                die("strange hostname '%s' blocked", ssh_host);
 
                        ssh = get_ssh_command();

-                       if (ssh)
-                               handle_ssh_variant(ssh, 1, &port_option,
-                                                  &needs_batch);
-                       else {
+                       if (ssh) {
+                               variant = determine_ssh_variant(ssh, 1);
+                       } else {

                                /*
                                 * GIT_SSH is the no-shell version of
                                 * GIT_SSH_COMMAND (and must remain so for
                                /*
                                 * GIT_SSH is the no-shell version of
                                 * GIT_SSH_COMMAND (and must remain so for


@@ -979,32 +987,38 @@ struct child_process *git_connect(int fd[2], const char *url,
                                ssh = getenv("GIT_SSH");
                                if (!ssh)
                                        ssh = "ssh";
                                ssh = getenv("GIT_SSH");
                                if (!ssh)
                                        ssh = "ssh";

-                               else
-                                       handle_ssh_variant(ssh, 0,
-                                                          &port_option,
-                                                          &needs_batch);
+                               variant = determine_ssh_variant(ssh, 0);

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

-                       if (get_protocol_version_config() > 0) {
+                       if (variant == VARIANT_SSH &&
+                           get_protocol_version_config() > 0) {

                                argv_array_push(&conn->args, "-o");
                                argv_array_push(&conn->args, "SendEnv=" GIT_PROTOCOL_ENVIRONMENT);
                                argv_array_pushf(&conn->env_array, GIT_PROTOCOL_ENVIRONMENT "=version=%d",
                                                 get_protocol_version_config());
                        }
 
                                argv_array_push(&conn->args, "-o");
                                argv_array_push(&conn->args, "SendEnv=" GIT_PROTOCOL_ENVIRONMENT);
                                argv_array_pushf(&conn->env_array, GIT_PROTOCOL_ENVIRONMENT "=version=%d",
                                                 get_protocol_version_config());
                        }
 

-                       if (flags & CONNECT_IPV4)
-                               argv_array_push(&conn->args, "-4");
-                       else if (flags & CONNECT_IPV6)
-                               argv_array_push(&conn->args, "-6");
-                       if (needs_batch)
+                       if (variant != VARIANT_SIMPLE) {
+                               if (flags & CONNECT_IPV4)
+                                       argv_array_push(&conn->args, "-4");
+                               else if (flags & CONNECT_IPV6)
+                                       argv_array_push(&conn->args, "-6");
+                       }
+
+                       if (variant == VARIANT_TORTOISEPLINK)

                                argv_array_push(&conn->args, "-batch");
                                argv_array_push(&conn->args, "-batch");

-                       if (port) {
-                               argv_array_pushf(&conn->args,
-                                                "-%c", port_option);
+
+                       if (port && variant != VARIANT_SIMPLE) {
+                               if (variant == VARIANT_SSH)
+                                       argv_array_push(&conn->args, "-p");
+                               else
+                                       argv_array_push(&conn->args, "-P");
+

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

+

                        argv_array_push(&conn->args, ssh_host);
                } else {
                        transport_check_allowed("file");
                        argv_array_push(&conn->args, ssh_host);
                } else {
                        transport_check_allowed("file");

diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 9c56f771b619e4bb931ce3d34a5f9325b8f61e90..86811a0c35f0a1baf23ce81a5829adf7b896548a 100755 (executable)
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -309,8 +309,8 @@ test_expect_success 'clone checking out a tag' '
 setup_ssh_wrapper () {
        test_expect_success 'setup ssh wrapper' '
                cp "$GIT_BUILD_DIR/t/helper/test-fake-ssh$X" \
 setup_ssh_wrapper () {
        test_expect_success 'setup ssh wrapper' '
                cp "$GIT_BUILD_DIR/t/helper/test-fake-ssh$X" \

-                       "$TRASH_DIRECTORY/ssh-wrapper$X" &&
-               GIT_SSH="$TRASH_DIRECTORY/ssh-wrapper$X" &&
+                       "$TRASH_DIRECTORY/ssh$X" &&
+               GIT_SSH="$TRASH_DIRECTORY/ssh$X" &&

                export GIT_SSH &&
                export TRASH_DIRECTORY &&
                >"$TRASH_DIRECTORY"/ssh-output
                export GIT_SSH &&
                export TRASH_DIRECTORY &&
                >"$TRASH_DIRECTORY"/ssh-output


@@ -318,7 +318,7 @@ setup_ssh_wrapper () {
 }
 
 copy_ssh_wrapper_as () {
 }
 
 copy_ssh_wrapper_as () {

-       cp "$TRASH_DIRECTORY/ssh-wrapper$X" "${1%$X}$X" &&
+       cp "$TRASH_DIRECTORY/ssh$X" "${1%$X}$X" &&

        GIT_SSH="${1%$X}$X" &&
        export GIT_SSH
 }
        GIT_SSH="${1%$X}$X" &&
        export GIT_SSH
 }


@@ -362,10 +362,26 @@ 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' '
+test_expect_success 'OpenSSH variant passes -4' '
+       git clone -4 "[myhost:123]:src" ssh-ipv4-clone &&
+       expect_ssh "-4 -p 123" myhost src
+'
+
+test_expect_success 'variant can be overriden' '
+       git -c ssh.variant=simple clone -4 "[myhost:123]:src" ssh-simple-clone &&
+       expect_ssh myhost src
+'
+
+test_expect_success 'simple is treated as simple' '
+       copy_ssh_wrapper_as "$TRASH_DIRECTORY/simple" &&
+       git clone -4 "[myhost:123]:src" ssh-bracket-clone-simple &&
+       expect_ssh myhost src
+'
+
+test_expect_success 'uplink is treated as simple' '

        copy_ssh_wrapper_as "$TRASH_DIRECTORY/uplink" &&
        git clone "[myhost:123]:src" ssh-bracket-clone-uplink &&
        copy_ssh_wrapper_as "$TRASH_DIRECTORY/uplink" &&
        git clone "[myhost:123]:src" ssh-bracket-clone-uplink &&

-       expect_ssh "-p 123" myhost src
+       expect_ssh myhost src

 '
 
 test_expect_success 'plink is treated specially (as putty)' '
 '
 
 test_expect_success 'plink is treated specially (as putty)' '

diff --git a/t/t5700-protocol-v1.sh b/t/t5700-protocol-v1.sh
index b0779d3623910b283662ea6f73f678aa85c3bfb1..ba86a44eb183921e844dff6f9d6a5bb27c7d5e91 100755 (executable)
--- a/t/t5700-protocol-v1.sh
+++ b/t/t5700-protocol-v1.sh
@@ -147,6 +147,8 @@ test_expect_success 'push with file:// using protocol v1' '
 test_expect_success 'setup ssh wrapper' '
        GIT_SSH="$GIT_BUILD_DIR/t/helper/test-fake-ssh" &&
        export GIT_SSH &&
 test_expect_success 'setup ssh wrapper' '
        GIT_SSH="$GIT_BUILD_DIR/t/helper/test-fake-ssh" &&
        export GIT_SSH &&

+       GIT_SSH_VARIANT=ssh &&
+       export GIT_SSH_VARIANT &&

        export TRASH_DIRECTORY &&
        >"$TRASH_DIRECTORY"/ssh-output
 '
        export TRASH_DIRECTORY &&
        >"$TRASH_DIRECTORY"/ssh-output
 '