src: Split NULL-separated values

src/HttpServer.cc

@@ -707,12 +707,12 @@ void prepare_response(Request *req, Http2Handler *hd)
 } // namespace
 
 namespace {
-void append_nv(Request *req, const std::vector<const nghttp2_nv*>& nva)
+void append_nv(Request *req, const std::vector<nghttp2_nv>& nva)
 {
-  for(auto nv : nva) {
+  for(auto& nv : nva) {
     req->headers.push_back(std::make_pair
-                           (std::string(nv->name, nv->name + nv->namelen),
-                            std::string(nv->value, nv->value + nv->valuelen)));
+                           (std::string(nv.name, nv.name + nv.namelen),
+                            std::string(nv.value, nv.value + nv.valuelen)));
   }
 }
 } // namespace

src/app_helper.cc

@@ -45,6 +45,7 @@
 
 #include "app_helper.h"
 #include "util.h"
+#include "http2.h"
 
 namespace nghttp2 {
 
@@ -163,13 +164,12 @@ const char* ansi_escend()
 
 void print_nv(nghttp2_nv *nva, size_t nvlen)
 {
-  size_t i;
-  for(i = 0; i < nvlen; ++i) {
+  for(auto& nv : http2::sort_nva(nva, nvlen)) {
     print_frame_attr_indent();
     printf("%s", ansi_esc("\033[1;34m"));
-    fwrite(nva[i].name, nva[i].namelen, 1, stdout);
+    fwrite(nv.name, nv.namelen, 1, stdout);
     printf("%s: ", ansi_escend());
-    fwrite(nva[i].value, nva[i].valuelen, 1, stdout);
+    fwrite(nv.value, nv.valuelen, 1, stdout);
     printf("\n");
   }
 }

src/http2.cc

@@ -196,69 +196,81 @@ size_t HTTP1_IGN_HDLEN = sizeof(HTTP1_IGN_HD)/sizeof(HTTP1_IGN_HD[0]);
 } // namespace
 
 namespace {
-auto nv_name_less = [](const nghttp2_nv *lhs, const nghttp2_nv *rhs)
+auto nv_name_less = [](const nghttp2_nv& lhs, const nghttp2_nv& rhs)
 {
-  return nghttp2_nv_compare_name(lhs, rhs) < 0;
+  return nghttp2_nv_compare_name(&lhs, &rhs) < 0;
 };
 } // namespace
 
-bool check_http2_headers(const std::vector<const nghttp2_nv*>& nva)
+bool check_http2_headers(const std::vector<nghttp2_nv>& nva)
 {
   for(size_t i = 0; i < DISALLOWED_HDLEN; ++i) {
     nghttp2_nv nv = {(uint8_t*)DISALLOWED_HD[i], nullptr,
                      (uint16_t)strlen(DISALLOWED_HD[i]), 0};
-    if(std::binary_search(std::begin(nva), std::end(nva), &nv, nv_name_less)) {
+    if(std::binary_search(std::begin(nva), std::end(nva), nv, nv_name_less)) {
       return false;
     }
   }
   return true;
 }
 
-std::vector<const nghttp2_nv*> sort_nva(const nghttp2_nv *nva, size_t nvlen)
+std::vector<nghttp2_nv> sort_nva(const nghttp2_nv *nva, size_t nvlen)
 {
-  auto res = std::vector<const nghttp2_nv*>();
+  auto v = std::vector<nghttp2_nv>(&nva[0], &nva[nvlen]);
+  std::sort(std::begin(v), std::end(v), nv_name_less);
+  auto res = std::vector<nghttp2_nv>();
   res.reserve(nvlen);
   for(size_t i = 0; i < nvlen; ++i) {
-    res.push_back(&nva[i]);
+    if(v[i].valuelen == 0) {
+      res.push_back(v[i]);
+      continue;
+    }
+    auto j = v[i].value;
+    auto end = v[i].value + v[i].valuelen;
+    for(;;) {
+      // Skip 0 length value
+      j = std::find_if_not(j, end,
+                           [](uint8_t c)
+                           {
+                             return c == '\0';
+                           });
+      if(j == end) {
+        break;
+      }
+      auto l = std::find(j, end, '\0');
+      res.push_back({v[i].name, j, v[i].namelen, static_cast<uint16_t>(l-j)});
+      j = l;
+    }
   }
-  std::sort(std::begin(res), std::end(res),
-            [](const nghttp2_nv *lhs, const nghttp2_nv *rhs)
-            {
-              auto rv = nghttp2_nv_compare_name(lhs, rhs);
-              if(rv == 0) {
-                return lhs < rhs;
-              }
-              return rv < 0;
-            });
   return res;
 }
 
-const nghttp2_nv* get_unique_header(const std::vector<const nghttp2_nv*>& nva,
+const nghttp2_nv* get_unique_header(const std::vector<nghttp2_nv>& nva,
                                     const char *name)
 {
   size_t namelen = strlen(name);
   nghttp2_nv nv = {(uint8_t*)name, nullptr, (uint16_t)namelen, 0};
-  auto i = std::lower_bound(std::begin(nva), std::end(nva), &nv, nv_name_less);
-  if(i != std::end(nva) && util::streq((*i)->name, (*i)->namelen,
+  auto i = std::lower_bound(std::begin(nva), std::end(nva), nv, nv_name_less);
+  if(i != std::end(nva) && util::streq((*i).name, (*i).namelen,
                                        (const uint8_t*)name, namelen)) {
     auto j = i + 1;
-    if(j == std::end(nva) || !util::streq((*j)->name, (*j)->namelen,
+    if(j == std::end(nva) || !util::streq((*j).name, (*j).namelen,
                                           (const uint8_t*)name, namelen)) {
-      return *i;
+      return &(*i);
     }
   }
   return nullptr;
 }
 
-const nghttp2_nv* get_header(const std::vector<const nghttp2_nv*>& nva,
+const nghttp2_nv* get_header(const std::vector<nghttp2_nv>& nva,
                              const char *name)
 {
   size_t namelen = strlen(name);
   nghttp2_nv nv = {(uint8_t*)name, nullptr, (uint16_t)namelen, 0};
-  auto i = std::lower_bound(std::begin(nva), std::end(nva), &nv, nv_name_less);
-  if(i != std::end(nva) && util::streq((*i)->name, (*i)->namelen,
+  auto i = std::lower_bound(std::begin(nva), std::end(nva), nv, nv_name_less);
+  if(i != std::end(nva) && util::streq((*i).name, (*i).namelen,
                                        (const uint8_t*)name, namelen)) {
-    return *i;
+    return &(*i);
   }
   return nullptr;
 }

src/http2.h

@@ -80,22 +80,25 @@ bool check_http2_allowed_header(const char *name);
 // Checks that headers |nva| including |nvlen| entries do not contain
 // disallowed header fields in HTTP/2.0 spec. This function returns
 // true if |nva| does not contains such headers.
-bool check_http2_headers(const std::vector<const nghttp2_nv*>& nva);
+bool check_http2_headers(const std::vector<nghttp2_nv>& nva);
 
-// Returns sorted |nva| with |nvlen| elements. This sort is stable
-// sort.
-std::vector<const nghttp2_nv*> sort_nva(const nghttp2_nv *nva, size_t nvlen);
+// Returns sorted |nva| with |nvlen| elements. The headers are sorted
+// by name only and not necessarily stable. In addition to the
+// sorting, this function splits values concatenated with NULL. The
+// ordering of the concatenated values are preserved. The element of
+// the returned vector refers to the memory pointed by |nva|.
+std::vector<nghttp2_nv> sort_nva(const nghttp2_nv *nva, size_t nvlen);
 
 // Returns the pointer to the entry in |nva| which has name |name| and
 // the |name| is uinque in the |nva|. If no such entry exist, returns
 // nullptr.
-const nghttp2_nv* get_unique_header(const std::vector<const nghttp2_nv*>& nva,
+const nghttp2_nv* get_unique_header(const std::vector<nghttp2_nv>& nva,
                                     const char *name);
 
 // Returns the poiter to the entry in |nva| which has name |name|. If
 // more than one entries which have the name |name|, first occurrence
 // in |nva| is returned. If no such entry exist, returns nullptr.
-const nghttp2_nv* get_header(const std::vector<const nghttp2_nv*>& nva,
+const nghttp2_nv* get_header(const std::vector<nghttp2_nv>& nva,
                              const char *name);
 
 // Returns std::string version of nv->name with nv->namelen bytes.

src/http2_test.cc

@@ -24,6 +24,7 @@
  */
 #include "http2_test.h"
 
+#include <cassert>
 #include <cstring>
 #include <iostream>
 
@@ -35,39 +36,38 @@
 using namespace nghttp2;
 
 #define MAKE_NV(K, V) {(uint8_t*)K, (uint8_t*)V,        \
-      (uint16_t)strlen(K), (uint16_t)strlen(V)}
+      (uint16_t)(sizeof(K)-1), (uint16_t)(sizeof(V)-1)}
 
 namespace shrpx {
 
 namespace {
-template<typename cstr1, typename cstr2>
-int bstrcmp(cstr1 *a, cstr2 *b)
+void check_nv(const std::pair<std::string, std::string>& a,
+              const nghttp2_nv *b)
 {
-  return strcmp((const char*)a, (const char*)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_sort_nva(void)
 {
+  // Last 0 is stripped in MAKE_NV
+  const uint8_t concatval[] = { '4', 0x00, 0x00, '6', 0x00, '5', 0x00 };
   nghttp2_nv nv[] = {MAKE_NV("alpha", "1"),
-                     MAKE_NV("bravo", "9"),
-                     MAKE_NV("bravo", "8"),
-                     MAKE_NV("charlie", "5"),
-                     MAKE_NV("bravo", "3"),
-                     MAKE_NV("bravo", "4")};
+                     MAKE_NV("charlie", "3"),
+                     MAKE_NV("bravo", "2"),
+                     MAKE_NV("delta", concatval)};
   auto nvlen = sizeof(nv)/sizeof(nv[0]);
   auto nva = http2::sort_nva(nv, nvlen);
-  CU_ASSERT(nvlen == nva.size());
-  CU_ASSERT(0 == bstrcmp("alpha", nva[0]->name));
-  CU_ASSERT(0 == bstrcmp("bravo", nva[1]->name));
-  CU_ASSERT(0 == bstrcmp("9", nva[1]->value));
-  CU_ASSERT(0 == bstrcmp("bravo", nva[2]->name));
-  CU_ASSERT(0 == bstrcmp("8", nva[2]->value));
-  CU_ASSERT(0 == bstrcmp("bravo", nva[3]->name));
-  CU_ASSERT(0 == bstrcmp("3", nva[3]->value));
-  CU_ASSERT(0 == bstrcmp("bravo", nva[4]->name));
-  CU_ASSERT(0 == bstrcmp("4", nva[4]->value));
-  CU_ASSERT(0 == bstrcmp("charlie", nva[5]->name));
+  CU_ASSERT(6 == nva.size());
+  check_nv({"alpha", "1"}, &nva[0]);
+  check_nv({"bravo", "2"}, &nva[1]);
+  check_nv({"charlie", "3"}, &nva[2]);
+  check_nv({"delta", "4"}, &nva[3]);
+  check_nv({"delta", "6"}, &nva[4]);
+  check_nv({"delta", "5"}, &nva[5]);
 }
 
 void test_http2_check_http2_headers(void)
@@ -164,17 +164,6 @@ auto headers = std::vector<std::pair<std::string, std::string>>
    {"zulu", "12"}};
 } // namespace
 
-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<nghttp2_nv> nva;

src/nghttp.cc

@@ -68,6 +68,7 @@
 #include "HtmlParser.h"
 #include "util.h"
 #include "base64.h"
+#include "http2.h"
 
 #ifndef O_BINARY
 # define O_BINARY (0)
@@ -1125,14 +1126,14 @@ void check_response_header
     // Server-pushed stream does not have stream user data
     return;
   }
+  auto nva = http2::sort_nva(frame->headers.nva, frame->headers.nvlen);
   bool gzip = false;
-  for(size_t i = 0; i < frame->headers.nvlen; ++i) {
-    auto nv = &frame->headers.nva[i];
-    if(util::strieq("content-encoding", nv->name, nv->namelen)) {
-      gzip = util::strieq("gzip", nv->value, nv->valuelen) ||
-        util::strieq("deflate", nv->value, nv->valuelen);
-    } else if(util::strieq(":status", nv->name, nv->namelen)) {
-      req->status.assign(nv->value, nv->value + nv->valuelen);
+  for(auto& nv : nva) {
+    if(util::strieq("content-encoding", nv.name, nv.namelen)) {
+      gzip = util::strieq("gzip", nv.value, nv.valuelen) ||
+        util::strieq("deflate", nv.value, nv.valuelen);
+    } else if(util::strieq(":status", nv.name, nv.namelen)) {
+      req->status.assign(nv.value, nv.value + nv.valuelen);
     }
   }
   if(gzip) {

src/shrpx_http2_session.cc

@@ -832,10 +832,10 @@ int on_frame_recv_callback
       return 0;
     }
 
-    for(auto nv : nva) {
-      if(nv->namelen > 0 && nv->name[0] != ':') {
-        downstream->add_response_header(http2::name_to_str(nv),
-                                        http2::value_to_str(nv));
+    for(auto& nv : nva) {
+      if(nv.namelen > 0 && nv.name[0] != ':') {
+        downstream->add_response_header(http2::name_to_str(&nv),
+                                        http2::value_to_str(&nv));
       }
     }
 
@@ -879,11 +879,11 @@ int on_frame_recv_callback
 
     if(LOG_ENABLED(INFO)) {
       std::stringstream ss;
-      for(auto nv : nva) {
+      for(auto& nv : nva) {
         ss << TTY_HTTP_HD;
-        ss.write(reinterpret_cast<char*>(nv->name), nv->namelen);
+        ss.write(reinterpret_cast<char*>(nv.name), nv.namelen);
         ss << TTY_RST << ": ";
-        ss.write(reinterpret_cast<char*>(nv->value), nv->valuelen);
+        ss.write(reinterpret_cast<char*>(nv.value), nv.valuelen);
         ss << "\n";
       }
       SSLOG(INFO, http2session) << "HTTP response headers. stream_id="

src/shrpx_http2_upstream.cc

@@ -242,11 +242,11 @@ int on_frame_recv_callback
 
     if(LOG_ENABLED(INFO)) {
       std::stringstream ss;
-      for(auto nv : nva) {
+      for(auto& nv : nva) {
         ss << TTY_HTTP_HD;
-        ss.write(reinterpret_cast<char*>(nv->name), nv->namelen);
+        ss.write(reinterpret_cast<char*>(nv.name), nv.namelen);
         ss << TTY_RST << ": ";
-        ss.write(reinterpret_cast<char*>(nv->value), nv->valuelen);
+        ss.write(reinterpret_cast<char*>(nv.value), nv.valuelen);
         ss << "\n";
       }
       ULOG(INFO, upstream) << "HTTP request headers. stream_id="
@@ -256,7 +256,7 @@ int on_frame_recv_callback
 
     if(get_config()->http2_upstream_dump_request_header) {
       http2::dump_nv(get_config()->http2_upstream_dump_request_header,
-                     frame->headers.nva, frame->headers.nvlen);
+                     nva.data(), nva.size());
     }
 
     if(!http2::check_http2_headers(nva)) {
@@ -264,10 +264,10 @@ int on_frame_recv_callback
       return 0;
     }
 
-    for(auto nv : nva) {
-      if(nv->namelen > 0 && nv->name[0] != ':') {
-        downstream->add_request_header(http2::name_to_str(nv),
-                                       http2::value_to_str(nv));
+    for(auto& nv : nva) {
+      if(nv.namelen > 0 && nv.name[0] != ':') {
+        downstream->add_request_header(http2::name_to_str(&nv),
+                                       http2::value_to_str(&nv));
       }
     }