[PATCH v2] Fix dir sep handling of GIT_ASKPASS on Windows

From: András Kucsma via GitGitGadget <gitgitgadget@gmail.com>
Date: Mar 25, 2020, 13:45 UTC
Message-ID: <pull.587.v2.git.1585143910604.gitgitgadget@gmail.com>
In-Reply-To: <pull.587.git.1584997990694.gitgitgadget@gmail.com>

From: Andras Kucsma <r0maikx02b@gmail.com>

On Windows with git installed through cygwin, GIT_ASKPASS failed to run for relative and absolute paths containing only backslashes as directory separators.

The reason was that git assumed that if there are no forward slashes in the executable path, it has to search for the executable on the PATH.

The fix is to look for OS specific directory separators, not just forward slashes.

Signed-off-by: Andras Kucsma <r0maikx02b@gmail.com>
---
    Fix dir sep handling of GIT_ASKPASS on Windows
    
    On Windows with git installed through cygwin, GIT_ASKPASS failed to run
    for relative and absolute paths containing only backslashes as directory
    separators.
    
    The reason was that git assumed that if there are no forward slashes in
    the executable path, it has to search for the executable on the PATH.
    
    The fix is to look for OS specific directory separators, not just
    forward slashes.
    
    Signed-off-by: Andras Kucsma r0maikx02b@gmail.com [r0maikx02b@gmail.com]
    
    Changes since v1:
    
     * Avoid scanning the whole path for a directory separator even if one
       is found earlier as suggested by Junio C Hamano.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-587%2Fr0mai%2Ffix-prepare_cmd-windows-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-587/r0mai/fix-prepare_cmd-windows-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/587

Range-diff vs v1:

 1:  8fbfbec0d38 ! 1:  947931ac568 Fix dir sep handling of GIT_ASKPASS on Windows
     @@ -14,6 +14,47 @@
      
          Signed-off-by: Andras Kucsma <r0maikx02b@gmail.com>
      
     + diff --git a/compat/win32/path-utils.h b/compat/win32/path-utils.h
     + --- a/compat/win32/path-utils.h
     + +++ b/compat/win32/path-utils.h
     +@@
     + 	return ret;
     + }
     + #define find_last_dir_sep win32_find_last_dir_sep
     ++static inline int win32_has_dir_sep(const char *path)
     ++{
     ++	/*
     ++	 * See how long the non-separator part of the given path is, and
     ++	 * if and only if it covers the whole path (i.e. path[len] is NULL),
     ++	 * there is no separator in the path---otherwise there is a separator.
     ++	 */
     ++	size_t len = strcspn(path, "/\\");
     ++	return !!path[len];
     ++}
     ++#define has_dir_sep(path) win32_has_dir_sep(path)
     + int win32_offset_1st_component(const char *path);
     + #define offset_1st_component win32_offset_1st_component
     + 
     +
     + diff --git a/git-compat-util.h b/git-compat-util.h
     + --- a/git-compat-util.h
     + +++ b/git-compat-util.h
     +@@
     + #define find_last_dir_sep git_find_last_dir_sep
     + #endif
     + 
     ++#ifndef has_dir_sep
     ++static inline int git_has_dir_sep(const char *path)
     ++{
     ++	return !!strchr(path, '/');
     ++}
     ++#define has_dir_sep(path) git_has_dir_sep(path)
     ++#endif
     ++
     + #ifndef query_user_email
     + #define query_user_email() NULL
     + #endif
     +
       diff --git a/run-command.c b/run-command.c
       --- a/run-command.c
       +++ b/run-command.c
     @@ -31,7 +72,7 @@
      +	 * the command directly.
       	 */
      -	if (!strchr(out->argv[1], '/')) {
     -+	if (find_last_dir_sep(out->argv[1]) == NULL) {
     ++	if (!has_dir_sep(out->argv[1])) {
       		char *program = locate_in_PATH(out->argv[1]);
       		if (program) {
       			free((char *)out->argv[1]);

 compat/win32/path-utils.h | 11 +++++++++++
 git-compat-util.h         |  8 ++++++++
 run-command.c             | 10 +++++-----
 3 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/compat/win32/path-utils.h b/compat/win32/path-utils.h
index f2e70872cd2..18eff7899e9 100644
--- a/compat/win32/path-utils.h
+++ b/compat/win32/path-utils.h
@@ -20,6 +20,17 @@ static inline char *win32_find_last_dir_sep(const char *path)
 	return ret;
 }
 #define find_last_dir_sep win32_find_last_dir_sep
+static inline int win32_has_dir_sep(const char *path)
+{
+	/*
+	 * See how long the non-separator part of the given path is, and
+	 * if and only if it covers the whole path (i.e. path[len] is NULL),
+	 * there is no separator in the path---otherwise there is a separator.
+	 */
+	size_t len = strcspn(path, "/\\");
+	return !!path[len];
+}
+#define has_dir_sep(path) win32_has_dir_sep(path)
 int win32_offset_1st_component(const char *path);
 #define offset_1st_component win32_offset_1st_component
 
diff --git a/git-compat-util.h b/git-compat-util.h
index aed0b5d4f90..8ba576e81e3 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -389,6 +389,14 @@ static inline char *git_find_last_dir_sep(const char *path)
 #define find_last_dir_sep git_find_last_dir_sep
 #endif
 
+#ifndef has_dir_sep
+static inline int git_has_dir_sep(const char *path)
+{
+	return !!strchr(path, '/');
+}
+#define has_dir_sep(path) git_has_dir_sep(path)
+#endif
+
 #ifndef query_user_email
 #define query_user_email() NULL
 #endif
diff --git a/run-command.c b/run-command.c
index f5e1149f9b3..0f41af3b550 100644
--- a/run-command.c
+++ b/run-command.c
@@ -421,12 +421,12 @@ static int prepare_cmd(struct argv_array *out, const struct child_process *cmd)
 	}
 
 	/*
-	 * If there are no '/' characters in the command then perform a path
-	 * lookup and use the resolved path as the command to exec.  If there
-	 * are '/' characters, we have exec attempt to invoke the command
-	 * directly.
+	 * If there are no dir separator characters in the command then perform
+	 * a path lookup and use the resolved path as the command to exec. If
+	 * there are dir separator characters, we have exec attempt to invoke
+	 * the command directly.
 	 */
-	if (!strchr(out->argv[1], '/')) {
+	if (!has_dir_sep(out->argv[1])) {
 		char *program = locate_in_PATH(out->argv[1]);
 		if (program) {
 			free((char *)out->argv[1]);

base-commit: 274b9cc25322d9ee79aa8e6d4e86f0ffe5ced925

-- 
gitgitgadget