threads / patch / 49648

patch, 2 partsWork around case-insensitivity issues with cwd on Windows

Subject: [PATCH 0/2] Work around case-insensitivity issues with cwd on Windows

## tl;dr

12 messages between Oct 23, 2018 and Oct 25, 2018. Diffs are folded; open one to read it.

replies: 11people: 4as markdown or json

Johannes Schindelin via GitGitGadget· Oct 23, 2018, 10:52 UTC · lore

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· Oct 23, 2018, 10:52 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH 1/2] mingw: ensure `getcwd()` reports the correct case

From: Johannes Schindelin <johannes.schindelin@gmx.de>

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 <johannes.schindelin@gmx.de>
---
 compat/mingw.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
Show changes to compat/mingw.c +9 −2
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· Oct 23, 2018, 10:52 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH 2/2] mingw: fix getcwd when the parent directory cannot be queried

From: Anton Serbulov <aserbulov@plesk.com>

`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 <aserbulov@plesk.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 compat/mingw.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)
Show changes to compat/mingw.c +39 −0
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· Oct 23, 2018, 13:16 UTC · re: Anton Serbulov via GitGitGadget · lore

Re: [PATCH 2/2] mingw: fix getcwd when the parent directory cannot be queried

On Tuesday, October 23, 2018 3:52:49 AM MST you wrote:
Show 5 quoted lines
> From: Anton Serbulov <aserbulov@plesk.com>
> 
> `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· Oct 23, 2018, 13:20 UTC · re: Stephen & Linda Smith · lore

Re: [PATCH 2/2] mingw: fix getcwd when the parent directory cannot be queried

On Tuesday, October 23, 2018 6:16:43 AM MST Stephen Smith wrote:
Show 12 quoted lines
> On Tuesday, October 23, 2018 3:52:49 AM MST you wrote:
> > From: Anton Serbulov <aserbulov@plesk.com>
> > 
> > `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· Oct 24, 2018, 09:19 UTC · re: Stephen & Linda Smith · lore

Re: [PATCH 2/2] mingw: fix getcwd when the parent directory cannot be queried

Hi Stephen,
On Tue, 23 Oct 2018, Stephen & Linda Smith wrote:
Show 7 quoted lines
> On Tuesday, October 23, 2018 3:52:49 AM MST you wrote:
> > From: Anton Serbulov <aserbulov@plesk.com>
> > 
> > `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

Show 10 quoted lines
> 
> > 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· Oct 24, 2018, 09:19 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH v2 0/2] Work around case-insensitivity issues with cwd on Windows

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· Oct 24, 2018, 09:19 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH v2 2/2] mingw: fix getcwd when the parent directory cannot be queried

From: Anton Serbulov <aserbulov@plesk.com>

`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 <aserbulov@plesk.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 compat/mingw.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)
Show changes to compat/mingw.c +39 −0
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· Oct 24, 2018, 09:19 UTC · re: Johannes Schindelin via GitGitGadget · lore

[PATCH v2 1/2] mingw: ensure `getcwd()` reports the correct case

From: Johannes Schindelin <johannes.schindelin@gmx.de>

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 <johannes.schindelin@gmx.de>
---
 compat/mingw.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
Show changes to compat/mingw.c +9 −2
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· Oct 24, 2018, 10:22 UTC · re: Johannes Schindelin via GitGitGadget · lore

Re: [PATCH v2 0/2] Work around case-insensitivity issues with cwd on Windows

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> 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· Oct 24, 2018, 15:20 UTC · re: Junio C Hamano · lore

Re: [PATCH v2 0/2] Work around case-insensitivity issues with cwd on Windows

Hi Junio,
On Wed, 24 Oct 2018, Junio C Hamano wrote:
Show 9 quoted lines
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> 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· Oct 25, 2018, 04:11 UTC · re: Johannes Schindelin · lore

Re: [PATCH v2 0/2] Work around case-insensitivity issues with cwd on Windows

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 13 quoted lines
> On Wed, 24 Oct 2018, Junio C Hamano wrote:
>
>> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
>> 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.

← back to recent threads