From 78f4858e988a6d0bd22f086d5f145ab1b0687510 Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Sat, 10 Jun 2017 10:12:29 -0700 Subject: [PATCH] Fix read_size_helper usage: read_size_helper can return zero if the buffer reaches its maximum size, causing infinite loops in HTTP. The function maybe_read_size_helper is provided to throw an exception instead of returning a value for this case. --- CHANGELOG.md | 1 + include/beast/core/detail/ostream.hpp | 6 ++-- .../beast/core/detail/read_size_helper.hpp | 36 ++++++++++++++++++- include/beast/http/impl/read.ipp | 7 ++-- .../beast/websocket/detail/pmd_extension.hpp | 3 +- test/core/flat_buffer.cpp | 15 +++----- 6 files changed, 47 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b14ad70d..ba709f93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ Version 53: * Fix basic_parser::maybe_flatten +* Fix read_size_helper usage -------------------------------------------------------------------------------- diff --git a/include/beast/core/detail/ostream.hpp b/include/beast/core/detail/ostream.hpp index 24828cd6..d5d4cb38 100644 --- a/include/beast/core/detail/ostream.hpp +++ b/include/beast/core/detail/ostream.hpp @@ -128,9 +128,8 @@ private: { using boost::asio::buffer_cast; using boost::asio::buffer_size; - using beast::detail::read_size_helper; auto mbs = buf_.prepare( - read_size_helper(buf_, max_size)); + maybe_read_size_helper(buf_, max_size)); auto const mb = *mbs.begin(); auto const p = buffer_cast(mb); this->setp(p, @@ -209,9 +208,8 @@ private: { using boost::asio::buffer_cast; using boost::asio::buffer_size; - using beast::detail::read_size_helper; auto mbs = buf_.prepare( - read_size_helper(buf_, max_size)); + maybe_read_size_helper(buf_, max_size)); auto const mb = *mbs.begin(); auto const p = buffer_cast(mb); this->setp(p, diff --git a/include/beast/core/detail/read_size_helper.hpp b/include/beast/core/detail/read_size_helper.hpp index 9087432e..92f9aa8d 100644 --- a/include/beast/core/detail/read_size_helper.hpp +++ b/include/beast/core/detail/read_size_helper.hpp @@ -17,9 +17,27 @@ namespace beast { namespace detail { +/** Returns a natural read size. + + This function inspects the capacity, size, and maximum + size of the dynamic buffer. Then it computes a natural + read size given the passed-in upper limit. It favors + a read size that does not require a reallocation, subject + to a reasonable minimum to avoid tiny reads. + + Calls to @ref read_size_helper should be made without + namespace qualification, i.e. allowing argument dependent + lookup to take effect, so that overloads of this function + for specific buffer types may be found. + + @param buffer The dynamic buffer to inspect. + + @param max_size An upper limit on the returned value. +*/ template std::size_t -read_size_helper(DynamicBuffer const& buffer, std::size_t max_size) +read_size_helper( + DynamicBuffer const& buffer, std::size_t max_size) { static_assert(beast::is_dynamic_buffer::value, "DynamicBuffer requirements not met"); @@ -34,6 +52,22 @@ read_size_helper(DynamicBuffer const& buffer, std::size_t max_size) std::min(max_size, limit)); } +/** Return a non-zero natural read size. +*/ +template +std::size_t +maybe_read_size_helper( + DynamicBuffer const& buffer, std::size_t max_size) +{ + static_assert(beast::is_dynamic_buffer::value, + "DynamicBuffer requirements not met"); + auto const n = read_size_helper(buffer, max_size); + if(n == 0) + BOOST_THROW_EXCEPTION(std::length_error{ + "buffer overflow"}); + return n; +} + } // detail } // beast diff --git a/include/beast/http/impl/read.ipp b/include/beast/http/impl/read.ipp index 06f354aa..e99aa52d 100644 --- a/include/beast/http/impl/read.ipp +++ b/include/beast/http/impl/read.ipp @@ -145,9 +145,8 @@ operator()(error_code ec, std::size_t bytes_transferred) do_read: try { - using beast::detail::read_size_helper; mb_.emplace(b_.prepare( - read_size_helper(b_, 65536))); + maybe_read_size_helper(b_, 65536))); } catch(std::length_error const&) { @@ -475,9 +474,9 @@ read_some( DynamicBuffer::mutable_buffers_type> b; try { - using beast::detail::read_size_helper; b.emplace(buffer.prepare( - read_size_helper(buffer, 65536))); + beast::detail::maybe_read_size_helper( + buffer, 65536))); } catch(std::length_error const&) { diff --git a/include/beast/websocket/detail/pmd_extension.hpp b/include/beast/websocket/detail/pmd_extension.hpp index 489a7f38..dab7adcf 100644 --- a/include/beast/websocket/detail/pmd_extension.hpp +++ b/include/beast/websocket/detail/pmd_extension.hpp @@ -369,9 +369,8 @@ inflate( for(;;) { // VFALCO we could be smarter about the size - using beast::detail::read_size_helper; auto const bs = buffer.prepare( - read_size_helper(buffer, 65536)); + maybe_read_size_helper(buffer, 65536)); auto const out = *bs.begin(); zs.avail_out = buffer_size(out); zs.next_out = buffer_cast(out); diff --git a/test/core/flat_buffer.cpp b/test/core/flat_buffer.cpp index b336033a..d4a2b896 100644 --- a/test/core/flat_buffer.cpp +++ b/test/core/flat_buffer.cpp @@ -143,16 +143,11 @@ public: BEAST_EXPECT(b.size() == 1); } { - flat_buffer b{10}; - try - { - b.reserve(11); - fail("", __FILE__, __LINE__); - } - catch(std::length_error const&) - { - pass(); - } + flat_buffer b{20}; + ostream(b) << "12345"; + b.consume(3); + ostream(b) << "67890123"; + BEAST_EXPECT(to_string(b.data()) == "4567890123"); } }