diff --git a/include/boost/http_proto/impl/serializer.hpp b/include/boost/http_proto/impl/serializer.hpp index 5fc5c8b4..ca1622c6 100644 --- a/include/boost/http_proto/impl/serializer.hpp +++ b/include/boost/http_proto/impl/serializer.hpp @@ -18,100 +18,96 @@ namespace boost { namespace http_proto { -class serializer::const_buf_gen_base +class serializer::cbs_gen { public: - // Return the next non-empty buffer, - // or an empty buffer if none remain. + struct stats_t + { + std::size_t size = 0; + std::size_t count = 0; + }; + + // Return the next non-empty buffer or an + // empty buffer if none remain. virtual buffers::const_buffer next() = 0; - // Size of remaining buffers - virtual - std::size_t - size() const = 0; - - // Count of remaining non-empty buffers + // Return the total size and count of + // remaining non-empty buffers. virtual - std::size_t - count() const = 0; + stats_t + stats() const = 0; - // Return true when there is no buffer or - // the remaining buffers are empty + // Return true if there are no remaining + // non-empty buffers. virtual bool is_empty() const = 0; }; template -class serializer::const_buf_gen - : public const_buf_gen_base +class serializer::cbs_gen_impl + : public cbs_gen { using it_t = decltype(buffers::begin( std::declval())); ConstBufferSequence cbs_; - it_t current_; + it_t curr_; public: using const_buffer = buffers::const_buffer; explicit - const_buf_gen(ConstBufferSequence cbs) + cbs_gen_impl(ConstBufferSequence cbs) : cbs_(std::move(cbs)) - , current_(buffers::begin(cbs_)) + , curr_(buffers::begin(cbs_)) { } const_buffer next() override { - while(current_ != buffers::end(cbs_)) + while(curr_ != buffers::end(cbs_)) { - const_buffer buf = *current_++; + // triggers conversion operator + const_buffer buf = *curr_++; if(buf.size() != 0) return buf; } return {}; } - std::size_t - size() const override - { - return std::accumulate( - current_, - buffers::end(cbs_), - std::size_t{}, - [](std::size_t sum, const_buffer cb) - { - return sum + cb.size(); - }); - } - - std::size_t - count() const override + stats_t + stats() const override { - return std::count_if( - current_, - buffers::end(cbs_), - [](const_buffer cb) + stats_t r; + for(auto it = curr_; it != buffers::end(cbs_); ++it) + { + // triggers conversion operator + const_buffer buf = *it; + if(buf.size() != 0) { - return cb.size() != 0; - }); + r.size += buf.size(); + r.count += 1; + } + } + return r; } bool is_empty() const override { - return std::all_of( - current_, - buffers::end(cbs_), - [](const_buffer cb) - { - return cb.size() == 0; - }); + for(auto it = curr_; it != buffers::end(cbs_); ++it) + { + // triggers conversion operator + const_buffer buf = *it; + if(buf.size() != 0) + return false; + } + return true; } }; @@ -131,8 +127,8 @@ start( "ConstBufferSequence type requirements not met"); start_init(m); - buf_gen_ = std::addressof( - ws_.emplace::type>>( std::forward(cbs))); start_buffers(m); diff --git a/include/boost/http_proto/serializer.hpp b/include/boost/http_proto/serializer.hpp index 876a21dd..6b4d3323 100644 --- a/include/boost/http_proto/serializer.hpp +++ b/include/boost/http_proto/serializer.hpp @@ -553,9 +553,9 @@ class serializer } private: - class const_buf_gen_base; + class cbs_gen; template - class const_buf_gen; + class cbs_gen_impl; detail::array_of_const_buffers make_array(std::size_t n); @@ -648,7 +648,7 @@ class serializer detail::workspace ws_; detail::filter* filter_ = nullptr; - const_buf_gen_base* buf_gen_ = nullptr; + cbs_gen* cbs_gen_ = nullptr; source* source_ = nullptr; buffers::circular_buffer out_; diff --git a/src/detail/impl/array_of_const_buffers.cpp b/src/detail/array_of_const_buffers.cpp similarity index 95% rename from src/detail/impl/array_of_const_buffers.cpp rename to src/detail/array_of_const_buffers.cpp index b719d26b..b10c0368 100644 --- a/src/detail/impl/array_of_const_buffers.cpp +++ b/src/detail/array_of_const_buffers.cpp @@ -20,11 +20,11 @@ namespace detail { array_of_const_buffers:: array_of_const_buffers( value_type* p, - std::uint16_t n) noexcept + std::uint16_t capacity) noexcept : base_(p) - , cap_(n) + , cap_(capacity) , pos_(0) - , size_(n) + , size_(0) { } diff --git a/src/serializer.cpp b/src/serializer.cpp index c99f729d..435a0dfc 100644 --- a/src/serializer.cpp +++ b/src/serializer.cpp @@ -37,9 +37,17 @@ namespace http_proto { namespace { -constexpr -std::size_t -crlf_len = 2; +const +buffers::const_buffer +crlf_and_final_chunk = {"\r\n0\r\n\r\n", 7}; + +const +buffers::const_buffer +crlf = {"\r\n", 2}; + +const +buffers::const_buffer +final_chunk = {"0\r\n\r\n", 5}; constexpr std::uint8_t @@ -49,13 +57,9 @@ chunk_header_len( return static_cast( (core::bit_width(max_chunk_size) + 3) / 4 + - crlf_len); + 2); // crlf }; -constexpr -std::size_t -final_chunk_len = 1 + crlf_len + crlf_len; - void write_chunk_header( const buffers::mutable_buffer_pair& mbs, @@ -66,7 +70,7 @@ write_chunk_header( char buf[18]; auto p = buf + 16; auto const n = buffers::size(mbs); - for(std::size_t i = n - crlf_len; i--;) + for(std::size_t i = n - 2; i--;) { *--p = hexdig[size & 0xf]; size >>= 4; @@ -80,30 +84,6 @@ write_chunk_header( BOOST_ASSERT(copied == n); } -void -write_crlf( - const buffers::mutable_buffer_pair& mbs) noexcept -{ - auto n = buffers::copy( - mbs, - buffers::const_buffer( - "\r\n", 2)); - ignore_unused(n); - BOOST_ASSERT(n == 2); -} - -void -write_final_chunk( - const buffers::mutable_buffer_pair& mbs) noexcept -{ - auto n = buffers::copy( - mbs, - buffers::const_buffer( - "0\r\n\r\n", 5)); - ignore_unused(n); - BOOST_ASSERT(n == 5); -} - //------------------------------------------------ class zlib_filter @@ -237,6 +217,18 @@ class brotli_filter } }; +template +std::size_t +clamp( + UInt x, + std::size_t limit = (std::numeric_limits< + std::size_t>::max)()) noexcept +{ + if(x >= limit) + return limit; + return static_cast(x); +} + } // namespace namespace detail { @@ -301,7 +293,7 @@ serializer( , svc_(other.svc_) , ws_(std::move(other.ws_)) , filter_(other.filter_) - , buf_gen_(other.buf_gen_) + , cbs_gen_(other.cbs_gen_) , source_(other.source_) , out_(other.out_) , in_(other.in_) @@ -333,7 +325,7 @@ operator=( svc_ = other.svc_; ws_ = std::move(other.ws_); filter_ = other.filter_; - buf_gen_ = other.buf_gen_; + cbs_gen_ = other.cbs_gen_; source_ = other.source_; out_ = other.out_; in_ = other.in_; @@ -410,24 +402,27 @@ prepare() -> prepped_.slide_to_front(); while(prepped_.capacity() != 0) { - auto buf = buf_gen_->next(); - if(buf.size() != 0) - { - prepped_.append(buf); - } - else // buf_gen_ is empty + auto buf = cbs_gen_->next(); + if(buf.size() == 0) + break; + prepped_.append(buf); + } + if(cbs_gen_->is_empty()) + { + if(is_chunked_) { - // append crlf and final chunk - if(is_chunked_) + if(prepped_.capacity() != 0) { - prepped_.append(tmp_); + prepped_.append( + crlf_and_final_chunk); more_input_ = false; } - break; + } + else + { + more_input_ = false; } } - if(buf_gen_->is_empty() && !is_chunked_) - more_input_ = false; } return const_buffers_type( prepped_.begin(), @@ -506,8 +501,8 @@ prepare() -> { if(more_input_ && tmp_.size() == 0) { - tmp_ = buf_gen_->next(); - if(tmp_.size() == 0) // buf_gen_ is empty + tmp_ = cbs_gen_->next(); + if(tmp_.size() == 0) // cbs_gen_ is empty more_input_ = false; } @@ -684,8 +679,7 @@ detail::array_of_const_buffers serializer:: make_array(std::size_t n) { - if(n > std::numeric_limits::max()) - detail::throw_length_error(); + BOOST_ASSERT(n <= std::uint16_t(-1)); return { ws_.push_array(n, @@ -702,8 +696,8 @@ start_init( if(state_ != state::start) detail::throw_logic_error(); - // TODO: to hold strong exception guarantee - // `state_` should be set to state::start if an + // TODO: To uphold the strong exception guarantee, + // `state_` must be reset to `state::start` if an // exception is thrown during the start operation. state_ = state::header; @@ -779,7 +773,7 @@ start_empty( if(!filter_) out_finish(); - prepped_[0] = { m.ph_->cbuf, m.ph_->size }; + prepped_.append({ m.ph_->cbuf, m.ph_->size }); more_input_ = false; } @@ -794,107 +788,32 @@ start_buffers( // start_init() already called style_ = style::buffers; - const auto buffers_max = (std::min)( - std::size_t{ 16 }, - buf_gen_->count()); - if(!filter_) { - if(!is_chunked_) - { - // no filter and no chunked - - prepped_ = make_array( - 1 + // header - buffers_max ); // buffers - - prepped_[0] = { m.ph_->cbuf, m.ph_->size }; - std::generate( - prepped_.begin() + 1, - prepped_.end(), - [this](){ return buf_gen_->next(); }); - more_input_ = !buf_gen_->is_empty(); - return; - } - - // no filter and chunked - - if(buf_gen_->is_empty()) - { - prepped_ = make_array( - 1 + // header - 1); // final chunk - - mutable_buffer final_chunk = { - ws_.reserve_front( - final_chunk_len), - final_chunk_len }; - write_final_chunk( - { final_chunk, {} }); - - prepped_[0] = { m.ph_->cbuf, m.ph_->size }; - prepped_[1] = final_chunk; - more_input_ = false; - return; - } - - // Write entire buffers as a single chunk - // since total size is known - - auto const buf_size = buf_gen_->size(); - auto const header_len = - chunk_header_len(buf_size); - - mutable_buffer chunk_header = { - ws_.reserve_front(header_len), - header_len }; - - write_chunk_header({ chunk_header, {} }, buf_size); - - mutable_buffer crlf_and_final_chunk = { - ws_.reserve_front( - crlf_len + final_chunk_len), - crlf_len + final_chunk_len }; - - write_crlf({ - buffers::prefix( - crlf_and_final_chunk, - crlf_len) - , {} }); - - write_final_chunk({ - buffers::sans_prefix( - crlf_and_final_chunk, - crlf_len) - , {}}); + auto stats = cbs_gen_->stats(); + auto batch_size = clamp(stats.count, 16); prepped_ = make_array( 1 + // header - 1 + // chunk header - buffers_max + // buffers - 1); // buffer or (crlf and final chunk) - - prepped_[0] = { m.ph_->cbuf, m.ph_->size }; - prepped_[1] = chunk_header; - std::generate( - prepped_.begin() + 2, - prepped_.end() - 1, - [this](){ return buf_gen_->next(); }); - - more_input_ = !buf_gen_->is_empty(); - // assigning the last slot - if(more_input_) - { - prepped_[prepped_.size() - 1] = - buf_gen_->next(); + batch_size + // buffers + (is_chunked_ ? 2 : 0)); // chunk header + final chunk - // deferred until buf_gen_ is drained - tmp_ = crlf_and_final_chunk; - } - else + prepped_.append({ m.ph_->cbuf, m.ph_->size }); + more_input_ = (batch_size != 0); + + if(is_chunked_) { - prepped_[prepped_.size() - 1] = - crlf_and_final_chunk; + if(!more_input_) + { + prepped_.append(final_chunk); + } + else + { + auto h_len = chunk_header_len(stats.size); + mutable_buffer mb(ws_.reserve_front(h_len), h_len); + write_chunk_header({ mb, {} }, stats.size); + prepped_.append(mb); + } } return; } @@ -907,7 +826,7 @@ start_buffers( out_init(); - prepped_[0] = { m.ph_->cbuf, m.ph_->size }; + prepped_.append({ m.ph_->cbuf, m.ph_->size }); tmp_ = {}; more_input_ = true; } @@ -933,7 +852,7 @@ start_source( out_init(); - prepped_[0] = { m.ph_->cbuf, m.ph_->size }; + prepped_.append({ m.ph_->cbuf, m.ph_->size }); more_input_ = true; } @@ -959,7 +878,7 @@ start_stream( out_init(); - prepped_[0] = { m.ph_->cbuf, m.ph_->size }; + prepped_.append({ m.ph_->cbuf, m.ph_->size }); more_input_ = true; return stream{ *this }; } @@ -994,7 +913,7 @@ out_prepare() noexcept buffers::sans_prefix( out_.prepare(out_.capacity()), chunk_header_len_), - crlf_len + final_chunk_len ); + crlf_and_final_chunk.size()); } return out_.prepare(out_.capacity()); } @@ -1015,8 +934,8 @@ out_commit( out_.prepare(n); out_.commit(n); - write_crlf(out_.prepare(crlf_len)); - out_.commit(crlf_len); + buffers::copy(out_.prepare(crlf.size()), crlf); + out_.commit(crlf.size()); } else { @@ -1030,10 +949,8 @@ out_capacity() const noexcept { if(is_chunked_) { - auto const overhead = - chunk_header_len_ + - crlf_len + - final_chunk_len; + auto const overhead = chunk_header_len_ + + crlf_and_final_chunk.size(); if(out_.capacity() < overhead) return 0; return out_.capacity() - overhead; @@ -1047,9 +964,9 @@ out_finish() noexcept { if(is_chunked_) { - write_final_chunk( - out_.prepare(final_chunk_len)); - out_.commit(final_chunk_len); + buffers::copy( + out_.prepare(final_chunk.size()), final_chunk); + out_.commit(final_chunk.size()); } } diff --git a/test/unit/serializer.cpp b/test/unit/serializer.cpp index bb931bcc..3cf9a96c 100644 --- a/test/unit/serializer.cpp +++ b/test/unit/serializer.cpp @@ -535,6 +535,20 @@ struct serializer_test std::string(2048, '*') + std::string("\r\n0\r\n\r\n")); + // buffers chunked empty + check_buffers( + "HTTP/1.1 200 OK\r\n" + "Server: test\r\n" + "Transfer-Encoding: chunked\r\n" + "\r\n", + "", + //-------------------------- + "HTTP/1.1 200 OK\r\n" + "Server: test\r\n" + "Transfer-Encoding: chunked\r\n" + "\r\n", + "0\r\n\r\n"); + // source check_src( "HTTP/1.1 200 OK\r\n"