nghttpx: About implicit conversion from ImmutableString and std::string to StringRef

This is required to avoid creation of temporary ImmutableString
like so:

std::string x;
ImmutableString y = ...;
StringRef ref = !x.empty() ? x : y;

First, temporary ImmutableString is created with x since
ImmutableString has constructor to accept std::string.  After
StringRef gets this, the temporary ImmutableString is destroyed, and
ref has dangling pointer.

src/shrpx.cc

@@ -730,7 +730,8 @@ pid_t fork_worker_process(SignalServer *ssv) {
 
     LOG(NOTICE) << "Worker process shutting down momentarily";
 
-    _Exit(EXIT_SUCCESS);
+    // call exit(...) instead of _Exit to get leak sanitizer report
+    exit(EXIT_SUCCESS);
   }
 
   // parent process

src/shrpx_client_handler.cc

@@ -839,26 +839,27 @@ void ClientHandler::write_accesslog(Downstream *downstream) {
   upstream_accesslog(
       get_config()->logging.access.format,
       LogSpec{
-          downstream, ipaddr_, http2::to_method_string(req.method),
+          downstream, StringRef(ipaddr_), http2::to_method_string(req.method),
 
           req.method == HTTP_CONNECT
-              ? req.authority
+              ? StringRef(req.authority)
               : (get_config()->http2_proxy || get_config()->client_proxy)
-                    ? construct_absolute_request_uri(req)
+                    ? StringRef(construct_absolute_request_uri(req))
                     : req.path.empty()
                           ? req.method == HTTP_OPTIONS
                                 ? StringRef::from_lit("*")
                                 : StringRef::from_lit("-")
-                          : req.path,
+                          : StringRef(req.path),
 
-          alpn_, nghttp2::ssl::get_tls_session_info(&tls_info, conn_.tls.ssl),
+          StringRef(alpn_),
+          nghttp2::ssl::get_tls_session_info(&tls_info, conn_.tls.ssl),
 
           std::chrono::system_clock::now(),          // time_now
           downstream->get_request_start_time(),      // request_start_time
           std::chrono::high_resolution_clock::now(), // request_end_time
 
           req.http_major, req.http_minor, resp.http_status,
-          downstream->response_sent_body_length, port_,
+          downstream->response_sent_body_length, StringRef(port_),
           get_config()->conn.listener.port, get_config()->pid,
       });
 }
@@ -869,21 +870,21 @@ void ClientHandler::write_accesslog(int major, int minor, unsigned int status,
   auto highres_now = std::chrono::high_resolution_clock::now();
   nghttp2::ssl::TLSSessionInfo tls_info;
 
-  upstream_accesslog(
-      get_config()->logging.access.format,
-      LogSpec{
-          nullptr, ipaddr_,
-          StringRef::from_lit("-"), // method
-          StringRef::from_lit("-"), // path,
-          alpn_, nghttp2::ssl::get_tls_session_info(&tls_info, conn_.tls.ssl),
-          time_now,
-          highres_now,  // request_start_time TODO is
-                        // there a better value?
-          highres_now,  // request_end_time
-          major, minor, // major, minor
-          status, body_bytes_sent, port_, get_config()->conn.listener.port,
-          get_config()->pid,
-      });
+  upstream_accesslog(get_config()->logging.access.format,
+                     LogSpec{
+                         nullptr, StringRef(ipaddr_),
+                         StringRef::from_lit("-"), // method
+                         StringRef::from_lit("-"), // path,
+                         StringRef(alpn_), nghttp2::ssl::get_tls_session_info(
+                                               &tls_info, conn_.tls.ssl),
+                         time_now,
+                         highres_now,  // request_start_time TODO is
+                                       // there a better value?
+                         highres_now,  // request_end_time
+                         major, minor, // major, minor
+                         status, body_bytes_sent, StringRef(port_),
+                         get_config()->conn.listener.port, get_config()->pid,
+                     });
 }
 
 ClientHandler::ReadBuf *ClientHandler::get_rb() { return &rb_; }

src/shrpx_http2_downstream_connection.cc

@@ -275,7 +275,7 @@ int Http2DownstreamConnection::push_request_headers() {
   auto authority = StringRef(downstream_hostport);
 
   if (no_host_rewrite && !req.authority.empty()) {
-    authority = req.authority;
+    authority = StringRef(req.authority);
   }
 
   downstream_->set_request_downstream_host(authority.str());

src/shrpx_log.cc

@@ -192,6 +192,14 @@ std::pair<OutputIterator, size_t> copy(const StringRef &src, size_t avail,
 }
 } // namespace
 
+namespace {
+template <typename OutputIterator>
+std::pair<OutputIterator, size_t> copy(const ImmutableString &src, size_t avail,
+                                       OutputIterator oitr) {
+  return copy(src.c_str(), src.size(), avail, oitr);
+}
+} // namespace
+
 namespace {
 template <size_t N, typename OutputIterator>
 std::pair<OutputIterator, size_t> copy_l(const char(&src)[N], size_t avail,
@@ -276,7 +284,7 @@ void upstream_accesslog(const std::vector<LogFragment> &lfv,
       break;
     case SHRPX_LOGF_HTTP:
       if (req) {
-        auto hd = req->fs.header(lf.value);
+        auto hd = req->fs.header(StringRef(lf.value));
         if (hd) {
           std::tie(p, avail) = copy((*hd).value, avail, p);
           break;

src/template.h

@@ -288,6 +288,12 @@ public:
     return ImmutableString(s, N - 1);
   }
 
+  const_iterator begin() const { return base; };
+  const_iterator cbegin() const { return base; };
+
+  const_iterator end() const { return base + len; };
+  const_iterator cend() const { return base + len; };
+
   const char *c_str() const { return base; }
   size_type size() const { return len; }
   bool empty() const { return len == 0; }
@@ -307,6 +313,40 @@ private:
   const char *base;
 };
 
+inline bool operator==(const ImmutableString &lhs, const std::string &rhs) {
+  return lhs.size() == rhs.size() &&
+         std::equal(std::begin(lhs), std::end(lhs), std::begin(rhs));
+}
+
+inline bool operator==(const std::string &lhs, const ImmutableString &rhs) {
+  return rhs == lhs;
+}
+
+inline bool operator==(const ImmutableString &lhs, const char *rhs) {
+  return lhs.size() == strlen(rhs) &&
+         std::equal(std::begin(lhs), std::end(lhs), rhs);
+}
+
+inline bool operator==(const char *lhs, const ImmutableString &rhs) {
+  return rhs == lhs;
+}
+
+inline bool operator!=(const ImmutableString &lhs, const std::string &rhs) {
+  return !(lhs == rhs);
+}
+
+inline bool operator!=(const std::string &lhs, const ImmutableString &rhs) {
+  return !(rhs == lhs);
+}
+
+inline bool operator!=(const ImmutableString &lhs, const char *rhs) {
+  return !(lhs == rhs);
+}
+
+inline bool operator!=(const char *lhs, const ImmutableString &rhs) {
+  return !(rhs == lhs);
+}
+
 // StringRef is a reference to a string owned by something else.  So
 // it behaves like simple string, but it does not own pointer.  When
 // it is default constructed, it has empty string.  You can freely
@@ -325,8 +365,9 @@ public:
   using const_iterator = const_pointer;
 
   StringRef() : base(""), len(0) {}
-  StringRef(const std::string &s) : base(s.c_str()), len(s.size()) {}
-  StringRef(const ImmutableString &s) : base(s.c_str()), len(s.size()) {}
+  explicit StringRef(const std::string &s) : base(s.c_str()), len(s.size()) {}
+  explicit StringRef(const ImmutableString &s)
+      : base(s.c_str()), len(s.size()) {}
   StringRef(const char *s) : base(s), len(strlen(s)) {}
   StringRef(const char *s, size_t n) : base(s), len(n) {}
 

neverbleed @ 84b3ac48

-Subproject commit 5c47587bc2855f2b9577a9bd369ed70088b77fec
+Subproject commit 84b3ac48f35e8ac2990c6bfeb04cf2036d883df7