From 70c1d361fd2b11d72139e44c95a340a44de830bb Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Thu, 27 Jul 2017 14:14:47 -0700 Subject: [PATCH] Refactor accept, handshake ops --- CHANGELOG.md | 1 + include/boost/beast/websocket/impl/accept.ipp | 534 +++++++++++------- .../boost/beast/websocket/impl/handshake.ipp | 32 ++ include/boost/beast/websocket/impl/stream.ipp | 68 --- 4 files changed, 350 insertions(+), 285 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a3a8c390..b683f67d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ Version 91: WebSocket: * Tidy up websocket javadocs +* Refactor accept, handshake ops -------------------------------------------------------------------------------- diff --git a/include/boost/beast/websocket/impl/accept.ipp b/include/boost/beast/websocket/impl/accept.ipp index 68394ab6..fcf6df32 100644 --- a/include/boost/beast/websocket/impl/accept.ipp +++ b/include/boost/beast/websocket/impl/accept.ipp @@ -38,38 +38,18 @@ class stream::response_op { struct data { - bool cont; stream& ws; response_type res; - int state = 0; + int step = 0; template data(Handler&, stream& ws_, http::request< Body, http::basic_fields> const& req, - Decorator const& decorator, bool cont_) - : cont(cont_) - , ws(ws_) + Decorator const& decorator) + : ws(ws_) , res(ws_.build_response(req, decorator)) { } - - template - data(Handler&, stream& ws_, http::request< - Body, http::basic_fields> const& req, - Buffers const& buffers, Decorator const& decorator, - bool cont_) - : cont(cont_) - , ws(ws_) - , res(ws_.build_response(req, decorator)) - { - using boost::asio::buffer_copy; - using boost::asio::buffer_size; - // VFALCO What about catch(std::length_error const&)? - ws.stream_.buffer().commit(buffer_copy( - ws.stream_.buffer().prepare( - buffer_size(buffers)), buffers)); - } }; handler_ptr d_; @@ -84,11 +64,12 @@ public: : d_(std::forward(h), ws, std::forward(args)...) { - (*this)(error_code{}, false); } - void operator()( - error_code ec, bool again = true); + template + void operator()(Buffers const& buffers); + + void operator()(error_code ec); friend void* asio_handler_allocate( @@ -111,7 +92,8 @@ public: friend bool asio_handler_is_continuation(response_op* op) { - return op->d_->cont; + // VFALCO This will go away in Net-TS + return false; } template @@ -126,36 +108,67 @@ public: template template +template void -stream::response_op:: -operator()(error_code ec, bool again) +stream:: +response_op:: +operator()(Buffers const& buffers) +{ + using boost::asio::buffer_copy; + using boost::asio::buffer_size; + auto& d = *d_; + error_code ec; + boost::optional mb; + auto const len = buffer_size(buffers); + try + { + mb.emplace(d.ws.stream_.buffer().prepare(len)); + } + catch(std::length_error const&) + { + d.step = 2; + ec = error::buffer_overflow; + return d.ws.get_io_service().post( + bind_handler(std::move(*this), ec)); + } + d.ws.stream_.buffer().commit( + buffer_copy(*mb, buffers)); + (*this)(ec); +} + +template +template +void +stream:: +response_op:: +operator()(error_code ec) { auto& d = *d_; - d.cont = d.cont || again; - while(! ec && d.state != 99) + switch(d.step) { - switch(d.state) - { - case 0: - // send response - d.state = 1; - http::async_write(d.ws.next_layer(), - d.res, std::move(*this)); - return; + case 0: + // send response + d.step = 1; + http::async_write(d.ws.next_layer(), + d.res, std::move(*this)); + return; - // sent response - case 1: - d.state = 99; - if(d.res.result() != - http::status::switching_protocols) - ec = error::handshake_failed; - if(! ec) - { - pmd_read(d.ws.pmd_config_, d.res); - d.ws.open(role_type::server); - } - break; + // sent response + case 1: + if(d.res.result() != + http::status::switching_protocols) + ec = error::handshake_failed; + if(! ec) + { + pmd_read(d.ws.pmd_config_, d.res); + d.ws.open(role_type::server); } + break; + + // call handler + case 2: + break; } d_.invoke(ec); } @@ -173,28 +186,13 @@ class stream::accept_op stream& ws; Decorator decorator; http::request_parser p; - + int step = 0; data(Handler&, stream& ws_, Decorator const& decorator_) : ws(ws_) , decorator(decorator_) { } - - template - data(Handler&, stream& ws_, - Buffers const& buffers, - Decorator const& decorator_) - : ws(ws_) - , decorator(decorator_) - { - using boost::asio::buffer_copy; - using boost::asio::buffer_size; - // VFALCO What about catch(std::length_error const&)? - ws.stream_.buffer().commit(buffer_copy( - ws.stream_.buffer().prepare( - buffer_size(buffers)), buffers)); - } }; handler_ptr d_; @@ -211,7 +209,8 @@ public: { } - void operator()(); + template + void operator()(Buffers const& buffers); void operator()(error_code ec); @@ -253,44 +252,73 @@ public: template template +template void -stream::accept_op:: -operator()() +stream:: +accept_op:: +operator()(Buffers const& buffers) { + using boost::asio::buffer_copy; + using boost::asio::buffer_size; auto& d = *d_; - http::async_read_header(d.ws.next_layer(), - d.ws.stream_.buffer(), d.p, - std::move(*this)); + error_code ec; + boost::optional mb; + auto const len = buffer_size(buffers); + try + { + mb.emplace(d.ws.stream_.buffer().prepare(len)); + } + catch(std::length_error const&) + { + d.step = 2; + ec = error::buffer_overflow; + return d.ws.get_io_service().post( + bind_handler(std::move(*this), ec)); + } + d.ws.stream_.buffer().commit( + buffer_copy(*mb, buffers)); + (*this)(ec); } template template void -stream::accept_op:: +stream:: +accept_op:: operator()(error_code ec) { auto& d = *d_; - if(! ec) + switch(d.step) { - BOOST_ASSERT(d.p.is_header_done()); - // Arguments from our state must be + case 0: + d.step = 1; + return http::async_read( + d.ws.next_layer(), d.ws.stream_.buffer(), + d.p, std::move(*this)); + + case 1: + { + if(ec) + break; + // Arguments from our step must be // moved to the stack before releasing // the handler. auto& ws = d.ws; auto const req = d.p.release(); auto const decorator = d.decorator; #if 1 - response_op{ + return response_op{ d_.release_handler(), - ws, req, decorator, true}; + ws, req, decorator}(ec); #else // VFALCO This *should* work but breaks // coroutine invariants in the unit test. // Also it calls reset() when it shouldn't. - ws.async_accept_ex( + return ws.async_accept_ex( req, decorator, d_.release_handler()); #endif - return; + } } d_.invoke(ec); } @@ -606,107 +634,126 @@ accept_ex(http::request -template -async_return_type< - AcceptHandler, void(error_code)> -stream:: -async_accept(AcceptHandler&& handler) -{ - static_assert(is_async_stream::value, - "AsyncStream requirements requirements not met"); - async_completion init{handler}; - reset(); - accept_op>{ - init.completion_handler, *this, &default_decorate_res}(); - return init.result.get(); -} - -template -template -async_return_type< - AcceptHandler, void(error_code)> -stream:: -async_accept_ex(ResponseDecorator const& decorator, - AcceptHandler&& handler) -{ - static_assert(is_async_stream::value, - "AsyncStream requirements requirements not met"); - static_assert(detail::is_ResponseDecorator< - ResponseDecorator>::value, - "ResponseDecorator requirements not met"); - async_completion init{handler}; - reset(); - accept_op>{ - init.completion_handler, *this, decorator}(); - return init.result.get(); -} - -template -template -typename std::enable_if< - ! http::detail::is_header::value, - async_return_type>::type -stream:: -async_accept(ConstBufferSequence const& buffers, - AcceptHandler&& handler) -{ - static_assert(is_async_stream::value, - "AsyncStream requirements requirements not met"); - static_assert(is_const_buffer_sequence< - ConstBufferSequence>::value, - "ConstBufferSequence requirements not met"); - async_completion init{handler}; - reset(); - accept_op>{ - init.completion_handler, *this, buffers, - &default_decorate_res}(); - return init.result.get(); -} - -template -template -typename std::enable_if< - ! http::detail::is_header::value, - async_return_type>::type -stream:: -async_accept_ex(ConstBufferSequence const& buffers, - ResponseDecorator const& decorator, - AcceptHandler&& handler) -{ - static_assert(is_async_stream::value, - "AsyncStream requirements requirements not met"); - static_assert(is_const_buffer_sequence< - ConstBufferSequence>::value, - "ConstBufferSequence requirements not met"); - static_assert(detail::is_ResponseDecorator< - ResponseDecorator>::value, - "ResponseDecorator requirements not met"); - async_completion init{handler}; - reset(); - accept_op>{ - init.completion_handler, *this, buffers, - decorator}(); - return init.result.get(); -} - -template -template -async_return_type< - AcceptHandler, void(error_code)> +async_return_type stream:: -async_accept(http::request> const& req, - AcceptHandler&& handler) +async_accept( + AcceptHandler&& handler) +{ + static_assert(is_async_stream::value, + "AsyncStream requirements requirements not met"); + async_completion init{handler}; + reset(); + accept_op< + decltype(&default_decorate_res), + handler_type>{ + init.completion_handler, + *this, + &default_decorate_res}({}); + return init.result.get(); +} + +template +template< + class ResponseDecorator, + class AcceptHandler> +async_return_type +stream:: +async_accept_ex( + ResponseDecorator const& decorator, + AcceptHandler&& handler) +{ + static_assert(is_async_stream::value, + "AsyncStream requirements requirements not met"); + static_assert(detail::is_ResponseDecorator< + ResponseDecorator>::value, + "ResponseDecorator requirements not met"); + async_completion init{handler}; + reset(); + accept_op< + ResponseDecorator, + handler_type>{ + init.completion_handler, + *this, + decorator}({}); + return init.result.get(); +} + +template +template< + class ConstBufferSequence, + class AcceptHandler> +typename std::enable_if< + ! http::detail::is_header::value, + async_return_type>::type +stream:: +async_accept( + ConstBufferSequence const& buffers, + AcceptHandler&& handler) +{ + static_assert(is_async_stream::value, + "AsyncStream requirements requirements not met"); + static_assert(is_const_buffer_sequence< + ConstBufferSequence>::value, + "ConstBufferSequence requirements not met"); + async_completion init{handler}; + reset(); + accept_op< + decltype(&default_decorate_res), + handler_type>{ + init.completion_handler, + *this, + &default_decorate_res}(buffers); + return init.result.get(); +} + +template +template< + class ConstBufferSequence, + class ResponseDecorator, + class AcceptHandler> +typename std::enable_if< + ! http::detail::is_header::value, + async_return_type>::type +stream:: +async_accept_ex( + ConstBufferSequence const& buffers, + ResponseDecorator const& decorator, + AcceptHandler&& handler) +{ + static_assert(is_async_stream::value, + "AsyncStream requirements requirements not met"); + static_assert(is_const_buffer_sequence< + ConstBufferSequence>::value, + "ConstBufferSequence requirements not met"); + static_assert(detail::is_ResponseDecorator< + ResponseDecorator>::value, + "ResponseDecorator requirements not met"); + async_completion init{handler}; + reset(); + accept_op< + ResponseDecorator, + handler_type>{ + init.completion_handler, + *this, + decorator}(buffers); + return init.result.get(); +} + +template +template< + class Body, class Allocator, + class AcceptHandler> +async_return_type +stream:: +async_accept( + http::request> const& req, + AcceptHandler&& handler) { static_assert(is_async_stream::value, "AsyncStream requirements requirements not met"); @@ -714,23 +761,26 @@ async_accept(http::request init{handler}; reset(); using boost::asio::asio_handler_is_continuation; - response_op>{init.completion_handler, - *this, req, &default_decorate_res, - asio_handler_is_continuation( - std::addressof(init.completion_handler))}; + response_op< + handler_type>{ + init.completion_handler, + *this, + req, + &default_decorate_res}({}); return init.result.get(); } template -template -async_return_type< - AcceptHandler, void(error_code)> +template< + class Body, class Allocator, + class ResponseDecorator, + class AcceptHandler> +async_return_type stream:: -async_accept_ex(http::request> const& req, - ResponseDecorator const& decorator, AcceptHandler&& handler) +async_accept_ex( + http::request> const& req, + ResponseDecorator const& decorator, + AcceptHandler&& handler) { static_assert(is_async_stream::value, "AsyncStream requirements requirements not met"); @@ -741,24 +791,26 @@ async_accept_ex(http::request init{handler}; reset(); using boost::asio::asio_handler_is_continuation; - response_op>{ - init.completion_handler, *this, req, decorator, - asio_handler_is_continuation( - std::addressof(init.completion_handler))}; + response_op< + handler_type>{ + init.completion_handler, + *this, + req, + decorator}({}); return init.result.get(); } template -template -async_return_type< - AcceptHandler, void(error_code)> +template< + class Body, class Allocator, + class ConstBufferSequence, + class AcceptHandler> +async_return_type stream:: -async_accept(http::request> const& req, - ConstBufferSequence const& buffers, - AcceptHandler&& handler) +async_accept( + http::request> const& req, + ConstBufferSequence const& buffers, + AcceptHandler&& handler) { static_assert(is_async_stream::value, "AsyncStream requirements requirements not met"); @@ -769,18 +821,21 @@ async_accept(http::request init{handler}; reset(); using boost::asio::asio_handler_is_continuation; - response_op>{ - init.completion_handler, *this, req, buffers, - &default_decorate_res, asio_handler_is_continuation( - std::addressof(init.completion_handler))}; + response_op< + handler_type>{ + init.completion_handler, + *this, + req, + &default_decorate_res}(buffers); return init.result.get(); } template -template +template< + class Body, class Allocator, + class ConstBufferSequence, + class ResponseDecorator, + class AcceptHandler> async_return_type< AcceptHandler, void(error_code)> stream:: @@ -802,13 +857,58 @@ async_accept_ex(http::request init{handler}; reset(); using boost::asio::asio_handler_is_continuation; - response_op>{init.completion_handler, - *this, req, buffers, decorator, asio_handler_is_continuation( - std::addressof(init.completion_handler))}; + response_op< + handler_type>{ + init.completion_handler, + *this, + req, + decorator}(buffers); return init.result.get(); } +//------------------------------------------------------------------------------ + +template +template +void +stream:: +do_accept( + Decorator const& decorator, + error_code& ec) +{ + http::request_parser p; + http::read(next_layer(), + stream_.buffer(), p, ec); + if(ec) + return; + do_accept(p.get(), decorator, ec); +} + +template +template +void +stream:: +do_accept( + http::request> const& req, + Decorator const& decorator, + error_code& ec) +{ + auto const res = build_response(req, decorator); + http::write(stream_, res, ec); + if(ec) + return; + if(res.result() != http::status::switching_protocols) + { + ec = error::handshake_failed; + // VFALCO TODO Respect keep alive setting, perform + // teardown if Connection: close. + return; + } + pmd_read(pmd_config_, req); + open(role_type::server); +} + } // websocket } // beast } // boost diff --git a/include/boost/beast/websocket/impl/handshake.ipp b/include/boost/beast/websocket/impl/handshake.ipp index aeb0bfbf..4c035295 100644 --- a/include/boost/beast/websocket/impl/handshake.ipp +++ b/include/boost/beast/websocket/impl/handshake.ipp @@ -391,6 +391,38 @@ handshake_ex(response_type& res, //------------------------------------------------------------------------------ +template +template +void +stream:: +do_handshake( + response_type* res_p, + string_view host, + string_view target, + RequestDecorator const& decorator, + error_code& ec) +{ + response_type res; + reset(); + detail::sec_ws_key_type key; + { + auto const req = build_request( + key, host, target, decorator); + pmd_read(pmd_config_, req); + http::write(stream_, req, ec); + } + if(ec) + return; + http::read(next_layer(), stream_.buffer(), res, ec); + if(ec) + return; + do_response(res, key, ec); + if(res_p) + *res_p = std::move(res); +} + +//------------------------------------------------------------------------------ + } // websocket } // beast } // boost diff --git a/include/boost/beast/websocket/impl/stream.ipp b/include/boost/beast/websocket/impl/stream.ipp index 0c345289..a480c7b1 100644 --- a/include/boost/beast/websocket/impl/stream.ipp +++ b/include/boost/beast/websocket/impl/stream.ipp @@ -818,74 +818,6 @@ build_response(http::request -template -void -stream:: -do_accept( - Decorator const& decorator, error_code& ec) -{ - http::request_parser p; - http::read_header(next_layer(), - stream_.buffer(), p, ec); - if(ec) - return; - do_accept(p.get(), decorator, ec); -} - -template -template -void -stream:: -do_accept(http::request> const& req, - Decorator const& decorator, error_code& ec) -{ - auto const res = build_response(req, decorator); - http::write(stream_, res, ec); - if(ec) - return; - if(res.result() != http::status::switching_protocols) - { - ec = error::handshake_failed; - // VFALCO TODO Respect keep alive setting, perform - // teardown if Connection: close. - return; - } - pmd_read(pmd_config_, req); - open(role_type::server); -} - -template -template -void -stream:: -do_handshake(response_type* res_p, - string_view host, - string_view target, - RequestDecorator const& decorator, - error_code& ec) -{ - response_type res; - reset(); - detail::sec_ws_key_type key; - { - auto const req = build_request( - key, host, target, decorator); - pmd_read(pmd_config_, req); - http::write(stream_, req, ec); - } - if(ec) - return; - http::read(next_layer(), stream_.buffer(), res, ec); - if(ec) - return; - do_response(res, key, ec); - if(res_p) - *res_p = std::move(res); -} - template void stream::