From 3b0a25ee85f97c88d1840b664c47e0fa0e90e2fb Mon Sep 17 00:00:00 2001 From: Mohammad Nejati Date: Sun, 6 Jul 2025 07:15:24 +0000 Subject: [PATCH 1/2] Refactor serializer::stream --- include/boost/http_proto/serializer.hpp | 134 ++++++++---------------- src/serializer.cpp | 110 +++++++++++-------- test/unit/serializer.cpp | 42 +++++--- 3 files changed, 139 insertions(+), 147 deletions(-) diff --git a/include/boost/http_proto/serializer.hpp b/include/boost/http_proto/serializer.hpp index 7cfed73a..b968430d 100644 --- a/include/boost/http_proto/serializer.hpp +++ b/include/boost/http_proto/serializer.hpp @@ -159,7 +159,7 @@ class serializer std::size_t max_type_erase = 1024; }; - struct stream; + class stream; /** Destructor */ @@ -401,124 +401,80 @@ install_serializer_service( //------------------------------------------------ -/** The type used for caller-provided body data during - serialization. - - @code{.cpp} - http_proto::serializer sr(128); - - http_proto::request req; - auto stream = sr.start_stream(req); - - std::string_view msg = "Hello, world!"; - auto n = buffers::copy( - stream.prepare(), - buffers::make_buffer( - msg.data(), msg.size())); - - stream.commit(n); - - auto cbs = sr.prepare().value(); - (void)cbs; - @endcode +/** Used for streaming body data during serialization. */ -struct serializer::stream +class serializer::stream { - /** Constructor. - - The only valid operations on default constructed - streams are assignment and destruction. +public: + /** The type used to represent a sequence of mutable buffers */ - stream() = default; + using mutable_buffers_type = + buffers::mutable_buffer_pair; - /** Constructor. + /** Constructor - The constructed stream will share the same - serializer as `other`. + A default-constructed stream is considered closed. */ - stream(stream const& other) = default; - - /** Assignment. + stream() = default; - The current stream will share the same serializer - as `other`. + /** Move constructor */ - stream& operator= ( - stream const& other) = default; - - /** A MutableBufferSequence consisting of a buffer pair. - */ - using buffers_type = - buffers::mutable_buffer_pair; - - /** Returns the remaining available capacity. + stream(stream&& other) + : sr_(other.sr_) + { + other.sr_ = nullptr; + } - The returned value represents the available free - space in the backing fixed-sized buffers used by the - serializer associated with this stream. + /** Move assignment + */ + stream& + operator=(stream&& other) + { + std::swap(sr_, other.sr_); + return *this; + } - The capacity is absolute and does not do any - accounting for any octets required by a chunked - transfer encoding. + /** Returns `true` if the stream is open */ BOOST_HTTP_PROTO_DECL - std::size_t - capacity() const noexcept; - - /** Return true if the stream cannot currently hold - additional output data. - - The fixed-sized buffers maintained by the associated - serializer can be sufficiently full from previous - calls to @ref stream::commit. + bool + is_open() const noexcept; - This function can be called to determine if the caller - should drain the serializer via @ref serializer::consume calls - before attempting to fill the buffer sequence - returned from @ref stream::prepare. + /** Returns the available capacity */ BOOST_HTTP_PROTO_DECL - bool - is_full() const noexcept; + std::size_t + capacity() const noexcept; - /** Returns a MutableBufferSequence for storing - serializer input. If `n` bytes are written to the - buffer sequence, @ref stream::commit must be called - with `n` to update the backing serializer's buffers. + /** Prepares a buffer for writing - The returned buffer sequence is as wide as is - possible. + Use @ref commit to make the written data available + to the serializer. - @exception std::length_error Thrown if the stream - has insufficient capacity and a chunked transfer - encoding is being used + @return An object of type @ref mutable_buffers_type + that satisfies MutableBufferSequence requirements, + the underlying memory is owned by the serializer. */ BOOST_HTTP_PROTO_DECL - buffers_type - prepare() const; + mutable_buffers_type + prepare() noexcept; - /** Make `n` bytes available to the serializer. + /** Commits data to the serializer - Once the buffer sequence returned from @ref stream::prepare - has been filled, the input can be marked as ready - for serialization by using this function. + @param n Number of bytes to commit. - @exception std::logic_error Thrown if `commit` is - called with 0. + @throw std::invalid_argument if `n > capacity()`. + @throw std::logic_error if `!is_open()`. */ BOOST_HTTP_PROTO_DECL void - commit(std::size_t n) const; - - /** Indicate that no more data is coming and that the - body should be treated as complete. + commit(std::size_t n); - @excpeption std::logic_error Thrown if the stream - has been previously closed. + /** Closes the stream */ BOOST_HTTP_PROTO_DECL void - close() const; + close(); private: friend class serializer; diff --git a/src/serializer.cpp b/src/serializer.cpp index d8b3de4f..3c972ceb 100644 --- a/src/serializer.cpp +++ b/src/serializer.cpp @@ -107,22 +107,34 @@ write_final_chunk( BOOST_ASSERT(n == 5); } +buffers::mutable_buffer_pair +prepare_chunked( + buffers::circular_buffer& cb) noexcept +{ + return + buffers::sans_suffix( + buffers::sans_prefix( + cb.prepare(cb.capacity()), + chunk_header_len), + final_chunk_len + crlf_len); +} + //------------------------------------------------ class appender { buffers::circular_buffer& cb_; buffers::mutable_buffer_pair mbp_; - std::size_t n_ = 0; bool is_chunked_ = false; bool more_input_ = true; public: appender( buffers::circular_buffer& cb, - bool is_chunked) + bool is_chunked) noexcept : cb_(cb) - , mbp_(cb.prepare(cb.capacity())) + , mbp_(is_chunked ? + prepare_chunked(cb) : cb.prepare(cb.capacity())) , is_chunked_(is_chunked) { } @@ -130,32 +142,20 @@ class appender bool is_full() const noexcept { - auto remaining = cb_.capacity() - n_; - if(is_chunked_) - return remaining <= chunked_overhead_; - - return remaining == 0; + return buffers::size(mbp_) == 0; } buffers::mutable_buffer_pair prepare() noexcept { - if(is_chunked_) - { - return buffers::sans_suffix( - buffers::sans_prefix( - mbp_, - chunk_header_len + n_) - , final_chunk_len + crlf_len); - } - return buffers::sans_prefix(mbp_, n_); + return mbp_; } void commit(std::size_t n, bool more) noexcept { BOOST_ASSERT(more_input_); - n_ += n; + mbp_ = buffers::sans_prefix(mbp_, n); more_input_ = more; } @@ -163,10 +163,18 @@ class appender { if(is_chunked_) { - if(n_) + auto consumed = cb_.capacity() - + buffers::size(mbp_) - chunked_overhead_; + + if(consumed != 0) { - write_chunk_header(mbp_, n_); - cb_.commit(n_ + chunk_header_len); + write_chunk_header( + cb_.prepare(chunk_header_len), + consumed); + cb_.commit(chunk_header_len); + + cb_.prepare(consumed); + cb_.commit(consumed); write_crlf( cb_.prepare(crlf_len)); @@ -182,7 +190,7 @@ class appender } else // is_chunked_ == false { - cb_.commit(n_); + cb_.commit(cb_.capacity() - buffers::size(mbp_)); } } }; @@ -1038,11 +1046,25 @@ start_stream( //------------------------------------------------ +bool +serializer:: +stream:: +is_open() const noexcept +{ + if(sr_ == nullptr) + return false; + + return sr_->more_input_; +} + std::size_t serializer:: stream:: capacity() const noexcept { + if(!is_open()) + return 0; + if(sr_->filter_) return sr_->cb1_.capacity(); @@ -1057,20 +1079,15 @@ capacity() const noexcept return 0; } -bool -serializer:: -stream:: -is_full() const noexcept -{ - return capacity() == 0; -} - auto serializer:: stream:: -prepare() const -> - buffers_type +prepare() noexcept -> + mutable_buffers_type { + if(!is_open()) + return {}; + if(sr_->filter_) return sr_->cb1_.prepare( sr_->cb1_.capacity()); @@ -1080,21 +1097,25 @@ prepare() const -> sr_->cb0_.capacity()); // chunked with no filter - const auto cap = sr_->cb0_.capacity(); - if(cap <= chunked_overhead_) - detail::throw_length_error(); - - return buffers::sans_prefix( - sr_->cb0_.prepare( - cap - crlf_len - final_chunk_len), - chunk_header_len); + return prepare_chunked(sr_->cb0_); } void serializer:: stream:: -commit(std::size_t n) const +commit(std::size_t n) { + // Precondition violation + if(!is_open()) + detail::throw_logic_error(); + + if(n > capacity()) + { + // n can't be greater than size of + // the buffers returned by prepare() + detail::throw_invalid_argument(); + } + if(sr_->filter_) return sr_->cb1_.commit(n); @@ -1123,11 +1144,10 @@ commit(std::size_t n) const void serializer:: stream:: -close() const +close() { - // Precondition violation - if(!sr_->more_input_) - detail::throw_logic_error(); + if(!is_open()) + return; // no-op; sr_->more_input_ = false; diff --git a/test/unit/serializer.cpp b/test/unit/serializer.cpp index 096fdd68..ecfb2435 100644 --- a/test/unit/serializer.cpp +++ b/test/unit/serializer.cpp @@ -338,13 +338,12 @@ struct serializer_test std::vector s; // stores complete output - auto prepare_chunk = [&] + auto prepare = [&]() { - BOOST_TEST(!stream.is_full()); + BOOST_TEST(stream.capacity() != 0); auto mbs = stream.prepare(); - auto bs = buffers::size(mbs); - BOOST_TEST_GT(bs, 0); + BOOST_TEST_EQ(bs, stream.capacity()); if( bs > body.size() ) bs = body.size(); @@ -354,15 +353,14 @@ struct serializer_test stream.commit(bs); if( bs < body.size() ) - BOOST_TEST(stream.is_full()); + BOOST_TEST(stream.capacity() == 0); else - BOOST_TEST(!stream.is_full()); + BOOST_TEST(stream.capacity() != 0); body.remove_prefix(bs); }; - auto consume_body_buffer = [&]( - buffers::const_buffer buf) + auto consume = [&](buffers::const_buffer buf) { // we have the prepared buffer sequence // representing the serializer's output but we @@ -392,7 +390,7 @@ struct serializer_test while(! sr.is_done() ) { if(! body.empty() ) - prepare_chunk(); + prepare(); if( body.empty() && !closed ) { @@ -409,11 +407,9 @@ struct serializer_test BOOST_TEST_GT(size, 0); for(auto pos = cbs.begin(); pos != end; ++pos) - consume_body_buffer(*pos); + consume(*pos); } - BOOST_TEST_THROWS(stream.close(), std::logic_error); - f(core::string_view(s.data(), s.size())); } @@ -758,6 +754,12 @@ struct serializer_test void testStreamErrors() { + // Default constructor + { + serializer::stream stream; + BOOST_TEST(!stream.is_open()); + } + { core::string_view sv = "HTTP/1.1 200 OK\r\n" @@ -768,11 +770,17 @@ struct serializer_test install_serializer_service(ctx, {}); serializer sr(ctx); auto stream = sr.start_stream(res); + BOOST_TEST(stream.is_open()); auto mbs = stream.prepare(); BOOST_TEST_GT( buffers::size(mbs), 0); - BOOST_TEST(!stream.is_full()); + BOOST_TEST(stream.capacity() != 0); + + // commit with `n > stream.capacity()` + BOOST_TEST_THROWS( + stream.commit(buffers::size(mbs) + 1), + std::invalid_argument); // commit 0 bytes must be possible stream.commit(0); @@ -790,6 +798,14 @@ struct serializer_test mcbs.error() == error::need_data); stream.close(); + BOOST_TEST(!stream.is_open()); + BOOST_TEST(stream.capacity() == 0); + BOOST_TEST(buffers::size(stream.prepare()) == 0); + BOOST_TEST_THROWS( + stream.commit(1), + std::logic_error); + + stream.close(); // fine no-op mcbs = sr.prepare(); std::string body; From d4044dedcfd956d33a4062b454ab26fdc88ad3d3 Mon Sep 17 00:00:00 2001 From: Mohammad Nejati Date: Sun, 6 Jul 2025 13:01:56 +0000 Subject: [PATCH 2/2] Rename test/limits macro --- include/boost/http_proto/detail/header.hpp | 2 +- test/limits/CMakeLists.txt | 2 +- test/limits/Jamfile | 2 +- test/limits/limits.cpp | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/include/boost/http_proto/detail/header.hpp b/include/boost/http_proto/detail/header.hpp index a106955f..a8929d9f 100644 --- a/include/boost/http_proto/detail/header.hpp +++ b/include/boost/http_proto/detail/header.hpp @@ -51,7 +51,7 @@ struct header // ^ ^ ^ ^ // buf buf+prefix buf+size buf+cap -#ifdef BOOST_HTTP_PROTO_TEST_FORCE_8BIT_OFFSET +#ifdef BOOST_HTTP_PROTO_TEST_LIMITS using offset_type = std::uint8_t; #else using offset_type = std::uint32_t; diff --git a/test/limits/CMakeLists.txt b/test/limits/CMakeLists.txt index 179d2d9a..44172085 100644 --- a/test/limits/CMakeLists.txt +++ b/test/limits/CMakeLists.txt @@ -18,7 +18,7 @@ add_executable(boost_http_proto_limits limits.cpp Jamfile ${TEST_MAIN} ${LIMITS_ target_include_directories(boost_http_proto_limits PRIVATE ../../) target_include_directories(boost_http_proto_limits PRIVATE ../../include ../../../url/extra/test_suite ../../..) target_compile_definitions(boost_http_proto_limits PRIVATE - BOOST_HTTP_PROTO_TEST_FORCE_8BIT_OFFSET + BOOST_HTTP_PROTO_TEST_LIMITS BOOST_HTTP_PROTO_NO_LIB BOOST_HTTP_PROTO_STATIC_LINK ) diff --git a/test/limits/Jamfile b/test/limits/Jamfile index b1cb798f..1c59cc10 100644 --- a/test/limits/Jamfile +++ b/test/limits/Jamfile @@ -25,7 +25,7 @@ project run limits.cpp /boost/http_proto//http_proto_sources : requirements - BOOST_HTTP_PROTO_TEST_FORCE_8BIT_OFFSET + BOOST_HTTP_PROTO_TEST_LIMITS BOOST_HTTP_PROTO_NO_LIB BOOST_HTTP_PROTO_STATIC_LINK static diff --git a/test/limits/limits.cpp b/test/limits/limits.cpp index b870205c..96aeaef4 100644 --- a/test/limits/limits.cpp +++ b/test/limits/limits.cpp @@ -25,8 +25,8 @@ #error "Limits should not be built with shared linking." #endif -#ifndef BOOST_HTTP_PROTO_TEST_FORCE_8BIT_OFFSET -#error "Limits should be built with BOOST_HTTP_PROTO_TEST_FORCE_8BIT_OFFSET." +#ifndef BOOST_HTTP_PROTO_TEST_LIMITS +#error "Limits should be built with BOOST_HTTP_PROTO_TEST_LIMITS." #endif namespace boost {