nghttpx: Rewrite create_forwarded to use BlockAllocator

src/shrpx_http.cc

@@ -50,58 +50,75 @@ std::string create_error_html(unsigned int status_code) {
   return res;
 }
 
-std::string create_forwarded(int params, const StringRef &node_by,
+StringRef create_forwarded(BlockAllocator &balloc, int params,
-                             const StringRef &node_for, const StringRef &host,
+                           const StringRef &node_by, const StringRef &node_for,
-                             const StringRef &proto) {
+                           const StringRef &host, const StringRef &proto) {
-  std::string res;
+  size_t len = 0;
+  if ((params & FORWARDED_BY) && !node_by.empty()) {
+    len += str_size("by=\"") + node_by.size() + str_size("\";");
+  }
+  if ((params & FORWARDED_FOR) && !node_for.empty()) {
+    len += str_size("for=\"") + node_for.size() + str_size("\";");
+  }
+  if ((params & FORWARDED_HOST) && !host.empty()) {
+    len += str_size("host=\"") + host.size() + str_size("\";");
+  }
+  if ((params & FORWARDED_PROTO) && !proto.empty()) {
+    len += str_size("proto=") + proto.size() + str_size(";");
+  }
+
+  auto iov = make_byte_ref(balloc, len + 1);
+  auto p = iov.base;
+
   if ((params & FORWARDED_BY) && !node_by.empty()) {
     // This must be quoted-string unless it is obfuscated version
     // (which starts with "_") or some special value (e.g.,
     // "localhost" for UNIX domain socket), since ':' is not allowed
     // in token.  ':' is used to separate host and port.
     if (node_by[0] == '_' || node_by[0] == 'l') {
-      res += "by=";
+      p = util::copy_lit(p, "by=");
-      res += node_by;
+      p = std::copy(std::begin(node_by), std::end(node_by), p);
-      res += ";";
+      p = util::copy_lit(p, ";");
     } else {
-      res += "by=\"";
+      p = util::copy_lit(p, "by=\"");
-      res += node_by;
+      p = std::copy(std::begin(node_by), std::end(node_by), p);
-      res += "\";";
+      p = util::copy_lit(p, "\";");
     }
   }
   if ((params & FORWARDED_FOR) && !node_for.empty()) {
     // We only quote IPv6 literal address only, which starts with '['.
     if (node_for[0] == '[') {
-      res += "for=\"";
+      p = util::copy_lit(p, "for=\"");
-      res += node_for;
+      p = std::copy(std::begin(node_for), std::end(node_for), p);
-      res += "\";";
+      p = util::copy_lit(p, "\";");
     } else {
-      res += "for=";
+      p = util::copy_lit(p, "for=");
-      res += node_for;
+      p = std::copy(std::begin(node_for), std::end(node_for), p);
-      res += ";";
+      p = util::copy_lit(p, ";");
     }
   }
   if ((params & FORWARDED_HOST) && !host.empty()) {
     // Just be quoted to skip checking characters.
-    res += "host=\"";
+    p = util::copy_lit(p, "host=\"");
-    res += host;
+    p = std::copy(std::begin(host), std::end(host), p);
-    res += "\";";
+    p = util::copy_lit(p, "\";");
   }
   if ((params & FORWARDED_PROTO) && !proto.empty()) {
     // Scheme production rule only allow characters which are all in
     // token.
-    res += "proto=";
+    p = util::copy_lit(p, "proto=");
-    res += proto;
+    p = std::copy(std::begin(proto), std::end(proto), p);
-    res += ";";
+    *p++ = ';';
   }
 
-  if (res.empty()) {
+  if (iov.base == p) {
-    return res;
+    return StringRef{};
   }
 
-  res.erase(res.size() - 1);
+  --p;
+  *p = '\0';
 
-  return res;
+  return StringRef{iov.base, p};
 }
 
 std::string colorizeHeaders(const char *hdrs) {

src/shrpx_http.h

@@ -32,6 +32,7 @@
 #include <nghttp2/nghttp2.h>
 
 #include "util.h"
+#include "allocator.h"
 
 namespace shrpx {
 
@@ -52,9 +53,9 @@ OutputIt create_via_header_value(OutputIt dst, int major, int minor) {
 // Returns generated RFC 7239 Forwarded header field value.  The
 // |params| is bitwise-OR of zero or more of shrpx_forwarded_param
 // defined in shrpx_config.h.
-std::string create_forwarded(int params, const StringRef &node_by,
+StringRef create_forwarded(BlockAllocator &balloc, int params,
-                             const StringRef &node_for, const StringRef &host,
+                           const StringRef &node_by, const StringRef &node_for,
-                             const StringRef &proto);
+                           const StringRef &host, const StringRef &proto);
 
 // Adds ANSI color codes to HTTP headers |hdrs|.
 std::string colorizeHeaders(const char *hdrs);

src/shrpx_http2_downstream_connection.cc

@@ -347,8 +347,6 @@ int Http2DownstreamConnection::push_request_headers() {
   auto upstream = downstream_->get_upstream();
   auto handler = upstream->get_client_handler();
 
-  std::string forwarded_value;
-
   auto &fwdconf = httpconf.forwarded;
 
   auto fwd =
@@ -361,25 +359,24 @@ int Http2DownstreamConnection::push_request_headers() {
       params &= ~FORWARDED_PROTO;
     }
 
-    auto value = http::create_forwarded(params, handler->get_forwarded_by(),
+    auto value = http::create_forwarded(
-                                        handler->get_forwarded_for(),
+        balloc, params, handler->get_forwarded_by(),
-                                        req.authority, req.scheme);
+        handler->get_forwarded_for(), req.authority, req.scheme);
+
     if (fwd || !value.empty()) {
       if (fwd) {
-        forwarded_value = fwd->value.str();
+        if (value.empty()) {
-
+          value = fwd->value;
-        if (!value.empty()) {
+        } else {
-          forwarded_value += ", ";
+          value = concat_string_ref(balloc, fwd->value,
+                                    StringRef::from_lit(", "), value);
         }
       }
 
-      forwarded_value += value;
+      nva.push_back(http2::make_nv_ls_nocopy("forwarded", value));
-
-      nva.push_back(http2::make_nv_ls("forwarded", forwarded_value));
     }
   } else if (fwd) {
     nva.push_back(http2::make_nv_ls_nocopy("forwarded", fwd->value));
-    forwarded_value = fwd->value.str();
   }
 
   auto &xffconf = httpconf.xff;

src/shrpx_http_downstream_connection.cc

@@ -272,6 +272,8 @@ int HttpDownstreamConnection::push_request_headers() {
   const auto &downstream_hostport = addr_->hostport;
   const auto &req = downstream_->request();
 
+  auto &balloc = downstream_->get_block_allocator();
+
   auto connect_method = req.method == HTTP_CONNECT;
 
   auto &httpconf = get_config()->http;
@@ -366,9 +368,10 @@ int HttpDownstreamConnection::push_request_headers() {
       params &= ~FORWARDED_PROTO;
     }
 
-    auto value = http::create_forwarded(params, handler->get_forwarded_by(),
+    auto value = http::create_forwarded(
-                                        handler->get_forwarded_for(),
+        balloc, params, handler->get_forwarded_by(),
-                                        req.authority, req.scheme);
+        handler->get_forwarded_for(), req.authority, req.scheme);
+
     if (fwd || !value.empty()) {
       buf->append("Forwarded: ");
       if (fwd) {

src/shrpx_http_test.cc

@@ -38,38 +38,40 @@
 namespace shrpx {
 
 void test_shrpx_http_create_forwarded(void) {
+  BlockAllocator balloc(1024, 1024);
+
   CU_ASSERT("by=\"example.com:3000\";for=\"[::1]\";host=\"www.example.com\";"
             "proto=https" ==
-            http::create_forwarded(FORWARDED_BY | FORWARDED_FOR |
+            http::create_forwarded(balloc, FORWARDED_BY | FORWARDED_FOR |
-                                       FORWARDED_HOST | FORWARDED_PROTO,
+                                               FORWARDED_HOST | FORWARDED_PROTO,
                                    StringRef::from_lit("example.com:3000"),
                                    StringRef::from_lit("[::1]"),
                                    StringRef::from_lit("www.example.com"),
                                    StringRef::from_lit("https")));
 
   CU_ASSERT("for=192.168.0.1" ==
-            http::create_forwarded(FORWARDED_FOR, StringRef::from_lit("alpha"),
+            http::create_forwarded(
-                                   StringRef::from_lit("192.168.0.1"),
+                balloc, FORWARDED_FOR, StringRef::from_lit("alpha"),
-                                   StringRef::from_lit("bravo"),
+                StringRef::from_lit("192.168.0.1"),
-                                   StringRef::from_lit("charlie")));
+                StringRef::from_lit("bravo"), StringRef::from_lit("charlie")));
 
   CU_ASSERT("by=_hidden;for=\"[::1]\"" ==
             http::create_forwarded(
-                FORWARDED_BY | FORWARDED_FOR, StringRef::from_lit("_hidden"),
+                balloc, FORWARDED_BY | FORWARDED_FOR,
-                StringRef::from_lit("[::1]"), StringRef::from_lit(""),
+                StringRef::from_lit("_hidden"), StringRef::from_lit("[::1]"),
-                StringRef::from_lit("")));
+                StringRef::from_lit(""), StringRef::from_lit("")));
 
   CU_ASSERT("by=\"[::1]\";for=_hidden" ==
             http::create_forwarded(
-                FORWARDED_BY | FORWARDED_FOR, StringRef::from_lit("[::1]"),
+                balloc, FORWARDED_BY | FORWARDED_FOR,
-                StringRef::from_lit("_hidden"), StringRef::from_lit(""),
+                StringRef::from_lit("[::1]"), StringRef::from_lit("_hidden"),
-                StringRef::from_lit("")));
-
-  CU_ASSERT("" ==
-            http::create_forwarded(
-                FORWARDED_BY | FORWARDED_FOR | FORWARDED_HOST | FORWARDED_PROTO,
-                StringRef::from_lit(""), StringRef::from_lit(""),
                 StringRef::from_lit(""), StringRef::from_lit("")));
+
+  CU_ASSERT("" == http::create_forwarded(
+                      balloc, FORWARDED_BY | FORWARDED_FOR | FORWARDED_HOST |
+                                  FORWARDED_PROTO,
+                      StringRef::from_lit(""), StringRef::from_lit(""),
+                      StringRef::from_lit(""), StringRef::from_lit("")));
 }
 
 void test_shrpx_http_create_via_header_value(void) {