Make argument quotation compatible with cygwin/msys2/gitbash on Windows #97
Conversation
|
@AkihiroSuda could you take a look? |
| @@ -252,6 +252,16 @@ func (rsf *ReverseSSHFS) Start() error { | |||
| return nil | |||
| } | |||
|
|
|||
| func addQuotes(input string) string { | |||
| input = strconv.Quote(input) | |||
| if runtime.GOOS == "windows" { | |||
There was a problem hiding this comment.
Could you add a comment to explain the reason?
Also, can we have a unit test and/or an integration test?
There was a problem hiding this comment.
I will try to sketch the unit test. It will take some time, I will ping you, when it is done.
There was a problem hiding this comment.
I managed to get a reproducer with a command line example. The command is extracted using recording processes started on the host.
"C:\Program Files\Git\usr\bin\ssh.exe" -i /c/Users/User/.local/share/containers/podman/machine/machine -p 60251 core@localhost -- mkdir -p \"/c/Users/runneradmin\"
bash: -c: line 1: unexpected EOF while looking for matching `"'I don't think this can be reasonably covered by integration test - this would recover spinning VM just for that test.
And the method in question is not unit testable as it actually executes the resulting external commands within itself.
It would be only possible to cover private addQuotes with a unit test, which is actually a single standard library call on all platforms other than Windows, but I will add it and add the comment.
There was a problem hiding this comment.
Added basic unit test and also extended test matrix to include Windows.
arixmkii
force-pushed
the
fix-windows-escape
branch
2 times, most recently
from
d6572fa to
6c1fa18
Compare
Signed-off-by: Arthur Sengileyev <arthur.sengileyev@gmail.com>
arixmkii
force-pushed
the
fix-windows-escape
branch
from
6c1fa18 to
f469c85
Compare
It is the same situation like in https://github.com/lima-vm/lima/blob/786c1f5f3c6e8ff276c0c5d6789cdd9243d70f49/pkg/sshutil/sshutil.go#L271
Windows based tooling can't correctly escape
".The implemented workaround probably will not work for some extreme cases, but at least it is not failing outright from the start.
It has been tested to work on Windows with Git Bash and WSL2 based tooling.