Removes unnecessary dynamic buffer.

Author: mzimbres

include/boost/redis/config.hpp

@@ -10,6 +10,7 @@
 #include <string>
 #include <chrono>
 #include <optional>
+#include <limits>
 
 namespace boost::redis
 {
@@ -78,6 +79,13 @@ struct config {
     *  To disable reconnection pass zero as duration.
     */
    std::chrono::steady_clock::duration reconnect_wait_interval = std::chrono::seconds{1};
+
+   /** @brief Maximum size of a socket read.
+    *  
+    *  Sets a limit on how much data is allowed to be read into the
+    *  read buffer. It can be used to prevent DDOS.
+    */
+   std::size_t max_read_size = (std::numeric_limits<std::size_t>::max)();
 };
 
 } // boost::redis

include/boost/redis/connection.hpp

@@ -50,7 +50,6 @@
 #include <cstddef>
 #include <deque>
 #include <functional>
-#include <limits>
 #include <memory>
 #include <string_view>
 #include <utility>
@@ -59,13 +58,6 @@ namespace boost::redis {
 namespace detail
 {
 
-template <class DynamicBuffer>
-std::string_view buffer_view(DynamicBuffer buf) noexcept
-{
-   char const* start = static_cast<char const*>(buf.data(0, buf.size()).data());
-   return std::string_view{start, std::size(buf)};
-}
-
 template <class AsyncReadStream, class DynamicBuffer>
 class append_some_op {
 private:
@@ -250,6 +242,8 @@ template <class Conn, class Logger>
 struct reader_op {
    using parse_result = typename Conn::parse_result;
    using parse_ret_type = typename Conn::parse_ret_type;
+   using dyn_buffer_type = asio::dynamic_string_buffer<char, std::char_traits<char>, std::allocator<char>>;
+
    Conn* conn_;
    Logger logger_;
    parse_ret_type res_{parse_result::resp, 0};
@@ -270,14 +264,14 @@ struct reader_op {
                BOOST_ASIO_CORO_YIELD
                async_append_some(
                   conn_->next_layer(),
-                  conn_->dbuf_,
+                  dyn_buffer_type{conn_->read_buffer_, conn_->cfg_.max_read_size},
                   conn_->get_suggested_buffer_growth(),
                   std::move(self));
             } else {
                BOOST_ASIO_CORO_YIELD
                async_append_some(
                   conn_->next_layer().next_layer(),
-                  conn_->dbuf_,
+                  dyn_buffer_type{conn_->read_buffer_, conn_->cfg_.max_read_size},
                   conn_->get_suggested_buffer_growth(),
                   std::move(self));
             }
@@ -302,7 +296,7 @@ struct reader_op {
             }
          }
 
-         res_ = conn_->on_read(buffer_view(conn_->dbuf_), ec);
+         res_ = conn_->on_read(ec);
          if (ec) {
             logger_.trace("reader_op (3)", ec);
             conn_->cancel(operation::run);
@@ -501,21 +495,17 @@ class basic_connection {
     *
     *  @param ex Executor on which connection operation will run.
     *  @param ctx SSL context.
-    *  @param max_read_size Maximum read size that is passed to
-    *  the internal `asio::dynamic_buffer` constructor.
     */
    explicit
    basic_connection(
       executor_type ex,
-      asio::ssl::context ctx = asio::ssl::context{asio::ssl::context::tlsv12_client},
-      std::size_t max_read_size = (std::numeric_limits<std::size_t>::max)())
+      asio::ssl::context ctx = asio::ssl::context{asio::ssl::context::tlsv12_client})
    : ctx_{std::move(ctx)}
    , stream_{std::make_unique<next_layer_type>(ex, ctx_)}
    , writer_timer_{ex}
    , receive_channel_{ex, 256}
    , resv_{ex}
    , health_checker_{ex}
-   , dbuf_{read_buffer_, max_read_size}
    {
       set_receive_response(ignore);
       writer_timer_.expires_at((std::chrono::steady_clock::time_point::max)());
@@ -525,9 +515,8 @@ class basic_connection {
    explicit
    basic_connection(
       asio::io_context& ioc,
-      asio::ssl::context ctx = asio::ssl::context{asio::ssl::context::tlsv12_client},
-      std::size_t max_read_size = (std::numeric_limits<std::size_t>::max)())
-   : basic_connection(ioc.get_executor(), std::move(ctx), max_read_size)
+      asio::ssl::context ctx = asio::ssl::context{asio::ssl::context::tlsv12_client})
+   : basic_connection(ioc.get_executor(), std::move(ctx))
    { }
 
    /** @brief Starts underlying connection operations.
@@ -1126,13 +1115,13 @@ class basic_connection {
       }
 
       on_push_ = false;
-      dbuf_.consume(parser_.get_consumed());
+      read_buffer_.erase(0, parser_.get_consumed());
       auto const res = std::make_pair(t, parser_.get_consumed());
       parser_.reset();
       return res;
    }
 
-   parse_ret_type on_read(std::string_view data, system::error_code& ec)
+   parse_ret_type on_read(system::error_code& ec)
    {
       // We arrive here in two states:
       //
@@ -1148,7 +1137,7 @@ class basic_connection {
          on_push_ = is_next_push();
 
       if (on_push_) {
-         if (!resp3::parse(parser_, data, receive_adapter_, ec))
+         if (!resp3::parse(parser_, read_buffer_, receive_adapter_, ec))
             return std::make_pair(parse_result::needs_more, 0);
 
          if (ec)
@@ -1162,7 +1151,7 @@ class basic_connection {
       BOOST_ASSERT(reqs_.front() != nullptr);
       BOOST_ASSERT(reqs_.front()->expected_responses_ != 0);
 
-      if (!resp3::parse(parser_, data, reqs_.front()->adapter_, ec))
+      if (!resp3::parse(parser_, read_buffer_, reqs_.front()->adapter_, ec))
          return std::make_pair(parse_result::needs_more, 0);
 
       if (ec) {
@@ -1205,11 +1194,8 @@ class basic_connection {
    resp3_handshaker_type handshaker_;
    receiver_adapter_type receive_adapter_;
 
-   using dyn_buffer_type = asio::dynamic_string_buffer<char, std::char_traits<char>, std::allocator<char>>;
-
    config cfg_;
    std::string read_buffer_;
-   dyn_buffer_type dbuf_;
    std::string write_buffer_;
    reqs_type reqs_;
    resp3::parser parser_{};
@@ -1237,15 +1223,13 @@ class connection {
    explicit
    connection(
       executor_type ex,
-      asio::ssl::context ctx = asio::ssl::context{asio::ssl::context::tlsv12_client},
-      std::size_t max_read_size = (std::numeric_limits<std::size_t>::max)());
+      asio::ssl::context ctx = asio::ssl::context{asio::ssl::context::tlsv12_client});
 
    /// Contructs from a context.
    explicit
    connection(
       asio::io_context& ioc,
-      asio::ssl::context ctx = asio::ssl::context{asio::ssl::context::tlsv12_client},
-      std::size_t max_read_size = (std::numeric_limits<std::size_t>::max)());
+      asio::ssl::context ctx = asio::ssl::context{asio::ssl::context::tlsv12_client});
 
    /// Returns the underlying executor.
    executor_type get_executor() noexcept

include/boost/redis/impl/connection.ipp

@@ -9,18 +9,12 @@
 
 namespace boost::redis {
 
-connection::connection(
-   executor_type ex,
-   asio::ssl::context ctx,
-   std::size_t max_read_size)
-: impl_{ex, std::move(ctx), max_read_size}
+connection::connection(executor_type ex, asio::ssl::context ctx)
+: impl_{ex, std::move(ctx)}
 { }
 
-connection::connection(
-   asio::io_context& ioc,
-   asio::ssl::context ctx,
-   std::size_t max_read_size)
-: impl_{ioc.get_executor(), std::move(ctx), max_read_size}
+connection::connection(asio::io_context& ioc, asio::ssl::context ctx)
+: impl_{ioc.get_executor(), std::move(ctx)}
 { }
 
 void

test/common.cpp

@@ -52,6 +52,7 @@ boost::redis::config make_test_config()
 {
    boost::redis::config cfg;
    cfg.addr.host = get_server_hostname();
+   cfg.max_read_size = 1000000;
    return cfg;
 }
 

test/test_issue_181.cpp

@@ -36,7 +36,7 @@ BOOST_AUTO_TEST_CASE(issue_181)
    auto const level = boost::redis::logger::level::debug;
    net::io_context ioc;
    auto ctx = net::ssl::context{net::ssl::context::tlsv12_client};
-   basic_connection conn{ioc.get_executor(), std::move(ctx), 1000000};
+   basic_connection conn{ioc.get_executor(), std::move(ctx)};
    net::steady_timer timer{ioc};
    timer.expires_after(std::chrono::seconds{1});