nghttpx: Use nghttp2_submit_{request, response}2 API

src/http2.cc

@@ -298,16 +298,24 @@ bool non_empty_value(const nghttp2_nv* nv)
   return nv && !http2::value_lws(nv) && http2::check_header_value(nv);
 }
 
-void copy_norm_headers_to_nv
-(std::vector<const char*>& nv,
+nghttp2_nv make_nv(const std::string& name, const std::string& value)
+{
+  return {
+    (uint8_t*)name.c_str(),
+      (uint8_t*)value.c_str(),
+      (uint16_t)name.size(), (uint16_t)value.size()
+      };
+}
+
+void copy_norm_headers_to_nva
+(std::vector<nghttp2_nv>& nva,
  const std::vector<std::pair<std::string, std::string>>& headers)
 {
   size_t i, j;
   for(i = 0, j = 0; i < headers.size() && j < IGN_HDLEN;) {
     int rv = strcmp(headers[i].first.c_str(), IGN_HD[j]);
     if(rv < 0) {
-      nv.push_back(headers[i].first.c_str());
-      nv.push_back(headers[i].second.c_str());
+      nva.push_back(make_nv(headers[i].first, headers[i].second));
       ++i;
     } else if(rv > 0) {
       ++j;
@@ -316,8 +324,7 @@ void copy_norm_headers_to_nv
     }
   }
   for(; i < headers.size(); ++i) {
-    nv.push_back(headers[i].first.c_str());
-    nv.push_back(headers[i].second.c_str());
+    nva.push_back(make_nv(headers[i].first, headers[i].second));
   }
 }
 

src/http2.h

@@ -39,6 +39,18 @@ namespace nghttp2 {
 
 namespace http2 {
 
+// Create nghttp2_nv from string literal |NAME| and std::string
+// |VALUE|.
+#define MAKE_NV_LS(NAME, VALUE)                                 \
+  { (uint8_t*)NAME, (uint8_t*)VALUE.c_str(),                    \
+      (uint16_t)(sizeof(NAME) - 1), (uint16_t)VALUE.size() }
+
+// Create nghttp2_nv from string literal |NAME| and |VALUE|.
+#define MAKE_NV_LS_LS(NAME, VALUE)                                      \
+  { (uint8_t*)NAME, (uint8_t*)VALUE,                                    \
+      (uint16_t)(sizeof(NAME) - 1), (uint16_t)(sizeof(VALUE) - 1) }
+
+
 std::string get_status_string(unsigned int status_code);
 
 void capitalize(std::string& s, size_t offset);
@@ -98,11 +110,16 @@ bool value_lws(const nghttp2_nv *nv);
 // and not contain illegal characters.
 bool non_empty_value(const nghttp2_nv* nv);
 
+// Creates nghttp2_nv using |name| and |value| and returns it. The
+// returned value only references the data pointer to name.c_str() and
+// value.c_str().
+nghttp2_nv make_nv(const std::string& name, const std::string& value);
+
 // Appends headers in |headers| to |nv|. Certain headers, including
 // disallowed headers in HTTP/2.0 spec and headers which require
 // special handling (i.e. via), are not copied.
-void copy_norm_headers_to_nv
-(std::vector<const char*>& nv,
+void copy_norm_headers_to_nva
+(std::vector<nghttp2_nv>& nva,
  const std::vector<std::pair<std::string, std::string>>& headers);
 
 // Appends HTTP/1.1 style header lines to |hdrs| from headers in

src/http2_test.cc

@@ -164,23 +164,26 @@ auto headers = std::vector<std::pair<std::string, std::string>>
    {"zulu", "12"}};
 } // namespace
 
-void test_http2_copy_norm_headers_to_nv(void)
+namespace {
+void check_nv(const std::pair<std::string, std::string>& a,
+              const nghttp2_nv *b)
+{
+  CU_ASSERT(a.first.size() == b->namelen);
+  CU_ASSERT(a.second.size() == b->valuelen);
+  CU_ASSERT(memcmp(a.first.c_str(), b->name, b->namelen) == 0);
+  CU_ASSERT(memcmp(a.second.c_str(), b->value, b->valuelen) == 0);
+}
+} // namespace
+
+void test_http2_copy_norm_headers_to_nva(void)
 {
-  std::vector<const char*> nv;
-  http2::copy_norm_headers_to_nv(nv, headers);
-  CU_ASSERT(12 == nv.size());
-  CU_ASSERT(strcmp(nv[0], "alpha") == 0);
-  CU_ASSERT(strcmp(nv[1], "0") == 0);
-  CU_ASSERT(strcmp(nv[2], "bravo") == 0);
-  CU_ASSERT(strcmp(nv[3], "1") == 0);
-  CU_ASSERT(strcmp(nv[4], "delta") == 0);
-  CU_ASSERT(strcmp(nv[5], "4") == 0);
-  CU_ASSERT(strcmp(nv[6], "foxtrot") == 0);
-  CU_ASSERT(strcmp(nv[7], "6") == 0);
-  CU_ASSERT(strcmp(nv[8], "tango") == 0);
-  CU_ASSERT(strcmp(nv[9], "7") == 0);
-  CU_ASSERT(strcmp(nv[10], "zulu") == 0);
-  CU_ASSERT(strcmp(nv[11], "12") == 0);
+  std::vector<nghttp2_nv> nva;
+  http2::copy_norm_headers_to_nva(nva, headers);
+  CU_ASSERT(6 == nva.size());
+  auto ans = std::vector<int>{0, 1, 4, 6, 7, 12};
+  for(size_t i = 0; i < ans.size(); ++i) {
+    check_nv(headers[ans[i]], &nva[i]);
+  }
 }
 
 void test_http2_build_http1_headers_from_norm_headers(void)

src/http2_test.h

@@ -32,7 +32,7 @@ void test_http2_check_http2_headers(void);
 void test_http2_get_unique_header(void);
 void test_http2_get_header(void);
 void test_http2_value_lws(void);
-void test_http2_copy_norm_headers_to_nv(void);
+void test_http2_copy_norm_headers_to_nva(void);
 void test_http2_build_http1_headers_from_norm_headers(void);
 void test_http2_check_header_value(void);
 

src/shrpx-unittest.cc

@@ -78,8 +78,8 @@ int main(int argc, char* argv[])
                    shrpx::test_http2_get_header) ||
       !CU_add_test(pSuite, "http2_value_lws",
                    shrpx::test_http2_value_lws) ||
-      !CU_add_test(pSuite, "http2_copy_norm_headers_to_nv",
-                   shrpx::test_http2_copy_norm_headers_to_nv) ||
+      !CU_add_test(pSuite, "http2_copy_norm_headers_to_nva",
+                   shrpx::test_http2_copy_norm_headers_to_nva) ||
       !CU_add_test(pSuite, "http2_build_http1_headers_from_norm_headers",
                    shrpx::test_http2_build_http1_headers_from_norm_headers) ||
       !CU_add_test(pSuite, "http2_check_header_value",

src/shrpx_http2_downstream_connection.cc

@@ -233,35 +233,36 @@ int Http2DownstreamConnection::push_request_headers()
   }
   downstream_->normalize_request_headers();
   auto end_headers = std::end(downstream_->get_request_headers());
-  // 12 means:
+  // 6 means:
   // 1. :method
   // 2. :scheme
   // 3. :path
   // 4. :authority (optional)
   // 5. via (optional)
   // 6. x-forwarded-for (optional)
-  auto nv = std::vector<const char*>();
-  nv.reserve(nheader * 2 + 10 + 1);
+  auto nva = std::vector<nghttp2_nv>();
+  nva.reserve(nheader + 6);
   std::string via_value;
   std::string xff_value;
   std::string scheme, authority, path, query;
   if(downstream_->get_request_method() == "CONNECT") {
     // The upstream may be HTTP/2 or HTTP/1
-    nv.push_back(":authority");
     if(!downstream_->get_request_http2_authority().empty()) {
-      nv.push_back(downstream_->get_request_http2_authority().c_str());
+      nva.push_back(MAKE_NV_LS(":authority",
+                               downstream_->get_request_http2_authority()));
     } else {
-      nv.push_back(downstream_->get_request_path().c_str());
+      nva.push_back(MAKE_NV_LS(":authority",
+                               downstream_->get_request_path()));
     }
   } else if(!downstream_->get_request_http2_scheme().empty()) {
     // Here the upstream is HTTP/2
-    nv.push_back(":scheme");
-    nv.push_back(downstream_->get_request_http2_scheme().c_str());
-    nv.push_back(":path");
-    nv.push_back(downstream_->get_request_path().c_str());
+    nva.push_back(MAKE_NV_LS(":scheme",
+                             downstream_->get_request_http2_scheme()));
+    nva.push_back(MAKE_NV_LS(":path",
+                             downstream_->get_request_path()));
     if(!downstream_->get_request_http2_authority().empty()) {
-      nv.push_back(":authority");
-      nv.push_back(downstream_->get_request_http2_authority().c_str());
+      nva.push_back(MAKE_NV_LS(":authority",
+                               downstream_->get_request_http2_authority()));
     } else if(downstream_->get_norm_request_header("host") == end_headers) {
       if(LOG_ENABLED(INFO)) {
         DCLOG(INFO, this) << "host header field missing";
@@ -289,19 +290,17 @@ int Http2DownstreamConnection::push_request_headers()
         path += query;
       }
     }
-    nv.push_back(":scheme");
     if(scheme.empty()) {
       // The default scheme is http. For HTTP2 upstream, the path must
       // be absolute URI, so scheme should be provided.
-      nv.push_back("http");
+      nva.push_back(MAKE_NV_LS_LS(":scheme", "http"));
     } else {
-      nv.push_back(scheme.c_str());
+      nva.push_back(MAKE_NV_LS(":scheme", scheme));
     }
-    nv.push_back(":path");
     if(path.empty()) {
-      nv.push_back(downstream_->get_request_path().c_str());
+      nva.push_back(MAKE_NV_LS(":path", downstream_->get_request_path()));
     } else {
-      nv.push_back(path.c_str());
+      nva.push_back(MAKE_NV_LS(":path", path));
     }
     if(!authority.empty()) {
       // TODO properly check IPv6 numeric address
@@ -313,8 +312,7 @@ int Http2DownstreamConnection::push_request_headers()
         authority += ":";
         authority += util::utos(u.port);
       }
-      nv.push_back(":authority");
-      nv.push_back(authority.c_str());
+      nva.push_back(MAKE_NV_LS(":authority", authority));
     } else if(downstream_->get_norm_request_header("host") == end_headers) {
       if(LOG_ENABLED(INFO)) {
         DCLOG(INFO, this) << "host header field missing";
@@ -323,10 +321,9 @@ int Http2DownstreamConnection::push_request_headers()
     }
   }
 
-  nv.push_back(":method");
-  nv.push_back(downstream_->get_request_method().c_str());
+  nva.push_back(MAKE_NV_LS(":method", downstream_->get_request_method()));
 
-  http2::copy_norm_headers_to_nv(nv, downstream_->get_request_headers());
+  http2::copy_norm_headers_to_nva(nva, downstream_->get_request_headers());
 
   bool content_length = false;
   if(downstream_->get_norm_request_header("content-length") != end_headers) {
@@ -336,8 +333,7 @@ int Http2DownstreamConnection::push_request_headers()
   auto expect = downstream_->get_norm_request_header("expect");
   if(expect != end_headers &&
      !util::strifind((*expect).second.c_str(), "100-continue")) {
-    nv.push_back("expect");
-    nv.push_back((*expect).second.c_str());
+    nva.push_back(MAKE_NV_LS("expect", (*expect).second));
   }
 
   bool chunked_encoding = false;
@@ -350,24 +346,21 @@ int Http2DownstreamConnection::push_request_headers()
 
   auto xff = downstream_->get_norm_request_header("x-forwarded-for");
   if(get_config()->add_x_forwarded_for) {
-    nv.push_back("x-forwarded-for");
     if(xff != end_headers) {
       xff_value = (*xff).second;
       xff_value += ", ";
     }
     xff_value += downstream_->get_upstream()->get_client_handler()->
       get_ipaddr();
-    nv.push_back(xff_value.c_str());
+    nva.push_back(MAKE_NV_LS("x-forwarded-for", xff_value));
   } else if(xff != end_headers) {
-    nv.push_back("x-forwarded-for");
-    nv.push_back((*xff).second.c_str());
+    nva.push_back(MAKE_NV_LS("x-forwarded-for", (*xff).second));
   }
 
   auto via = downstream_->get_norm_request_header("via");
   if(get_config()->no_via) {
     if(via != end_headers) {
-      nv.push_back("via");
-      nv.push_back((*via).second.c_str());
+      nva.push_back(MAKE_NV_LS("via", (*via).second));
     }
   } else {
     if(via != end_headers) {
@@ -376,15 +369,17 @@ int Http2DownstreamConnection::push_request_headers()
     }
     via_value += http::create_via_header_value
       (downstream_->get_request_major(), downstream_->get_request_minor());
-    nv.push_back("via");
-    nv.push_back(via_value.c_str());
+    nva.push_back(MAKE_NV_LS("via", via_value));
   }
-  nv.push_back(nullptr);
 
   if(LOG_ENABLED(INFO)) {
     std::stringstream ss;
-    for(size_t i = 0; nv[i]; i += 2) {
-      ss << TTY_HTTP_HD << nv[i] << TTY_RST << ": " << nv[i+1] << "\n";
+    for(auto& nv : nva) {
+      ss << TTY_HTTP_HD;
+      ss.write(reinterpret_cast<const char*>(nv.name), nv.namelen);
+      ss << TTY_RST << ": ";
+      ss.write(reinterpret_cast<const char*>(nv.value), nv.valuelen);
+      ss << "\n";
     }
     DCLOG(INFO, this) << "HTTP request headers\n" << ss.str();
   }
@@ -396,10 +391,10 @@ int Http2DownstreamConnection::push_request_headers()
     data_prd.source.ptr = this;
     data_prd.read_callback = http2_data_read_callback;
     rv = http2session_->submit_request(this, downstream_->get_priorty(),
-                                       nv.data(), &data_prd);
+                                       nva.data(), nva.size(), &data_prd);
   } else {
     rv = http2session_->submit_request(this, downstream_->get_priorty(),
-                                       nv.data(), nullptr);
+                                       nva.data(), nva.size(), nullptr);
   }
   if(rv != 0) {
     DCLOG(FATAL, this) << "nghttp2_submit_request() failed";

src/shrpx_http2_session.cc

@@ -569,12 +569,14 @@ void Http2Session::remove_stream_data(StreamData *sd)
 }
 
 int Http2Session::submit_request(Http2DownstreamConnection *dconn,
-                                uint8_t pri, const char **nv,
-                                const nghttp2_data_provider *data_prd)
+                                 uint8_t pri,
+                                 const nghttp2_nv *nva, size_t nvlen,
+                                 const nghttp2_data_provider *data_prd)
 {
   assert(state_ == CONNECTED);
   auto sd = util::make_unique<StreamData>();
-  int rv = nghttp2_submit_request(session_, pri, nv, data_prd, sd.get());
+  int rv = nghttp2_submit_request2(session_, pri, nva, nvlen,
+                                   data_prd, sd.get());
   if(rv == 0) {
     dconn->attach_stream_data(sd.get());
     streams_.insert(sd.release());

src/shrpx_http2_session.h

@@ -65,7 +65,7 @@ public:
   void remove_stream_data(StreamData *sd);
 
   int submit_request(Http2DownstreamConnection *dconn,
-                     uint8_t pri, const char **nv,
+                     uint8_t pri, const nghttp2_nv *nva, size_t nvlen,
                      const nghttp2_data_provider *data_prd);
 
   int submit_rst_stream(int32_t stream_id, nghttp2_error_code error_code);

src/shrpx_http2_upstream.cc

@@ -945,20 +945,18 @@ int Http2Upstream::on_downstream_header_complete(Downstream *downstream)
   downstream->normalize_response_headers();
   auto end_headers = std::end(downstream->get_response_headers());
   size_t nheader = downstream->get_response_headers().size();
-  // 4 means :status and possible via header field.
-  auto nv = std::vector<const char*>();
-  nv.reserve(nheader * 2 + 4 + 1);
+  auto nva = std::vector<nghttp2_nv>();
+  // 2 means :status and possible via header field.
+  nva.reserve(nheader + 2);
   std::string via_value;
   auto response_status = util::utos(downstream->get_response_http_status());
-  nv.push_back(":status");
-  nv.push_back(response_status.c_str());
+  nva.push_back(MAKE_NV_LS(":status", response_status));
 
-  http2::copy_norm_headers_to_nv(nv, downstream->get_response_headers());
+  http2::copy_norm_headers_to_nva(nva, downstream->get_response_headers());
   auto via = downstream->get_norm_response_header("via");
   if(get_config()->no_via) {
     if(via != end_headers) {
-      nv.push_back("via");
-      nv.push_back((*via).second.c_str());
+      nva.push_back(MAKE_NV_LS("via", (*via).second));
     }
   } else {
     if(via != end_headers) {
@@ -967,14 +965,16 @@ int Http2Upstream::on_downstream_header_complete(Downstream *downstream)
     }
     via_value += http::create_via_header_value
       (downstream->get_response_major(), downstream->get_response_minor());
-    nv.push_back("via");
-    nv.push_back(via_value.c_str());
+    nva.push_back(MAKE_NV_LS("via", via_value));
   }
-  nv.push_back(nullptr);
   if(LOG_ENABLED(INFO)) {
     std::stringstream ss;
-    for(size_t i = 0; nv[i]; i += 2) {
-      ss << TTY_HTTP_HD << nv[i] << TTY_RST << ": " << nv[i+1] << "\n";
+    for(auto& nv : nva) {
+      ss << TTY_HTTP_HD;
+      ss.write(reinterpret_cast<const char*>(nv.name), nv.namelen);
+      ss << TTY_RST << ": ";
+      ss.write(reinterpret_cast<const char*>(nv.value), nv.valuelen);
+      ss << "\n";
     }
     ULOG(INFO, this) << "HTTP response headers. stream_id="
                      << downstream->get_stream_id() << "\n"
@@ -983,7 +983,7 @@ int Http2Upstream::on_downstream_header_complete(Downstream *downstream)
 
   if(get_config()->http2_upstream_dump_response_header) {
     http2::dump_nv(get_config()->http2_upstream_dump_response_header,
-                   nv.data());
+                   nva.data(), nva.size());
   }
 
   nghttp2_data_provider data_prd;
@@ -991,8 +991,8 @@ int Http2Upstream::on_downstream_header_complete(Downstream *downstream)
   data_prd.read_callback = downstream_data_read_callback;
 
   int rv;
-  rv = nghttp2_submit_response(session_, downstream->get_stream_id(),
-                               nv.data(), &data_prd);
+  rv = nghttp2_submit_response2(session_, downstream->get_stream_id(),
+                                nva.data(), nva.size(), &data_prd);
   if(rv != 0) {
     ULOG(FATAL, this) << "nghttp2_submit_response() failed";
     return -1;