nghttpx: Write h1 header into downstream buffer directly

src/http2.cc

@@ -132,13 +132,13 @@ std::string get_status_string(unsigned int status_code) {
   }
 }
 
-void capitalize(std::string &s, size_t offset) {
+void capitalize(DefaultMemchunks *buf, const std::string &s) {
-  s[offset] = util::upcase(s[offset]);
+  buf->append(util::upcase(s[0]));
-  for (size_t i = offset + 1, eoi = s.size(); i < eoi; ++i) {
+  for (size_t i = 1; i < s.size(); ++i) {
     if (s[i - 1] == '-') {
-      s[i] = util::upcase(s[i]);
+      buf->append(util::upcase(s[i]));
     } else {
-      s[i] = util::lowcase(s[i]);
+      buf->append(s[i]);
     }
   }
 }
@@ -242,7 +242,7 @@ void copy_headers_to_nva(std::vector<nghttp2_nv> &nva, const Headers &headers) {
   }
 }
 
-void build_http1_headers_from_headers(std::string &hdrs,
+void build_http1_headers_from_headers(DefaultMemchunks *buf,
                                       const Headers &headers) {
   for (auto &kv : headers) {
     if (kv.name.empty() || kv.name[0] == ':') {
@@ -262,11 +262,10 @@ void build_http1_headers_from_headers(std::string &hdrs,
     case HD_X_FORWARDED_PROTO:
       continue;
     }
-    hdrs += kv.name;
+    capitalize(buf, kv.name);
-    capitalize(hdrs, hdrs.size() - kv.name.size());
+    buf->append(": ");
-    hdrs += ": ";
+    buf->append(kv.value);
-    hdrs += kv.value;
+    buf->append("\r\n");
-    hdrs += "\r\n";
   }
 }
 

src/http2.h

@@ -38,6 +38,7 @@
 #include "http-parser/http_parser.h"
 
 #include "util.h"
+#include "memchunk.h"
 
 namespace nghttp2 {
 
@@ -69,7 +70,7 @@ namespace http2 {
 
 std::string get_status_string(unsigned int status_code);
 
-void capitalize(std::string &s, size_t offset);
+void capitalize(DefaultMemchunks *buf, const std::string &s);
 
 // Returns true if |value| is LWS
 bool lws(const char *value);
@@ -137,11 +138,11 @@ nghttp2_nv make_nv_ls(const char (&name)[N], const std::string &value) {
 // which require special handling (i.e. via), are not copied.
 void copy_headers_to_nva(std::vector<nghttp2_nv> &nva, const Headers &headers);
 
-// Appends HTTP/1.1 style header lines to |hdrs| from headers in
+// Appends HTTP/1.1 style header lines to |buf| from headers in
 // |headers|.  |headers| must be indexed before this call (its
 // element's token field is assigned).  Certain headers, which
 // requires special handling (i.e. via and cookie), are not appended.
-void build_http1_headers_from_headers(std::string &hdrs,
+void build_http1_headers_from_headers(DefaultMemchunks *buf,
                                       const Headers &headers);
 
 // Return positive window_size_increment if WINDOW_UPDATE should be

src/http2_test.cc

@@ -167,9 +167,11 @@ void test_http2_copy_headers_to_nva(void) {
 }
 
 void test_http2_build_http1_headers_from_headers(void) {
-  std::string hdrs;
+  MemchunkPool pool;
-  http2::build_http1_headers_from_headers(hdrs, headers);
+  DefaultMemchunks buf(&pool);
-  CU_ASSERT(hdrs == "Alpha: 0\r\n"
+  http2::build_http1_headers_from_headers(&buf, headers);
+  auto hdrs = std::string(buf.head->pos, buf.head->last);
+  CU_ASSERT("Alpha: 0\r\n"
             "Bravo: 1\r\n"
             "Delta: 4\r\n"
             "Expect: 5\r\n"
@@ -177,7 +179,7 @@ void test_http2_build_http1_headers_from_headers(void) {
             "Tango: 7\r\n"
             "Te: 8\r\n"
             "Te: 9\r\n"
-                    "Zulu: 12\r\n");
+            "Zulu: 12\r\n" == hdrs);
 }
 
 void test_http2_lws(void) {

src/memchunk.h

@@ -35,6 +35,7 @@
 #include <memory>
 #include <array>
 #include <algorithm>
+#include <string>
 
 #include "template.h"
 
@@ -126,6 +127,17 @@ template <typename Memchunk> struct Memchunks {
       m = next;
     }
   }
+  size_t append(char c) {
+    if (!tail) {
+      head = tail = pool->get();
+    }
+    if (tail->left() == 0) {
+      tail->next = pool->get();
+      tail = tail->next;
+    }
+    *tail->last++ = c;
+    return 1;
+  }
   size_t append(const void *src, size_t count) {
     if (count == 0) {
       return 0;
@@ -156,6 +168,7 @@ template <typename Memchunk> struct Memchunks {
   template <size_t N> size_t append(const char (&s)[N]) {
     return append(s, N - 1);
   }
+  size_t append(const std::string &s) { return append(s.c_str(), s.size()); }
   size_t remove(void *dest, size_t count) {
     if (!tail || count == 0) {
       return 0;

src/shrpx_http_downstream_connection.cc

@@ -227,135 +227,137 @@ int HttpDownstreamConnection::push_request_headers() {
   if (no_host_rewrite && !req_authority.empty()) {
     authority = req_authority.c_str();
   }
+  auto authoritylen = strlen(authority);
 
   downstream_->set_request_downstream_host(authority);
 
   downstream_->assemble_request_cookie();
 
+  auto buf = downstream_->get_request_buf();
+
   // Assume that method and request path do not contain \r\n.
-  std::string hdrs = http2::to_method_string(method);
+  auto meth = http2::to_method_string(method);
-  hdrs += ' ';
+  buf->append(meth, strlen(meth));
+  buf->append(" ");
 
   auto &scheme = downstream_->get_request_http2_scheme();
   auto &path = downstream_->get_request_path();
 
   if (connect_method) {
-    hdrs += authority;
+    buf->append(authority, authoritylen);
   } else if (get_config()->http2_proxy || get_config()->client_proxy) {
     // Construct absolute-form request target because we are going to
     // send a request to a HTTP/1 proxy.
     assert(!scheme.empty());
-    hdrs += scheme;
+    buf->append(scheme);
-    hdrs += "://";
+    buf->append("://");
-    hdrs += authority;
+    buf->append(authority, authoritylen);
-    hdrs += path;
+    buf->append(path);
   } else if (method == HTTP_OPTIONS && path.empty()) {
     // Server-wide OPTIONS
-    hdrs += "*";
+    buf->append("*");
   } else {
-    hdrs += path;
+    buf->append(path);
   }
-  hdrs += " HTTP/1.1\r\nHost: ";
+  buf->append(" HTTP/1.1\r\nHost: ");
-  hdrs += authority;
+  buf->append(authority, authoritylen);
-  hdrs += "\r\n";
+  buf->append("\r\n");
 
-  http2::build_http1_headers_from_headers(hdrs,
+  http2::build_http1_headers_from_headers(buf,
                                           downstream_->get_request_headers());
 
   if (!downstream_->get_assembled_request_cookie().empty()) {
-    hdrs += "Cookie: ";
+    buf->append("Cookie: ");
-    hdrs += downstream_->get_assembled_request_cookie();
+    buf->append(downstream_->get_assembled_request_cookie());
-    hdrs += "\r\n";
+    buf->append("\r\n");
   }
 
   if (!connect_method && downstream_->get_request_http2_expect_body() &&
       !downstream_->get_request_header(http2::HD_CONTENT_LENGTH)) {
 
     downstream_->set_chunked_request(true);
-    hdrs += "Transfer-Encoding: chunked\r\n";
+    buf->append("Transfer-Encoding: chunked\r\n");
   }
 
   if (downstream_->get_request_connection_close()) {
-    hdrs += "Connection: close\r\n";
+    buf->append("Connection: close\r\n");
   }
 
   if (!connect_method && downstream_->get_upgrade_request()) {
     auto connection = downstream_->get_request_header(http2::HD_CONNECTION);
     if (connection) {
-      hdrs += "Connection: ";
+      buf->append("Connection: ");
-      hdrs += (*connection).value;
+      buf->append((*connection).value);
-      hdrs += "\r\n";
+      buf->append("\r\n");
     }
 
     auto upgrade = downstream_->get_request_header(http2::HD_UPGRADE);
     if (upgrade) {
-      hdrs += "Upgrade: ";
+      buf->append("Upgrade: ");
-      hdrs += (*upgrade).value;
+      buf->append((*upgrade).value);
-      hdrs += "\r\n";
+      buf->append("\r\n");
     }
   }
 
   auto xff = downstream_->get_request_header(http2::HD_X_FORWARDED_FOR);
   if (get_config()->add_x_forwarded_for) {
-    hdrs += "X-Forwarded-For: ";
+    buf->append("X-Forwarded-For: ");
     if (xff && !get_config()->strip_incoming_x_forwarded_for) {
-      hdrs += (*xff).value;
+      buf->append((*xff).value);
-      hdrs += ", ";
+      buf->append(", ");
     }
-    hdrs += client_handler_->get_ipaddr();
+    buf->append(client_handler_->get_ipaddr());
-    hdrs += "\r\n";
+    buf->append("\r\n");
   } else if (xff && !get_config()->strip_incoming_x_forwarded_for) {
-    hdrs += "X-Forwarded-For: ";
+    buf->append("X-Forwarded-For: ");
-    hdrs += (*xff).value;
+    buf->append((*xff).value);
-    hdrs += "\r\n";
+    buf->append("\r\n");
   }
   if (!get_config()->http2_proxy && !get_config()->client_proxy &&
       !connect_method) {
-    hdrs += "X-Forwarded-Proto: ";
+    buf->append("X-Forwarded-Proto: ");
     assert(!scheme.empty());
-    hdrs += scheme;
+    buf->append(scheme);
-    hdrs += "\r\n";
+    buf->append("\r\n");
   }
   auto via = downstream_->get_request_header(http2::HD_VIA);
   if (get_config()->no_via) {
     if (via) {
-      hdrs += "Via: ";
+      buf->append("Via: ");
-      hdrs += (*via).value;
+      buf->append((*via).value);
-      hdrs += "\r\n";
+      buf->append("\r\n");
     }
   } else {
-    hdrs += "Via: ";
+    buf->append("Via: ");
     if (via) {
-      hdrs += (*via).value;
+      buf->append((*via).value);
-      hdrs += ", ";
+      buf->append(", ");
     }
-    hdrs += http::create_via_header_value(downstream_->get_request_major(),
+    buf->append(http::create_via_header_value(
-                                          downstream_->get_request_minor());
+        downstream_->get_request_major(), downstream_->get_request_minor()));
-    hdrs += "\r\n";
+    buf->append("\r\n");
   }
 
   for (auto &p : get_config()->add_request_headers) {
-    hdrs += p.first;
+    buf->append(p.first);
-    hdrs += ": ";
+    buf->append(": ");
-    hdrs += p.second;
+    buf->append(p.second);
-    hdrs += "\r\n";
+    buf->append("\r\n");
   }
 
-  hdrs += "\r\n";
+  buf->append("\r\n");
+
   if (LOG_ENABLED(INFO)) {
-    const char *hdrp;
     std::string nhdrs;
+    for (auto chunk = buf->head; chunk; chunk = chunk->next) {
+      nhdrs.append(chunk->pos, chunk->last);
+    }
     if (log_config()->errorlog_tty) {
-      nhdrs = http::colorizeHeaders(hdrs.c_str());
+      nhdrs = http::colorizeHeaders(nhdrs.c_str());
-      hdrp = nhdrs.c_str();
-    } else {
-      hdrp = hdrs.c_str();
     }
     DCLOG(INFO, this) << "HTTP request headers. stream_id="
-                      << downstream_->get_stream_id() << "\n" << hdrp;
+                      << downstream_->get_stream_id() << "\n" << nhdrs;
   }
-  auto output = downstream_->get_request_buf();
-  output->append(hdrs.c_str(), hdrs.size());
 
   signal_write();
 
@@ -395,9 +397,7 @@ int HttpDownstreamConnection::end_upload_data() {
     output->append("0\r\n\r\n");
   } else {
     output->append("0\r\n");
-    std::string trailer_part;
+    http2::build_http1_headers_from_headers(output, trailers);
-    http2::build_http1_headers_from_headers(trailer_part, trailers);
-    output->append(trailer_part.c_str(), trailer_part.size());
     output->append("\r\n");
   }
 

src/shrpx_https_upstream.cc

@@ -797,20 +797,17 @@ int HttpsUpstream::send_reply(Downstream *downstream, const uint8_t *body,
   auto output = downstream->get_response_buf();
 
   output->append("HTTP/1.1 ");
-  auto status_str =
+  output->append(
-      http2::get_status_string(downstream->get_response_http_status());
+      http2::get_status_string(downstream->get_response_http_status()));
-  output->append(status_str.c_str(), status_str.size());
   output->append("\r\n");
 
   for (auto &kv : downstream->get_response_headers()) {
     if (kv.name.empty() || kv.name[0] == ':') {
       continue;
     }
-    auto name = kv.name;
+    http2::capitalize(output, kv.name);
-    http2::capitalize(name, 0);
-    output->append(name.c_str(), name.size());
     output->append(": ");
-    output->append(kv.value.c_str(), kv.value.size());
+    output->append(kv.value);
     output->append("\r\n");
   }
 
@@ -889,6 +886,17 @@ std::unique_ptr<Downstream> HttpsUpstream::pop_downstream() {
   return std::unique_ptr<Downstream>(downstream_.release());
 }
 
+namespace {
+void write_altsvc(DefaultMemchunks *buf, const AltSvc &altsvc) {
+  buf->append(util::percent_encode_token(altsvc.protocol_id));
+  buf->append("=\"");
+  buf->append(util::quote_string(altsvc.host));
+  buf->append(":");
+  buf->append(altsvc.service);
+  buf->append("\"");
+}
+} // namespace
+
 int HttpsUpstream::on_downstream_header_complete(Downstream *downstream) {
   if (LOG_ENABLED(INFO)) {
     if (downstream->get_non_final_response()) {
@@ -916,13 +924,15 @@ int HttpsUpstream::on_downstream_header_complete(Downstream *downstream) {
 
   auto connect_method = downstream->get_request_method() == HTTP_CONNECT;
 
-  std::string hdrs = "HTTP/";
+  auto buf = downstream->get_response_buf();
-  hdrs += util::utos(downstream->get_request_major());
+
-  hdrs += ".";
+  buf->append("HTTP/");
-  hdrs += util::utos(downstream->get_request_minor());
+  buf->append(util::utos(downstream->get_request_major()));
-  hdrs += " ";
+  buf->append(".");
-  hdrs += http2::get_status_string(downstream->get_response_http_status());
+  buf->append(util::utos(downstream->get_request_minor()));
-  hdrs += "\r\n";
+  buf->append(" ");
+  buf->append(http2::get_status_string(downstream->get_response_http_status()));
+  buf->append("\r\n");
 
   if (!get_config()->http2_proxy && !get_config()->client_proxy &&
       !get_config()->no_location_rewrite) {
@@ -930,20 +940,16 @@ int HttpsUpstream::on_downstream_header_complete(Downstream *downstream) {
         get_client_handler()->get_upstream_scheme());
   }
 
-  http2::build_http1_headers_from_headers(hdrs,
+  http2::build_http1_headers_from_headers(buf,
                                           downstream->get_response_headers());
 
-  auto output = downstream->get_response_buf();
-
   if (downstream->get_non_final_response()) {
-    hdrs += "\r\n";
+    buf->append("\r\n");
 
     if (LOG_ENABLED(INFO)) {
-      log_response_headers(hdrs);
+      log_response_headers(buf);
     }
 
-    output->append(hdrs.c_str(), hdrs.size());
-
     downstream->clear_response_headers();
 
     return 0;
@@ -964,93 +970,87 @@ int HttpsUpstream::on_downstream_header_complete(Downstream *downstream) {
     if (downstream->get_request_major() <= 0 ||
         downstream->get_request_minor() <= 0) {
       // We add this header for HTTP/1.0 or HTTP/0.9 clients
-      hdrs += "Connection: Keep-Alive\r\n";
+      buf->append("Connection: Keep-Alive\r\n");
     }
   } else if (!downstream->get_upgraded()) {
-    hdrs += "Connection: close\r\n";
+    buf->append("Connection: close\r\n");
   }
 
   if (!connect_method && downstream->get_upgraded()) {
     auto connection = downstream->get_response_header(http2::HD_CONNECTION);
     if (connection) {
-      hdrs += "Connection: ";
+      buf->append("Connection: ");
-      hdrs += (*connection).value;
+      buf->append((*connection).value);
-      hdrs += "\r\n";
+      buf->append("\r\n");
     }
 
     auto upgrade = downstream->get_response_header(http2::HD_UPGRADE);
     if (upgrade) {
-      hdrs += "Upgrade: ";
+      buf->append("Upgrade: ");
-      hdrs += (*upgrade).value;
+      buf->append((*upgrade).value);
-      hdrs += "\r\n";
+      buf->append("\r\n");
     }
   }
 
   if (!downstream->get_response_header(http2::HD_ALT_SVC)) {
     // We won't change or alter alt-svc from backend for now
     if (!get_config()->altsvcs.empty()) {
-      hdrs += "Alt-Svc: ";
+      buf->append("Alt-Svc: ");
 
-      for (const auto &altsvc : get_config()->altsvcs) {
+      auto &altsvcs = get_config()->altsvcs;
-        hdrs += util::percent_encode_token(altsvc.protocol_id);
+      write_altsvc(buf, altsvcs[0]);
-        hdrs += "=\"";
+      for (size_t i = 1; i < altsvcs.size(); ++i) {
-        hdrs += util::quote_string(altsvc.host);
+        buf->append(", ");
-        hdrs += ":";
+        write_altsvc(buf, altsvcs[i]);
-        hdrs += altsvc.service;
-        hdrs += "\", ";
       }
-
+      buf->append("\r\n");
-      hdrs[hdrs.size() - 2] = '\r';
-      hdrs[hdrs.size() - 1] = '\n';
     }
   }
 
   if (!get_config()->http2_proxy && !get_config()->client_proxy) {
-    hdrs += "Server: ";
+    buf->append("Server: ");
-    hdrs += get_config()->server_name;
+    buf->append(get_config()->server_name, strlen(get_config()->server_name));
-    hdrs += "\r\n";
+    buf->append("\r\n");
   } else {
     auto server = downstream->get_response_header(http2::HD_SERVER);
     if (server) {
-      hdrs += "Server: ";
+      buf->append("Server: ");
-      hdrs += (*server).value;
+      buf->append((*server).value);
-      hdrs += "\r\n";
+      buf->append("\r\n");
     }
   }
 
   auto via = downstream->get_response_header(http2::HD_VIA);
   if (get_config()->no_via) {
     if (via) {
-      hdrs += "Via: ";
+      buf->append("Via: ");
-      hdrs += (*via).value;
+      buf->append((*via).value);
-      hdrs += "\r\n";
+      buf->append("\r\n");
     }
   } else {
-    hdrs += "Via: ";
+    buf->append("Via: ");
     if (via) {
-      hdrs += (*via).value;
+      buf->append((*via).value);
-      hdrs += ", ";
+      buf->append(", ");
     }
-    hdrs += http::create_via_header_value(downstream->get_response_major(),
+    buf->append(http::create_via_header_value(
-                                          downstream->get_response_minor());
+        downstream->get_response_major(), downstream->get_response_minor()));
-    hdrs += "\r\n";
+    buf->append("\r\n");
   }
 
   for (auto &p : get_config()->add_response_headers) {
-    hdrs += p.first;
+    buf->append(p.first);
-    hdrs += ": ";
+    buf->append(": ");
-    hdrs += p.second;
+    buf->append(p.second);
-    hdrs += "\r\n";
+    buf->append("\r\n");
   }
 
-  hdrs += "\r\n";
+  buf->append("\r\n");
 
   if (LOG_ENABLED(INFO)) {
-    log_response_headers(hdrs);
+    log_response_headers(buf);
   }
 
-  output->append(hdrs.c_str(), hdrs.size());
-
   return 0;
 }
 
@@ -1085,9 +1085,7 @@ int HttpsUpstream::on_downstream_body_complete(Downstream *downstream) {
       output->append("0\r\n\r\n");
     } else {
       output->append("0\r\n");
-      std::string trailer_part;
+      http2::build_http1_headers_from_headers(output, trailers);
-      http2::build_http1_headers_from_headers(trailer_part, trailers);
-      output->append(trailer_part.c_str(), trailer_part.size());
       output->append("\r\n");
     }
   }
@@ -1114,16 +1112,15 @@ int HttpsUpstream::on_downstream_abort_request(Downstream *downstream,
   return 0;
 }
 
-void HttpsUpstream::log_response_headers(const std::string &hdrs) const {
+void HttpsUpstream::log_response_headers(DefaultMemchunks *buf) const {
-  const char *hdrp;
   std::string nhdrs;
-  if (log_config()->errorlog_tty) {
+  for (auto chunk = buf->head; chunk; chunk = chunk->next) {
-    nhdrs = http::colorizeHeaders(hdrs.c_str());
+    nhdrs.append(chunk->pos, chunk->last);
-    hdrp = nhdrs.c_str();
-  } else {
-    hdrp = hdrs.c_str();
   }
-  ULOG(INFO, this) << "HTTP response headers\n" << hdrp;
+  if (log_config()->errorlog_tty) {
+    nhdrs = http::colorizeHeaders(nhdrs.c_str());
+  }
+  ULOG(INFO, this) << "HTTP response headers\n" << nhdrs;
 }
 
 void HttpsUpstream::on_handler_delete() {

src/shrpx_https_upstream.h

@@ -80,7 +80,7 @@ public:
                             size_t len);
 
   void reset_current_header_length();
-  void log_response_headers(const std::string &hdrs) const;
+  void log_response_headers(DefaultMemchunks *buf) const;
 
 private:
   ClientHandler *handler_;