src: Remove 0x00 concatenation for headers

Now concatenating header values with 0x00 as delimiter is not
necessary because HPACK reference set is removed and the order of
header field fed into HPACK encoder is preserved when they are
decoded.

src/HttpServer.cc

@@ -94,9 +94,9 @@ namespace {
 void append_nv(Stream *stream, const std::vector<nghttp2_nv>& nva)
 {
   for(auto& nv : nva) {
-    http2::split_add_header(stream->headers,
+    http2::add_header(stream->headers,
-                            nv.name, nv.namelen, nv.value, nv.valuelen,
+                      nv.name, nv.namelen, nv.value, nv.valuelen,
-                            nv.flags & NGHTTP2_NV_FLAG_NO_INDEX);
+                      nv.flags & NGHTTP2_NV_FLAG_NO_INDEX);
   }
 }
 } // namespace
@@ -1158,8 +1158,8 @@ int on_header_callback(nghttp2_session *session,
   if(!http2::check_nv(name, namelen, value, valuelen)) {
     return 0;
   }
-  http2::split_add_header(stream->headers, name, namelen, value, valuelen,
+  http2::add_header(stream->headers, name, namelen, value, valuelen,
-                          flags & NGHTTP2_NV_FLAG_NO_INDEX);
+                    flags & NGHTTP2_NV_FLAG_NO_INDEX);
   return 0;
 }
 } // namespace

src/http2.cc

@@ -275,31 +275,12 @@ Headers::value_type to_header(const uint8_t *name, size_t namelen,
                 no_index);
 }
 
-void split_add_header(Headers& nva,
+void add_header(Headers& nva,
-                      const uint8_t *name, size_t namelen,
+                const uint8_t *name, size_t namelen,
-                      const uint8_t *value, size_t valuelen,
+                const uint8_t *value, size_t valuelen,
-                      bool no_index)
+                bool no_index)
 {
-  if(valuelen == 0) {
+  nva.push_back(to_header(name, namelen, value, valuelen, no_index));
-    nva.push_back(to_header(name, namelen, value, valuelen, no_index));
-    return;
-  }
-  auto j = value;
-  auto end = value + valuelen;
-  for(;;) {
-    // Skip 0 length value
-    j = std::find_if(j, end,
-                     [](uint8_t c)
-                     {
-                       return c != '\0';
-                     });
-    if(j == end) {
-      break;
-    }
-    auto l = std::find(j, end, '\0');
-    nva.push_back(to_header(name, namelen, j, l-j, no_index));
-    j = l;
-  }
 }
 
 const Headers::value_type* get_unique_header(const Headers& nva,
@@ -355,29 +336,6 @@ nghttp2_nv make_nv(const std::string& name, const std::string& value,
       name.size(), value.size(), flags};
 }
 
-Headers concat_norm_headers(Headers headers)
-{
-  auto res = Headers();
-  res.reserve(headers.size());
-  for(auto& kv : headers) {
-    if(!res.empty() && res.back().name == kv.name &&
-       kv.name != "cookie" && kv.name != "set-cookie") {
-
-      auto& last = res.back();
-
-      if(!kv.value.empty()) {
-        last.value.append(1, '\0');
-        last.value += kv.value;
-      }
-      // We do ORing nv flags.  This is done even if value is empty.
-      last.no_index |= kv.no_index;
-    } else {
-      res.push_back(std::move(kv));
-    }
-  }
-  return res;
-}
-
 void copy_norm_headers_to_nva
 (std::vector<nghttp2_nv>& nva, const Headers& headers)
 {

src/http2.h

@@ -104,15 +104,13 @@ Headers::value_type to_header(const uint8_t *name, size_t namelen,
                               const uint8_t *value, size_t valuelen,
                               bool no_index);
 
-// Add name/value pairs to |nva|. The name is given in the |name| with
+// Add name/value pairs to |nva|.  If |no_index| is true, this
-// |namelen| bytes. This function inspects the |value| and split it
+// name/value pair won't be indexed when it is forwarded to the next
-// using '\0' as delimiter. Each token is added to the |nva| with the
+// hop.
-// name |name|.  If |no_index| is true, this name/value pair won't be
+void add_header(Headers& nva,
-// indexed when it is forwarded to the next hop.
+                const uint8_t *name, size_t namelen,
-void split_add_header(Headers& nva,
+                const uint8_t *value, size_t valuelen,
-                      const uint8_t *name, size_t namelen,
+                bool no_index);
-                      const uint8_t *value, size_t valuelen,
-                      bool no_index);
 
 // Returns sorted |nva| with |nvlen| elements. The headers are sorted
 // by name only and not necessarily stable. In addition to the
@@ -143,12 +141,6 @@ bool value_lws(const Headers::value_type *nv);
 // and not contain illegal characters.
 bool non_empty_value(const Headers::value_type *nv);
 
-// Concatenates field with same value by NULL as delimiter and returns
-// new vector containing the resulting header fields. cookie and
-// set-cookie header fields won't be concatenated. This function
-// assumes that the |headers| is sorted by name.
-Headers concat_norm_headers(Headers headers);
-
 // 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().  If |no_index| is true, nghttp2_nv flags member has

src/http2_test.cc

@@ -72,27 +72,19 @@ void test_http2_sort_nva(void)
   check_nv({"delta", "5"}, &nva[5]);
 }
 
-void test_http2_split_add_header(void)
+void test_http2_add_header(void)
 {
-  const uint8_t concatval[] = { '4', 0x00, 0x00, '6', 0x00, '5', '9', 0x00 };
   auto nva = Headers();
-  http2::split_add_header(nva, (const uint8_t*)"delta", 5,
-                          concatval, sizeof(concatval), false);
-  CU_ASSERT(Headers::value_type("delta", "4") == nva[0]);
-  CU_ASSERT(Headers::value_type("delta", "6") == nva[1]);
-  CU_ASSERT(Headers::value_type("delta", "59") == nva[2]);
 
-  nva.clear();
+  http2::add_header(nva, (const uint8_t*)"alpha", 5,
-
+                    (const uint8_t*)"123", 3, false);
-  http2::split_add_header(nva, (const uint8_t*)"alpha", 5,
-                          (const uint8_t*)"123", 3, false);
   CU_ASSERT(Headers::value_type("alpha", "123") == nva[0]);
   CU_ASSERT(!nva[0].no_index);
 
   nva.clear();
 
-  http2::split_add_header(nva, (const uint8_t*)"alpha", 5,
+  http2::add_header(nva, (const uint8_t*)"alpha", 5,
-                          (const uint8_t*)"", 0, true);
+                    (const uint8_t*)"", 0, true);
   CU_ASSERT(Headers::value_type("alpha", "") == nva[0]);
   CU_ASSERT(nva[0].no_index);
 }
@@ -199,18 +191,6 @@ auto headers = Headers
    {"zulu", "12"}};
 } // namespace
 
-void test_http2_concat_norm_headers(void)
-{
-  auto hds = headers;
-  hds.emplace_back("cookie", "foo");
-  hds.emplace_back("cookie", "bar");
-  hds.emplace_back("set-cookie", "baz");
-  hds.emplace_back("set-cookie", "buzz");
-  auto res = http2::concat_norm_headers(hds);
-  CU_ASSERT(14 == res.size());
-  CU_ASSERT(std::string("2") + '\0' + std::string("3") == res[2].value);
-}
-
 void test_http2_copy_norm_headers_to_nva(void)
 {
   std::vector<nghttp2_nv> nva;

src/http2_test.h

@@ -27,13 +27,12 @@
 
 namespace shrpx {
 
-void test_http2_split_add_header(void);
+void test_http2_add_header(void);
 void test_http2_sort_nva(void);
 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_concat_norm_headers(void);
 void test_http2_copy_norm_headers_to_nva(void);
 void test_http2_build_http1_headers_from_norm_headers(void);
 void test_http2_lws(void);

src/nghttp.cc

@@ -1048,8 +1048,6 @@ int submit_request
                      return lhs.name < rhs.name;
                    });
 
-  build_headers = http2::concat_norm_headers(std::move(build_headers));
-
   auto nva = std::vector<nghttp2_nv>();
   nva.reserve(build_headers.size());
 
@@ -1264,8 +1262,8 @@ int on_header_callback(nghttp2_session *session,
     if(!req) {
       break;
     }
-    http2::split_add_header(req->res_nva, name, namelen, value, valuelen,
+    http2::add_header(req->res_nva, name, namelen, value, valuelen,
-                            flags & NGHTTP2_NV_FLAG_NO_INDEX);
+                      flags & NGHTTP2_NV_FLAG_NO_INDEX);
     break;
   }
   case NGHTTP2_PUSH_PROMISE: {
@@ -1274,8 +1272,8 @@ int on_header_callback(nghttp2_session *session,
     if(!req) {
       break;
     }
-    http2::split_add_header(req->push_req_nva, name, namelen, value, valuelen,
+    http2::add_header(req->push_req_nva, name, namelen, value, valuelen,
-                            flags & NGHTTP2_NV_FLAG_NO_INDEX);
+                      flags & NGHTTP2_NV_FLAG_NO_INDEX);
     break;
   }
   }

src/shrpx-unittest.cc

@@ -71,8 +71,7 @@ int main(int argc, char* argv[])
                    shrpx::test_shrpx_ssl_create_lookup_tree) ||
       !CU_add_test(pSuite, "ssl_cert_lookup_tree_add_cert_from_file",
                    shrpx::test_shrpx_ssl_cert_lookup_tree_add_cert_from_file) ||
-      !CU_add_test(pSuite, "http2_split_add_header",
+      !CU_add_test(pSuite, "http2_add_header", shrpx::test_http2_add_header) ||
-                   shrpx::test_http2_split_add_header) ||
       !CU_add_test(pSuite, "http2_sort_nva", shrpx::test_http2_sort_nva) ||
       !CU_add_test(pSuite, "http2_check_http2_headers",
                    shrpx::test_http2_check_http2_headers) ||
@@ -82,8 +81,6 @@ 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_concat_norm_headers",
-                   shrpx::test_http2_concat_norm_headers) ||
       !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",

src/shrpx_downstream.cc

@@ -230,11 +230,6 @@ Headers::const_iterator Downstream::get_norm_request_header
   return get_norm_header(request_headers_, name);
 }
 
-void Downstream::concat_norm_request_headers()
-{
-  request_headers_ = http2::concat_norm_headers(std::move(request_headers_));
-}
-
 void Downstream::add_request_header(std::string name, std::string value)
 {
   request_header_key_prev_ = true;
@@ -256,8 +251,8 @@ void Downstream::split_add_request_header
  bool no_index)
 {
   request_headers_sum_ += namelen + valuelen;
-  http2::split_add_header(request_headers_, name, namelen, value, valuelen,
+  http2::add_header(request_headers_, name, namelen, value, valuelen,
-                          no_index);
+                    no_index);
 }
 
 bool Downstream::get_request_header_key_prev() const
@@ -485,11 +480,6 @@ void Downstream::normalize_response_headers()
   http2::normalize_headers(response_headers_);
 }
 
-void Downstream::concat_norm_response_headers()
-{
-  response_headers_ = http2::concat_norm_headers(std::move(response_headers_));
-}
-
 Headers::const_iterator Downstream::get_norm_response_header
 (const std::string& name) const
 {
@@ -551,8 +541,8 @@ void Downstream::split_add_response_header
  bool no_index)
 {
   response_headers_sum_ += namelen + valuelen;
-  http2::split_add_header(response_headers_, name, namelen, value, valuelen,
+  http2::add_header(response_headers_, name, namelen, value, valuelen,
-                          no_index);
+                    no_index);
 }
 
 bool Downstream::get_response_header_key_prev() const

src/shrpx_downstream.h

@@ -99,10 +99,6 @@ public:
   // called after calling normalize_request_headers().
   Headers::const_iterator get_norm_request_header
   (const std::string& name) const;
-  // Concatenates request header fields with same name by NULL as
-  // delimiter. See http2::concat_norm_headers(). This function must
-  // be called after calling normalize_request_headers().
-  void concat_norm_request_headers();
   void add_request_header(std::string name, std::string value);
   void set_last_request_header_value(std::string value);
 
@@ -162,10 +158,6 @@ public:
   const Headers& get_response_headers() const;
   // Makes key lowercase and sort headers by name using <
   void normalize_response_headers();
-  // Concatenates response header fields with same name by NULL as
-  // delimiter. See http2::concat_norm_headers(). This function must
-  // be called after calling normalize_response_headers().
-  void concat_norm_response_headers();
   // Returns iterator pointing to the response header with the name
   // |name|. If multiple header have |name| as name, return first
   // occurrence from the beginning. If no such header is found,

src/shrpx_http2_downstream_connection.cc

@@ -244,7 +244,7 @@ int Http2DownstreamConnection::push_request_headers()
     downstream_->crumble_request_cookie();
   }
   downstream_->normalize_request_headers();
-  downstream_->concat_norm_request_headers();
+
   auto end_headers = std::end(downstream_->get_request_headers());
 
   // 7 means:

src/shrpx_http2_upstream.cc

@@ -1161,7 +1161,7 @@ int Http2Upstream::on_downstream_header_complete(Downstream *downstream)
     downstream->rewrite_norm_location_response_header
       (get_client_handler()->get_upstream_scheme(), get_config()->port);
   }
-  downstream->concat_norm_response_headers();
+
   auto end_headers = std::end(downstream->get_response_headers());
   size_t nheader = downstream->get_response_headers().size();
   auto nva = std::vector<nghttp2_nv>();