From 94ce8e7e710b9a3544f97ccd5ff82903d3158978 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 19 Jun 2017 17:11:16 +0200 Subject: [PATCH 1/3] mingw (t5580): document bug when cloning from backslashed UNC paths Due to a quirk in Git's method to spawn git-upload-pack, there is a problem when passing paths with backslashes in them: Git will force the command-line through the shell, which has different quoting semantics in Git for Windows (being an MSYS2 program) than regular Win32 executables such as git.exe itself. The symptom is that the first of the two backslashes in UNC paths of the form \\myserver\folder\repository.git is *stripped off*. Document this bug by introducing a test case. Signed-off-by: Johannes Schindelin --- t/t5580-clone-push-unc.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/t/t5580-clone-push-unc.sh b/t/t5580-clone-push-unc.sh index ba548df4a9..c3703765f4 100755 --- a/t/t5580-clone-push-unc.sh +++ b/t/t5580-clone-push-unc.sh @@ -40,6 +40,11 @@ test_expect_success clone ' git clone "file://$UNCPATH" clone ' +test_expect_failure 'clone with backslashed path' ' + BACKSLASHED="$(echo "$UNCPATH" | tr / \\\\)" && + git clone "$BACKSLASHED" backslashed +' + test_expect_success push ' ( cd clone && From cac536a08ec4457d388caffeae3ca6c20c42fee2 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 19 Jun 2017 16:35:17 +0200 Subject: [PATCH 2/3] 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. We need to take care to quote not only whitespace, but also curly brackets. As aliases frequently go through the MSYS2 Bash, and as aliases frequently get parameters such as HEAD@{yesterday}, let's make sure that this does not regress by adding a test case for that. Helped-by: Kim Gybels Signed-off-by: Johannes Schindelin --- compat/mingw.c | 63 ++++++++++++++++++++++++++++++++++++++- t/t0061-run-command.sh | 10 +++++++ t/t5580-clone-push-unc.sh | 2 +- 3 files changed, 73 insertions(+), 2 deletions(-) diff --git a/compat/mingw.c b/compat/mingw.c index 858ca14a57..3bb5f3ea3a 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -900,7 +900,7 @@ char *mingw_getcwd(char *pointer, int len) * See http://msdn2.microsoft.com/en-us/library/17w5ykft(vs.71).aspx * (Parsing C++ Command-Line Arguments) */ -static const char *quote_arg(const char *arg) +static const char *quote_arg_msvc(const char *arg) { /* count chars to quote */ int len = 0, n = 0; @@ -955,6 +955,37 @@ static const char *quote_arg(const char *arg) return q; } +#include "quote.h" + +static const char *quote_arg_msys2(const char *arg) +{ + struct strbuf buf = STRBUF_INIT; + const char *p2 = arg, *p; + + for (p = arg; *p; p++) { + int ws = isspace(*p); + if (!ws && *p != '\\' && *p != '"' && *p != '{') + continue; + if (!buf.len) + strbuf_addch(&buf, '"'); + if (p != p2) + strbuf_add(&buf, p2, p - p2); + if (!ws && *p != '{') + strbuf_addch(&buf, '\\'); + p2 = p; + } + + if (p == arg) + strbuf_addch(&buf, '"'); + else if (!buf.len) + return arg; + else + strbuf_add(&buf, p2, p - p2), + + strbuf_addch(&buf, '"'); + return strbuf_detach(&buf, 0); +} + static const char *parse_interpreter(const char *cmd) { static char buf[100]; @@ -1088,6 +1119,34 @@ struct pinfo_t { static struct pinfo_t *pinfo = NULL; CRITICAL_SECTION pinfo_cs; +static int is_msys2_sh(const char *cmd) +{ + if (cmd && !strcmp(cmd, "sh")) { + static int ret = -1; + char *p; + + if (ret >= 0) + return ret; + + p = path_lookup(cmd, 0); + if (!p) + ret = 0; + else { + size_t len = strlen(p); + ret = len > 15 && + is_dir_sep(p[len - 15]) && + !strncasecmp(p + len - 14, "usr", 3) && + is_dir_sep(p[len - 11]) && + !strncasecmp(p + len - 10, "bin", 3) && + is_dir_sep(p[len - 7]) && + !strcasecmp(p + len - 6, "sh.exe"); + free(p); + } + return ret; + } + return 0; +} + static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaenv, const char *dir, int prepend_cmd, int fhin, int fhout, int fherr) @@ -1098,6 +1157,8 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaen wchar_t wcmd[MAX_PATH], wdir[MAX_PATH], *wargs, *wenvblk = NULL; unsigned flags = CREATE_UNICODE_ENVIRONMENT; BOOL ret; + const char *(*quote_arg)(const char *arg) = + is_msys2_sh(*argv) ? quote_arg_msys2 : quote_arg_msvc; /* Determine whether or not we are associated to a console */ HANDLE cons = CreateFile("CONOUT$", GENERIC_WRITE, diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh index 3e131c5325..bc74073746 100755 --- a/t/t0061-run-command.sh +++ b/t/t0061-run-command.sh @@ -177,4 +177,14 @@ test_expect_success 'GIT_TRACE with environment variables' ' ) ' +test_expect_success MINGW 'verify curlies are quoted properly' ' + : force the rev-parse through the MSYS2 Bash && + git -c alias.r="!git rev-parse" r -- a{b}c >actual && + cat >expect <<-\EOF && + -- + a{b}c + EOF + test_cmp expect actual +' + test_done diff --git a/t/t5580-clone-push-unc.sh b/t/t5580-clone-push-unc.sh index c3703765f4..217adf3a63 100755 --- a/t/t5580-clone-push-unc.sh +++ b/t/t5580-clone-push-unc.sh @@ -40,7 +40,7 @@ test_expect_success clone ' git clone "file://$UNCPATH" clone ' -test_expect_failure 'clone with backslashed path' ' +test_expect_success 'clone with backslashed path' ' BACKSLASHED="$(echo "$UNCPATH" | tr / \\\\)" && git clone "$BACKSLASHED" backslashed ' From 292ea52a9fd93be0ccf74a2e3afe718b78f1723c Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 13 Jul 2017 14:28:42 +0200 Subject: [PATCH 3/3] t5580: test cloning without file://, test fetching via UNC paths It gets a bit silly to add the commands to the name of the test script, so let's just rename it while we're testing more UNC stuff. Signed-off-by: Johannes Schindelin --- t/{t5580-clone-push-unc.sh => t5580-unc-paths.sh} | 12 ++++++++++++ 1 file changed, 12 insertions(+) rename t/{t5580-clone-push-unc.sh => t5580-unc-paths.sh} (88%) diff --git a/t/t5580-clone-push-unc.sh b/t/t5580-unc-paths.sh similarity index 88% rename from t/t5580-clone-push-unc.sh rename to t/t5580-unc-paths.sh index 217adf3a63..254fefccde 100755 --- a/t/t5580-clone-push-unc.sh +++ b/t/t5580-unc-paths.sh @@ -40,11 +40,23 @@ test_expect_success clone ' git clone "file://$UNCPATH" clone ' +test_expect_success 'clone without file://' ' + git clone "$UNCPATH" clone-without-file +' + test_expect_success 'clone with backslashed path' ' BACKSLASHED="$(echo "$UNCPATH" | tr / \\\\)" && git clone "$BACKSLASHED" backslashed ' +test_expect_success fetch ' + git init to-fetch && + ( + cd to-fetch && + git fetch "$UNCPATH" master + ) +' + test_expect_success push ' ( cd clone &&