Diff options
| author | Bryce Kwon <bryce@brycekwon.com> | |
|---|---|---|
| committer | Bryce Kwon <bryce@brycekwon.com> | |
| commit | ||
| parent | ||
| tree | ||
| download | ||
Fail cleanly when a filter cannot run
A filter program that could not be run answered with two responses,
and a filter that exited without reading its input ended cgit with
the page half written.
| -rw-r--r-- | source/cache.c | 3 | |
| -rw-r--r-- | source/filter.c | 22 |
2 files changed, 20 insertions, 5 deletions
diff --git a/source/cache.c b/source/cache.c index 28558b1..8123e5e 100644 --- a/source/cache.c +++ b/source/cache.c @@ -314,6 +314,9 @@ static int fill_slot(struct cache_slot *slot) slot->saved_stdout = dup(STDOUT_FILENO); if (slot->saved_stdout == -1) return errno; + // A filter program that outlives the request must not hold the client + // connection open. + fcntl(slot->saved_stdout, F_SETFD, FD_CLOEXEC); if (dup2(slot->lock_fd, STDOUT_FILENO) == -1) return errno; diff --git a/source/filter.c b/source/filter.c index 81c4681..1cfdde2 100644 --- a/source/filter.c +++ b/source/filter.c @@ -18,6 +18,10 @@ #include <lauxlib.h> #endif +// Exit status of a child that could not run the filter, the shell's own value +// for a command not found. +#define EXEC_FAILED 127 + static int open_exec_filter(struct cgit_filter *base, va_list ap) { struct cgit_exec_filter *filter = (struct cgit_exec_filter *)base; @@ -32,11 +36,18 @@ static int open_exec_filter(struct cgit_filter *base, va_list ap) filter->pid = cgit_die_unless_non_negative(fork(), "Unable to create subprocess"); if (filter->pid == 0) { close(pipefd[1]); - cgit_die_unless_non_negative(dup2(pipefd[0], STDIN_FILENO), "Unable to use pipe as STDIN"); + if (dup2(pipefd[0], STDIN_FILENO) < 0) + _exit(EXEC_FAILED); execvp(filter->cmd, filter->argv); - die_errno("Unable to exec subprocess %s", filter->cmd); + // The child shares the parent's page buffer, cache lock and exit + // handlers, so it must not die through them. + fprintf(stderr, "[cgit] Unable to exec filter %s: %s\n", filter->cmd, strerror(errno)); + _exit(EXEC_FAILED); } close(pipefd[0]); + // The child keeps the page's stdout, but not the descriptor the parent + // needs it back from. + fcntl(filter->old_stdout, F_SETFD, FD_CLOEXEC); cgit_die_unless_non_negative(dup2(pipefd[1], STDOUT_FILENO), "Unable to use pipe as STDOUT"); close(pipefd[1]); return 0; @@ -52,9 +63,10 @@ static int close_exec_filter(struct cgit_filter *base) if (filter->pid < 0) goto done; waitpid(filter->pid, &exit_status, 0); - if (WIFEXITED(exit_status)) - goto done; - die("Subprocess %s exited abnormally", filter->cmd); + if (!WIFEXITED(exit_status)) + die("Subprocess %s exited abnormally", filter->cmd); + if (WEXITSTATUS(exit_status) == EXEC_FAILED) + die("Unable to run filter %s", filter->cmd); done: for (i = 0; i < filter->base.argument_count; i++) |