nghttpx: Add BlockAllocator to ClientHandler

src/shrpx-unittest.cc

@@ -176,6 +176,8 @@ int main(int argc, char *argv[]) {
       !CU_add_test(pSuite, "util_make_hostport",
                    shrpx::test_util_make_hostport) ||
       !CU_add_test(pSuite, "util_strifind", shrpx::test_util_strifind) ||
+      !CU_add_test(pSuite, "util_random_alpha_digit",
+                   shrpx::test_util_random_alpha_digit) ||
       !CU_add_test(pSuite, "gzip_inflate", test_nghttp2_gzip_inflate) ||
       !CU_add_test(pSuite, "buffer_write", nghttp2::test_buffer_write) ||
       !CU_add_test(pSuite, "pool_recycle", nghttp2::test_pool_recycle) ||

src/shrpx.cc

@@ -2569,8 +2569,10 @@ int process_options(Config *config,
       fwdconf.by_obfuscated.empty()) {
     std::mt19937 gen(rd());
     auto &dst = fwdconf.by_obfuscated;
-    dst = "_";
+    dst.resize(1 + SHRPX_OBFUSCATED_NODE_LENGTH);
-    dst += util::random_alpha_digit(gen, SHRPX_OBFUSCATED_NODE_LENGTH);
+    auto p = std::begin(dst);
+    *p++ = '_';
+    util::random_alpha_digit(p, std::end(dst), gen);
   }
 
   if (config->http2.upstream.debug.frame_debug) {

src/shrpx_client_handler.cc

@@ -376,17 +376,18 @@ int ClientHandler::upstream_http1_connhd_read() {
 }
 
 ClientHandler::ClientHandler(Worker *worker, int fd, SSL *ssl,
-                             const char *ipaddr, const char *port, int family,
+                             const StringRef &ipaddr, const StringRef &port,
-                             const UpstreamAddr *faddr)
+                             int family, const UpstreamAddr *faddr)
-    : conn_(worker->get_loop(), fd, ssl, worker->get_mcpool(),
+    : balloc_(128, 128),
+      conn_(worker->get_loop(), fd, ssl, worker->get_mcpool(),
             get_config()->conn.upstream.timeout.write,
             get_config()->conn.upstream.timeout.read,
             get_config()->conn.upstream.ratelimit.write,
             get_config()->conn.upstream.ratelimit.read, writecb, readcb,
             timeoutcb, this, get_config()->tls.dyn_rec.warmup_threshold,
             get_config()->tls.dyn_rec.idle_timeout, PROTO_NONE),
-      ipaddr_(ipaddr),
+      ipaddr_(make_string_ref(balloc_, ipaddr)),
-      port_(port),
+      port_(make_string_ref(balloc_, port)),
       faddr_(faddr),
       worker_(worker),
       left_connhd_len_(NGHTTP2_CLIENT_MAGIC_LEN),
@@ -416,13 +417,28 @@ ClientHandler::ClientHandler(Worker *worker, int fd, SSL *ssl,
 
   if (fwdconf.params & FORWARDED_FOR) {
     if (fwdconf.for_node_type == FORWARDED_NODE_OBFUSCATED) {
-      forwarded_for_ = "_";
+      // 1 for '_'
-      forwarded_for_ += util::random_alpha_digit(worker_->get_randgen(),
+      auto len = SHRPX_OBFUSCATED_NODE_LENGTH + 1;
-                                                 SHRPX_OBFUSCATED_NODE_LENGTH);
+      // 1 for terminating NUL.
+      auto buf = make_byte_ref(balloc_, len + 1);
+      auto p = buf.base;
+      *p++ = '_';
+      p = util::random_alpha_digit(p, p + len - 1, worker_->get_randgen());
+      *p = '\0';
+
+      forwarded_for_ = StringRef{buf.base, p};
     } else if (family == AF_INET6) {
-      forwarded_for_ = "[";
+      // 2 for '[' and ']'
-      forwarded_for_ += ipaddr_;
+      auto len = 2 + ipaddr_.size();
-      forwarded_for_ += ']';
+      // 1 for terminating NUL.
+      auto buf = make_byte_ref(balloc_, len + 1);
+      auto p = buf.base;
+      *p++ = '[';
+      p = std::copy(std::begin(ipaddr_), std::end(ipaddr_), p);
+      *p++ = ']';
+      *p = '\0';
+
+      forwarded_for_ = StringRef{buf.base, p};
     } else {
       // family == AF_INET or family == AF_UNIX
       forwarded_for_ = ipaddr_;
@@ -441,7 +457,7 @@ void ClientHandler::setup_upstream_io_callback() {
     // upgraded to HTTP/2 through HTTP Upgrade or direct HTTP/2
     // connection.
     upstream_ = make_unique<HttpsUpstream>(this);
-    alpn_ = "http/1.1";
+    alpn_ = StringRef::from_lit("http/1.1");
     read_ = &ClientHandler::read_clear;
     write_ = &ClientHandler::write_clear;
     on_read_ = &ClientHandler::upstream_http1_connhd_read;
@@ -524,7 +540,7 @@ int ClientHandler::validate_next_proto() {
     }
 
     upstream_ = make_unique<HttpsUpstream>(this);
-    alpn_ = "http/1.1";
+    alpn_ = StringRef::from_lit("http/1.1");
 
     // At this point, input buffer is already filled with some bytes.
     // The read callback is not called until new data come. So consume
@@ -555,7 +571,7 @@ int ClientHandler::validate_next_proto() {
     auto http2_upstream = make_unique<Http2Upstream>(this);
 
     upstream_ = std::move(http2_upstream);
-    alpn_.assign(std::begin(proto), std::end(proto));
+    alpn_ = make_string_ref(balloc_, proto);
 
     // At this point, input buffer is already filled with some bytes.
     // The read callback is not called until new data come. So consume
@@ -574,16 +590,16 @@ int ClientHandler::validate_next_proto() {
 
     switch (spdy_version) {
     case SPDYLAY_PROTO_SPDY2:
-      alpn_ = "spdy/2";
+      alpn_ = StringRef::from_lit("spdy/2");
       break;
     case SPDYLAY_PROTO_SPDY3:
-      alpn_ = "spdy/3";
+      alpn_ = StringRef::from_lit("spdy/3");
       break;
     case SPDYLAY_PROTO_SPDY3_1:
-      alpn_ = "spdy/3.1";
+      alpn_ = StringRef::from_lit("spdy/3.1");
       break;
     default:
-      alpn_ = "spdy/unknown";
+      alpn_ = StringRef::from_lit("spdy/unknown");
     }
 
     // At this point, input buffer is already filled with some bytes.
@@ -599,7 +615,7 @@ int ClientHandler::validate_next_proto() {
 
   if (proto == StringRef::from_lit("http/1.1")) {
     upstream_ = make_unique<HttpsUpstream>(this);
-    alpn_ = proto.str();
+    alpn_ = StringRef::from_lit("http/1.1");
 
     // At this point, input buffer is already filled with some bytes.
     // The read callback is not called until new data come. So consume
@@ -629,7 +645,7 @@ int ClientHandler::on_read() {
 }
 int ClientHandler::on_write() { return on_write_(*this); }
 
-const std::string &ClientHandler::get_ipaddr() const { return ipaddr_; }
+const StringRef &ClientHandler::get_ipaddr() const { return ipaddr_; }
 
 bool ClientHandler::get_should_close_after_write() const {
   return should_close_after_write_;
@@ -974,7 +990,7 @@ ClientHandler::get_downstream_connection(Downstream *downstream) {
 
   if (shared_addr->affinity == AFFINITY_IP) {
     if (!affinity_hash_computed_) {
-      affinity_hash_ = compute_affinity_from_ip(StringRef{ipaddr_});
+      affinity_hash_ = compute_affinity_from_ip(ipaddr_);
       affinity_hash_computed_ = true;
     }
 
@@ -1093,7 +1109,7 @@ SSL *ClientHandler::get_ssl() const { return conn_.tls.ssl; }
 
 void ClientHandler::direct_http2_upgrade() {
   upstream_ = make_unique<Http2Upstream>(this);
-  alpn_ = NGHTTP2_CLEARTEXT_PROTO_VERSION_ID;
+  alpn_ = StringRef::from_lit(NGHTTP2_CLEARTEXT_PROTO_VERSION_ID);
   on_read_ = &ClientHandler::upstream_read;
   write_ = &ClientHandler::write_clear;
 }
@@ -1118,7 +1134,7 @@ int ClientHandler::perform_http2_upgrade(HttpsUpstream *http) {
   upstream_.release();
   // TODO We might get other version id in HTTP2-settings, if we
   // support aliasing for h2, but we just use library default for now.
-  alpn_ = NGHTTP2_CLEARTEXT_PROTO_VERSION_ID;
+  alpn_ = StringRef::from_lit(NGHTTP2_CLEARTEXT_PROTO_VERSION_ID);
   on_read_ = &ClientHandler::upstream_http2_connhd_read;
   write_ = &ClientHandler::write_clear;
 
@@ -1199,7 +1215,7 @@ void ClientHandler::write_accesslog(Downstream *downstream) {
   upstream_accesslog(
       get_config()->logging.access.format,
       LogSpec{
-          downstream, downstream->get_addr(), StringRef{ipaddr_},
+          downstream, downstream->get_addr(), ipaddr_,
           http2::to_method_string(req.method),
 
           req.method == HTTP_CONNECT
@@ -1212,15 +1228,14 @@ void ClientHandler::write_accesslog(Downstream *downstream) {
                                 : StringRef::from_lit("-")
                           : StringRef(req.path),
 
-          StringRef(alpn_),
+          alpn_, nghttp2::ssl::get_tls_session_info(&tls_info, conn_.tls.ssl),
-          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, StringRef(port_), faddr_->port,
+          downstream->response_sent_body_length, port_, faddr_->port,
           get_config()->pid,
       });
 }
@@ -1231,21 +1246,20 @@ 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,
+  upstream_accesslog(
-                     LogSpec{
+      get_config()->logging.access.format,
-                         nullptr, nullptr, StringRef(ipaddr_),
+      LogSpec{
-                         StringRef::from_lit("-"), // method
+          nullptr, nullptr, ipaddr_,
-                         StringRef::from_lit("-"), // path,
+          StringRef::from_lit("-"), // method
-                         StringRef(alpn_), nghttp2::ssl::get_tls_session_info(
+          StringRef::from_lit("-"), // path,
-                                               &tls_info, conn_.tls.ssl),
+          alpn_, nghttp2::ssl::get_tls_session_info(&tls_info, conn_.tls.ssl),
-                         time_now,
+          time_now,
-                         highres_now,  // request_start_time TODO is
+          highres_now,  // request_start_time TODO is
-                                       // there a better value?
+                        // there a better value?
-                         highres_now,  // request_end_time
+          highres_now,  // request_end_time
-                         major, minor, // major, minor
+          major, minor, // major, minor
-                         status, body_bytes_sent, StringRef(port_),
+          status, body_bytes_sent, port_, faddr_->port, get_config()->pid,
-                         faddr_->port, get_config()->pid,
+      });
-                     });
 }
 
 ClientHandler::ReadBuf *ClientHandler::get_rb() { return &rb_; }
@@ -1474,8 +1488,9 @@ int ClientHandler::proxy_protocol_read() {
 
   rb_.drain(end + 2 - rb_.pos);
 
-  ipaddr_.assign(src_addr, src_addr + src_addrlen);
+  ipaddr_ =
-  port_.assign(src_port, src_port + src_portlen);
+      make_string_ref(balloc_, StringRef{src_addr, src_addr + src_addrlen});
+  port_ = make_string_ref(balloc_, StringRef{src_port, src_port + src_portlen});
 
   if (LOG_ENABLED(INFO)) {
     CLOG(INFO, this) << "PROXY-protocol-v1: Finished, " << (rb_.pos - first)
@@ -1495,16 +1510,16 @@ StringRef ClientHandler::get_forwarded_by() const {
   return StringRef{faddr_->hostport};
 }
 
-StringRef ClientHandler::get_forwarded_for() const {
+StringRef ClientHandler::get_forwarded_for() const { return forwarded_for_; }
-  return StringRef{forwarded_for_};
-}
 
 const UpstreamAddr *ClientHandler::get_upstream_addr() const { return faddr_; }
 
 Connection *ClientHandler::get_connection() { return &conn_; };
 
-void ClientHandler::set_tls_sni(const StringRef &sni) { sni_ = sni.str(); }
+void ClientHandler::set_tls_sni(const StringRef &sni) {
+  sni_ = make_string_ref(balloc_, sni);
+}
 
-StringRef ClientHandler::get_tls_sni() const { return StringRef{sni_}; }
+StringRef ClientHandler::get_tls_sni() const { return sni_; }
 
 } // namespace shrpx

src/shrpx_client_handler.h

@@ -37,6 +37,7 @@
 #include "shrpx_connection.h"
 #include "buffer.h"
 #include "memchunk.h"
+#include "allocator.h"
 
 using namespace nghttp2;
 
@@ -54,8 +55,8 @@ struct DownstreamAddr;
 
 class ClientHandler {
 public:
-  ClientHandler(Worker *worker, int fd, SSL *ssl, const char *ipaddr,
+  ClientHandler(Worker *worker, int fd, SSL *ssl, const StringRef &ipaddr,
-                const char *port, int family, const UpstreamAddr *faddr);
+                const StringRef &port, int family, const UpstreamAddr *faddr);
   ~ClientHandler();
 
   int noop();
@@ -90,8 +91,7 @@ public:
   void reset_upstream_write_timeout(ev_tstamp t);
 
   int validate_next_proto();
-  const std::string &get_ipaddr() const;
+  const StringRef &get_ipaddr() const;
-  const std::string &get_port() const;
   bool get_should_close_after_write() const;
   void set_should_close_after_write(bool f);
   Upstream *get_upstream();
@@ -163,20 +163,21 @@ public:
   StringRef get_tls_sni() const;
 
 private:
+  BlockAllocator balloc_;
   Connection conn_;
   ev_timer reneg_shutdown_timer_;
   std::unique_ptr<Upstream> upstream_;
   // IP address of client.  If UNIX domain socket is used, this is
   // "localhost".
-  std::string ipaddr_;
+  StringRef ipaddr_;
-  std::string port_;
+  StringRef port_;
   // The ALPN identifier negotiated for this connection.
-  std::string alpn_;
+  StringRef alpn_;
   // The client address used in "for" parameter of Forwarded header
   // field.
-  std::string forwarded_for_;
+  StringRef forwarded_for_;
   // lowercased TLS SNI which client sent.
-  std::string sni_;
+  StringRef sni_;
   std::function<int(ClientHandler &)> read_, write_;
   std::function<int(ClientHandler &)> on_read_, on_write_;
   // Address of frontend listening socket

src/shrpx_http2_downstream_connection.cc

@@ -357,7 +357,7 @@ int Http2DownstreamConnection::push_request_headers() {
 
   if (xffconf.add) {
     StringRef xff_value;
-    auto addr = StringRef{upstream->get_client_handler()->get_ipaddr()};
+    auto addr = upstream->get_client_handler()->get_ipaddr();
     if (xff) {
       xff_value = concat_string_ref(balloc, xff->value,
                                     StringRef::from_lit(", "), addr);

src/shrpx_ssl.cc

@@ -829,16 +829,16 @@ SSL *create_ssl(SSL_CTX *ssl_ctx) {
 
 ClientHandler *accept_connection(Worker *worker, int fd, sockaddr *addr,
                                  int addrlen, const UpstreamAddr *faddr) {
-  char host[NI_MAXHOST];
+  std::array<char, NI_MAXHOST> host;
-  char service[NI_MAXSERV];
+  std::array<char, NI_MAXSERV> service;
   int rv;
 
   if (addr->sa_family == AF_UNIX) {
-    std::copy_n("localhost", sizeof("localhost"), host);
+    std::copy_n("localhost", sizeof("localhost"), std::begin(host));
     service[0] = '\0';
   } else {
-    rv = getnameinfo(addr, addrlen, host, sizeof(host), service,
+    rv = getnameinfo(addr, addrlen, host.data(), host.size(), service.data(),
-                     sizeof(service), NI_NUMERICHOST | NI_NUMERICSERV);
+                     service.size(), NI_NUMERICHOST | NI_NUMERICSERV);
     if (rv != 0) {
       LOG(ERROR) << "getnameinfo() failed: " << gai_strerror(rv);
 
@@ -867,8 +867,8 @@ ClientHandler *accept_connection(Worker *worker, int fd, sockaddr *addr,
     }
   }
 
-  return new ClientHandler(worker, fd, ssl, host, service, addr->sa_family,
+  return new ClientHandler(worker, fd, ssl, StringRef{host.data()},
-                           faddr);
+                           StringRef{service.data()}, addr->sa_family, faddr);
 }
 
 bool tls_hostname_match(const StringRef &pattern, const StringRef &hostname) {

src/util.h

@@ -665,16 +665,17 @@ uint64_t get_uint64(const uint8_t *data);
 int read_mime_types(std::map<std::string, std::string> &res,
                     const char *filename);
 
-template <typename Generator>
+// Fills random alpha and digit byte to the range [|first|, |last|).
-std::string random_alpha_digit(Generator &gen, size_t len) {
+// Returns the one beyond the |last|.
-  std::string res;
+template <typename OutputIt, typename Generator>
-  res.reserve(len);
+OutputIt random_alpha_digit(OutputIt first, OutputIt last, Generator &gen) {
+  constexpr uint8_t s[] =
+      "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789";
   std::uniform_int_distribution<> dis(0, 26 * 2 + 10 - 1);
-  for (; len > 0; --len) {
+  for (; first != last; ++first) {
-    res += "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"[dis(
+    *first = s[dis(gen)];
-        gen)];
   }
-  return res;
+  return first;
 }
 
 template <typename OutputIterator, typename CharT, size_t N>

src/util_test.cc

@@ -26,6 +26,7 @@
 
 #include <cstring>
 #include <iostream>
+#include <random>
 
 #include <CUnit/CUnit.h>
 
@@ -528,4 +529,19 @@ void test_util_strifind(void) {
                             StringRef::from_lit("http1")));
 }
 
+void test_util_random_alpha_digit(void) {
+  std::random_device rd;
+  std::mt19937 gen(rd());
+  std::array<uint8_t, 19> data;
+
+  auto p = util::random_alpha_digit(std::begin(data), std::end(data), gen);
+
+  CU_ASSERT(std::end(data) == p);
+
+  for (auto b : data) {
+    CU_ASSERT(('A' <= b && b <= 'Z') || ('a' <= b && b <= 'z') ||
+              ('0' <= b && b <= '9'));
+  }
+}
+
 } // namespace shrpx

src/util_test.h

@@ -62,6 +62,7 @@ void test_util_parse_config_str_list(void);
 void test_util_make_http_hostport(void);
 void test_util_make_hostport(void);
 void test_util_strifind(void);
+void test_util_random_alpha_digit(void);
 
 } // namespace shrpx