[prev in list] [next in list] [prev in thread] [next in thread] 

List:       git
Subject:    Quoting of paths when pushing to Windows over SSH
From:       Ben Boeckel <mathstuf () gmail ! com>
Date:       2022-02-15 18:15:28
Message-ID: YgvtwEPMCH3974e7 () erythro ! dev ! benboeckel ! internal
[Download RAW message or body]

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


[prev in list] [next in list] [prev in thread] [next in thread]