nghttpx: Refactor ocsp command execution

We have now generic read-only command execution in shrpx_exec.{h,cc}.

src/CMakeLists.txt

@@ -111,6 +111,7 @@ if(ENABLE_APP)
     shrpx_router.cc
     shrpx_api_downstream_connection.cc
     shrpx_health_monitor_downstream_connection.cc
+    shrpx_exec.cc
   )
   if(HAVE_SPDYLAY)
     list(APPEND NGHTTPX_SRCS

src/Makefile.am

@@ -138,6 +138,7 @@ NGHTTPX_SRCS = \
 	shrpx_api_downstream_connection.cc shrpx_api_downstream_connection.h \
 	shrpx_health_monitor_downstream_connection.cc \
 	shrpx_health_monitor_downstream_connection.h \
+	shrpx_exec.cc shrpx_exec.h \
 	buffer.h memchunk.h template.h allocator.h
 
 if HAVE_SPDYLAY

src/shrpx_connection_handler.cc

@@ -44,6 +44,7 @@
 #include "shrpx_accept_handler.h"
 #include "shrpx_memcached_dispatcher.h"
 #include "shrpx_signal.h"
+#include "shrpx_exec.h"
 #include "util.h"
 #include "template.h"
 
@@ -506,7 +507,6 @@ void ConnectionHandler::cancel_ocsp_update() {
 // https://github.com/h2o/h2o
 int ConnectionHandler::start_ocsp_update(const char *cert_file) {
   int rv;
-  int pfd[2];
 
   if (LOG_ENABLED(INFO)) {
     LOG(INFO) << "Start ocsp update for " << cert_file;
@@ -520,92 +520,15 @@ int ConnectionHandler::start_ocsp_update(const char *cert_file) {
           get_config()->tls.ocsp.fetch_ocsp_response_file.c_str()),
       const_cast<char *>(cert_file), nullptr};
 
-#ifdef O_CLOEXEC
+  Process proc;
-  if (pipe2(pfd, O_CLOEXEC) == -1) {
+  rv = exec_read_command(proc, argv);
-    return -1;
-  }
-#else  // !O_CLOEXEC
-  if (pipe(pfd) == -1) {
-    return -1;
-  }
-  util::make_socket_closeonexec(pfd[0]);
-  util::make_socket_closeonexec(pfd[1]);
-#endif // !O_CLOEXEC
-
-  auto closer = defer([&pfd]() {
-    if (pfd[0] != -1) {
-      close(pfd[0]);
-    }
-
-    if (pfd[1] != -1) {
-      close(pfd[1]);
-    }
-  });
-
-  sigset_t oldset;
-
-  rv = shrpx_signal_block_all(&oldset);
   if (rv != 0) {
-    auto error = errno;
-    LOG(ERROR) << "Blocking all signals failed: " << strerror(error);
-
     return -1;
   }
 
-  auto pid = fork();
+  ocsp_.pid = proc.pid;
+  ocsp_.fd = proc.rfd;
 
-  if (pid == 0) {
-    // child process
-    shrpx_signal_unset_worker_proc_ign_handler();
-
-    rv = shrpx_signal_unblock_all();
-    if (rv != 0) {
-      auto error = errno;
-      LOG(FATAL) << "Unblocking all signals failed: " << strerror(error);
-
-      _Exit(EXIT_FAILURE);
-    }
-
-    dup2(pfd[1], 1);
-    close(pfd[0]);
-
-    rv = execv(argv[0], argv);
-    if (rv == -1) {
-      auto error = errno;
-      LOG(ERROR) << "Could not execute ocsp query command: " << argv[0]
-                 << ", execve() faild, errno=" << error;
-      _Exit(EXIT_FAILURE);
-    }
-    // unreachable
-  }
-
-  // parent process
-  if (pid == -1) {
-    auto error = errno;
-    LOG(ERROR) << "Could not execute ocsp query command for " << cert_file
-               << ": " << argv[0] << ", fork() failed, errno=" << error;
-  }
-
-  rv = shrpx_signal_set(&oldset);
-  if (rv != 0) {
-    auto error = errno;
-    LOG(FATAL) << "Restoring all signals failed: " << strerror(error);
-
-    _Exit(EXIT_FAILURE);
-  }
-
-  if (pid == -1) {
-    return -1;
-  }
-
-  close(pfd[1]);
-  pfd[1] = -1;
-
-  ocsp_.pid = pid;
-  ocsp_.fd = pfd[0];
-  pfd[0] = -1;
-
-  util::make_socket_nonblocking(ocsp_.fd);
   ev_io_set(&ocsp_.rev, ocsp_.fd, EV_READ);
   ev_io_start(loop_, &ocsp_.rev);
 

src/shrpx_exec.cc

@@ -0,0 +1,134 @@
+/*
+ * nghttp2 - HTTP/2 C Library
+ *
+ * Copyright (c) 2016 Tatsuhiro Tsujikawa
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining
+ * a copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sublicense, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
+ * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
+ * OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+ * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+#include "shrpx_exec.h"
+
+#include <cerrno>
+
+#include "shrpx_signal.h"
+#include "util.h"
+#include "template.h"
+
+using namespace nghttp2;
+
+namespace shrpx {
+
+// inspired by h2o_read_command function from h2o project:
+// https://github.com/h2o/h2o
+int exec_read_command(Process &proc, char *const argv[]) {
+  int rv;
+  int pfd[2];
+
+#ifdef O_CLOEXEC
+  if (pipe2(pfd, O_CLOEXEC) == -1) {
+    return -1;
+  }
+#else  // !O_CLOEXEC
+  if (pipe(pfd) == -1) {
+    return -1;
+  }
+  util::make_socket_closeonexec(pfd[0]);
+  util::make_socket_closeonexec(pfd[1]);
+#endif // !O_CLOEXEC
+
+  auto closer = defer([&pfd]() {
+    if (pfd[0] != -1) {
+      close(pfd[0]);
+    }
+
+    if (pfd[1] != -1) {
+      close(pfd[1]);
+    }
+  });
+
+  sigset_t oldset;
+
+  rv = shrpx_signal_block_all(&oldset);
+  if (rv != 0) {
+    auto error = errno;
+    LOG(ERROR) << "Blocking all signals failed: errno=" << error;
+
+    return -1;
+  }
+
+  auto pid = fork();
+
+  if (pid == 0) {
+    // child process
+    shrpx_signal_unset_worker_proc_ign_handler();
+
+    rv = shrpx_signal_unblock_all();
+    if (rv != 0) {
+      auto error = errno;
+      LOG(FATAL) << "Unblocking all signals failed: errno=" << error;
+
+      _Exit(EXIT_FAILURE);
+    }
+
+    dup2(pfd[1], 1);
+    close(pfd[0]);
+
+    rv = execv(argv[0], argv);
+    if (rv == -1) {
+      auto error = errno;
+      LOG(ERROR) << "Could not execute command: " << argv[0]
+                 << ", execve() faild, errno=" << error;
+      _Exit(EXIT_FAILURE);
+    }
+    // unreachable
+  }
+
+  // parent process
+  if (pid == -1) {
+    auto error = errno;
+    LOG(ERROR) << "Could not execute command: " << argv[0]
+               << ", fork() failed, errno=" << error;
+  }
+
+  rv = shrpx_signal_set(&oldset);
+  if (rv != 0) {
+    auto error = errno;
+    LOG(FATAL) << "Restoring all signals failed: errno=" << error;
+
+    _Exit(EXIT_FAILURE);
+  }
+
+  if (pid == -1) {
+    return -1;
+  }
+
+  close(pfd[1]);
+  pfd[1] = -1;
+
+  util::make_socket_nonblocking(pfd[0]);
+
+  proc.pid = pid;
+  proc.rfd = pfd[0];
+
+  pfd[0] = -1;
+
+  return 0;
+}
+
+} // namespace shrpx

src/shrpx_exec.h

@@ -0,0 +1,47 @@
+/*
+ * nghttp2 - HTTP/2 C Library
+ *
+ * Copyright (c) 2016 Tatsuhiro Tsujikawa
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining
+ * a copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sublicense, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
+ * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
+ * OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+ * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+#ifndef SHRPX_EXEC_H
+#define SHRPX_EXEC_H
+
+#include "unistd.h"
+
+namespace shrpx {
+
+struct Process {
+  pid_t pid;
+  // fd to read from process
+  int rfd;
+};
+
+// Executes command |argv| after forking current process.  The command
+// should not expect to read from stdin.  Parent process can read the
+// stdout from command using proc.rfd.  On success, this function
+// returns 0, and process information is stored in |proc|.  Otherwise,
+// returns -1.
+int exec_read_command(Process &proc, char *const argv[]);
+
+} // namespace shrpx
+
+#endif // SHRPX_EXEC_H