nghttpx: Realloc header buffer

src/allocator.h

@@ -29,6 +29,8 @@
 
 #include <sys/uio.h>
 
+#include <cassert>
+
 #include "template.h"
 
 namespace nghttp2 {
@@ -45,14 +47,18 @@ struct MemBlock {
 
 // BlockAllocator allocates memory block with given size at once, and
 // cuts the region from it when allocation is requested.  If the
-// requested size is larger than given threshold, it will be allocated
+// requested size is larger than given threshold (plus small internal
-// in a distinct buffer on demand.
+// overhead), it will be allocated in a distinct buffer on demand.
+// The |isolation_threshold| must be less than or equal to
+// |block_size|.
 struct BlockAllocator {
   BlockAllocator(size_t block_size, size_t isolation_threshold)
       : retain(nullptr),
         head(nullptr),
         block_size(block_size),
-        isolation_threshold(std::min(block_size, isolation_threshold)) {}
+        isolation_threshold(std::min(block_size, isolation_threshold)) {
+    assert(isolation_threshold <= block_size);
+  }
 
   ~BlockAllocator() { reset(); }
 
@@ -105,20 +111,60 @@ struct BlockAllocator {
   }
 
   void *alloc(size_t size) {
-    if (size >= isolation_threshold) {
+    if (size + sizeof(size_t) >= isolation_threshold) {
-      auto mb = alloc_mem_block(size);
+      auto len = std::max(static_cast<size_t>(16), size);
+      // We will store the allocated size in size_t field.
+      auto mb = alloc_mem_block(len + sizeof(size_t));
+      auto sp = reinterpret_cast<size_t *>(mb->begin);
+      *sp = len;
       mb->last = mb->end;
-      return mb->begin;
+      return mb->begin + sizeof(size_t);
     }
 
-    if (!head || head->end - head->last < static_cast<ssize_t>(size)) {
+    if (!head ||
+        head->end - head->last < static_cast<ssize_t>(size + sizeof(size_t))) {
       head = alloc_mem_block(block_size);
     }
 
-    auto res = head->last;
+    // We will store the allocated size in size_t field.
+    auto res = head->last + sizeof(size_t);
+    auto sp = reinterpret_cast<size_t *>(head->last);
+    *sp = size;
 
     head->last = reinterpret_cast<uint8_t *>(
-        (reinterpret_cast<intptr_t>(head->last + size) + 0xf) & ~0xf);
+        (reinterpret_cast<intptr_t>(res + size) + 0xf) & ~0xf);
+
+    return res;
+  }
+
+  // Returns allocated size for memory pointed by |ptr|.  We assume
+  // that |ptr| was returned from alloc() or realloc().
+  size_t get_alloc_length(void *ptr) {
+    return *reinterpret_cast<size_t *>(static_cast<uint8_t *>(ptr) -
+                                       sizeof(size_t));
+  }
+
+  // Allocates memory of at least |size| bytes.  If |ptr| is nullptr,
+  // this is equivalent to alloc(size).  If |ptr| is not nullptr,
+  // obtain the allocated size for |ptr|, assuming that |ptr| was
+  // returned from alloc() or realloc().  If the allocated size is
+  // greater than or equal to size, |ptr| is returned.  Otherwise,
+  // allocates at least |size| bytes of memory, and the original
+  // content pointed by |ptr| is copied to the newly allocated memory.
+  void *realloc(void *ptr, size_t size) {
+    if (!ptr) {
+      return alloc(size);
+    }
+    auto alloclen = get_alloc_length(ptr);
+    auto p = reinterpret_cast<uint8_t *>(ptr);
+    if (size <= alloclen) {
+      return ptr;
+    }
+
+    auto nalloclen = std::max(size + 1, alloclen * 2);
+
+    auto res = alloc(nalloclen);
+    std::copy_n(p, alloclen, static_cast<uint8_t *>(res));
 
     return res;
   }
@@ -186,6 +232,30 @@ StringRef concat_string_ref(BlockAllocator &alloc, Args &&... args) {
   return StringRef{dst, len};
 }
 
+// Returns the string which is the concatenation of |value| and |args|
+// in the given order.  The resulting string will be NULL-terminated.
+// This function assumes that the pointer value value.c_str() was
+// obtained from alloc.alloc() or alloc.realloc(), and attempts to use
+// unused memory region by using alloc.realloc().  If value is empty,
+// then just call concat_string_ref().
+template <typename BlockAllocator, typename... Args>
+StringRef realloc_concat_string_ref(BlockAllocator &alloc,
+                                    const StringRef &value, Args &&... args) {
+  if (value.empty()) {
+    return concat_string_ref(alloc, std::forward<Args>(args)...);
+  }
+
+  auto len =
+      value.size() + concat_string_ref_count(0, std::forward<Args>(args)...);
+  auto dst = static_cast<uint8_t *>(
+      alloc.realloc(const_cast<uint8_t *>(value.byte()), len + 1));
+  auto p = dst + value.size();
+  p = concat_string_ref_copy(p, std::forward<Args>(args)...);
+  *p = '\0';
+
+  return StringRef{dst, len};
+}
+
 struct ByteRef {
   // The pointer to the beginning of the buffer.
   uint8_t *base;

src/shrpx_downstream.cc

@@ -359,21 +359,31 @@ void add_header(bool &key_prev, size_t &sum, HeaderRefs &headers,
 }
 } // namespace
 
+namespace {
+StringRef alloc_header_name(BlockAllocator &balloc, const StringRef &name) {
+  auto iov = make_byte_ref(balloc, name.size() + 1);
+  auto p = iov.base;
+  p = std::copy(std::begin(name), std::end(name), p);
+  util::inp_strlower(iov.base, p);
+  *p = '\0';
+
+  return StringRef{iov.base, p};
+}
+} // namespace
+
 namespace {
 void append_last_header_key(BlockAllocator &balloc, bool &key_prev, size_t &sum,
                             HeaderRefs &headers, const char *data, size_t len) {
   assert(key_prev);
   sum += len;
   auto &item = headers.back();
-  auto iov = make_byte_ref(balloc, item.name.size() + len + 1);
+  auto name =
-  auto p = iov.base;
+      realloc_concat_string_ref(balloc, item.name, StringRef{data, len});
-  p = std::copy(std::begin(item.name), std::end(item.name), p);
-  p = std::copy_n(data, len, p);
-  util::inp_strlower(p - len, p);
-  *p = '\0';
 
-  item.name = StringRef{iov.base, p};
+  auto p = const_cast<uint8_t *>(name.byte());
+  util::inp_strlower(p + name.size() - len, p + name.size());
 
+  item.name = name;
   item.token = http2::lookup_token(item.name);
 }
 } // namespace
@@ -385,7 +395,8 @@ void append_last_header_value(BlockAllocator &balloc, bool &key_prev,
   key_prev = false;
   sum += len;
   auto &item = headers.back();
-  item.value = concat_string_ref(balloc, item.value, StringRef{data, len});
+  item.value =
+      realloc_concat_string_ref(balloc, item.value, StringRef{data, len});
 }
 } // namespace
 
@@ -439,6 +450,12 @@ void FieldStore::add_header_token(const StringRef &name, const StringRef &value,
                     no_index, token);
 }
 
+void FieldStore::alloc_add_header_name(const StringRef &name) {
+  auto name_ref = alloc_header_name(balloc_, name);
+  auto token = http2::lookup_token(name_ref);
+  add_header_token(name_ref, StringRef{}, false, token);
+}
+
 void FieldStore::append_last_header_key(const char *data, size_t len) {
   shrpx::append_last_header_key(balloc_, header_key_prev_, buffer_size_,
                                 headers_, data, len);
@@ -460,6 +477,12 @@ void FieldStore::add_trailer_token(const StringRef &name,
                     no_index, token);
 }
 
+void FieldStore::alloc_add_trailer_name(const StringRef &name) {
+  auto name_ref = alloc_header_name(balloc_, name);
+  auto token = http2::lookup_token(name_ref);
+  add_trailer_token(name_ref, StringRef{}, false, token);
+}
+
 void FieldStore::append_last_trailer_key(const char *data, size_t len) {
   shrpx::append_last_header_key(balloc_, trailer_key_prev_, buffer_size_,
                                 trailers_, data, len);

src/shrpx_downstream.h

@@ -86,6 +86,11 @@ public:
   void add_header_token(const StringRef &name, const StringRef &value,
                         bool no_index, int32_t token);
 
+  // Adds header field name |name|.  First, the copy of header field
+  // name pointed by name.c_str() of length name.size() is made, and
+  // stored.
+  void alloc_add_header_name(const StringRef &name);
+
   void append_last_header_key(const char *data, size_t len);
   void append_last_header_value(const char *data, size_t len);
 
@@ -101,6 +106,11 @@ public:
   void add_trailer_token(const StringRef &name, const StringRef &value,
                          bool no_index, int32_t token);
 
+  // Adds trailer field name |name|.  First, the copy of trailer field
+  // name pointed by name.c_str() of length name.size() is made, and
+  // stored.
+  void alloc_add_trailer_name(const StringRef &name);
+
   void append_last_trailer_key(const char *data, size_t len);
   void append_last_trailer_value(const char *data, size_t len);
 

src/shrpx_downstream_test.cc

@@ -33,26 +33,35 @@
 namespace shrpx {
 
 void test_downstream_field_store_append_last_header(void) {
-  BlockAllocator balloc(4096, 4096);
+  BlockAllocator balloc(16, 16);
   FieldStore fs(balloc, 0);
-  fs.add_header_token(StringRef::from_lit("alpha"), StringRef{}, false, -1);
+  fs.alloc_add_header_name(StringRef::from_lit("alpha"));
   auto bravo = StringRef::from_lit("BRAVO");
   fs.append_last_header_key(bravo.c_str(), bravo.size());
+  // Add more characters so that relloc occurs
+  auto golf = StringRef::from_lit("golF0123456789");
+  fs.append_last_header_key(golf.c_str(), golf.size());
+
   auto charlie = StringRef::from_lit("Charlie");
   fs.append_last_header_value(charlie.c_str(), charlie.size());
   auto delta = StringRef::from_lit("deltA");
   fs.append_last_header_value(delta.c_str(), delta.size());
+  // Add more characters so that relloc occurs
+  auto echo = StringRef::from_lit("echo0123456789");
+  fs.append_last_header_value(echo.c_str(), echo.size());
+
   fs.add_header_token(StringRef::from_lit("echo"),
                       StringRef::from_lit("foxtrot"), false, -1);
 
-  auto ans = HeaderRefs{
+  auto ans =
-      {StringRef::from_lit("alphabravo"), StringRef::from_lit("CharliedeltA")},
+      HeaderRefs{{StringRef::from_lit("alphabravogolf0123456789"),
-      {StringRef::from_lit("echo"), StringRef::from_lit("foxtrot")}};
+                  StringRef::from_lit("CharliedeltAecho0123456789")},
+                 {StringRef::from_lit("echo"), StringRef::from_lit("foxtrot")}};
   CU_ASSERT(ans == fs.headers());
 }
 
 void test_downstream_field_store_header(void) {
-  BlockAllocator balloc(4096, 4096);
+  BlockAllocator balloc(16, 16);
   FieldStore fs(balloc, 0);
   fs.add_header_token(StringRef::from_lit("alpha"), StringRef::from_lit("0"),
                       false, -1);

src/shrpx_https_upstream.cc

@@ -123,7 +123,6 @@ int htp_hdr_keycb(http_parser *htp, const char *data, size_t len) {
   auto downstream = upstream->get_downstream();
   auto &req = downstream->request();
   auto &httpconf = get_config()->http;
-  auto &balloc = downstream->get_block_allocator();
 
   if (req.fs.buffer_size() + len > httpconf.request_header_field_buffer) {
     if (LOG_ENABLED(INFO)) {
@@ -148,9 +147,7 @@ int htp_hdr_keycb(http_parser *htp, const char *data, size_t len) {
             Downstream::HTTP1_REQUEST_HEADER_TOO_LARGE);
         return -1;
       }
-      auto name = http2::copy_lower(balloc, StringRef{data, len});
+      req.fs.alloc_add_header_name(StringRef{data, len});
-      auto token = http2::lookup_token(name);
-      req.fs.add_header_token(name, StringRef{}, false, token);
     }
   } else {
     // trailer part
@@ -164,9 +161,7 @@ int htp_hdr_keycb(http_parser *htp, const char *data, size_t len) {
         }
         return -1;
       }
-      auto name = http2::copy_lower(balloc, StringRef{data, len});
+      req.fs.alloc_add_trailer_name(StringRef{data, len});
-      auto token = http2::lookup_token(name);
-      req.fs.add_trailer_token(name, StringRef{}, false, token);
     }
   }
   return 0;