From f9f6e1e0cc45ce075197011e14f0348936e4de09 Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Mon, 19 Jun 2017 14:35:32 -0700 Subject: [PATCH] Refine FieldsReader concept (API Change) * FieldsReader now requires chunked() and keep_alive() * serializer logic calls into the reader to calculate the message metadata * Removed some of the previous requirements of FieldsReader Actions Required: * Implement chunked() and keep_alive() for user defined FieldsReader types. --- CHANGELOG.md | 3 + doc/concept/Body.qbk | 5 +- doc/concept/BodyReader.qbk | 7 +- doc/concept/BodyWriter.qbk | 7 +- doc/concept/Fields.qbk | 62 ++++++++++----- doc/concept/FieldsReader.qbk | 32 +++++++- include/beast/http/fields.hpp | 9 --- include/beast/http/impl/fields.ipp | 102 ++++++++++++++----------- include/beast/http/impl/message.ipp | 27 ------- include/beast/http/impl/serializer.ipp | 6 +- include/beast/http/message.hpp | 18 ----- test/http/write.cpp | 25 +++--- test/websocket/stream.cpp | 2 +- 13 files changed, 167 insertions(+), 138 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 520cc3a6..6d45b13c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ Version 62: API Changes: * parser requires basic_fields +* Refine FieldsReader concept Actions Required: @@ -18,6 +19,8 @@ Actions Required: will need to create their own subclass of basic_parser to work with their custom fields type. +* Implement chunked() and keep_alive() for user defined FieldsReader types. + -------------------------------------------------------------------------------- Version 61: diff --git a/doc/concept/Body.qbk b/doc/concept/Body.qbk index 8a334ecb..15bcab8c 100644 --- a/doc/concept/Body.qbk +++ b/doc/concept/Body.qbk @@ -67,9 +67,10 @@ In this table: ] ] -Exemplar: +[heading Exemplar] + ``` -struct body +struct Body { using value_type = implementation_defined; diff --git a/doc/concept/BodyReader.qbk b/doc/concept/BodyReader.qbk index bfbd9596..56eb5c85 100644 --- a/doc/concept/BodyReader.qbk +++ b/doc/concept/BodyReader.qbk @@ -118,9 +118,10 @@ In this table: the implementation. ] -Exemplar: +[heading Exemplar] + ``` -struct reader +struct BodyReader { public: /** Controls when the implementation requests buffers. @@ -140,7 +141,7 @@ public: */ template explicit - reader(message const& msg); + BodyReader(message const& msg); /** Initialization. diff --git a/doc/concept/BodyWriter.qbk b/doc/concept/BodyWriter.qbk index 95e648b7..d4ced348 100644 --- a/doc/concept/BodyWriter.qbk +++ b/doc/concept/BodyWriter.qbk @@ -94,9 +94,10 @@ In the table below: the implementation. ] -Exemplar: +[heading Exemplar] + ``` -struct writer +struct BodyWriter { /** Construct the writer. @@ -104,7 +105,7 @@ struct writer */ template explicit - writer(message& msg); + BodyWriter(message& msg); /** Initialization. diff --git a/doc/concept/Fields.qbk b/doc/concept/Fields.qbk index 414d99b7..9d4497ad 100644 --- a/doc/concept/Fields.qbk +++ b/doc/concept/Fields.qbk @@ -33,24 +33,6 @@ In this table: [ A type which meets the requirements of __FieldsReader__. ] -][ - [`c.has_close_impl()`] - [`bool`] - [ - Returns `true` if the value for Connection has "close" in the list. - ] -][ - [`c.has_chunked_impl()`] - [`bool`] - [ - Returns `true` if "chunked" is the last Transfer-Encoding. - ] -][ - [`c.has_content_length_impl()`] - [`bool`] - [ - Returns `true` if the Content-Length field is present. - ] ][ [`c.get_method_impl()`] [`string_view`] @@ -109,4 +91,48 @@ In this table: ] ] +[heading Exemplar] + +``` +class Fields +{ +protected: + /** Set or clear the method string. + + @note Only called for requests. + */ + void set_method_impl(string_view s); + + /** Set or clear the target string. + + @note Only called for requests. + */ + void set_target_impl(string_view s); + + /** Set or clear the reason string. + + @note Only called for responses. + */ + void set_reason_impl(string_view s); + + /** Returns the request-method string. + + @note Only called for requests. + */ + string_view get_method_impl() const; + + /** Returns the request-target string. + + @note Only called for requests. + */ + string_view get_target_impl() const; + + /** Returns the response reason-phrase string. + + @note Only called for responses. + */ + string_view get_reason_impl() const; +}; +``` + [endsect] diff --git a/doc/concept/FieldsReader.qbk b/doc/concept/FieldsReader.qbk index 3a8e52d8..a95060f6 100644 --- a/doc/concept/FieldsReader.qbk +++ b/doc/concept/FieldsReader.qbk @@ -56,6 +56,27 @@ In this table: response. The lifetime of `f` is guaranteed to end no earlier than after the `X` is destroyed. ] +][ + [`a.chunked()`] + [`bool`] + [ + Called once after construction, this function returns `true` + if the [*Transfer-Encoding] field is present, and the last + token in its value is [*"chunked"]. + ] +][ + [`a.keep_alive()`] + [`bool`] + [ + Called once after construction, this function returns `true` + if the semantics of the mesage indicate that the connection + should remain open after processing the message. When the + HTTP version is below 1.1, `keep_alive` returns `true` when + the [*Connection] field is present, and its value contains the + "keep-alive" token. Otherwise, `keep_alive` returns `true` + when either the [*Connection] field is absent, or if the + value does not contain the "close" token. + ] ][ [`a.get()`] [X::const_buffers_type] @@ -75,7 +96,8 @@ In this table: ] ] -Exemplar: +[heading Exemplar] + ``` struct FieldsReader { @@ -88,6 +110,14 @@ struct FieldsReader // Constructor for responses FieldsReader(F const& f, unsigned version, unsigned status); + // Returns `true` if the payload uses the chunked Transfer-Encoding + bool + chunked(); + + // Returns `true` if keep-alive is indicated + bool + keep_alive(); + // Returns the serialized header buffers const_buffers_type get(); diff --git a/include/beast/http/fields.hpp b/include/beast/http/fields.hpp index ffc12c8d..418cd989 100644 --- a/include/beast/http/fields.hpp +++ b/include/beast/http/fields.hpp @@ -555,15 +555,6 @@ public: } protected: - /// Returns `true` if the value for Connection has "close" in the list. - bool has_close_impl() const; - - /// Returns `true` if "chunked" is the last Transfer-Encoding - bool has_chunked_impl() const; - - /// Returns `true` if the Content-Length field is present - bool has_content_length_impl() const; - /** Set or clear the method string. @note Only called for requests. diff --git a/include/beast/http/impl/fields.ipp b/include/beast/http/impl/fields.ipp index 43cae160..64cf1efc 100644 --- a/include/beast/http/impl/fields.ipp +++ b/include/beast/http/impl/fields.ipp @@ -132,6 +132,12 @@ public: } }; + bool + get_chunked() const; + + bool + get_keep_alive(int version) const; + template void prepare(String& s, basic_fields const& f, @@ -146,6 +152,8 @@ public: static_string ss_; string_view sv_; std::string s_; + bool chunked_; + bool keep_alive_; public: using const_buffers_type = @@ -160,6 +168,18 @@ public: reader(basic_fields const& f, unsigned version, unsigned code); + bool + chunked() + { + return chunked_; + } + + bool + keep_alive() + { + return keep_alive_; + } + const_buffers_type get() const { @@ -170,6 +190,41 @@ public: } }; +template +bool +basic_fields::reader:: +get_chunked() const +{ + auto const te = token_list{ + f_[field::transfer_encoding]}; + for(auto it = te.begin(); it != te.end();) + { + auto next = std::next(it); + if(next == te.end()) + return iequals(*it, "chunked"); + it = next; + } + return false; +} + +template +bool +basic_fields::reader:: +get_keep_alive(int version) const +{ + if(version < 11) + { + auto const it = f_.find(field::connection); + if(it == f_.end()) + return false; + return token_list{it->value()}.exists("keep-alive"); + } + auto const it = f_.find(field::connection); + if(it == f_.end()) + return true; + return ! token_list{it->value()}.exists("close"); +} + template template void @@ -258,6 +313,8 @@ basic_fields::reader:: reader(basic_fields const& f, unsigned version, verb v) : f_(f) + , chunked_(get_chunked()) + , keep_alive_(get_keep_alive(version)) { try { @@ -276,6 +333,8 @@ basic_fields::reader:: reader(basic_fields const& f, unsigned version, unsigned code) : f_(f) + , chunked_(get_chunked()) + , keep_alive_(get_keep_alive(version)) { try { @@ -753,49 +812,6 @@ equal_range(string_view name) const -> // Fields -template -bool -basic_fields:: -has_close_impl() const -{ - auto const fit = set_.find( - to_string(field::connection), key_compare{}); - if(fit == set_.end()) - return false; - return token_list{fit->value()}.exists("close"); -} - -template -bool -basic_fields:: -has_chunked_impl() const -{ - auto const fit = set_.find(to_string( - field::transfer_encoding), key_compare{}); - if(fit == set_.end()) - return false; - token_list const v{fit->value()}; - auto it = v.begin(); - if(it == v.end()) - return false; - for(;;) - { - auto cur = it++; - if(it == v.end()) - return iequals(*cur, "chunked"); - } -} - -template -bool -basic_fields:: -has_content_length_impl() const -{ - auto const fit = set_.find( - to_string(field::content_length), key_compare{}); - return fit != set_.end(); -} - template inline void diff --git a/include/beast/http/impl/message.ipp b/include/beast/http/impl/message.ipp index cd43a277..4660b2a1 100644 --- a/include/beast/http/impl/message.ipp +++ b/include/beast/http/impl/message.ipp @@ -249,33 +249,6 @@ message(std::piecewise_construct_t, { } -template -inline -bool -message:: -has_close() const -{ - return this->has_close_impl(); -} - -template -inline -bool -message:: -has_chunked() const -{ - return this->has_chunked_impl(); -} - -template -inline -bool -message:: -has_content_length() const -{ - return this->has_content_length_impl(); -} - template boost::optional message:: diff --git a/include/beast/http/impl/serializer.ipp b/include/beast/http/impl/serializer.ipp index 8de92b67..22299a90 100644 --- a/include/beast/http/impl/serializer.ipp +++ b/include/beast/http/impl/serializer.ipp @@ -59,10 +59,8 @@ get(error_code& ec, Visit&& visit) { frdinit(std::integral_constant{}); - close_ = m_.has_close() || ( - m_.version < 11 && - ! m_.has_content_length()); - if(m_.has_chunked()) + close_ = ! frd_->keep_alive(); + if(frd_->chunked()) goto go_init_c; s_ = do_init; BOOST_FALLTHROUGH; diff --git a/include/beast/http/message.hpp b/include/beast/http/message.hpp index f30416d2..827f3618 100644 --- a/include/beast/http/message.hpp +++ b/include/beast/http/message.hpp @@ -483,24 +483,6 @@ struct message : header return *this; } - /// Returns `true` if "close" is specified in the Connection field. - bool - has_close() const; - - /// Returns `true` if "chunked" is the last Transfer-Encoding. - bool - has_chunked() const; - - /** Returns `true` if the Content-Length field is present. - - This function checks the fields to determine if the content - length field is present, regardless of the actual value. - - @note The contents of the body payload are not inspected. - */ - bool - has_content_length() const; - /** Returns the payload size of the body in octets if possible. This function invokes the @b Body algorithm to measure diff --git a/test/http/write.cpp b/test/http/write.cpp index e9fbda6c..6846eb88 100644 --- a/test/http/write.cpp +++ b/test/http/write.cpp @@ -303,7 +303,10 @@ public: str(message const& m) { test::string_ostream ss(ios_); - write(ss, m); + error_code ec; + write(ss, m, ec); + if(ec && ec != error::end_of_stream) + BOOST_THROW_EXCEPTION(system_error{ec}); return ss.str; } @@ -321,7 +324,7 @@ public: error_code ec; test::string_ostream ss{ios_}; async_write(ss, m, do_yield[ec]); - if(BEAST_EXPECTS(! ec, ec.message())) + if(BEAST_EXPECTS(ec == error::end_of_stream, ec.message())) BEAST_EXPECT(ss.str == "HTTP/1.0 200 OK\r\n" "Server: test\r\n" @@ -368,7 +371,8 @@ public: m.target("/"); m.version = 10; m.insert(field::user_agent, "test"); - m.insert("Content-Length", "5"); + m.set(field::connection, "keep-alive"); + m.set(field::content_length, "5"); m.body = "*****"; try { @@ -376,6 +380,7 @@ public: BEAST_EXPECT(fs.next_layer().str == "GET / HTTP/1.0\r\n" "User-Agent: test\r\n" + "Connection: keep-alive\r\n" "Content-Length: 5\r\n" "\r\n" "*****" @@ -465,7 +470,8 @@ public: m.target("/"); m.version = 10; m.insert(field::user_agent, "test"); - m.insert("Content-Length", "5"); + m.set(field::connection, "keep-alive"); + m.set(field::content_length, "5"); m.body = "*****"; error_code ec = test::error::fail_error; write(fs, m, ec); @@ -474,6 +480,7 @@ public: BEAST_EXPECT(fs.next_layer().str == "GET / HTTP/1.0\r\n" "User-Agent: test\r\n" + "Connection: keep-alive\r\n" "Content-Length: 5\r\n" "\r\n" "*****" @@ -493,7 +500,8 @@ public: m.target("/"); m.version = 10; m.insert(field::user_agent, "test"); - m.insert("Content-Length", "5"); + m.set(field::connection, "keep-alive"); + m.set(field::content_length, "5"); m.body = "*****"; error_code ec = test::error::fail_error; async_write(fs, m, do_yield[ec]); @@ -502,6 +510,7 @@ public: BEAST_EXPECT(fs.next_layer().str == "GET / HTTP/1.0\r\n" "User-Agent: test\r\n" + "Connection: keep-alive\r\n" "Content-Length: 5\r\n" "\r\n" "*****" @@ -858,10 +867,8 @@ public: void run() override { - yield_to([&](yield_context yield){ - testAsyncWrite(yield); }); - yield_to([&](yield_context yield){ - testFailures(yield); }); + yield_to([&](yield_context yield){ testAsyncWrite(yield); }); + yield_to([&](yield_context yield){ testFailures(yield); }); testOutput(); test_std_ostream(); testIoService(); diff --git a/test/websocket/stream.cpp b/test/websocket/stream.cpp index 76efc091..2c670a60 100644 --- a/test/websocket/stream.cpp +++ b/test/websocket/stream.cpp @@ -1004,7 +1004,7 @@ public: } }; // wrong version - check(error::handshake_failed, + check(http::error::end_of_stream, "GET / HTTP/1.0\r\n" "Host: localhost:80\r\n" "Upgrade: WebSocket\r\n"