Quoting of paths when pushing to Windows over SSH

From: Ben Boeckel <mathstuf@gmail.com>
Date: Feb 15, 2022, 18:15 UTC
Message-ID: <YgvtwEPMCH3974e7@erythro.dev.benboeckel.internal>

Hi,

It seems that some quoting when creating the SSH command line for the SSH transport needs some extra logic to handle remote hosts which are not using POSIX shells (namely `cmd`). With the Windows "Enable SSH Server" built-in feature, it ends up failing with this being attempted (as reported by `GIT_TRACE=2`):

    run_command: unset GIT_PREFIX; GIT_PROTOCOL=version=2 ssh -o SendEnv=GIT_PROTOCOL hostname 'C:/Progra~1/Git/mingw64/libexec/git-core/git-receive-pack.exe '\''remote/path'\'''

Of note here:

  - I could not figure out a way to have a path with a space for
    `git-receive-pack` (it just kept saying `C:\Program` is not a
    program regardless of what kinds of quoting I tried), but DOS
    shortnames suffice here at least.
  - the remote path is single-quoted which is not stripped on the other
    side, so the error that `'remote/path'` does not exist ends up
    happening.

I do *not* want to change the default shell on the other side because it is actually useful to be `cmd` in this instance (for `vcvarsall.bat` namely) when logging in manually. Some potential solutions:

  - a way to set the shell to use on the remote for ssh command;
  - a way to say "use double quotes, not single quotes" (or some other
    way to tell `sq_*` that we're not talking to POSIX on the other
    side).

I was able to move forward for now with this patch that I'm using in a specific build to talk to this remote (on top of 2.34.1) because I know I don't have any special characters in my instance:

diff --git a/connect.c b/connect.c
index eaf7d6d261..a286feb812 100644
--- a/connect.c
+++ b/connect.c
@@ -1393,7 +1393,7 @@ struct child_process *git_connect(int fd[2], const char *url,
 
                strbuf_addstr(&cmd, prog);
                strbuf_addch(&cmd, ' ');
-               sq_quote_buf(&cmd, path);
+               strbuf_addstr(&cmd, path);
 
                /* remove repo-local variables from the environment */
                for (var = local_repo_env; *var; var++)

What might be the best way of making this work more widely?

--Ben