mingw: special-case arguments to `sh`

The MSYS2 runtime does its best to emulate the command-line wildcard expansion and de-quoting which would be performed by the calling Unix shell on Unix systems. Those Unix shell quoting rules differ from the quoting rules applying to Windows' cmd and Powershell, making it a little awkward to quote command-line parameters properly when spawning other processes. In particular, git.exe passes arguments to subprocesses that are *not* intended to be interpreted as wildcards, and if they contain backslashes, those are not to be interpreted as escape characters, e.g. when passing Windows paths. Note: this is only a problem when calling MSYS2 executables, not when calling MINGW executables such as git.exe. However, we do call MSYS2 executables frequently, most notably when setting the use_shell flag in the child_process structure. There is no elegant way to determine whether the .exe file to be executed is an MSYS2 program or a MINGW one. But since the use case of passing a command line through the shell is so prevalent, we need to work around this issue at least when executing sh.exe. Let's introduce an ugly, hard-coded test whether argv[0] is "sh", and whether it refers to the MSYS2 Bash, to determine whether we need to quote the arguments differently than usual. That still does not fix the issue completely, but at least it is something. Incidentally, this also fixes the problem where `git clone \\server\repo` failed due to incorrect handling of the backslashes when handing the path to the git-upload-pack process. Further, we need to take care to quote not only whitespace and backslashes, but also curly brackets. As aliases frequently go through the MSYS2 Bash, and as aliases frequently get parameters such as HEAD@{yesterday}, this is really important. As an early version of this patch broke this, let's make sure that this does not regress by adding a test case for that. Helped-by: Kim Gybels <kgybels@infogroep.be> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Johannes Schindelin committed Jan 17, 2019 at 12:14 UTC 9e9da23c2765050ff30d34540fbab62a1b4e5d01
3 files changed +87 -2
compat/mingw.c
+76 -1
@@ -7,6 +7,7 @@
7 #include "../cache.h"
8 #include "win32/lazyload.h"
9 #include "../config.h"
10 +#include "dir.h"
11
12 #define HCAST(type, handle) ((type)(intptr_t)handle)
13
@@ -1031,7 +1032,7 @@ char *mingw_getcwd(char *pointer, int len)
1032 * See "Parsing C++ Command-Line Arguments" at Microsoft's Docs:
1033 * https://docs.microsoft.com/en-us/cpp/cpp/parsing-cpp-command-line-arguments
1034 */
1034 -static const char *quote_arg(const char *arg)
1035 +static const char *quote_arg_msvc(const char *arg)
1036 {
1037 /* count chars to quote */
1038 int len = 0, n = 0;
@@ -1086,6 +1087,37 @@ static const char *quote_arg(const char *arg)
1087 return q;
1088 }
1089
1090 +#include "quote.h"
1091 +
1092 +static const char *quote_arg_msys2(const char *arg)
1093 +{
1094 + struct strbuf buf = STRBUF_INIT;
1095 + const char *p2 = arg, *p;
1096 +
1097 + for (p = arg; *p; p++) {
1098 + int ws = isspace(*p);
1099 + if (!ws && *p != '\\' && *p != '"' && *p != '{')
1100 + continue;
1101 + if (!buf.len)
1102 + strbuf_addch(&buf, '"');
1103 + if (p != p2)
1104 + strbuf_add(&buf, p2, p - p2);
1105 + if (!ws && *p != '{')
1106 + strbuf_addch(&buf, '\\');
1107 + p2 = p;
1108 + }
1109 +
1110 + if (p == arg)
1111 + strbuf_addch(&buf, '"');
1112 + else if (!buf.len)
1113 + return arg;
1114 + else
1115 + strbuf_add(&buf, p2, p - p2),
1116 +
1117 + strbuf_addch(&buf, '"');
1118 + return strbuf_detach(&buf, 0);
1119 +}
1120 +
1121 static const char *parse_interpreter(const char *cmd)
1122 {
1123 static char buf[100];
@@ -1317,6 +1349,47 @@ struct pinfo_t {
1349 static struct pinfo_t *pinfo = NULL;
1350 CRITICAL_SECTION pinfo_cs;
1351
1352 +/* Used to match and chomp off path components */
1353 +static inline int match_last_path_component(const char *path, size_t *len,
1354 + const char *component)
1355 +{
1356 + size_t component_len = strlen(component);
1357 + if (*len < component_len + 1 ||
1358 + !is_dir_sep(path[*len - component_len - 1]) ||
1359 + fspathncmp(path + *len - component_len, component, component_len))
1360 + return 0;
1361 + *len -= component_len + 1;
1362 + /* chomp off repeated dir separators */
1363 + while (*len > 0 && is_dir_sep(path[*len - 1]))
1364 + (*len)--;
1365 + return 1;
1366 +}
1367 +
1368 +static int is_msys2_sh(const char *cmd)
1369 +{
1370 + if (cmd && !strcmp(cmd, "sh")) {
1371 + static int ret = -1;
1372 + char *p;
1373 +
1374 + if (ret >= 0)
1375 + return ret;
1376 +
1377 + p = path_lookup(cmd, 0);
1378 + if (!p)
1379 + ret = 0;
1380 + else {
1381 + size_t len = strlen(p);
1382 +
1383 + ret = match_last_path_component(p, &len, "sh.exe") &&
1384 + match_last_path_component(p, &len, "bin") &&
1385 + match_last_path_component(p, &len, "usr");
1386 + free(p);
1387 + }
1388 + return ret;
1389 + }
1390 + return 0;
1391 +}
1392 +
1393 static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaenv,
1394 const char *dir,
1395 int prepend_cmd, int fhin, int fhout, int fherr)
@@ -1328,6 +1401,8 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaen
1401 unsigned flags = CREATE_UNICODE_ENVIRONMENT;
1402 BOOL ret;
1403 HANDLE cons;
1404 + const char *(*quote_arg)(const char *arg) =
1405 + is_msys2_sh(*argv) ? quote_arg_msys2 : quote_arg_msvc;
1406
1407 do_unset_environment_variables();
1408
t/t0061-run-command.sh
+10
@@ -199,4 +199,14 @@ test_expect_success 'GIT_TRACE with environment variables' '
199 )
200 '
201
202 +test_expect_success MINGW 'verify curlies are quoted properly' '
203 + : force the rev-parse through the MSYS2 Bash &&
204 + git -c alias.r="!git rev-parse" r -- a{b}c >actual &&
205 + cat >expect <<-\EOF &&
206 + --
207 + a{b}c
208 + EOF
209 + test_cmp expect actual
210 +'
211 +
212 test_done
t/t5580-clone-push-unc.sh
+1 -1
@@ -40,7 +40,7 @@ test_expect_success clone '
40 git clone "file://$UNCPATH" clone
41 '
42
43 -test_expect_failure 'clone with backslashed path' '
43 +test_expect_success 'clone with backslashed path' '
44 BACKSLASHED="$(echo "$UNCPATH" | tr / \\\\)" &&
45 git clone "$BACKSLASHED" backslashed
46 '