From 1696d72321492c05bebd1e823de0708c13ec7d72 Mon Sep 17 00:00:00 2001 From: Ramsay Jones Date: Thu, 5 Apr 2012 18:48:46 +0100 Subject: [PATCH 1/2] compat/mingw.[ch]: Change return type of exec functions to int The POSIX standard specifies a return type of int for all six exec functions. In addition, all exec functions return -1 on error, and simply do not return on success. However, the current emulation of the exec functions on mingw are declared with a void return type. This would cause a problem should any code attempt to call the exec function in a non-void context. In particular, if an exec function were used in a conditional it would fail to compile. In order to improve the fidelity of the emulation, we change the return type of the mingw_execv[p] functions to int and return -1 on error. Signed-off-by: Ramsay Jones Signed-off-by: Junio C Hamano --- compat/mingw.c | 6 ++++-- compat/mingw.h | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/compat/mingw.c b/compat/mingw.c index a0ac487c0c..afc892d6b1 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -1003,7 +1003,7 @@ static void mingw_execve(const char *cmd, char *const *argv, char *const *env) } } -void mingw_execvp(const char *cmd, char *const *argv) +int mingw_execvp(const char *cmd, char *const *argv) { char **path = get_path_split(); char *prog = path_lookup(cmd, path, 0); @@ -1015,11 +1015,13 @@ void mingw_execvp(const char *cmd, char *const *argv) errno = ENOENT; free_path_split(path); + return -1; } -void mingw_execv(const char *cmd, char *const *argv) +int mingw_execv(const char *cmd, char *const *argv) { mingw_execve(cmd, argv, environ); + return -1; } int mingw_kill(pid_t pid, int sig) diff --git a/compat/mingw.h b/compat/mingw.h index 0ff1e04812..ef5b15014e 100644 --- a/compat/mingw.h +++ b/compat/mingw.h @@ -274,9 +274,9 @@ int mingw_utime(const char *file_name, const struct utimbuf *times); pid_t mingw_spawnvpe(const char *cmd, const char **argv, char **env, const char *dir, int fhin, int fhout, int fherr); -void mingw_execvp(const char *cmd, char *const *argv); +int mingw_execvp(const char *cmd, char *const *argv); #define execvp mingw_execvp -void mingw_execv(const char *cmd, char *const *argv); +int mingw_execv(const char *cmd, char *const *argv); #define execv mingw_execv static inline unsigned int git_ntohl(unsigned int x) From 38f865c27d1f2560afb48efd2b7b105c1278c4b5 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 30 Mar 2012 03:52:18 -0400 Subject: [PATCH 2/2] run-command: treat inaccessible directories as ENOENT When execvp reports EACCES, it can be one of two things: 1. We found a file to execute, but did not have permissions to do so. 2. We did not have permissions to look in some directory in the $PATH. In the former case, we want to consider this a permissions problem and report it to the user as such (since getting this for something like "git foo" is likely a configuration error). In the latter case, there is a good chance that the inaccessible directory does not contain anything of interest. Reporting "permission denied" is confusing to the user (and prevents our usual "did you mean...?" lookup). It also prevents git from trying alias lookup, since we do so only when an external command does not exist (not when it exists but has an error). This patch detects EACCES from execvp, checks whether we are in case (2), and if so converts errno to ENOENT. This behavior matches that of "bash" (but not of simpler shells that use execvp more directly, like "dash"). Test stolen from Junio. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- cache.h | 2 ++ exec_cmd.c | 2 +- run-command.c | 66 ++++++++++++++++++++++++++++++++++++++++-- t/t0061-run-command.sh | 13 +++++++++ 4 files changed, 80 insertions(+), 3 deletions(-) diff --git a/cache.h b/cache.h index 10afd71d43..5067226c2f 100644 --- a/cache.h +++ b/cache.h @@ -1261,4 +1261,6 @@ extern struct startup_info *startup_info; /* builtin/merge.c */ int checkout_fast_forward(const unsigned char *from, const unsigned char *to); +int sane_execvp(const char *file, char *const argv[]); + #endif /* CACHE_H */ diff --git a/exec_cmd.c b/exec_cmd.c index 171e841531..125fa6fabf 100644 --- a/exec_cmd.c +++ b/exec_cmd.c @@ -134,7 +134,7 @@ int execv_git_cmd(const char **argv) { trace_argv_printf(nargv, "trace: exec:"); /* execvp() can only ever return if it fails */ - execvp("git", (char **)nargv); + sane_execvp("git", (char **)nargv); trace_printf("trace: exec failed: %s\n", strerror(errno)); diff --git a/run-command.c b/run-command.c index 1c51043884..805d41f93d 100644 --- a/run-command.c +++ b/run-command.c @@ -18,6 +18,68 @@ static inline void dup_devnull(int to) } #endif +static char *locate_in_PATH(const char *file) +{ + const char *p = getenv("PATH"); + struct strbuf buf = STRBUF_INIT; + + if (!p || !*p) + return NULL; + + while (1) { + const char *end = strchrnul(p, ':'); + + strbuf_reset(&buf); + + /* POSIX specifies an empty entry as the current directory. */ + if (end != p) { + strbuf_add(&buf, p, end - p); + strbuf_addch(&buf, '/'); + } + strbuf_addstr(&buf, file); + + if (!access(buf.buf, F_OK)) + return strbuf_detach(&buf, NULL); + + if (!*end) + break; + p = end + 1; + } + + strbuf_release(&buf); + return NULL; +} + +static int exists_in_PATH(const char *file) +{ + char *r = locate_in_PATH(file); + free(r); + return r != NULL; +} + +int sane_execvp(const char *file, char * const argv[]) +{ + if (!execvp(file, argv)) + return 0; /* cannot happen ;-) */ + + /* + * When a command can't be found because one of the directories + * listed in $PATH is unsearchable, execvp reports EACCES, but + * careful usability testing (read: analysis of occasional bug + * reports) reveals that "No such file or directory" is more + * intuitive. + * + * We avoid commands with "/", because execvp will not do $PATH + * lookups in that case. + * + * The reassignment of EACCES to errno looks like a no-op below, + * but we need to protect against exists_in_PATH overwriting errno. + */ + if (errno == EACCES && !strchr(file, '/')) + errno = exists_in_PATH(file) ? EACCES : ENOENT; + return -1; +} + static const char **prepare_shell_cmd(const char **argv) { int argc, nargc = 0; @@ -56,7 +118,7 @@ static int execv_shell_cmd(const char **argv) { const char **nargv = prepare_shell_cmd(argv); trace_argv_printf(nargv, "trace: exec:"); - execvp(nargv[0], (char **)nargv); + sane_execvp(nargv[0], (char **)nargv); free(nargv); return -1; } @@ -278,7 +340,7 @@ fail_pipe: } else if (cmd->use_shell) { execv_shell_cmd(cmd->argv); } else { - execvp(cmd->argv[0], (char *const*) cmd->argv); + sane_execvp(cmd->argv[0], (char *const*) cmd->argv); } if (errno == ENOENT) { if (!cmd->silent_exec_failure) diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh index 8d4938f019..17e969df60 100755 --- a/t/t0061-run-command.sh +++ b/t/t0061-run-command.sh @@ -34,4 +34,17 @@ test_expect_success POSIXPERM 'run_command reports EACCES' ' grep "fatal: cannot exec.*hello.sh" err ' +test_expect_success POSIXPERM 'unreadable directory in PATH' ' + mkdir local-command && + test_when_finished "chmod u+rwx local-command && rm -fr local-command" && + git config alias.nitfol "!echo frotz" && + chmod a-rx local-command && + ( + PATH=./local-command:$PATH && + git nitfol >actual + ) && + echo frotz >expect && + test_cmp expect actual +' + test_done