@matheusbsbs / matheus-bagliano / commits / 38f865c27d

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 <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Mar 30, 2012 at 03:52 UTC 38f865c27d1f2560afb48efd2b7b105c1278c4b5

4 files changed +80 -3

cache.h

@@ -1261,4 +1261,6 @@ extern struct startup_info *startup_info;

1261 /* builtin/merge.c */

1262 int checkout_fast_forward(const unsigned char *from, const unsigned char *to);

1263

1264 +int sane_execvp(const char *file, char *const argv[]);

1265 +

1266 #endif /* CACHE_H */

exec_cmd.c

+1 -1

@@ -134,7 +134,7 @@ int execv_git_cmd(const char **argv) {

134 trace_argv_printf(nargv, "trace: exec:");

135

136 /* execvp() can only ever return if it fails */

137 - execvp("git", (char **)nargv);

137 + sane_execvp("git", (char **)nargv);

138

139 trace_printf("trace: exec failed: %s\n", strerror(errno));

140

run-command.c

+64 -2

@@ -18,6 +18,68 @@ static inline void dup_devnull(int to)

18 }

19 #endif

21 +static char *locate_in_PATH(const char *file)

22 +{

23 + const char *p = getenv("PATH");

24 + struct strbuf buf = STRBUF_INIT;

25 +

26 + if (!p || !*p)

27 + return NULL;

28 +

29 + while (1) {

30 + const char *end = strchrnul(p, ':');

31 +

32 + strbuf_reset(&buf);

33 +

34 + /* POSIX specifies an empty entry as the current directory. */

35 + if (end != p) {

36 + strbuf_add(&buf, p, end - p);

37 + strbuf_addch(&buf, '/');

38 + }

39 + strbuf_addstr(&buf, file);

40 +

41 + if (!access(buf.buf, F_OK))

42 + return strbuf_detach(&buf, NULL);

43 +

44 + if (!*end)

45 + break;

46 + p = end + 1;

47 + }

48 +

49 + strbuf_release(&buf);

50 + return NULL;

51 +}

52 +

53 +static int exists_in_PATH(const char *file)

54 +{

55 + char *r = locate_in_PATH(file);

56 + free(r);

57 + return r != NULL;

58 +}

59 +

60 +int sane_execvp(const char *file, char * const argv[])

61 +{

62 + if (!execvp(file, argv))

63 + return 0; /* cannot happen ;-) */

64 +

65 + /*

66 + * When a command can't be found because one of the directories

67 + * listed in $PATH is unsearchable, execvp reports EACCES, but

68 + * careful usability testing (read: analysis of occasional bug

69 + * reports) reveals that "No such file or directory" is more

70 + * intuitive.

71 + *

72 + * We avoid commands with "/", because execvp will not do $PATH

73 + * lookups in that case.

74 + *

75 + * The reassignment of EACCES to errno looks like a no-op below,

76 + * but we need to protect against exists_in_PATH overwriting errno.

77 + */

78 + if (errno == EACCES && !strchr(file, '/'))

79 + errno = exists_in_PATH(file) ? EACCES : ENOENT;

80 + return -1;

81 +}

82 +

83 static const char **prepare_shell_cmd(const char **argv)

84 {

85 int argc, nargc = 0;

@@ -56,7 +118,7 @@ static int execv_shell_cmd(const char **argv)

118 {

119 const char **nargv = prepare_shell_cmd(argv);

120 trace_argv_printf(nargv, "trace: exec:");

59 - execvp(nargv[0], (char **)nargv);

121 + sane_execvp(nargv[0], (char **)nargv);

122 free(nargv);

123 return -1;

124 }

@@ -278,7 +340,7 @@ fail_pipe:

340 } else if (cmd->use_shell) {

341 execv_shell_cmd(cmd->argv);

342 } else {

281 - execvp(cmd->argv[0], (char *const*) cmd->argv);

343 + sane_execvp(cmd->argv[0], (char *const*) cmd->argv);

344 }

345 if (errno == ENOENT) {

346 if (!cmd->silent_exec_failure)

t/t0061-run-command.sh

+13

@@ -34,4 +34,17 @@ test_expect_success POSIXPERM 'run_command reports EACCES' '

34 grep "fatal: cannot exec.*hello.sh" err

35 '

37 +test_expect_success POSIXPERM 'unreadable directory in PATH' '

38 + mkdir local-command &&

39 + test_when_finished "chmod u+rwx local-command && rm -fr local-command" &&

40 + git config alias.nitfol "!echo frotz" &&

41 + chmod a-rx local-command &&

42 + (

43 + PATH=./local-command:$PATH &&

44 + git nitfol >actual

45 + ) &&

46 + echo frotz >expect &&

47 + test_cmp expect actual

48 +'

49 +

50 test_done