From 3bdae7228bdb4dee6d827e6b10f24d9e4ac8a509 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 c6edfc45f1..e9dbca89dd 100644 --- a/winsup/cygwin/syscalls.cc +++ b/winsup/cygwin/syscalls.cc @@ -326,7 +326,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 @@ -334,7 +334,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 23684c6c66e3e492e661358553db6886875a76f5 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 a18cec79d7fb1c8af5913f2cad3268a3f17759e5 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 970e1480b6b2914d19f9f8e6e68d50af71b57829 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 e20cef85ff9b0b25a9af62f72aa8b4b327a0bdda 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 c5a09527b5bf527cae4768d9eeede69d288ac5d0 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 602882ff4b..b79f604ee7 100644 --- a/winsup/cygwin/path.cc +++ b/winsup/cygwin/path.cc @@ -1211,9 +1211,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: */ @@ -1223,6 +1236,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 b94f13df8d..a56194e204 100644 --- a/winsup/cygwin/path.h +++ b/winsup/cygwin/path.h @@ -59,6 +59,7 @@ enum pathconv_arg PC_KEEP_HANDLE = _BIT (12), /* keep handle for later stat calls */ PC_NO_ACCESS_CHECK = _BIT (13), /* helper flag for error check */ PC_SYM_NOFOLLOW_DIR = _BIT (14), /* don't follow a trailing slash */ + PC_KEEP_FINAL_SLASH = _BIT (15), /* do not remove a trailing slash */ PC_DONT_USE = _BIT (31) /* conversion to signed happens. */ };