From 06a0f3480e42e1f06531772d338f15cf182185f1 Mon Sep 17 00:00:00 2001
From: Tomasz Buchert <tomasz@buchert.pl>
Date: Wed, 12 Aug 2015 20:05:10 +0200
Subject: [PATCH 1/3] nghttpx: better handle /dev/stderr and /dev/stdout

---
 src/shrpx_log.cc | 31 +++++++++++--------------------
 src/util.cc      | 16 +++++++++++++++-
 src/util.h       |  7 ++++++-
 3 files changed, 32 insertions(+), 22 deletions(-)

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<LogFragment> &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..6f72366f 100644
--- a/src/util.cc
+++ b/src/util.cc
@@ -657,7 +657,21 @@ std::string numeric_name(const struct sockaddr *sa, socklen_t salen) {
   return host.data();
 }
 
-int reopen_log_file(const char *path) {
+
+void close_log_file(int &fd) {
+  if (fd != STDERR_FILENO && fd != STDOUT_FILENO && fd != -1) {
+    close(fd);
+  }
+  fd = -1;
+}
+
+int open_log_file(const char *path) {
+  if (strcmp(path, "/dev/stdout") == 0) {
+    return STDOUT_FILENO;
+  }
+  if (strcmp(path, "/dev/stderr") == 0) {
+    return STDERR_FILENO;
+  }
 #if defined(__ANDROID__) || defined(ANDROID)
   int fd;
 
diff --git a/src/util.h b/src/util.h
index b945e9f2..b6860a07 100644
--- a/src/util.h
+++ b/src/util.h
@@ -516,10 +516,15 @@ bool numeric_host(const char *hostname);
 // failed, "unknown" is returned.
 std::string numeric_name(const struct sockaddr *sa, socklen_t salen);
 
+// 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 ".".

From 900dcf4ceda3853782fe3f2820e6298630ccdb18 Mon Sep 17 00:00:00 2001
From: Tomasz Buchert <tomasz@buchert.pl>
Date: Wed, 12 Aug 2015 20:11:20 +0200
Subject: [PATCH 2/3] nghttpx: remove Android-specific code

---
 src/shrpx.cc |  5 -----
 src/util.cc  | 24 +++++++-----------------
 2 files changed, 7 insertions(+), 22 deletions(-)

diff --git a/src/shrpx.cc b/src/shrpx.cc
index d66b5131..1efb0eea 100644
--- a/src/shrpx.cc
+++ b/src/shrpx.cc
@@ -1059,12 +1059,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;
diff --git a/src/util.cc b/src/util.cc
index 6f72366f..fc92039c 100644
--- a/src/util.cc
+++ b/src/util.cc
@@ -666,27 +666,17 @@ void close_log_file(int &fd) {
 }
 
 int open_log_file(const char *path) {
-  if (strcmp(path, "/dev/stdout") == 0) {
+
+  if (strcmp(path, "/dev/stdout") == 0 ||
+      strcmp(path, "/proc/self/fd/1") == 0) {
     return STDOUT_FILENO;
   }
-  if (strcmp(path, "/dev/stderr") == 0) {
+
+  if (strcmp(path, "/dev/stderr") == 0 ||
+      strcmp(path, "/proc/self/fd/2") == 0) {
     return STDERR_FILENO;
   }
-#if defined(__ANDROID__) || defined(ANDROID)
-  int fd;
-
-  if (strcmp("/proc/self/fd/1", path) == 0 ||
-      strcmp("/proc/self/fd/2", path) == 0) {
-
-    // 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);
-  }
-#elif defined O_CLOEXEC
+#if defined O_CLOEXEC
 
   auto fd = open(path, O_WRONLY | O_APPEND | O_CREAT | O_CLOEXEC,
                  S_IRUSR | S_IWUSR | S_IRGRP);

From 97566ce4e3b3584e4a4d67f17b467f00ea9b339b Mon Sep 17 00:00:00 2001
From: Tomasz Buchert <tomasz@buchert.pl>
Date: Thu, 13 Aug 2015 11:01:37 +0200
Subject: [PATCH 3/3] nghttpx: make early copy of stderr

---
 src/shrpx.cc |  6 ++++++
 src/util.cc  | 20 +++++++++++++++++---
 src/util.h   |  8 ++++++++
 3 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/src/shrpx.cc b/src/shrpx.cc
index 1efb0eea..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;
@@ -1763,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/util.cc b/src/util.cc
index fc92039c..bfa292a8 100644
--- a/src/util.cc
+++ b/src/util.cc
@@ -657,9 +657,23 @@ std::string numeric_name(const struct sockaddr *sa, socklen_t salen) {
   return host.data();
 }
 
+static int STDERR_COPY = -1;
+static int STDOUT_COPY = -1;
+
+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);
+}
+
+void restore_original_fds() {
+  dup2(STDERR_COPY, STDERR_FILENO);
+}
 
 void close_log_file(int &fd) {
-  if (fd != STDERR_FILENO && fd != STDOUT_FILENO && fd != -1) {
+  if (fd != STDERR_COPY && fd != STDOUT_COPY && fd != -1) {
     close(fd);
   }
   fd = -1;
@@ -669,12 +683,12 @@ int open_log_file(const char *path) {
 
   if (strcmp(path, "/dev/stdout") == 0 ||
       strcmp(path, "/proc/self/fd/1") == 0) {
-    return STDOUT_FILENO;
+    return STDOUT_COPY;
   }
 
   if (strcmp(path, "/dev/stderr") == 0 ||
       strcmp(path, "/proc/self/fd/2") == 0) {
-    return STDERR_FILENO;
+    return STDERR_COPY;
   }
 #if defined O_CLOEXEC
 
diff --git a/src/util.h b/src/util.h
index b6860a07..6463d19a 100644
--- a/src/util.h
+++ b/src/util.h
@@ -516,6 +516,14 @@ 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).