# [PATCH 0/2] Work around case-insensitivity issues with cwd on Windows 12 messages from 2018-10-23 to 2018-10-25. Participants: Johannes Schindelin via GitGitGadget, Anton Serbulov via GitGitGadget, Stephen & Linda Smith, Stephen Smith, Johannes Schindelin, Junio C Hamano. Thread: https://gitlist.dev/t/49648 ## Johannes Schindelin via GitGitGadget, 2018-10-23 10:52 Subject: [PATCH 0/2] Work around case-insensitivity issues with cwd on Windows Message-ID: URL: https://gitlist.dev/e/pull.54.git.gitgitgadget%40gmail.com ``` On Windows, file names are recorded case-sensitively, but looked up case-insensitively. Therefore, it is possible to switch to a directory by using incorrect case, e.g. cd documentation will still get you into the Documentation subdirectory. In Powershell, doing so will however report the current directory with the specified spelling rather than the one recorded on disk, and Git will get confused. To remedy that, we fixed this in Git for Windows more than three years ago, and needed only a small fix a couple of months later to accommodate for the diverse scenarios encountered by the many Git for Windows users. Not only to keep the story closer to what happened historically, but also to make it easier to follow, I refrained from squashing these two patches. Side note: the second patch is technically not battle-tested for that long: it uses an API function that requires Windows Vista or later, and we only recently started to clean up Git for Windows' code to drop fallbacks for Windows XP. Read: this code used to load the GetFinalPathNameByHandle() function dynamically, and that is the only difference to the code that has been "battle-tested" for close to three years. Anton Serbulov (1): mingw: fix getcwd when the parent directory cannot be queried Johannes Schindelin (1): mingw: ensure `getcwd()` reports the correct case compat/mingw.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 48 insertions(+), 2 deletions(-) base-commit: c4df23f7927d8d00e666a3c8d1b3375f1dc8a3c1 Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-54%2Fdscho%2Fmingw-getcwd-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-54/dscho/mingw-getcwd-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/54 -- gitgitgadget ``` ## Johannes Schindelin via GitGitGadget, 2018-10-23 10:52 Subject: [PATCH 1/2] mingw: ensure `getcwd()` reports the correct case Message-ID: URL: https://gitlist.dev/e/e13ae233822c8e4a4504e3294adf3e0de38a1990.1540291965.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Johannes Schindelin When switching the current working directory, say, in PowerShell, it is quite possible to use a different capitalization than the one that is recorded on disk. While doing the same in `cmd.exe` adjusts the capitalization magically, that does not happen in PowerShell so that `getcwd()` returns the current directory in a different way than is recorded on disk. Typically this creates no problems except when you call git log . in a subdirectory called, say, "GIT/" but you switched to "Git/" and your `getcwd()` reports the latter, then Git won't understand that you wanted to see the history as per the `GIT/` subdirectory but it thinks you wanted to see the history of some directory that may have existed in the past (but actually never did). So let's be extra careful to adjust the capitalization of the current directory before working with it. Reported by a few PowerShell power users ;-) Signed-off-by: Johannes Schindelin --- compat/mingw.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/compat/mingw.c b/compat/mingw.c index 18caf2196..2c3e27ce9 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -917,8 +917,15 @@ struct tm *localtime_r(const time_t *timep, struct tm *result) char *mingw_getcwd(char *pointer, int len) { - wchar_t wpointer[MAX_PATH]; - if (!_wgetcwd(wpointer, ARRAY_SIZE(wpointer))) + wchar_t cwd[MAX_PATH], wpointer[MAX_PATH]; + DWORD ret = GetCurrentDirectoryW(ARRAY_SIZE(cwd), cwd); + + if (!ret || ret >= ARRAY_SIZE(cwd)) { + errno = ret ? ENAMETOOLONG : err_win_to_posix(GetLastError()); + return NULL; + } + ret = GetLongPathNameW(cwd, wpointer, ARRAY_SIZE(wpointer)); + if (!ret || ret >= ARRAY_SIZE(wpointer)) return NULL; if (xwcstoutf(pointer, wpointer, len) < 0) return NULL; -- gitgitgadget ``` ## Anton Serbulov via GitGitGadget, 2018-10-23 10:52 Subject: [PATCH 2/2] mingw: fix getcwd when the parent directory cannot be queried Message-ID: <3e3b1c3b1ff3363b216574f09d2c9cf22d25ad30.1540291965.git.gitgitgadget@gmail.com> URL: https://gitlist.dev/e/3e3b1c3b1ff3363b216574f09d2c9cf22d25ad30.1540291965.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Anton Serbulov `GetLongPathName()` function may fail when it is unable to query the parent directory of a path component to determine the long name for that component. It happens, because of it uses `FindFirstFile()` function for each next short part of path. The `FindFirstFile()` requires `List Directory` and `Synchronize` desired access for a calling process. In case of lacking such permission for some part of path, the `GetLongPathName()` returns 0 as result and `GetLastError()` returns ERROR_ACCESS_DENIED. `GetFinalPathNameByHandle()` function can help in such cases, because it requires `Read Attributes` and `Synchronize` desired access to the target path only. The `GetFinalPathNameByHandle()` function was introduced on `Windows Server 2008/Windows Vista`. So we need to load it dynamically. `CreateFile()` parameters: `lpFileName` = path to the current directory `dwDesiredAccess` = 0 (it means `Read Attributes` and `Synchronize`) `dwShareMode` = FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE (it prevents `Sharing Violation`) `lpSecurityAttributes` = NULL (default security attributes) `dwCreationDisposition` = OPEN_EXISTING (required to obtain a directory handle) `dwFlagsAndAttributes` = FILE_FLAG_BACKUP_SEMANTICS (required to obtain a directory handle) `hTemplateFile` = NULL (when opening an existing file or directory, `CreateFile` ignores this parameter) The string that is returned by `GetFinalPathNameByHandle()` function uses the \\?\ syntax. To skip the prefix and convert backslashes to slashes, the `normalize_ntpath()` mingw function will be used. Note: `GetFinalPathNameByHandle()` function returns a final path. It is the path that is returned when a path is fully resolved. For example, for a symbolic link named "C:\tmp\mydir" that points to "D:\yourdir", the final path would be "D:\yourdir". Signed-off-by: Anton Serbulov Signed-off-by: Johannes Schindelin --- compat/mingw.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/compat/mingw.c b/compat/mingw.c index 2c3e27ce9..19addfa5d 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -202,6 +202,31 @@ static int ask_yes_no_if_possible(const char *format, ...) } } +/* Normalizes NT paths as returned by some low-level APIs. */ +static wchar_t *normalize_ntpath(wchar_t *wbuf) +{ + int i; + /* fix absolute path prefixes */ + if (wbuf[0] == '\\') { + /* strip NT namespace prefixes */ + if (!wcsncmp(wbuf, L"\\??\\", 4) || + !wcsncmp(wbuf, L"\\\\?\\", 4)) + wbuf += 4; + else if (!wcsnicmp(wbuf, L"\\DosDevices\\", 12)) + wbuf += 12; + /* replace remaining '...UNC\' with '\\' */ + if (!wcsnicmp(wbuf, L"UNC\\", 4)) { + wbuf += 2; + *wbuf = '\\'; + } + } + /* convert backslashes to slashes */ + for (i = 0; wbuf[i]; i++) + if (wbuf[i] == '\\') + wbuf[i] = '/'; + return wbuf; +} + int mingw_unlink(const char *pathname) { int ret, tries = 0; @@ -925,6 +950,20 @@ char *mingw_getcwd(char *pointer, int len) return NULL; } ret = GetLongPathNameW(cwd, wpointer, ARRAY_SIZE(wpointer)); + if (!ret && GetLastError() == ERROR_ACCESS_DENIED) { + HANDLE hnd = CreateFileW(cwd, 0, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL, + OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL); + if (hnd == INVALID_HANDLE_VALUE) + return NULL; + ret = GetFinalPathNameByHandleW(hnd, wpointer, ARRAY_SIZE(wpointer), 0); + CloseHandle(hnd); + if (!ret || ret >= ARRAY_SIZE(wpointer)) + return NULL; + if (xwcstoutf(pointer, normalize_ntpath(wpointer), len) < 0) + return NULL; + return pointer; + } if (!ret || ret >= ARRAY_SIZE(wpointer)) return NULL; if (xwcstoutf(pointer, wpointer, len) < 0) -- gitgitgadget ``` ## Stephen & Linda Smith, 2018-10-23 13:16 Subject: Re: [PATCH 2/2] mingw: fix getcwd when the parent directory cannot be queried Message-ID: <6700297.8bfoVLpaf1@thunderbird> URL: https://gitlist.dev/e/6700297.8bfoVLpaf1%40thunderbird In-Reply-To: <3e3b1c3b1ff3363b216574f09d2c9cf22d25ad30.1540291965.git.gitgitgadget@gmail.com> ``` On Tuesday, October 23, 2018 3:52:49 AM MST you wrote: > From: Anton Serbulov > > `GetLongPathName()` function may fail when it is unable to query > the parent directory of a path component to determine the long name > for that component. It happens, because of it uses `FindFirstFile()` s/of it/it/ > function for each next short part of path. The `FindFirstFile()` > requires `List Directory` and `Synchronize` desired access for a calling > process. ``` ## Stephen Smith, 2018-10-23 13:20 Subject: Re: [PATCH 2/2] mingw: fix getcwd when the parent directory cannot be queried Message-ID: <31438048.4iejHa56lk@thunderbird> URL: https://gitlist.dev/e/31438048.4iejHa56lk%40thunderbird In-Reply-To: <6700297.8bfoVLpaf1@thunderbird> ``` On Tuesday, October 23, 2018 6:16:43 AM MST Stephen Smith wrote: > On Tuesday, October 23, 2018 3:52:49 AM MST you wrote: > > From: Anton Serbulov > > > > `GetLongPathName()` function may fail when it is unable to query > > the parent directory of a path component to determine the long name > > for that component. It happens, because of it uses `FindFirstFile()` > > s/of it/it/ > > > function for each next short part of path. The `FindFirstFile()` > > requires `List Directory` and `Synchronize` desired access for a calling > > process. BTW - It was "Stephen" that sent the email. :-) ``` ## Johannes Schindelin, 2018-10-24 09:19 Subject: Re: [PATCH 2/2] mingw: fix getcwd when the parent directory cannot be queried Message-ID: URL: https://gitlist.dev/e/nycvar.QRO.7.76.6.1810241118460.4546%40tvgsbejvaqbjf.bet In-Reply-To: <6700297.8bfoVLpaf1@thunderbird> ``` Hi Stephen, On Tue, 23 Oct 2018, Stephen & Linda Smith wrote: > On Tuesday, October 23, 2018 3:52:49 AM MST you wrote: > > From: Anton Serbulov > > > > `GetLongPathName()` function may fail when it is unable to query > > the parent directory of a path component to determine the long name > > for that component. It happens, because of it uses `FindFirstFile()` > s/of it/it/ Thanks, fixed in v2, Dscho > > > function for each next short part of path. The `FindFirstFile()` > > requires `List Directory` and `Synchronize` desired access for a calling > > process. > > > > > > ``` ## Johannes Schindelin via GitGitGadget, 2018-10-24 09:19 Subject: [PATCH v2 0/2] Work around case-insensitivity issues with cwd on Windows Message-ID: URL: https://gitlist.dev/e/pull.54.v2.git.gitgitgadget%40gmail.com In-Reply-To: ``` On Windows, file names are recorded case-sensitively, but looked up case-insensitively. Therefore, it is possible to switch to a directory by using incorrect case, e.g. cd documentation will still get you into the Documentation subdirectory. In Powershell, doing so will however report the current directory with the specified spelling rather than the one recorded on disk, and Git will get confused. To remedy that, we fixed this in Git for Windows more than three years ago, and needed only a small fix a couple of months later to accommodate for the diverse scenarios encountered by the many Git for Windows users. Not only to keep the story closer to what happened historically, but also to make it easier to follow, I refrained from squashing these two patches. Side note: the second patch is technically not battle-tested for that long: it uses an API function that requires Windows Vista or later, and we only recently started to clean up Git for Windows' code to drop fallbacks for Windows XP. Read: this code used to load the GetFinalPathNameByHandle() function dynamically, and that is the only difference to the code that has been "battle-tested" for close to three years. Changes since v1: * Fixed a grammar mistake in the second commit message. Anton Serbulov (1): mingw: fix getcwd when the parent directory cannot be queried Johannes Schindelin (1): mingw: ensure `getcwd()` reports the correct case compat/mingw.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 48 insertions(+), 2 deletions(-) base-commit: c4df23f7927d8d00e666a3c8d1b3375f1dc8a3c1 Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-54%2Fdscho%2Fmingw-getcwd-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-54/dscho/mingw-getcwd-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/54 Range-diff vs v1: 1: e13ae2338 = 1: e13ae2338 mingw: ensure `getcwd()` reports the correct case 2: 3e3b1c3b1 ! 2: 87ef9ac63 mingw: fix getcwd when the parent directory cannot be queried @@ -4,7 +4,7 @@ `GetLongPathName()` function may fail when it is unable to query the parent directory of a path component to determine the long name - for that component. It happens, because of it uses `FindFirstFile()` + for that component. It happens, because it uses `FindFirstFile()` function for each next short part of path. The `FindFirstFile()` requires `List Directory` and `Synchronize` desired access for a calling process. -- gitgitgadget ``` ## Anton Serbulov via GitGitGadget, 2018-10-24 09:19 Subject: [PATCH v2 2/2] mingw: fix getcwd when the parent directory cannot be queried Message-ID: <87ef9ac6349fff6afc4d5cfc513f682f335a28f4.1540372778.git.gitgitgadget@gmail.com> URL: https://gitlist.dev/e/87ef9ac6349fff6afc4d5cfc513f682f335a28f4.1540372778.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Anton Serbulov `GetLongPathName()` function may fail when it is unable to query the parent directory of a path component to determine the long name for that component. It happens, because it uses `FindFirstFile()` function for each next short part of path. The `FindFirstFile()` requires `List Directory` and `Synchronize` desired access for a calling process. In case of lacking such permission for some part of path, the `GetLongPathName()` returns 0 as result and `GetLastError()` returns ERROR_ACCESS_DENIED. `GetFinalPathNameByHandle()` function can help in such cases, because it requires `Read Attributes` and `Synchronize` desired access to the target path only. The `GetFinalPathNameByHandle()` function was introduced on `Windows Server 2008/Windows Vista`. So we need to load it dynamically. `CreateFile()` parameters: `lpFileName` = path to the current directory `dwDesiredAccess` = 0 (it means `Read Attributes` and `Synchronize`) `dwShareMode` = FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE (it prevents `Sharing Violation`) `lpSecurityAttributes` = NULL (default security attributes) `dwCreationDisposition` = OPEN_EXISTING (required to obtain a directory handle) `dwFlagsAndAttributes` = FILE_FLAG_BACKUP_SEMANTICS (required to obtain a directory handle) `hTemplateFile` = NULL (when opening an existing file or directory, `CreateFile` ignores this parameter) The string that is returned by `GetFinalPathNameByHandle()` function uses the \\?\ syntax. To skip the prefix and convert backslashes to slashes, the `normalize_ntpath()` mingw function will be used. Note: `GetFinalPathNameByHandle()` function returns a final path. It is the path that is returned when a path is fully resolved. For example, for a symbolic link named "C:\tmp\mydir" that points to "D:\yourdir", the final path would be "D:\yourdir". Signed-off-by: Anton Serbulov Signed-off-by: Johannes Schindelin --- compat/mingw.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/compat/mingw.c b/compat/mingw.c index 2c3e27ce9..19addfa5d 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -202,6 +202,31 @@ static int ask_yes_no_if_possible(const char *format, ...) } } +/* Normalizes NT paths as returned by some low-level APIs. */ +static wchar_t *normalize_ntpath(wchar_t *wbuf) +{ + int i; + /* fix absolute path prefixes */ + if (wbuf[0] == '\\') { + /* strip NT namespace prefixes */ + if (!wcsncmp(wbuf, L"\\??\\", 4) || + !wcsncmp(wbuf, L"\\\\?\\", 4)) + wbuf += 4; + else if (!wcsnicmp(wbuf, L"\\DosDevices\\", 12)) + wbuf += 12; + /* replace remaining '...UNC\' with '\\' */ + if (!wcsnicmp(wbuf, L"UNC\\", 4)) { + wbuf += 2; + *wbuf = '\\'; + } + } + /* convert backslashes to slashes */ + for (i = 0; wbuf[i]; i++) + if (wbuf[i] == '\\') + wbuf[i] = '/'; + return wbuf; +} + int mingw_unlink(const char *pathname) { int ret, tries = 0; @@ -925,6 +950,20 @@ char *mingw_getcwd(char *pointer, int len) return NULL; } ret = GetLongPathNameW(cwd, wpointer, ARRAY_SIZE(wpointer)); + if (!ret && GetLastError() == ERROR_ACCESS_DENIED) { + HANDLE hnd = CreateFileW(cwd, 0, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL, + OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL); + if (hnd == INVALID_HANDLE_VALUE) + return NULL; + ret = GetFinalPathNameByHandleW(hnd, wpointer, ARRAY_SIZE(wpointer), 0); + CloseHandle(hnd); + if (!ret || ret >= ARRAY_SIZE(wpointer)) + return NULL; + if (xwcstoutf(pointer, normalize_ntpath(wpointer), len) < 0) + return NULL; + return pointer; + } if (!ret || ret >= ARRAY_SIZE(wpointer)) return NULL; if (xwcstoutf(pointer, wpointer, len) < 0) -- gitgitgadget ``` ## Johannes Schindelin via GitGitGadget, 2018-10-24 09:19 Subject: [PATCH v2 1/2] mingw: ensure `getcwd()` reports the correct case Message-ID: URL: https://gitlist.dev/e/e13ae233822c8e4a4504e3294adf3e0de38a1990.1540372778.git.gitgitgadget%40gmail.com In-Reply-To: ``` From: Johannes Schindelin When switching the current working directory, say, in PowerShell, it is quite possible to use a different capitalization than the one that is recorded on disk. While doing the same in `cmd.exe` adjusts the capitalization magically, that does not happen in PowerShell so that `getcwd()` returns the current directory in a different way than is recorded on disk. Typically this creates no problems except when you call git log . in a subdirectory called, say, "GIT/" but you switched to "Git/" and your `getcwd()` reports the latter, then Git won't understand that you wanted to see the history as per the `GIT/` subdirectory but it thinks you wanted to see the history of some directory that may have existed in the past (but actually never did). So let's be extra careful to adjust the capitalization of the current directory before working with it. Reported by a few PowerShell power users ;-) Signed-off-by: Johannes Schindelin --- compat/mingw.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/compat/mingw.c b/compat/mingw.c index 18caf2196..2c3e27ce9 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -917,8 +917,15 @@ struct tm *localtime_r(const time_t *timep, struct tm *result) char *mingw_getcwd(char *pointer, int len) { - wchar_t wpointer[MAX_PATH]; - if (!_wgetcwd(wpointer, ARRAY_SIZE(wpointer))) + wchar_t cwd[MAX_PATH], wpointer[MAX_PATH]; + DWORD ret = GetCurrentDirectoryW(ARRAY_SIZE(cwd), cwd); + + if (!ret || ret >= ARRAY_SIZE(cwd)) { + errno = ret ? ENAMETOOLONG : err_win_to_posix(GetLastError()); + return NULL; + } + ret = GetLongPathNameW(cwd, wpointer, ARRAY_SIZE(wpointer)); + if (!ret || ret >= ARRAY_SIZE(wpointer)) return NULL; if (xwcstoutf(pointer, wpointer, len) < 0) return NULL; -- gitgitgadget ``` ## Junio C Hamano, 2018-10-24 10:22 Subject: Re: [PATCH v2 0/2] Work around case-insensitivity issues with cwd on Windows Message-ID: URL: https://gitlist.dev/e/xmqqin1rmynw.fsf%40gitster-ct.c.googlers.com In-Reply-To: ``` "Johannes Schindelin via GitGitGadget" writes: > Changes since v1: > > * Fixed a grammar mistake in the second commit message. Thanks. I think this matches what I earlier queued on 'pu' with Stephen's typofix squashed in, so we are good already. ``` ## Johannes Schindelin, 2018-10-24 15:20 Subject: Re: [PATCH v2 0/2] Work around case-insensitivity issues with cwd on Windows Message-ID: URL: https://gitlist.dev/e/nycvar.QRO.7.76.6.1810241720200.4546%40tvgsbejvaqbjf.bet In-Reply-To: ``` Hi Junio, On Wed, 24 Oct 2018, Junio C Hamano wrote: > "Johannes Schindelin via GitGitGadget" > writes: > > > Changes since v1: > > > > * Fixed a grammar mistake in the second commit message. > > Thanks. I think this matches what I earlier queued on 'pu' with > Stephen's typofix squashed in, so we are good already. Perfect. Sorry that I forgot to check. Ciao, Dscho ``` ## Junio C Hamano, 2018-10-25 04:11 Subject: Re: [PATCH v2 0/2] Work around case-insensitivity issues with cwd on Windows Message-ID: URL: https://gitlist.dev/e/xmqq8t2mll6b.fsf%40gitster-ct.c.googlers.com In-Reply-To: ``` Johannes Schindelin writes: > On Wed, 24 Oct 2018, Junio C Hamano wrote: > >> "Johannes Schindelin via GitGitGadget" >> writes: >> >> > Changes since v1: >> > >> > * Fixed a grammar mistake in the second commit message. >> >> Thanks. I think this matches what I earlier queued on 'pu' with >> Stephen's typofix squashed in, so we are good already. > > Perfect. Sorry that I forgot to check. Thanks for double-checking, and I do appreciate the final version, even if it matches the version I would have in 'pu' if I perfectly followed all what was said in the review thread, as I am not always perfect. ```