Parser concept, fixes:

A new concept Parser is introduced with routines to read from a stream
into the parser. This solves a problem with the old read interface where
messages must be default constructible and move assignable.

Parser fixes:

* Fix detect invalid reason-phrase octets
* Fix write_eof to set the 'complete' state on success
* Fix consider parse complete if eof received on empty body

WebSocket:

* Increase coverage

include/beast/http/impl/basic_parser_v1.ipp

@@ -9,6 +9,7 @@
 #define BEAST_HTTP_IMPL_BASIC_PARSER_V1_IPP
 
 #include <beast/core/buffer_concepts.hpp>
+#include <cassert>
 
 namespace beast {
 namespace http {
@@ -88,6 +89,11 @@ write(boost::asio::const_buffer const& buffer, error_code& ec)
         s_ = s_closed;
         return used();
     };
+    auto errc = [&]
+    {
+        s_ = s_closed;
+        return used();
+    };
     auto piece = [&]
     {
         return boost::string_ref{
@@ -113,6 +119,7 @@ write(boost::asio::const_buffer const& buffer, error_code& ec)
         switch(s_)
         {
         case s_closed:
+        case s_closed_complete:
             return err(parse_error::connection_closed);
             break;
 
@@ -126,6 +133,9 @@ write(boost::asio::const_buffer const& buffer, error_code& ec)
         case s_req_method_start:
             if(! is_token(ch))
                 return err(parse_error::bad_method);
+            call_on_start(ec);
+            if(ec)
+                return errc();
             cb_ = &self::call_on_method;
             s_ = s_req_method;
             break;
@@ -134,7 +144,7 @@ write(boost::asio::const_buffer const& buffer, error_code& ec)
             if(! is_token(ch))
             {
                 if(cb(nullptr))
-                    return used();
+                    return errc();
                 s_ = s_req_space_before_url;
                 goto redo;
             }
@@ -147,21 +157,23 @@ write(boost::asio::const_buffer const& buffer, error_code& ec)
             break;
 
         case s_req_url_start:
+        {
             if(ch == ' ')
                 return err(parse_error::bad_uri);
             // VFALCO TODO Better checking for valid URL characters
             if(! is_text(ch))
                 return err(parse_error::bad_uri);
-            if(cb(&self::call_on_uri))
-                return used();
+            assert(! cb_);
+            cb(&self::call_on_uri);
             s_ = s_req_url;
             break;
+        }
 
         case s_req_url:
             if(ch == ' ')
             {
                 if(cb(nullptr))
-                    return used();
+                    return errc();
                 s_ = s_req_http_start;
                 break;
             }
@@ -245,7 +257,7 @@ write(boost::asio::const_buffer const& buffer, error_code& ec)
                 return err(parse_error::bad_crlf);
             call_on_request(ec);
             if(ec)
-                return used();
+                return errc();
             s_ = s_header_field_start;
             break;
 
@@ -257,7 +269,14 @@ write(boost::asio::const_buffer const& buffer, error_code& ec)
             content_length_ = no_content_length;
             switch(ch)
             {
-            case 'H': s_ = s_res_H; break;
+            case 'H':
+                call_on_start(ec);
+                if(ec)
+                    return errc();
+                s_ = s_res_H;
+                break;
+            // VFALCO NOTE this allows whitespace at the beginning,
+            //        need to check rfc7230
             case '\r':
             case '\n':
                 break;
@@ -365,13 +384,16 @@ write(boost::asio::const_buffer const& buffer, error_code& ec)
                 s_ = s_res_line_almost_done;
                 break;
             }
+            // VFALCO Is this up to spec?
             if(ch == '\n')
             {
                 s_ = s_header_field_start;
                 break;
             }
+            if(! is_text(ch))
+                return err(parse_error::bad_status);
             if(cb(&self::call_on_reason))
-                return used();
+                return errc();
             pos_ = 0;
             s_ = s_res_status;
             break;
@@ -380,17 +402,19 @@ write(boost::asio::const_buffer const& buffer, error_code& ec)
             if(ch == '\r')
             {
                 if(cb(nullptr))
-                    return used();
+                    return errc();
                 s_ = s_res_line_almost_done;
                 break;
             }
             if(ch == '\n')
             {
                 if(cb(nullptr))
-                    return used();
+                    return errc();
                 s_ = s_header_field_start;
                 break;
             }
+            if(! is_text(ch))
+                return err(parse_error::bad_status);
             break;
 
         case s_res_line_almost_done:
@@ -402,7 +426,7 @@ write(boost::asio::const_buffer const& buffer, error_code& ec)
         case s_res_line_done:
             call_on_response(ec);
             if(ec)
-                return used();
+                return errc();
             s_ = s_header_field_start;
             goto redo;
 
@@ -431,8 +455,8 @@ write(boost::asio::const_buffer const& buffer, error_code& ec)
                 fs_ = h_general;
                 break;
             }
-            if(cb(&self::call_on_field))
-                return used();
+            assert(! cb_);
+            cb(&self::call_on_field);
             s_ = s_header_field;
             break;
         }
@@ -529,7 +553,7 @@ write(boost::asio::const_buffer const& buffer, error_code& ec)
             if(ch == ':')
             {
                 if(cb(nullptr))
-                    return used();
+                    return errc();
                 s_ = s_header_value_start;
                 break;
             }
@@ -579,7 +603,7 @@ write(boost::asio::const_buffer const& buffer, error_code& ec)
             }
             call_on_value(ec, boost::string_ref{"", 0});
             if(ec)
-                return used();
+                return errc();
             s_ = s_header_field_start;
             goto redo;
 
@@ -629,7 +653,7 @@ write(boost::asio::const_buffer const& buffer, error_code& ec)
             }
             pos_ = 0;
             if(cb(&self::call_on_value))
-                return used();
+                return errc();
             s_ = s_header_value_text;
             break;
         }
@@ -641,7 +665,7 @@ write(boost::asio::const_buffer const& buffer, error_code& ec)
                 if(ch == '\r')
                 {
                     if(cb(nullptr))
-                        return used();
+                        return errc();
                     s_ = s_header_value_discard_lWs;
                     break;
                 }
@@ -775,9 +799,9 @@ write(boost::asio::const_buffer const& buffer, error_code& ec)
                 return err(parse_error::bad_value);
             call_on_value(ec, boost::string_ref(" ", 1));
             if(ec)
-                return used();
+                return errc();
             if(cb(&self::call_on_value))
-                return used();
+                return errc();
             s_ = s_header_value_text;
             break;
 
@@ -811,7 +835,7 @@ write(boost::asio::const_buffer const& buffer, error_code& ec)
                 return err(parse_error::bad_crlf);
             if(flags_ & parse_flag::trailing)
             {
-                //if(cb(&self::call_on_chunk_complete)) return used();
+                //if(cb(&self::call_on_chunk_complete)) return errc();
                 s_ = s_complete;
                 goto redo;
             }
@@ -821,7 +845,7 @@ write(boost::asio::const_buffer const& buffer, error_code& ec)
                 (parse_flag::upgrade | parse_flag::connection_upgrade)) /*|| method == "connect"*/;
             auto const maybe_skip = call_on_headers(ec);
             if(ec)
-                return used();
+                return errc();
             switch(maybe_skip)
             {
             case 0: break;
@@ -839,7 +863,7 @@ write(boost::asio::const_buffer const& buffer, error_code& ec)
             assert(! cb_);
             call_on_headers(ec);
             if(ec)
-                return used();
+                return errc();
             bool const hasBody =
                 (flags_ & parse_flag::chunked) || (content_length_ > 0 &&
                     content_length_ != no_content_length);
@@ -878,8 +902,8 @@ write(boost::asio::const_buffer const& buffer, error_code& ec)
         }
 
         case s_body_identity0:
-            if(cb(&self::call_on_body))
-                return used();
+            assert(! cb_);
+            cb(&self::call_on_body);
             s_ = s_body_identity;
             goto redo; // VFALCO fall through?
 
@@ -903,8 +927,8 @@ write(boost::asio::const_buffer const& buffer, error_code& ec)
         }
 
         case s_body_identity_eof0:
-            if(cb(&self::call_on_body))
-                return used();
+            assert(! cb_);
+            cb(&self::call_on_body);
             s_ = s_body_identity_eof;
             goto redo; // VFALCO fall through?
 
@@ -963,13 +987,13 @@ write(boost::asio::const_buffer const& buffer, error_code& ec)
                 s_ = s_header_field_start;
                 break;
             }
-            //call_chunk_header(ec); if(ec) return used();
+            //call_chunk_header(ec); if(ec) return errc();
             s_ = s_chunk_data_start;
             break;
 
         case s_chunk_data_start:
-            if(cb(&self::call_on_body))
-                return used();
+            assert(! cb_);
+            cb(&self::call_on_body);
             s_ = s_chunk_data;
             goto redo; // VFALCO fall through?
 
@@ -991,7 +1015,7 @@ write(boost::asio::const_buffer const& buffer, error_code& ec)
             if(ch != '\r')
                 return err(parse_error::bad_crlf);
             if(cb(nullptr))
-                return used();
+                return errc();
             s_ = s_chunk_data_done;
             break;
 
@@ -1005,10 +1029,10 @@ write(boost::asio::const_buffer const& buffer, error_code& ec)
         case s_complete:
             ++p;
             if(cb(nullptr))
-                return used();
+                return errc();
             call_on_complete(ec);
             if(ec)
-                return used();
+                return errc();
             s_ = s_restart;
             return used();
 
@@ -1024,7 +1048,7 @@ write(boost::asio::const_buffer const& buffer, error_code& ec)
     {
         (this->*cb_)(ec, piece());
         if(ec)
-            return used();
+            return errc();
     }
     return used();
 }
@@ -1036,17 +1060,31 @@ write_eof(error_code& ec)
 {
     switch(s_)
     {
+    case s_restart:
+        s_ = s_closed_complete;
+        break;
+
+    case s_closed:
+    case s_closed_complete:
+        break;
+
+    case s_body_identity_eof0:
     case s_body_identity_eof:
         cb_ = nullptr;
         call_on_complete(ec);
         if(ec)
-            return;
-        return;
+        {
+            s_ = s_closed;
+            break;
+        }
+        s_ = s_closed_complete;
+        break;
+
     default:
+        s_ = s_closed;
+        ec = parse_error::short_read;
         break;
     }
-    ec = parse_error::short_read;
-    s_ = s_closed;
 }
 
 template<bool isRequest, class Derived>

include/beast/http/impl/read.ipp

@@ -9,6 +9,7 @@
 #define BEAST_HTTP_IMPL_READ_IPP_HPP
 
 #include <beast/http/parser_v1.hpp>
+#include <beast/http/type_check.hpp>
 #include <beast/core/bind_handler.hpp>
 #include <beast/core/handler_alloc.hpp>
 #include <beast/core/stream_concepts.hpp>
@@ -19,6 +20,185 @@ namespace http {
 
 namespace detail {
 
+template<class Stream,
+    class Streambuf, class Parser, class Handler>
+class parse_op
+{
+    using alloc_type =
+        handler_alloc<char, Handler>;
+
+    struct data
+    {
+        Stream& s;
+        Streambuf& sb;
+        Parser& p;
+        Handler h;
+        bool started = false;
+        bool cont;
+        int state = 0;
+
+        template<class DeducedHandler>
+        data(DeducedHandler&& h_, Stream& s_,
+                Streambuf& sb_, Parser& p_)
+            : s(s_)
+            , sb(sb_)
+            , p(p_)
+            , h(std::forward<DeducedHandler>(h_))
+            , cont(boost_asio_handler_cont_helpers::
+                is_continuation(h))
+        {
+        }
+    };
+
+    std::shared_ptr<data> d_;
+
+public:
+    parse_op(parse_op&&) = default;
+    parse_op(parse_op const&) = default;
+
+    template<class DeducedHandler, class... Args>
+    parse_op(DeducedHandler&& h, Stream& s, Args&&... args)
+        : d_(std::allocate_shared<data>(alloc_type{h},
+            std::forward<DeducedHandler>(h), s,
+                std::forward<Args>(args)...))
+    {
+        (*this)(error_code{}, 0, false);
+    }
+
+    void
+    operator()(error_code ec,
+        std::size_t bytes_transferred, bool again = true);
+
+    friend
+    void* asio_handler_allocate(
+        std::size_t size, parse_op* op)
+    {
+        return boost_asio_handler_alloc_helpers::
+            allocate(size, op->d_->h);
+    }
+
+    friend
+    void asio_handler_deallocate(
+        void* p, std::size_t size, parse_op* op)
+    {
+        return boost_asio_handler_alloc_helpers::
+            deallocate(p, size, op->d_->h);
+    }
+
+    friend
+    bool asio_handler_is_continuation(parse_op* op)
+    {
+        return op->d_->cont;
+    }
+
+    template <class Function>
+    friend
+    void asio_handler_invoke(Function&& f, parse_op* op)
+    {
+        return boost_asio_handler_invoke_helpers::
+            invoke(f, op->d_->h);
+    }
+};
+
+template<class Stream,
+    class Streambuf, class Parser, class Handler>
+void
+parse_op<Stream, Streambuf, Parser, Handler>::
+operator()(error_code ec, std::size_t bytes_transferred, bool again)
+{
+    auto& d = *d_;
+    d.cont = d.cont || again;
+    while(d.state != 99)
+    {
+        switch(d.state)
+        {
+        case 0:
+        {
+            auto const used =
+                d.p.write(d.sb.data(), ec);
+            if(ec)
+            {
+                // call handler
+                d.state = 99;
+                d.s.get_io_service().post(
+                    bind_handler(std::move(*this), ec, 0));
+                return;
+            }
+            if(used > 0)
+                d.started = true;
+            d.sb.consume(used);
+            if(d.p.complete())
+            {
+                // call handler
+                d.state = 99;
+                d.s.get_io_service().post(
+                    bind_handler(std::move(*this), ec, 0));
+                return;
+            }
+            d.state = 1;
+            break;
+        }
+
+        case 1:
+            // read
+            d.state = 2;
+            d.s.async_read_some(d.sb.prepare(
+                read_size_helper(d.sb, 65536)),
+                    std::move(*this));
+            return;
+
+        // got data
+        case 2:
+        {
+            if(ec == boost::asio::error::eof)
+            {
+                if(! d.started)
+                {
+                    // call handler
+                    d.state = 99;
+                    break;
+                }
+                // Caller will see eof on next read.
+                ec = {};
+                d.p.write_eof(ec);
+                assert(ec || d.p.complete());
+                // call handler
+                d.state = 99;
+                break;
+            }
+            if(ec)
+            {
+                // call handler
+                d.state = 99;
+                break;
+            }
+            d.sb.commit(bytes_transferred);
+            auto const used = d.p.write(d.sb.data(), ec);
+            if(ec)
+            {
+                // call handler
+                d.state = 99;
+                break;
+            }
+            if(used > 0)
+                d.started = true;
+            d.sb.consume(used);
+            if(d.p.complete())
+            {
+                // call handler
+                d.state = 99;
+                break;
+            }
+            d.state = 1;
+            break;
+        }
+        }
+    }
+    d.h(ec);
+}
+
+//------------------------------------------------------------------------------
+
 template<class Stream, class Streambuf,
     bool isRequest, class Body, class Headers,
         class Handler>
@@ -69,12 +249,11 @@ public:
             std::forward<DeducedHandler>(h), s,
                 std::forward<Args>(args)...))
     {
-        (*this)(error_code{}, 0, false);
+        (*this)(error_code{}, false);
     }
 
     void
-    operator()(error_code ec,
-        std::size_t bytes_transferred, bool again = true);
+    operator()(error_code ec, bool again = true);
 
     friend
     void* asio_handler_allocate(
@@ -112,98 +291,25 @@ template<class Stream, class Streambuf,
         class Handler>
 void
 read_op<Stream, Streambuf, isRequest, Body, Headers, Handler>::
-operator()(error_code ec, std::size_t bytes_transferred, bool again)
+operator()(error_code ec, bool again)
 {
     auto& d = *d_;
     d.cont = d.cont || again;
-    while(d.state != 99)
+    while(! ec && d.state != 99)
     {
         switch(d.state)
         {
         case 0:
-        {
-            auto const used =
-                d.p.write(d.sb.data(), ec);
-            if(ec)
-            {
-                // call handler
-                d.state = 99;
-                d.s.get_io_service().post(
-                    bind_handler(std::move(*this), ec, 0));
-                return;
-            }
-            if(used > 0)
-                d.started = true;
-            d.sb.consume(used);
-            if(d.p.complete())
-            {
-                // call handler
-                d.state = 99;
-                d.m = d.p.release();
-                d.s.get_io_service().post(
-                    bind_handler(std::move(*this), ec, 0));
-                return;
-            }
             d.state = 1;
-            break;
-        }
-
-        case 1:
-            // read
-            d.state = 2;
-            d.s.async_read_some(d.sb.prepare(
-                read_size_helper(d.sb, 65536)),
-                    std::move(*this));
+            async_parse(d.s, d.sb, d.p, std::move(*this));
             return;
 
-        // got data
-        case 2:
-        {
-            if(ec == boost::asio::error::eof)
-            {
-                if(! d.started)
-                {
-                    // call handler
-                    d.state = 99;
-                    break;
-                }
-                // Caller will see eof on next read.
-                ec = {};
-                d.p.write_eof(ec);
-                if(! ec)
-                {
-                    assert(d.p.complete());
-                    d.m = d.p.release();
-                }
-                // call handler
-                d.state = 99;
-                break;
-            }
-            if(ec)
-            {
-                // call handler
-                d.state = 99;
-                break;
-            }
-            d.sb.commit(bytes_transferred);
-            d.sb.consume(d.p.write(d.sb.data(), ec));
-            if(ec)
-            {
-                // call handler
-                d.state = 99;
-                break;
-            }
-            if(d.p.complete())
-            {
-                // call handler
-                d.state = 99;
-                d.m = d.p.release();
-                break;
-            }
-            d.state = 1;
+        case 1:
+            // call handler
+            d.state = 99;
+            d.m = d.p.release();
             break;
         }
-        }
     }
     d.h(ec);
 }
@@ -212,12 +318,91 @@ operator()(error_code ec, std::size_t bytes_transferred, bool again)
 
 //------------------------------------------------------------------------------
 
+template<class SyncReadStream, class Streambuf, class Parser>
+void
+parse(SyncReadStream& stream,
+    Streambuf& streambuf, Parser& parser)
+{
+    error_code ec;
+    parse(stream, streambuf, parser, ec);
+    if(ec)
+        throw boost::system::system_error{ec};
+}
+
+template<class SyncReadStream, class Streambuf, class Parser>
+void
+parse(SyncReadStream& stream, Streambuf& streambuf,
+    Parser& parser, error_code& ec)
+{
+    static_assert(is_SyncReadStream<SyncReadStream>::value,
+        "SyncReadStream requirements not met");
+    static_assert(is_Streambuf<Streambuf>::value,
+        "Streambuf requirements not met");
+    static_assert(is_Parser<Parser>::value,
+        "Parser requirements not met");
+    bool started = false;
+    for(;;)
+    {
+        auto used =
+            parser.write(streambuf.data(), ec);
+        if(ec)
+            return;
+        streambuf.consume(used);
+        if(used > 0)
+            started = true;
+        if(parser.complete())
+            break;
+        streambuf.commit(stream.read_some(
+            streambuf.prepare(read_size_helper(
+                streambuf, 65536)), ec));
+        if(ec && ec != boost::asio::error::eof)
+            return;
+        if(ec == boost::asio::error::eof)
+        {
+            if(! started)
+                return;
+            // Caller will see eof on next read.
+            ec = {};
+            parser.write_eof(ec);
+            if(ec)
+                return;
+            assert(parser.complete());
+            break;
+        }
+    }
+}
+
+template<class AsyncReadStream,
+    class Streambuf, class Parser, class ReadHandler>
+typename async_completion<
+    ReadHandler, void(error_code)>::result_type
+async_parse(AsyncReadStream& stream,
+    Streambuf& streambuf, Parser& parser, ReadHandler&& handler)
+{
+    static_assert(is_AsyncReadStream<AsyncReadStream>::value,
+        "AsyncReadStream requirements not met");
+    static_assert(is_Streambuf<Streambuf>::value,
+        "Streambuf requirements not met");
+    static_assert(is_Parser<Parser>::value,
+        "Parser requirements not met");
+    beast::async_completion<ReadHandler,
+        void(error_code)> completion(handler);
+    detail::parse_op<AsyncReadStream, Streambuf,
+        Parser, decltype(completion.handler)>{
+            completion.handler, stream, streambuf, parser};
+    return completion.result.get();
+}
+
 template<class SyncReadStream, class Streambuf,
     bool isRequest, class Body, class Headers>
 void
 read(SyncReadStream& stream, Streambuf& streambuf,
     message_v1<isRequest, Body, Headers>& msg)
 {
+    static_assert(is_SyncReadStream<SyncReadStream>::value,
+        "SyncReadStream requirements not met");
+    static_assert(is_Streambuf<Streambuf>::value,
+        "Streambuf requirements not met");
     error_code ec;
     read(stream, streambuf, msg, ec);
     if(ec)
@@ -236,40 +421,11 @@ read(SyncReadStream& stream, Streambuf& streambuf,
     static_assert(is_Streambuf<Streambuf>::value,
         "Streambuf requirements not met");
     parser_v1<isRequest, Body, Headers> p;
-    bool started = false;
-    for(;;)
-    {
-        auto used =
-            p.write(streambuf.data(), ec);
-        if(ec)
-            return;
-        streambuf.consume(used);
-        if(used > 0)
-            started = true;
-        if(p.complete())
-        {
-            m = p.release();
-            break;
-        }
-        streambuf.commit(stream.read_some(
-            streambuf.prepare(read_size_helper(
-                streambuf, 65536)), ec));
-        if(ec && ec != boost::asio::error::eof)
-            return;
-        if(ec == boost::asio::error::eof)
-        {
-            if(! started)
-                return;
-            // Caller will see eof on next read.
-            ec = {};
-            p.write_eof(ec);
-            if(ec)
-                return;
-            assert(p.complete());
-            m = p.release();
-            break;
-        }
-    }
+    parse(stream, streambuf, p, ec);
+    if(ec)
+        return;
+    assert(p.complete());
+    m = p.release();
 }
 
 template<class AsyncReadStream, class Streambuf,