Remove the use of static_string from http::fields

The `temporary_buffer<A>` class template replaces the use of
`static_string` in `http::fields`, simplifying `set_chunked_impl` and
`set_keep_alive_impl`.

Signed-off-by: Damian Jarek <damian.jarek93@gmail.com>

CHANGELOG.md

@@ -1,6 +1,7 @@
 Version 259:
 
 * Reduce the number of instantiations of filter_token_list
+* Remove the use of `static_string` from `http::fields`
 
 --------------------------------------------------------------------------------
 

include/boost/beast/http/fields.hpp

@@ -62,8 +62,6 @@ class basic_fields
 
     friend class fields_test; // for `header`
 
-    static std::size_t constexpr max_static_buffer = 4096;
-
     struct element;
 
     using off_t = std::uint16_t;
@@ -72,7 +70,7 @@ public:
     /// The type of allocator used.
     using allocator_type = Allocator;
 
-    /// The type of element used to represent a field 
+    /// The type of element used to represent a field
     class value_type
     {
         friend class basic_fields;

include/boost/beast/http/impl/fields.hpp

@@ -12,7 +12,6 @@
 
 #include <boost/beast/core/buffers_cat.hpp>
 #include <boost/beast/core/string.hpp>
-#include <boost/beast/core/static_string.hpp>
 #include <boost/beast/core/detail/buffers_ref.hpp>
 #include <boost/beast/core/detail/clamp.hpp>
 #include <boost/beast/http/verb.hpp>
@@ -751,12 +750,99 @@ equal_range(string_view name) const ->
 
 namespace detail {
 
+struct temporary_buffer
+{
+    temporary_buffer() = default;
+
+    temporary_buffer(temporary_buffer const&) = delete;
+    temporary_buffer(temporary_buffer&&) = delete;
+
+    temporary_buffer& operator=(temporary_buffer const&) = delete;
+    temporary_buffer& operator=(temporary_buffer&&) = delete;
+
+    ~temporary_buffer() noexcept
+    {
+        deallocate(data_);
+    }
+
+    void append(string_view sv)
+    {
+        reserve(sv.size());
+        unsafe_append(sv);
+    }
+
+    void append(string_view sv1, string_view sv2)
+    {
+        reserve(sv1.size() + sv2.size());
+        unsafe_append(sv1);
+        unsafe_append(sv2);
+    }
+
+    string_view view() const noexcept
+    {
+        return {data_, size_};
+    }
+
+    std::size_t size() const noexcept
+    {
+        return size_;
+    }
+
+    std::size_t capacity() const noexcept
+    {
+        return capacity_;
+    }
+
+    bool empty() const noexcept
+    {
+        return size_ == 0;
+    }
+
+private:
+    void unsafe_append(string_view sv)
+    {
+        auto n = sv.size();
+        std::memcpy(&data_[size_], sv.data(), n);
+        size_ += n;
+    }
+
+    void reserve(std::size_t sv_size)
+    {
+        if (capacity_ - size_ < sv_size)
+        {
+            auto constexpr limit = (std::numeric_limits<std::size_t>::max)();
+            if (limit/2 < capacity_ || limit - size_ < sv_size)
+            {
+                BOOST_THROW_EXCEPTION(
+                    std::length_error{"temporary_buffer::append"});
+            }
+
+            auto const new_cap = (std::max)(size_ + sv_size, capacity_*2);
+            char* const p = new char[new_cap];
+            std::memcpy(p, data_, size_);
+            deallocate(boost::exchange(data_, p));
+            capacity_ = new_cap;
+        }
+    }
+
+    void deallocate(char* data) noexcept
+    {
+        if (data != buffer_)
+            delete[] data;
+    }
+
+    char buffer_[4096];
+    char* data_ = buffer_;
+    std::size_t capacity_ = sizeof(buffer_);
+    std::size_t size_ = 0;
+};
+
 // Filter a token list
 //
-template<class String, class Pred>
+template<class Pred>
 void
 filter_token_list(
-    String& s,
+    detail::temporary_buffer& s,
     string_view value,
     Pred&& pred)
 {
@@ -768,22 +854,21 @@ filter_token_list(
     while(pred(*it))
         if(++it == last)
             return;
-    s.append(it->data(), it->size());
+    s.append(*it);
     while(++it != last)
     {
         if(! pred(*it))
         {
-            s.append(", ");
+            s.append(", ", *it);
-            s.append(it->data(), it->size());
         }
     }
 }
 
 // Filter the last item in a token list
-template<class String, class Pred>
+template<class Pred>
 void
 filter_token_list_last(
-    String& s,
+    detail::temporary_buffer& s,
     string_view value,
     Pred&& pred)
 {
@@ -795,10 +880,10 @@ filter_token_list_last(
         if(next == te.end())
         {
             if(! pred(*it))
-                s.append(it->data(), it->size());
+                s.append(*it);
             return;
         }
-        s.append(it->data(), it->size());
+        s.append(*it);
         for(;;)
         {
             it = next;
@@ -807,34 +892,32 @@ filter_token_list_last(
             {
                 if(! pred(*it))
                 {
-                    s.append(", ");
+                    s.append(", ", *it);
-                    s.append(it->data(), it->size());
                 }
                 return;
             }
-            s.append(", ");
+            s.append(", ", *it);
-            s.append(it->data(), it->size());
         }
     }
 }
 
-template<class String>
+struct iequals_predicate
+{
+    bool operator()(string_view s)
+    {
+        return beast::iequals(s, sv1) || beast::iequals(s, sv2);
+    }
+
+    string_view sv1;
+    string_view sv2;
+};
+
+inline
 void
 keep_alive_impl(
-    String& s, string_view value,
+    detail::temporary_buffer& s, string_view value,
     unsigned version, bool keep_alive)
 {
-    struct iequals_predicate
-    {
-        bool operator()(string_view s)
-        {
-            return beast::iequals(s, sv1) || beast::iequals(s, sv2);
-        }
-
-        string_view sv1;
-        string_view sv2;
-    };
-
     if(version < 11)
     {
         if(keep_alive)
@@ -990,6 +1073,7 @@ void
 basic_fields<Allocator>::
 set_chunked_impl(bool value)
 {
+    detail::temporary_buffer buf;
     auto it = find(field::transfer_encoding);
     if(value)
     {
@@ -1011,78 +1095,21 @@ set_chunked_impl(bool value)
             }
             itt = next;
         }
-        static_string<max_static_buffer> buf;
+
-        if(! beast::detail::sum_exceeds(
+        buf.append(it->value(), ", chunked");
-            it->value().size(), 9u, buf.max_size()))
+        set(field::transfer_encoding, buf.view());
-        {
-            buf.append(it->value().data(), it->value().size());
-            buf.append(", chunked", 9);
-            set(field::transfer_encoding, buf);
-        }
-        else
-        {
-        #ifdef BOOST_BEAST_HTTP_NO_FIELDS_BASIC_STRING_ALLOCATOR
-            // Workaround for https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56437
-            std::string s;
-        #else
-            using A =
-                typename beast::detail::allocator_traits<
-                    Allocator>::template rebind_alloc<char>;
-            std::basic_string<
-                char,
-                std::char_traits<char>,
-                A> s{A{this->get()}};
-        #endif
-            s.reserve(it->value().size() + 9);
-            s.append(it->value().data(), it->value().size());
-            s.append(", chunked", 9);
-            set(field::transfer_encoding, s);
-        }
         return;
     }
     // filter "chunked"
     if(it == end())
         return;
-#ifndef BOOST_NO_EXCEPTIONS
+
-    try
+    detail::filter_token_list_last(buf, it->value(),
-    {
+        detail::iequals_predicate{"chunked"});
-        static_string<max_static_buffer> buf;
+    if(! buf.empty())
-        detail::filter_token_list_last(buf, it->value(),
+        set(field::transfer_encoding, buf.view());
-            [](string_view s)
+    else
-            {
+        erase(field::transfer_encoding);
-                return iequals(s, "chunked");
-            });
-        if(! buf.empty())
-            set(field::transfer_encoding, buf);
-        else
-            erase(field::transfer_encoding);
-    }
-    catch(std::length_error const&)
-#endif
-    {
-    #ifdef BOOST_BEAST_HTTP_NO_FIELDS_BASIC_STRING_ALLOCATOR
-        // Workaround for https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56437
-        std::string s;
-    #else
-        using A =
-            typename beast::detail::allocator_traits<
-                Allocator>::template rebind_alloc<char>;
-        std::basic_string<
-            char,
-            std::char_traits<char>,
-            A> s{A{this->get()}};
-    #endif
-        s.reserve(it->value().size());
-        detail::filter_token_list_last(s, it->value(),
-            [](string_view s)
-            {
-                return iequals(s, "chunked");
-            });
-        if(! s.empty())
-            set(field::transfer_encoding, s);
-        else
-            erase(field::transfer_encoding);
-    }
 }
 
 template<class Allocator>
@@ -1105,40 +1132,12 @@ set_keep_alive_impl(
 {
     // VFALCO What about Proxy-Connection ?
     auto const value = (*this)[field::connection];
-#ifndef BOOST_NO_EXCEPTIONS
+    detail::temporary_buffer buf;
-    try
+    detail::keep_alive_impl(buf, value, version, keep_alive);
-    {
+    if(buf.empty())
-        static_string<max_static_buffer> buf;
+        erase(field::connection);
-        detail::keep_alive_impl(
+    else
-            buf, value, version, keep_alive);
+        set(field::connection, buf.view());
-        if(buf.empty())
-            erase(field::connection);
-        else
-            set(field::connection, buf);
-    }
-    catch(std::length_error const&)
-#endif
-    {
-    #ifdef BOOST_BEAST_HTTP_NO_FIELDS_BASIC_STRING_ALLOCATOR
-        // Workaround for https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56437
-        std::string s;
-    #else
-        using A =
-            typename beast::detail::allocator_traits<
-                Allocator>::template rebind_alloc<char>;
-        std::basic_string<
-            char,
-            std::char_traits<char>,
-            A> s{A{this->get()}};
-    #endif
-        s.reserve(value.size());
-        detail::keep_alive_impl(
-            s, value, version, keep_alive);
-        if(s.empty())
-            erase(field::connection);
-        else
-            set(field::connection, s);
-    }
 }
 
 //------------------------------------------------------------------------------

test/beast/http/fields.cpp

@@ -24,6 +24,9 @@ namespace http {
 class fields_test : public beast::unit_test::suite
 {
 public:
+    static constexpr std::size_t max_static_buffer =
+        sizeof(http::detail::temporary_buffer);
+
     template<class T>
     class test_allocator
     {
@@ -685,8 +688,7 @@ public:
                     (! res.keep_alive() && ! v));
             };
 
-        BOOST_STATIC_ASSERT(fields::max_static_buffer == 4096);
+        std::string const big(max_static_buffer + 1, 'a');
-        std::string const big(4096 + 1, 'a');
 
         // HTTP/1.0
         res.version(10);
@@ -846,10 +848,10 @@ public:
 
         res.content_length(0);
         BEAST_EXPECT(res[field::content_length] == "0");
-        
+
         res.content_length(100);
         BEAST_EXPECT(res[field::content_length] == "100");
-        
+
         res.content_length(boost::none);
         BEAST_EXPECT(res.count(field::content_length) == 0);
 
@@ -857,12 +859,12 @@ public:
         res.content_length(0);
         BEAST_EXPECT(res[field::content_length] == "0");
         BEAST_EXPECT(res.count(field::transfer_encoding) == 0);
-        
+
         res.set(field::transfer_encoding, "chunked");
         res.content_length(100);
         BEAST_EXPECT(res[field::content_length] == "100");
         BEAST_EXPECT(res.count(field::transfer_encoding) == 0);
-        
+
         res.set(field::transfer_encoding, "chunked");
         res.content_length(boost::none);
         BEAST_EXPECT(res.count(field::content_length) == 0);
@@ -874,12 +876,12 @@ public:
             res.content_length(0);
             BEAST_EXPECT(res[field::content_length] == "0");
             BEAST_EXPECT(res[field::transfer_encoding] == s);
-        
+
             res.set(field::transfer_encoding, s);
             res.content_length(100);
             BEAST_EXPECT(res[field::content_length] == "100");
             BEAST_EXPECT(res[field::transfer_encoding] == s);
-        
+
             res.set(field::transfer_encoding, s);
             res.content_length(boost::none);
             BEAST_EXPECT(res.count(field::content_length) == 0);
@@ -889,12 +891,12 @@ public:
             res.content_length(0);
             BEAST_EXPECT(res[field::content_length] == "0");
             BEAST_EXPECT(res[field::transfer_encoding] == s);
-        
+
             res.set(field::transfer_encoding, s + ", chunked");
             res.content_length(100);
             BEAST_EXPECT(res[field::content_length] == "100");
             BEAST_EXPECT(res[field::transfer_encoding] == s);
-        
+
             res.set(field::transfer_encoding, s + ", chunked");
             res.content_length(boost::none);
             BEAST_EXPECT(res.count(field::content_length) == 0);
@@ -904,12 +906,12 @@ public:
             res.content_length(0);
             BEAST_EXPECT(res[field::content_length] == "0");
             BEAST_EXPECT(res[field::transfer_encoding] == "chunked, " + s);
-        
+
             res.set(field::transfer_encoding, "chunked, " + s);
             res.content_length(100);
             BEAST_EXPECT(res[field::content_length] == "100");
             BEAST_EXPECT(res[field::transfer_encoding] == "chunked, " + s);
-        
+
             res.set(field::transfer_encoding, "chunked, " + s);
             res.content_length(boost::none);
             BEAST_EXPECT(res.count(field::content_length) == 0);
@@ -918,8 +920,7 @@ public:
 
         check("foo");
 
-        BOOST_STATIC_ASSERT(fields::max_static_buffer == 4096);
+        std::string const big(max_static_buffer + 1, 'a');
-        std::string const big(4096 + 1, 'a');
 
         check(big);
     }