From 3b05f7a6eb98569f4f3cbc1065ca1c882cb47deb Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 15 Apr 2019 10:04:56 +0200 Subject: [PATCH 1/6] fixup??? Add MSYS triplets. This patch contains a forgotten change in a comment, and a fix where instead of ".msys", we would append ".smym". Signed-off-by: Johannes Schindelin --- winsup/cygwin/syscalls.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc index 17d532bc02..240777b757 100644 --- a/winsup/cygwin/syscalls.cc +++ b/winsup/cygwin/syscalls.cc @@ -327,7 +327,7 @@ try_to_bin (path_conv &pc, HANDLE &fh, ACCESS_MASK access, ULONG flags) } else { - /* Create unique filename. Start with a dot, followed by "cyg" + /* Create unique filename. Start with a dot, followed by "msys" transposed into the Unicode low surrogate area (U+dc00) on file systems supporting Unicode (except Samba), followed by the inode number in hex, followed by a path hash in hex. The combination @@ -335,7 +335,7 @@ try_to_bin (path_conv &pc, HANDLE &fh, ACCESS_MASK access, ULONG flags) RtlAppendUnicodeToString (&recycler, (pc.fs_flags () & FILE_UNICODE_ON_DISK && !pc.fs_is_samba ()) - ? L".\xdc73\xdc6d\xdc79\xdc6d" : L".msys"); + ? L".\xdc6d\xdc73\xdc79\xdc73" : L".msys"); pfii = (PFILE_INTERNAL_INFORMATION) infobuf; status = NtQueryInformationFile (fh, &io, pfii, sizeof *pfii, FileInternalInformation); From abfa904d9d636c4a190385ae6fc951758d4bb537 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 6 Jan 2017 16:12:05 +0100 Subject: [PATCH 2/6] fixup??? Add functionality for converting UNIX paths in arguments and environment variables to Windows form for native Win32 applications. There is a subtle buffer overrun in the convert() mechanism: after a sub_convert() completed, we may very well be at the end of the conversion, i.e. the "src" pointer may already point at the trailing NUL. Advancing that pointer any further shifts the pointer out of bounds, with the predictable result of copying bogus characters into the output buffer. Another very subtle buffer overrun happens when the the buffer passed to the find_path_start_and_type() function ends in a colon: in that case, the code advances one byte to see whether there is a slash, another colon, or a period, or if there is no equal sign in th rest of the buffer, but since there is only a NUL byte, the code enters the next loop iteration, advancing by one byte *again*, and consequently failing the `it2 != end` condition. Signed-off-by: Johannes Schindelin --- winsup/cygwin/msys2_path_conv.cc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/winsup/cygwin/msys2_path_conv.cc b/winsup/cygwin/msys2_path_conv.cc index 406b98ac81..cf01b8c8ad 100644 --- a/winsup/cygwin/msys2_path_conv.cc +++ b/winsup/cygwin/msys2_path_conv.cc @@ -310,6 +310,10 @@ const char* convert(char *dst, size_t dstlen, const char *src) { } sub_convert(&srcbeg, &srcit, &dstit, dstend, &in_string); + if (!*srcit) { + *dstit = '\0'; + return dst; + } srcbeg = srcit + 1; for (; *srcit != '\0'; ++srcit) { continue; @@ -441,7 +445,7 @@ path_type find_path_start_and_type(const char** src, int recurse, const char* en return find_path_start_and_type(src, true, end); } - if (ch == ':') { + if (ch == ':' && it2 + 1 != end) { it2 += 1; ch = *it2; if (ch == '/' || ch == ':' || ch == '.') { From 6353aa5427d672bf4a57c51ebfeeb0245679e441 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 19 Jan 2017 16:02:44 +0100 Subject: [PATCH 3/6] fixup??? Add functionality for converting UNIX paths in arguments and environment variables to Windows form for native Win32 applications. The purpose of the subp_convert() function is to convert POSIX path lists' individual components. As a special case, the code allows URLs to be left alone. However, the code in ppl_convert() to identify what constitutes is buggy, as it is too loose in what it interprets as URL (the mere substring "://" will fool it into believing that, say, "/c/://share/c$" constitutes a URL), and when subp_convert() then asks the find_path_start_and_type() function to determine the type, it correctly identifies this as a POSIX path list, and then subp_convert() calls ppl_convert() again, causing an infinite recursion. The symptom is a "Bad address" error message. As a stop-gap measure (leaving the fix that teaches ppl_convert() to be more careful in what it identifies as URLs for later), let's just force subp_convert() to trust the caller's judgement when the `is_url` parameter is non-zero. This fixes https://github.com/git-for-windows/git/issues/1033 Signed-off-by: Johannes Schindelin --- winsup/cygwin/msys2_path_conv.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/winsup/cygwin/msys2_path_conv.cc b/winsup/cygwin/msys2_path_conv.cc index cf01b8c8ad..1b07bebb25 100644 --- a/winsup/cygwin/msys2_path_conv.cc +++ b/winsup/cygwin/msys2_path_conv.cc @@ -540,7 +540,7 @@ void url_convert(const char** from, const char* to, char** dst, const char* dste void subp_convert(const char** from, const char* end, int is_url, char** dst, const char* dstend) { const char* begin = *from; - path_type type = find_path_start_and_type(from, 0, end); + path_type type = is_url ? URL : find_path_start_and_type(from, 0, end); copy_to_dst(begin, *from, dst, dstend); if (type == NONE) { From 8b8c603957bbb56da45905ba3d725dc9e94e9935 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 19 Jan 2017 16:21:00 +0100 Subject: [PATCH 4/6] fixup??? Add functionality for converting UNIX paths in arguments and environment variables to Windows form for native Win32 applications. When the ppl_convert() function (that is tasked with converting POSIX path lists to their Windows equivalents) looks at a "://", it immediately jumps to the conclusion that it is looking at a URL right now. However, this misidentifies, say, "/c/://server/share" as a URL. Let's not do that. Be more careful by having a closer look at the protocol part of the URL: it is safe to assume that it must start with a letter and continue with alpha-numeric characters (or a "+", to allow for something like "git+http://example.com"). Please note that we still do not go out of our way to verify that the *server* part of the URL makes sense: that would complexify the code dramatically (see https://url.spec.whatwg.org/#url-parsing for a glimpse how complicated URL parsing/validation actually is), so we just err on the side of "good enough for now". Please note that this has become an issue with PATHs containing UNC components only recently: it started with the introduction of ORIGINAL_PATH into MSYS2, as going through the POSIX->Windows barrier in nested calls *will* encounter "://" in ORIGINAL_PATH even if the original PATH only contained double *back*-slashes (i.e. ";\\"). Signed-off-by: Johannes Schindelin --- winsup/cygwin/msys2_path_conv.cc | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/winsup/cygwin/msys2_path_conv.cc b/winsup/cygwin/msys2_path_conv.cc index 1b07bebb25..ddeffa4d2b 100644 --- a/winsup/cygwin/msys2_path_conv.cc +++ b/winsup/cygwin/msys2_path_conv.cc @@ -570,9 +570,16 @@ void ppl_convert(const char** from, const char* to, char** dst, const char* dste if (prev_was_simc) { continue; } - if (*(it + 1) == '/' && *(it + 2) == '/') { + if (*(it + 1) == '/' && *(it + 2) == '/' && isalpha(*beg)) { is_url = 1; - continue; + /* double-check: protocol must be alnum (or +) */ + for (const char *p = beg; p != it; ++p) + if (!isalnum(*p) && *p != '+') { + is_url = 0; + break; + } + if (is_url) + continue; } prev_was_simc = 1; subp_convert(&beg, it, is_url, dst, dstend); From f795d07e46e2e332e4eed7b3c3e86f9942b6aabd Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 17 May 2017 18:13:32 +0200 Subject: [PATCH 5/6] strace --quiet: be *really* quiet The biggest problem with strace spitting out `create_child: ...` despite being asked to be real quiet is that its output can very well interfere with scripts' operations. For example, when running any of Git for Windows' shell scripts with `GIT_STRACE_COMMANDS=/path/to/logfile` (which is sadly an often needed debugging technique while trying to address the many MSYS2 issues Git for Windows faces), any time the output of any command is redirected into a variable, it will include that `create_child: ...` line, wreaking havoc with Git's expectations. So let's just really be quiet when we're asked to be quiet. Signed-off-by: Johannes Schindelin --- winsup/utils/strace.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/winsup/utils/strace.cc b/winsup/utils/strace.cc index 6c94158f3d..e7291bbc8d 100644 --- a/winsup/utils/strace.cc +++ b/winsup/utils/strace.cc @@ -351,7 +351,8 @@ create_child (char **argv) flags |= CREATE_NEW_CONSOLE | CREATE_NEW_PROCESS_GROUP; make_command_line (one_line, argv); - printf ("create_child: %s\n", one_line.buf); + if (!quiet) + printf ("create_child: %s\n", one_line.buf); SetConsoleCtrlHandler (NULL, 0); /* Commit message for this code was: From 9dcd6bbf5f244dae7d4441b1167b3c281d02b919 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 14 Mar 2018 17:04:19 +0100 Subject: [PATCH 6/6] Fix incorrect path conversion (trailing slash) The problem with the original approach is not that it is completely bogus. After all, when you pass the option --prefix=/tmp/ to Git, you do want to end up with a trailing slash. The real problem with the original approach is that it simply changed path_conv, completely oblivious and careless about other users of path_conv. That was really wrong, of course, and cost us time, sweat and tears. The appropriate approach is to *not* affect other users, but instead introduce a flag that we use in *our* caller, so that everybody gets what they want: - emulation of inodes on FAT by calculating the hash on the normalized path: works. - MSYS2's conversion of "POSIX" paths to Windows paths: works. Signed-off-by: Johannes Schindelin --- winsup/cygwin/msys2_path_conv.cc | 2 +- winsup/cygwin/path.cc | 16 +++++++++++++++- winsup/cygwin/path.h | 1 + 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/winsup/cygwin/msys2_path_conv.cc b/winsup/cygwin/msys2_path_conv.cc index ddeffa4d2b..646eef32c4 100644 --- a/winsup/cygwin/msys2_path_conv.cc +++ b/winsup/cygwin/msys2_path_conv.cc @@ -624,7 +624,7 @@ void posix_to_win32_path(const char* from, const char* to, char** dst, const cha strncpy(one_path, from, to-from); one_path[to-from] = '\0'; - path_conv conv (one_path, 0); + path_conv conv (one_path, PC_KEEP_FINAL_SLASH); if (conv.error) { set_errno(conv.error); diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc index ad152e461a..f311a04962 100644 --- a/winsup/cygwin/path.cc +++ b/winsup/cygwin/path.cc @@ -1236,9 +1236,22 @@ path_conv::check (const char *src, unsigned opt, cfree (wide_path); wide_path = NULL; } + + if (need_directory) + { + size_t n = strlen (this->path); + /* Do not add trailing \ to UNC device names like \\.\a: */ + if (this->path[n - 1] != '\\' && + (strncmp (this->path, "\\\\.\\", 4) != 0)) + { + this->modifiable_path ()[n] = '\\'; + this->modifiable_path ()[n + 1] = '\0'; + } + need_directory = 0; + } } - if (need_directory) + if ((opt & PC_KEEP_FINAL_SLASH) && need_directory) { size_t n = strlen (this->path); /* Do not add trailing \ to UNC device names like \\.\a: */ @@ -1248,6 +1261,7 @@ path_conv::check (const char *src, unsigned opt, this->modifiable_path ()[n] = '\\'; this->modifiable_path ()[n + 1] = '\0'; } + need_directory = 0; } if (opt & PC_OPEN) diff --git a/winsup/cygwin/path.h b/winsup/cygwin/path.h index 0c94c61522..69a78bcbca 100644 --- a/winsup/cygwin/path.h +++ b/winsup/cygwin/path.h @@ -58,6 +58,7 @@ enum pathconv_arg PC_SYM_NOFOLLOW_PROCFD = _BIT (11), PC_KEEP_HANDLE = _BIT (12), PC_NO_ACCESS_CHECK = _BIT (13), + PC_KEEP_FINAL_SLASH = _BIT (14), PC_DONT_USE = _BIT (31) /* conversion to signed happens. */ };