From dfeb9967b8c9dc6c085fbac6553d8200db758b3c Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Sun, 12 Oct 2014 19:10:33 -0700 Subject: [PATCH] Return error_code from beast::http::basic_parser: This changes the HTTP parser interface to return an error_code instead of a bool. This eliminates the need for the error() member function and simplifies calling code. --- src/beast/beast/http/basic_parser.h | 49 +++++++++++----------- src/beast/beast/http/impl/basic_parser.cpp | 25 +++++------ src/beast/beast/http/tests/parser.test.cpp | 10 ++--- src/ripple/http/impl/Peer.h | 12 +++--- src/ripple/overlay/impl/PeerImp.cpp | 11 +---- 5 files changed, 48 insertions(+), 59 deletions(-) diff --git a/src/beast/beast/http/basic_parser.h b/src/beast/beast/http/basic_parser.h index 208ed3c09..769bfbf9d 100644 --- a/src/beast/beast/http/basic_parser.h +++ b/src/beast/beast/http/basic_parser.h @@ -111,16 +111,12 @@ public: return complete_; } - /** Returns the error if write or write_eof returned false. */ - error_code - error() const noexcept; - /** Write data to the parser. @param data A buffer containing the data to write @param bytes The size of the buffer pointed to by data. @return A pair with bool success, and the number of bytes consumed. */ - std::pair + std::pair write (void const* data, std::size_t bytes); /** Write a set of buffer data to the parser. @@ -131,24 +127,8 @@ public: @return A pair with bool success, and the number of bytes consumed. */ template - std::pair - write (ConstBufferSequence const& buffers) - { - std::pair result (true, 0); - for (auto const& buffer : buffers) - { - std::size_t bytes_consumed; - std::tie (result.first, bytes_consumed) = - write (boost::asio::buffer_cast (buffer), - boost::asio::buffer_size (buffer)); - if (! result.first) - break; - result.second += bytes_consumed; - if (complete()) - break; - } - return result; - } + std::pair + write (ConstBufferSequence const& buffers); /** Called to indicate the end of file. HTTP needs to know where the end of the stream is. For example, @@ -158,7 +138,7 @@ public: @note This is typically called when a socket read returns eof. @return `true` if the message is complete. */ - bool + error_code write_eof(); protected: @@ -253,6 +233,27 @@ private: static int cb_message_complete (joyent::http_parser*); }; +template +auto +basic_parser::write (ConstBufferSequence const& buffers) -> + std::pair +{ + std::pair result ({}, 0); + for (auto const& buffer : buffers) + { + std::size_t bytes_consumed; + std::tie (result.first, bytes_consumed) = + write (boost::asio::buffer_cast (buffer), + boost::asio::buffer_size (buffer)); + if (result.first) + break; + result.second += bytes_consumed; + if (complete()) + break; + } + return result; +} + } // http } // beast diff --git a/src/beast/beast/http/impl/basic_parser.cpp b/src/beast/beast/http/impl/basic_parser.cpp index e34992bc5..8d9aefac7 100644 --- a/src/beast/beast/http/impl/basic_parser.cpp +++ b/src/beast/beast/http/impl/basic_parser.cpp @@ -114,32 +114,29 @@ basic_parser::operator= (basic_parser&& other) return *this; } -basic_parser::error_code -basic_parser::error() const noexcept +auto +basic_parser::write (void const* data, std::size_t bytes) -> + std::pair { - auto s (reinterpret_cast (&state_)); - return error_code{static_cast(s->http_errno), message_category()}; -} - -std::pair -basic_parser::write (void const* data, std::size_t bytes) -{ - std::pair result (false, 0); + std::pair result ({}, 0); auto s (reinterpret_cast (&state_)); auto h (reinterpret_cast (&hooks_)); result.second = joyent::http_parser_execute (s, h, static_cast (data), bytes); - result.first = s->http_errno == 0; + result.first = error_code{static_cast(s->http_errno), + message_category()}; return result; } -bool -basic_parser::write_eof() +auto +basic_parser::write_eof() -> + error_code { auto s (reinterpret_cast (&state_)); auto h (reinterpret_cast (&hooks_)); joyent::http_parser_execute (s, h, nullptr, 0); - return s->http_errno == 0; + return error_code{static_cast(s->http_errno), + message_category()}; } //------------------------------------------------------------------------------ diff --git a/src/beast/beast/http/tests/parser.test.cpp b/src/beast/beast/http/tests/parser.test.cpp index d9e53e463..d81946383 100644 --- a/src/beast/beast/http/tests/parser.test.cpp +++ b/src/beast/beast/http/tests/parser.test.cpp @@ -67,9 +67,9 @@ public: message m; parser p (m, true); auto result (p.write (boost::asio::buffer(text))); - expect (result.first); + expect (! result.first); auto result2 (p.write_eof()); - expect (result2); + expect (! result2); expect (p.complete()); } @@ -81,9 +81,9 @@ public: ; message m; parser p (m, true); - auto result (p.write (boost::asio::buffer(text))); - if (expect (! result.first)) - expect (p.error().message() == "invalid HTTP method"); + auto result = p.write (boost::asio::buffer(text)); + if (expect (result.first)) + expect (result.first.message() == "invalid HTTP method"); } } }; diff --git a/src/ripple/http/impl/Peer.h b/src/ripple/http/impl/Peer.h index b976402e6..76792c71c 100644 --- a/src/ripple/http/impl/Peer.h +++ b/src/ripple/http/impl/Peer.h @@ -514,16 +514,14 @@ Peer::do_read (boost::asio::yield_context yield) // should request that the handler compose a proper HTTP error // response. This requires refactoring HTTPReply() into // something sensible. - auto const result = parser.write (read_buf_.data()); - if (result.first) - read_buf_.consume (result.second); - else - ec = parser.error(); + std::size_t used; + std::tie (ec, used) = parser.write (read_buf_.data()); + if (! ec) + read_buf_.consume (used); } else { - if (! parser.write_eof()) - ec = parser.error(); + ec = parser.write_eof(); } } diff --git a/src/ripple/overlay/impl/PeerImp.cpp b/src/ripple/overlay/impl/PeerImp.cpp index 650e021c9..08bd19b3a 100644 --- a/src/ripple/overlay/impl/PeerImp.cpp +++ b/src/ripple/overlay/impl/PeerImp.cpp @@ -447,13 +447,9 @@ PeerImp::on_read_http_response (error_code ec, std::size_t bytes_transferred) if (! ec) { read_buffer_.commit (bytes_transferred); - bool success; std::size_t bytes_consumed; - std::tie (success, bytes_consumed) = http_parser_->write ( + std::tie (ec, bytes_consumed) = http_parser_->write ( read_buffer_.data()); - if (! success) - ec = http_parser_->error(); - if (! ec) { read_buffer_.consume (bytes_consumed); @@ -597,12 +593,9 @@ PeerImp::on_read_http_request (error_code ec, std::size_t bytes_transferred) if (! ec) { read_buffer_.commit (bytes_transferred); - bool success; std::size_t bytes_consumed; - std::tie (success, bytes_consumed) = http_parser_->write ( + std::tie (ec, bytes_consumed) = http_parser_->write ( read_buffer_.data()); - if (! success) - ec = http_parser_->error(); if (! ec) {