From 38f865c27d1f2560afb48efd2b7b105c1278c4b5 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 30 Mar 2012 03:52:18 -0400 Subject: [PATCH] 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 10afd71d4..5067226c2 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 171e84153..125fa6fab 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 1c5104388..805d41f93 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 8d4938f01..17e969df6 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 -- 2.26.2