diff --git a/src/shrpx.cc b/src/shrpx.cc index d66b5131..666c882a 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -564,6 +564,9 @@ void exec_binary_signal_cb(struct ev_loop *loop, ev_signal *w, int revents) { } } + // restores original stderr + util::restore_original_fds(); + if (execve(argv[0], argv.get(), envp.get()) == -1) { auto error = errno; LOG(ERROR) << "execve failed: errno=" << error; @@ -1059,12 +1062,7 @@ void fill_default_config() { mod_config()->accesslog_file = nullptr; mod_config()->accesslog_syslog = false; mod_config()->accesslog_format = parse_log_format(DEFAULT_ACCESSLOG_FORMAT); -#if defined(__ANDROID__) || defined(ANDROID) - // Android does not have /dev/stderr. Use /proc/self/fd/2 instead. - mod_config()->errorlog_file = strcopy("/proc/self/fd/2"); -#else // !__ANDROID__ && ANDROID mod_config()->errorlog_file = strcopy("/dev/stderr"); -#endif // !__ANDROID__ && ANDROID mod_config()->errorlog_syslog = false; mod_config()->conf_path = strcopy("/etc/nghttpx/nghttpx.conf"); mod_config()->syslog_facility = LOG_DAEMON; @@ -1768,6 +1766,9 @@ int main(int argc, char **argv) { create_config(); fill_default_config(); + // make copy of stderr + util::store_original_fds(); + // First open log files with default configuration, so that we can // log errors/warnings while reading configuration files. reopen_log_files(); diff --git a/src/shrpx_log.cc b/src/shrpx_log.cc index d31fa023..5c853e9b 100644 --- a/src/shrpx_log.cc +++ b/src/shrpx_log.cc @@ -326,31 +326,25 @@ void upstream_accesslog(const std::vector &lfv, int reopen_log_files() { int res = 0; + int new_accesslog_fd = -1; + int new_errorlog_fd = -1; auto lgconf = log_config(); - if (lgconf->accesslog_fd != -1) { - close(lgconf->accesslog_fd); - lgconf->accesslog_fd = -1; - } - if (!get_config()->accesslog_syslog && get_config()->accesslog_file) { - lgconf->accesslog_fd = - util::reopen_log_file(get_config()->accesslog_file.get()); + new_accesslog_fd = util::open_log_file(get_config()->accesslog_file.get()); - if (lgconf->accesslog_fd == -1) { + if (new_accesslog_fd == -1) { LOG(ERROR) << "Failed to open accesslog file " << get_config()->accesslog_file.get(); res = -1; } } - int new_errorlog_fd = -1; - if (!get_config()->errorlog_syslog && get_config()->errorlog_file) { - new_errorlog_fd = util::reopen_log_file(get_config()->errorlog_file.get()); + new_errorlog_fd = util::open_log_file(get_config()->errorlog_file.get()); if (new_errorlog_fd == -1) { if (lgconf->errorlog_fd != -1) { @@ -365,16 +359,13 @@ int reopen_log_files() { } } - if (lgconf->errorlog_fd != -1) { - close(lgconf->errorlog_fd); - lgconf->errorlog_fd = -1; - lgconf->errorlog_tty = false; - } + util::close_log_file(lgconf->accesslog_fd); + util::close_log_file(lgconf->errorlog_fd); - if (new_errorlog_fd != -1) { - lgconf->errorlog_fd = new_errorlog_fd; - lgconf->errorlog_tty = isatty(lgconf->errorlog_fd); - } + lgconf->accesslog_fd = new_accesslog_fd; + lgconf->errorlog_fd = new_errorlog_fd; + lgconf->errorlog_tty = (new_errorlog_fd == -1) ? + false : isatty(new_errorlog_fd); return res; } diff --git a/src/util.cc b/src/util.cc index a5865606..bfa292a8 100644 --- a/src/util.cc +++ b/src/util.cc @@ -657,22 +657,40 @@ std::string numeric_name(const struct sockaddr *sa, socklen_t salen) { return host.data(); } -int reopen_log_file(const char *path) { -#if defined(__ANDROID__) || defined(ANDROID) - int fd; +static int STDERR_COPY = -1; +static int STDOUT_COPY = -1; - if (strcmp("/proc/self/fd/1", path) == 0 || - strcmp("/proc/self/fd/2", path) == 0) { +void store_original_fds() { + // consider dup'ing stdout too + STDERR_COPY = dup(STDERR_FILENO); + STDOUT_COPY = STDOUT_FILENO; + // no race here, since it is called early + make_socket_closeonexec(STDERR_COPY); +} - // We will get permission denied error when O_APPEND is used for - // these paths. - fd = - open(path, O_WRONLY | O_CREAT | O_CLOEXEC, S_IRUSR | S_IWUSR | S_IRGRP); - } else { - fd = open(path, O_WRONLY | O_APPEND | O_CREAT | O_CLOEXEC, - S_IRUSR | S_IWUSR | S_IRGRP); +void restore_original_fds() { + dup2(STDERR_COPY, STDERR_FILENO); +} + +void close_log_file(int &fd) { + if (fd != STDERR_COPY && fd != STDOUT_COPY && fd != -1) { + close(fd); } -#elif defined O_CLOEXEC + fd = -1; +} + +int open_log_file(const char *path) { + + if (strcmp(path, "/dev/stdout") == 0 || + strcmp(path, "/proc/self/fd/1") == 0) { + return STDOUT_COPY; + } + + if (strcmp(path, "/dev/stderr") == 0 || + strcmp(path, "/proc/self/fd/2") == 0) { + return STDERR_COPY; + } +#if defined O_CLOEXEC auto fd = open(path, O_WRONLY | O_APPEND | O_CREAT | O_CLOEXEC, S_IRUSR | S_IWUSR | S_IRGRP); diff --git a/src/util.h b/src/util.h index b945e9f2..6463d19a 100644 --- a/src/util.h +++ b/src/util.h @@ -516,10 +516,23 @@ bool numeric_host(const char *hostname); // failed, "unknown" is returned. std::string numeric_name(const struct sockaddr *sa, socklen_t salen); +// Makes internal copy of stderr (and possibly stdout in the future), +// which is then used as pointer to /dev/stderr or /proc/self/fd/2 +void store_original_fds(); + +// Restores the original stderr that was stored with copy_original_fds +// Used just before execv +void restore_original_fds(); + +// Closes |fd| which was returned by open_log_file (see below) +// and sets it to -1. In the case that |fd| points to stdout or +// stderr, or is -1, the descriptor is not closed (but still set to -1). +void close_log_file(int &fd); + // Opens |path| with O_APPEND enabled. If file does not exist, it is // created first. This function returns file descriptor referring the // opened file if it succeeds, or -1. -int reopen_log_file(const char *path); +int open_log_file(const char *path); // Returns ASCII dump of |data| of length |len|. Only ASCII printable // characters are preserved. Other characters are replaced with ".".