diff --git a/CHANGELOG.md b/CHANGELOG.md index a6a8167c..bca89eb3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,12 +10,17 @@ Version 76: API Changes: * Rename to serializer::keep_alive +* BodyReader, BodyWriter use two-phase init Actions Required: * Use serializer::keep_alive instead of serializer::close and take the logical NOT of the return value. +* Modify instances of user-defined BodyReader and BodyWriter + types to perfrom two-phase initialization, as per the + updated documented type requirements. + -------------------------------------------------------------------------------- Version 75: diff --git a/doc/concept/BodyReader.qbk b/doc/concept/BodyReader.qbk index 4ede1415..470370df 100644 --- a/doc/concept/BodyReader.qbk +++ b/doc/concept/BodyReader.qbk @@ -36,8 +36,14 @@ In this table: * `R` is the type `boost::optional>`. -[table BodyReader requirements -[[expression] [type] [semantics, pre/post-conditions]] +[heading Associated Types] + +* __Body__ +* [link beast.ref.beast__http__is_body_reader `is_body_reader`] + +[heading BodyReader requirements] +[table Valid Expressions +[[Expression] [Type] [Semantics, Pre/Post-conditions]] [ [`X::const_buffers_type`] [] @@ -46,22 +52,33 @@ In this table: This is the type of buffer returned by `X::get`. ] ][ - [`X(m,ec);`] + [`X(m);`] [] [ Constructible from `m`. The lifetime of `m` is guaranteed to end no earlier than after the `X` is destroyed. + The reader shall not access the contents of `m` before the + first call to `init`, permitting lazy construction of the + message. + The constructor may optionally require that `m` is const, which has these consequences: * If `X` requires that `m` is a const reference, then serializers constructed for messages with this body type will also require a - `const` reference to a message, otherwise: + const reference to a message, otherwise: * If `X` requires that `m` is a non-const reference, then serializers constructed for messages with this body type will aso require a non-const reference to a message. - + ] +][ + [`a.init(ec)`] + [] + [ + Called once to fully initialize the object before any calls to + `get`. The message body becomes valid before entering this function, + and remains valid until the reader is destroyed. The function will ensure that `!ec` is `true` if there was no error or set to the appropriate error code if there was one. ] @@ -69,9 +86,9 @@ In this table: [`a.get(ec)`] [`R`] [ - Called repeatedly after `init` succeeds. This function returns - `boost::none` if all buffers representing the body have been - returned in previous calls or if it sets `ec` to indicate an + Called one or more times after `init` succeeds. This function + returns `boost::none` if all buffers representing the body have + been returned in previous calls or if it sets `ec` to indicate an error. Otherwise, if there are buffers remaining the function should return a pair with the first element containing a non-zero length buffer sequence representing the next set of octets in @@ -82,13 +99,6 @@ In this table: The function will ensure that `!ec` is `true` if there was no error or set to the appropriate error code if there was one. ] -][ - [`is_body_reader`] - [`std::true_type`] - [ - An alias for `std::true_type` for `B`, otherwise an alias - for `std::false_type`. - ] ] ] @@ -96,4 +106,11 @@ In this table: [concept_BodyReader] +[heading Models] + +* [link beast.ref.beast__http__basic_dynamic_body.reader `basic_dynamic_body::reader`] +* [link beast.ref.beast__http__basic_file_body__reader `basic_file_body::reader`] +* [link beast.ref.beast__http__empty_body.reader `empty_body::reader`] +* [link beast.ref.beast__http__string_body.reader `string_body::reader`] + [endsect] diff --git a/doc/concept/BodyWriter.qbk b/doc/concept/BodyWriter.qbk index 830d2509..50ae408d 100644 --- a/doc/concept/BodyWriter.qbk +++ b/doc/concept/BodyWriter.qbk @@ -38,20 +38,40 @@ In this table: * `ec` is a value of type [link beast.ref.beast__error_code `error_code&`]. +[heading Associated Types] + +* __Body__ +* [link beast.ref.beast__http__is_body_writer `is_body_writer`] + [table Writer requirements [[expression] [type] [semantics, pre/post-conditions]] [ - [`X(m,n,ec);`] + [`X(m);`] [] [ - Constructible from `m`. The lifetime of `m` is guaranteed - to end no earlier than after the `X` is destroyed. The constructor - will be called after a complete header is stored in `m`, and before - parsing body octets for messages indicating that a body is present. + Constructible from `m`. The lifetime of `m` is guaranteed to + end no earlier than after the `X` is destroyed. The constructor + will be called after a complete header is stored in `m`, and + before parsing body octets for messages indicating that a body + is present The writer shall not access the contents of `m` before + the first call to `init`, permitting lazy construction of the + message. + + The function will ensure that `!ec` is `true` if there was + no error or set to the appropriate error code if there was one. + ] +][ + [`a.init(n, ec)`] + [] + [ + Called once to fully initialize the object before any calls to + `put`. The message body is valid before entering this function, + and remains valid until the writer is destroyed. The value of `n` will be set to the content length of the body if known, otherwise `n` will be equal to `boost::none`. Implementations of [*BodyWriter] may use this information to optimize allocation. + The function will ensure that `!ec` is `true` if there was no error or set to the appropriate error code if there was one. ] @@ -89,4 +109,11 @@ In this table: [concept_BodyWriter] +[heading Models] + +* [link beast.ref.beast__http__basic_dynamic_body.writer `basic_dynamic_body::writer`] +* [link beast.ref.beast__http__basic_file_body__reader `basic_file_body::writer`] +* [link beast.ref.beast__http__empty_body.writer `empty_body::writer`] +* [link beast.ref.beast__http__string_body.writer `string_body::writer`] + [endsect] diff --git a/example/common/const_body.hpp b/example/common/const_body.hpp index c4a0f743..9549ff49 100644 --- a/example/common/const_body.hpp +++ b/example/common/const_body.hpp @@ -68,9 +68,14 @@ struct const_body template explicit - reader(beast::http::message const& msg, beast::error_code& ec) + reader(beast::http::message const& msg) : body_(msg.body) + { + } + + void + init(beast::error_code& ec) { ec.assign(0, ec.category()); } diff --git a/example/common/mutable_body.hpp b/example/common/mutable_body.hpp index 3d441e53..bf080356 100644 --- a/example/common/mutable_body.hpp +++ b/example/common/mutable_body.hpp @@ -71,9 +71,14 @@ struct mutable_body template explicit - reader(beast::http::message const& msg, beast::error_code& ec) + reader(beast::http::message const& msg) : body_(msg.body) + { + } + + void + init(beast::error_code& ec) { ec.assign(0, ec.category()); } @@ -99,15 +104,20 @@ struct mutable_body public: template explicit - writer(beast::http::message& m, - boost::optional const& content_length, - beast::error_code& ec) + writer(beast::http::message< + isRequest, mutable_body, Fields>& m) : body_(m.body) { - if(content_length) + } + + void + init(boost::optional< + std::uint64_t> const& length, beast::error_code& ec) + { + if(length) { - if(*content_length > (std::numeric_limits< - std::size_t>::max)()) + if(*length > (std::numeric_limits< + std::size_t>::max)()) { ec = beast::http::error::buffer_overflow; return; @@ -115,7 +125,7 @@ struct mutable_body try { body_.reserve(static_cast< - std::size_t>(*content_length)); + std::size_t>(*length)); } catch(std::exception const&) { @@ -129,7 +139,7 @@ struct mutable_body template std::size_t put(ConstBufferSequence const& buffers, - beast::error_code& ec) + beast::error_code& ec) { using boost::asio::buffer_size; using boost::asio::buffer_copy; diff --git a/include/beast/http/buffer_body.hpp b/include/beast/http/buffer_body.hpp index d889bd4e..3b4f660a 100644 --- a/include/beast/http/buffer_body.hpp +++ b/include/beast/http/buffer_body.hpp @@ -101,9 +101,14 @@ struct buffer_body template explicit - reader(message const& msg, error_code& ec) + reader(message const& msg) : body_(msg.body) + { + } + + void + init(error_code& ec) { ec.assign(0, ec.category()); } @@ -152,10 +157,13 @@ struct buffer_body public: template explicit - writer(message& m, - boost::optional const&, - error_code& ec) + writer(message& m) : body_(m.body) + { + } + + void + init(boost::optional const&, error_code& ec) { ec.assign(0, ec.category()); } diff --git a/include/beast/http/dynamic_body.hpp b/include/beast/http/dynamic_body.hpp index 43206e17..35fbaddc 100644 --- a/include/beast/http/dynamic_body.hpp +++ b/include/beast/http/dynamic_body.hpp @@ -55,9 +55,14 @@ struct basic_dynamic_body template explicit - reader(message const& m, error_code& ec) + reader(message const& m) : body_(m.body) + { + } + + void + init(error_code& ec) { ec.assign(0, ec.category()); } @@ -82,10 +87,13 @@ struct basic_dynamic_body public: template explicit - writer(message& msg, - boost::optional const&, - error_code& ec) + writer(message& msg) : body_(msg.body) + { + } + + void + init(boost::optional const&, error_code& ec) { ec.assign(0, ec.category()); } diff --git a/include/beast/http/empty_body.hpp b/include/beast/http/empty_body.hpp index 5ac2ca81..9468502c 100644 --- a/include/beast/http/empty_body.hpp +++ b/include/beast/http/empty_body.hpp @@ -54,8 +54,13 @@ struct empty_body template explicit - reader(message const&, error_code& ec) + reader(message const&) + { + } + + void + init(error_code& ec) { ec.assign(0, ec.category()); } @@ -77,9 +82,12 @@ struct empty_body { template explicit - writer(message&, - boost::optional const&, - error_code& ec) + writer(message&) + { + } + + void + init(boost::optional const&, error_code& ec) { ec.assign(0, ec.category()); } diff --git a/include/beast/http/file_body.hpp b/include/beast/http/file_body.hpp index ea59e270..17970126 100644 --- a/include/beast/http/file_body.hpp +++ b/include/beast/http/file_body.hpp @@ -235,9 +235,17 @@ public: // always have the `file_body` as the body type. // template - reader( - message const& m, - error_code& ec); + reader(message< + isRequest, basic_file_body, Fields> const& m); + + // Initializer + // + // This is called before the body is serialized and + // gives the reader a chance to do something that might + // need to return an error code. + // + void + init(error_code& ec); // This function is called zero or more times to // retrieve buffers. A return value of `boost::none` @@ -261,18 +269,29 @@ template template basic_file_body:: reader:: -reader( - message const& m, - error_code& ec) +reader(message const& m) : body_(m.body) { // The file must already be open BOOST_ASSERT(body_.file_.is_open()); // Get the size of the file - remain_ = body_.file_.size(ec); - if(ec) - return; + remain_ = body_.file_size_; +} + +// Initializer +template +void +basic_file_body:: +reader:: +init(error_code& ec) +{ + // The error_code specification requires that we + // either set the error to some value, or set it + // to indicate no error. + // + // We don't do anything fancy so set "no error" + ec.assign(0, ec.category()); } // This function is called repeatedly by the serializer to @@ -353,9 +372,18 @@ public: template explicit writer( - message& m, - boost::optional const& content_length, - error_code& ec); + message& m); + + // Initializer + // + // This is called before the body is parsed and + // gives the writer a chance to do something that might + // need to return an error code. It informs us of + // the payload size (`content_length`) which we can + // optionally use for optimization. + // + void + init(boost::optional const&, error_code& ec); // This function is called one or more times to store // buffer sequences corresponding to the incoming body. @@ -387,11 +415,18 @@ template template basic_file_body:: writer:: -writer( - message& m, +writer(message& m) + : body_(m.body) +{ +} + +template +void +basic_file_body:: +writer:: +init( boost::optional const& content_length, error_code& ec) - : body_(m.body) { // The file must already be open for writing BOOST_ASSERT(body_.file_.is_open()); @@ -401,7 +436,11 @@ writer( // to see if there is enough room to store the body. boost::ignore_unused(content_length); - // This is required by the error_code specification + // The error_code specification requires that we + // either set the error to some value, or set it + // to indicate no error. + // + // We don't do anything fancy so set "no error" ec.assign(0, ec.category()); } diff --git a/include/beast/http/impl/parser.ipp b/include/beast/http/impl/parser.ipp index 9f5ab5a2..73bacd9a 100644 --- a/include/beast/http/impl/parser.ipp +++ b/include/beast/http/impl/parser.ipp @@ -14,12 +14,20 @@ namespace beast { namespace http { +template +parser:: +parser() + : wr_(m_) +{ +} + template template parser:: parser(Arg1&& arg1, ArgN&&... argn) : m_(std::forward(arg1), std::forward(argn)...) + , wr_(m_) { } @@ -30,8 +38,9 @@ parser(parser&& p, Args&&... args) : base_type(std::move(p)) , m_(p.release(), std::forward(args)...) + , wr_(m_) { - if(p.wr_) + if(wr_inited_) BOOST_THROW_EXCEPTION(std::invalid_argument{ "moved-from parser has a body"}); } diff --git a/include/beast/http/impl/serializer.ipp b/include/beast/http/impl/serializer.ipp index 0e8f86c8..89f5c6da 100644 --- a/include/beast/http/impl/serializer.ipp +++ b/include/beast/http/impl/serializer.ipp @@ -59,6 +59,7 @@ template:: serializer(value_type& m) : m_(m) + , rd_(m_) { } @@ -67,6 +68,7 @@ template:: serializer(value_type& m, ChunkDecorator const& d) : m_(m) + , rd_(m_) , d_(d) { } @@ -96,12 +98,12 @@ next(error_code& ec, Visit&& visit) case do_init: { - if(split_) - goto go_header_only; - rd_.emplace(m_, ec); + rd_.init(ec); if(ec) return; - auto result = rd_->get(ec); + if(split_) + goto go_header_only; + auto result = rd_.get(ec); if(ec == error::need_more) goto go_header_only; if(ec) @@ -129,18 +131,12 @@ next(error_code& ec, Visit&& visit) break; case do_body: - if(! rd_) - { - rd_.emplace(m_, ec); - if(ec) - return; - } s_ = do_body + 1; BEAST_FALLTHROUGH; case do_body + 1: { - auto result = rd_->get(ec); + auto result = rd_.get(ec); if(ec) return; if(! result) @@ -161,12 +157,12 @@ next(error_code& ec, Visit&& visit) s_ = do_init_c; case do_init_c: { - if(split_) - goto go_header_only_c; - rd_.emplace(m_, ec); + rd_.init(ec); if(ec) return; - auto result = rd_->get(ec); + if(split_) + goto go_header_only_c; + auto result = rd_.get(ec); if(ec == error::need_more) goto go_header_only_c; if(ec) @@ -237,18 +233,12 @@ next(error_code& ec, Visit&& visit) break; case do_body_c: - if(! rd_) - { - rd_.emplace(m_, ec); - if(ec) - return; - } s_ = do_body_c + 1; BEAST_FALLTHROUGH; case do_body_c + 1: { - auto result = rd_->get(ec); + auto result = rd_.get(ec); if(ec) return; if(! result) diff --git a/include/beast/http/parser.hpp b/include/beast/http/parser.hpp index e630a996..108223ef 100644 --- a/include/beast/http/parser.hpp +++ b/include/beast/http/parser.hpp @@ -60,21 +60,17 @@ class parser parser>; message> m_; - boost::optional wr_; + typename Body::writer wr_; std::function cb_; + bool wr_inited_ = false; public: /// The type of message returned by the parser using value_type = message>; - /// Constructor (default) - parser() - { - // avoid `parser()=default` otherwise value-init - // for members can happen (i.e. a big memset on - // static_buffer_n). - } + /// Constructor + parser(); /// Copy constructor (disallowed) parser(parser const&) = delete; @@ -285,13 +281,14 @@ private: std::uint64_t> const& content_length, error_code& ec) { - wr_.emplace(m_, content_length, ec); + wr_.init(content_length, ec); + wr_inited_ = true; } std::size_t on_data(string_view s, error_code& ec) { - return wr_->put(boost::asio::buffer( + return wr_.put(boost::asio::buffer( s.data(), s.size()), ec); } @@ -305,10 +302,7 @@ private: void on_complete(error_code& ec) { - if(wr_) - wr_->finish(ec); - else - ec.assign(0, ec.category()); + wr_.finish(ec); } }; diff --git a/include/beast/http/serializer.hpp b/include/beast/http/serializer.hpp index b5668f19..d69bf711 100644 --- a/include/beast/http/serializer.hpp +++ b/include/beast/http/serializer.hpp @@ -134,7 +134,7 @@ public: static_assert(is_body_reader::value, "BodyReader requirements not met"); - /** The type of the message referenced by this object. + /** The type of message this serializer uses This may be const or non-const depending on the implementation of the corresponding @b BodyReader. @@ -145,11 +145,9 @@ public: using value_type = typename std::conditional< std::is_constructible&, - error_code&>::value && + message&>::value && ! std::is_constructible const&, - error_code&>::value, + message const&>::value, message, message const>::type; #endif @@ -248,8 +246,8 @@ private: using pcb8_t = buffer_prefix_view; value_type& m_; + reader rd_; boost::optional frd_; - boost::optional rd_; boost::variant explicit - reader(message const& msg, error_code& ec) + reader(message const& msg) : body_(msg.body) + { + } + + void + init(error_code& ec) { ec.assign(0, ec.category()); } @@ -80,23 +85,27 @@ struct string_body public: template explicit - writer(message& m, - boost::optional content_length, - error_code& ec) + writer(message& m) : body_(m.body) { - if(content_length) + } + + void + init(boost::optional< + std::uint64_t> const& length, error_code& ec) + { + if(length) { - if(*content_length > (std::numeric_limits< - std::size_t>::max)()) + if(*length > ( + std::numeric_limits::max)()) { ec = error::buffer_overflow; return; } try { - body_.reserve(static_cast< - std::size_t>(*content_length)); + body_.reserve( + static_cast(*length)); } catch(std::exception const&) { diff --git a/include/beast/http/string_view_body.hpp b/include/beast/http/string_view_body.hpp index eb5960de..0219372e 100644 --- a/include/beast/http/string_view_body.hpp +++ b/include/beast/http/string_view_body.hpp @@ -53,9 +53,14 @@ struct string_view_body template explicit - reader(message const& m, error_code& ec) + reader(message const& m) : body_(m.body) + { + } + + void + init(error_code& ec) { ec.assign(0, ec.category()); } diff --git a/include/beast/http/type_traits.hpp b/include/beast/http/type_traits.hpp index 6a4dd5c6..46f5674a 100644 --- a/include/beast/http/type_traits.hpp +++ b/include/beast/http/type_traits.hpp @@ -80,6 +80,7 @@ struct is_body_reader().init(std::declval()), std::declval>&>() = std::declval().get(std::declval()), @@ -87,11 +88,9 @@ struct is_body_reader::value && std::is_constructible&, - error_code&>::value && + message&>::value && std::is_constructible&, - error_code&>::value + message&>::value > {}; #endif @@ -124,6 +123,9 @@ struct is_body_writer : std::false_type {}; template struct is_body_writer().init( + boost::optional(), + std::declval()), std::declval() = std::declval().put( std::declval(), @@ -132,10 +134,10 @@ struct is_body_writer()), (void)0)>> : std::integral_constant&, - boost::optional, - error_code& - >::value> + message&>::value && + std::is_constructible&>::value + > { }; #endif diff --git a/test/exemplars.cpp b/test/exemplars.cpp index 1079475a..95f99971 100644 --- a/test/exemplars.cpp +++ b/test/exemplars.cpp @@ -58,10 +58,24 @@ public: @param ec Set to the error, if any occurred. */ - template + template explicit - BodyReader(message const& msg, - error_code &ec); + BodyReader(message const& msg); + + /** Initialize the reader + + This is called after construction and before the first + call to `get`. The message is valid and complete upon + entry. + + @param ec Set to the error, if any occurred. + */ + void + init(error_code& ec) + { + // The specification requires this to indicate "no error" + ec.assign(0, ec.category()); + } /** Returns the next buffer in the body. @@ -85,7 +99,7 @@ public: get(error_code& ec) { // The specification requires this to indicate "no error" - ec = {}; + ec.assign(0, ec.category()); return boost::none; // for exposition only } @@ -109,14 +123,25 @@ struct BodyWriter */ template explicit - BodyWriter(message& msg, - boost::optional content_length, - error_code& ec) + BodyWriter(message& msg); + + /** Initialize the writer + + This is called after construction and before the first + call to `put`. The message is valid and complete upon + entry. + + @param ec Set to the error, if any occurred. + */ + void + init( + boost::optional const& content_length, + error_code& ec) { - boost::ignore_unused(msg, content_length); + boost::ignore_unused(content_length); // The specification requires this to indicate "no error" - ec = {}; + ec.assign(0, ec.category()); } /** Store buffers. diff --git a/test/http/serializer.cpp b/test/http/serializer.cpp index 20826307..d39dbcb5 100644 --- a/test/http/serializer.cpp +++ b/test/http/serializer.cpp @@ -27,8 +27,10 @@ public: boost::asio::const_buffers_1; template - reader(message const&, - error_code&); + reader(message const&); + + void + init(error_code& ec); boost::optional> get(error_code&); @@ -45,8 +47,10 @@ public: boost::asio::const_buffers_1; template - reader(message&, - error_code&); + reader(message&); + + void + init(error_code& ec); boost::optional> get(error_code&); @@ -55,6 +59,7 @@ public: BOOST_STATIC_ASSERT(std::is_const< serializer< true, const_body>::value_type>::value); + BOOST_STATIC_ASSERT(! std::is_const::value_type>::value); diff --git a/test/http/write.cpp b/test/http/write.cpp index d072fa2b..bb0a7f63 100644 --- a/test/http/write.cpp +++ b/test/http/write.cpp @@ -47,13 +47,18 @@ public: template explicit - reader(message const& msg, error_code &ec) + reader(message const& msg) : body_(msg.body) + { + } + + void + init(error_code& ec) { ec.assign(0, ec.category()); } - + boost::optional> get(error_code& ec) { @@ -87,9 +92,14 @@ public: template explicit - reader(message const& msg, error_code& ec) + reader(message const& msg) : body_(msg.body) + { + } + + void + init(error_code& ec) { ec.assign(0, ec.category()); } @@ -219,9 +229,14 @@ public: template explicit - reader(message const& msg, error_code& ec) + reader(message const& msg) : body_(msg.body) + { + } + + void + init(error_code& ec) { body_.fc_.fail(ec); }