Starting operations while engaged in multi-function ops is no longer UB

Attempting to start an operation involving server communication after a
multi-function operation has been started and before all its associated
packets have been read now fails with
client_errc::engaged_in_multi_function, instead of causing undefined
behavior.
Attempting to start a read_some_rows or read_resultset_head operation
with no in-progress multi-function operation now fails with
client_errc::not_engaged_in_multi_function.
Attempting to start an operation that requires an established session
before a successful connect now fails with client_errc::not_connected.
Added the three client_errc enum values mentioned above.
Renamed detail::connection_status (connection_pool) to node_status.
Added detail::connection_status (connection_state_data).
Removed redundant spotcheck tests.

close #448 
close #450

Author: anarthal

doc/qbk/04_overview.qbk

@@ -287,10 +287,11 @@ table in batches, which can be the case in an ETL process:
 
 [overview_multifn]
 
-[warning
+[note
     Once you start a multi-function operation with [refmemunq any_connection async_start_execution],
     the server immediately sends all rows to the client. [*You must read all rows] before engaging in further operations.
-    Otherwise, you will encounter packet mismatches, which can lead to bugs and vulnerabilities!
+    If you try to initiate an unrelated operation before reading all rows, you will get a
+    `client_errc::engaged_in_multi_function` error.
 ]
 
 Multi-function operations are powerful but complex. Only use them when there is a strong reason to do so.

include/boost/mysql/client_errc.hpp

@@ -144,6 +144,25 @@ enum class client_errc : int
      * that a single connection does not run two asynchronous operations in parallel.
      */
     operation_in_progress,
+
+    /**
+     * \brief The requested operation requires an established session.
+     * Call `async_connect` before invoking other operations.
+     */
+    not_connected,
+
+    /**
+     * \brief The connection is currently engaged in a multi-function operation.
+     * Finish the current operation by calling `async_read_some_rows` and `async_read_resultset_head`
+     * before starting any other operation.
+     */
+    engaged_in_multi_function,
+
+    /**
+     * \brief The operation requires the connection to be engaged in a multi-function operation.
+     * Use `async_start_execution` to start one.
+     */
+    not_engaged_in_multi_function,
 };
 
 BOOST_MYSQL_DECL

include/boost/mysql/detail/connection_impl.hpp

@@ -580,6 +580,9 @@ class connection_impl
         const pipeline_request& req,
         std::vector<stage_response>& response
     );
+
+    // Exposed for testing
+    connection_state& get_state() { return *st_; }
 };
 
 // To use some completion tokens, like deferred, in C++11, the old macros

include/boost/mysql/impl/connection_impl.ipp

@@ -17,6 +17,7 @@
 #include <boost/mysql/detail/connection_impl.hpp>
 
 #include <boost/mysql/impl/internal/sansio/connection_state.hpp>
+#include <boost/mysql/impl/internal/sansio/connection_state_data.hpp>
 
 #include <boost/throw_exception.hpp>
 
@@ -94,7 +95,7 @@ boost::system::result<boost::mysql::character_set> boost::mysql::detail::connect
 boost::optional<std::uint32_t> boost::mysql::detail::connection_impl::connection_id() const
 {
     const auto& data = st_->data();
-    if (!data.is_connected)
+    if (data.status == connection_status::not_connected)
         return {};
     else
         return data.connection_id;

include/boost/mysql/impl/error_categories.ipp

@@ -89,7 +89,17 @@ inline const char* error_to_string(client_errc error)
     case client_errc::operation_in_progress:
         return "Another operation is currently in progress for this connection. Make sure that a single "
                "connection does not run two asynchronous operations in parallel.";
-
+    case client_errc::not_connected:
+        return "The requested operation requires an established session. Call async_connect before invoking "
+               "other operations.";
+    case client_errc::engaged_in_multi_function:
+        return "The connection is currently engaged in a multi-function operation."
+               "Finish the current operation by calling async_read_some_rows and async_read_resultset_head "
+               "before "
+               "starting any other operation.";
+    case client_errc::not_engaged_in_multi_function:
+        return "The operation requires the connection to be engaged in a multi-function operation. "
+               "Use async_start_execution to start one.";
     default: return "<unknown MySQL client error>";
     }
 }

include/boost/mysql/impl/internal/connection_pool/sansio_connection_node.hpp

@@ -22,7 +22,7 @@ namespace mysql {
 namespace detail {
 
 // The status the connection is in
-enum class connection_status
+enum class node_status
 {
     // Connection task hasn't initiated yet.
     // This status doesn't count as pending. This facilitates tracking pending connections.
@@ -51,7 +51,7 @@ enum class connection_status
 };
 
 // The next I/O action the connection should take. There's
-// no 1-1 mapping to connection_status
+// no 1-1 mapping to node_status
 enum class next_connection_action
 {
     // Do nothing, exit the loop
@@ -92,37 +92,37 @@ enum class collection_state
 template <class Derived>
 class sansio_connection_node
 {
-    connection_status status_;
+    node_status status_;
 
-    inline bool is_pending(connection_status status) noexcept
+    inline bool is_pending(node_status status) noexcept
     {
-        return status != connection_status::initial && status != connection_status::idle &&
-               status != connection_status::in_use && status != connection_status::terminated;
+        return status != node_status::initial && status != node_status::idle &&
+               status != node_status::in_use && status != node_status::terminated;
     }
 
-    inline static next_connection_action status_to_action(connection_status status) noexcept
+    inline static next_connection_action status_to_action(node_status status) noexcept
     {
         switch (status)
         {
-        case connection_status::connect_in_progress: return next_connection_action::connect;
-        case connection_status::sleep_connect_failed_in_progress:
+        case node_status::connect_in_progress: return next_connection_action::connect;
+        case node_status::sleep_connect_failed_in_progress:
             return next_connection_action::sleep_connect_failed;
-        case connection_status::ping_in_progress: return next_connection_action::ping;
-        case connection_status::reset_in_progress: return next_connection_action::reset;
-        case connection_status::idle:
-        case connection_status::in_use: return next_connection_action::idle_wait;
+        case node_status::ping_in_progress: return next_connection_action::ping;
+        case node_status::reset_in_progress: return next_connection_action::reset;
+        case node_status::idle:
+        case node_status::in_use: return next_connection_action::idle_wait;
         default: return next_connection_action::none;
         }
     }
 
-    next_connection_action set_status(connection_status new_status)
+    next_connection_action set_status(node_status new_status)
     {
         auto& derived = static_cast<Derived&>(*this);
 
         // Notify we're entering/leaving the idle status
-        if (new_status == connection_status::idle && status_ != connection_status::idle)
+        if (new_status == node_status::idle && status_ != node_status::idle)
             derived.entering_idle();
-        else if (new_status != connection_status::idle && status_ == connection_status::idle)
+        else if (new_status != node_status::idle && status_ == node_status::idle)
             derived.exiting_idle();
 
         // Notify we're entering/leaving a pending status
@@ -138,64 +138,63 @@ class sansio_connection_node
     }
 
 public:
-    sansio_connection_node(connection_status initial_status = connection_status::initial) noexcept
+    sansio_connection_node(node_status initial_status = node_status::initial) noexcept
         : status_(initial_status)
     {
     }
 
     void mark_as_in_use() noexcept
     {
-        BOOST_ASSERT(status_ == connection_status::idle);
-        set_status(connection_status::in_use);
+        BOOST_ASSERT(status_ == node_status::idle);
+        set_status(node_status::in_use);
     }
 
-    void cancel() { set_status(connection_status::terminated); }
+    void cancel() { set_status(node_status::terminated); }
 
     next_connection_action resume(error_code ec, collection_state col_st)
     {
         switch (status_)
         {
-        case connection_status::initial: return set_status(connection_status::connect_in_progress);
-        case connection_status::connect_in_progress:
-            return ec ? set_status(connection_status::sleep_connect_failed_in_progress)
-                      : set_status(connection_status::idle);
-        case connection_status::sleep_connect_failed_in_progress:
-            return set_status(connection_status::connect_in_progress);
-        case connection_status::idle:
+        case node_status::initial: return set_status(node_status::connect_in_progress);
+        case node_status::connect_in_progress:
+            return ec ? set_status(node_status::sleep_connect_failed_in_progress)
+                      : set_status(node_status::idle);
+        case node_status::sleep_connect_failed_in_progress:
+            return set_status(node_status::connect_in_progress);
+        case node_status::idle:
             // The wait finished with no interruptions, and the connection
             // is still idle. Time to ping.
-            return set_status(connection_status::ping_in_progress);
-        case connection_status::in_use:
+            return set_status(node_status::ping_in_progress);
+        case node_status::in_use:
             // If col_st != none, the user has notified us to collect the connection.
             // This happens after they return the connection to the pool.
             // Update status and continue
             if (col_st == collection_state::needs_collect)
             {
                 // No reset needed, we're idle
-                return set_status(connection_status::idle);
+                return set_status(node_status::idle);
             }
             else if (col_st == collection_state::needs_collect_with_reset)
             {
-                return set_status(connection_status::reset_in_progress);
+                return set_status(node_status::reset_in_progress);
             }
             else
             {
                 // The user is still using the connection (it's taking long, but can happen).
                 // Idle wait again until they return the connection.
                 return next_connection_action::idle_wait;
             }
-        case connection_status::ping_in_progress:
-        case connection_status::reset_in_progress:
+        case node_status::ping_in_progress:
+        case node_status::reset_in_progress:
             // Reconnect if there was an error. Otherwise, we're idle
-            return ec ? set_status(connection_status::connect_in_progress)
-                      : set_status(connection_status::idle);
-        case connection_status::terminated:
+            return ec ? set_status(node_status::connect_in_progress) : set_status(node_status::idle);
+        case node_status::terminated:
         default: return next_connection_action::none;
         }
     }
 
     // Exposed for testing
-    connection_status status() const noexcept { return status_; }
+    node_status status() const noexcept { return status_; }
 };
 
 // Composes a diagnostics object containing info about the last connect error.

include/boost/mysql/impl/internal/sansio/close_connection.hpp

@@ -38,8 +38,9 @@ class close_connection_algo
         {
         case 0:
 
-            // If we're not connected, we're done
-            if (!st.is_connected)
+            // If we're not connected, we're done.
+            // If we're in a multi-function operation, it's safe to proceed.
+            if (st.status == connection_status::not_connected)
                 return next_action();
 
             // Attempt quit

include/boost/mysql/impl/internal/sansio/connect.hpp

@@ -42,6 +42,7 @@ class connect_algo
         switch (resume_point_)
         {
         case 0:
+            // Handshake and connect wipe out state, so no state checks are performed.
 
             // Physical connect
             BOOST_MYSQL_YIELD(resume_point_, 1, next_action::connect(server_address_))

include/boost/mysql/impl/internal/sansio/connection_state_data.hpp

@@ -9,7 +9,9 @@
 #define BOOST_MYSQL_IMPL_INTERNAL_SANSIO_CONNECTION_STATE_DATA_HPP
 
 #include <boost/mysql/character_set.hpp>
+#include <boost/mysql/client_errc.hpp>
 #include <boost/mysql/diagnostics.hpp>
+#include <boost/mysql/error_code.hpp>
 #include <boost/mysql/field_view.hpp>
 #include <boost/mysql/metadata_mode.hpp>
 
@@ -21,6 +23,8 @@
 #include <boost/mysql/impl/internal/protocol/serialization.hpp>
 #include <boost/mysql/impl/internal/sansio/message_reader.hpp>
 
+#include <boost/assert.hpp>
+
 #include <array>
 #include <cstddef>
 #include <cstdint>
@@ -30,15 +34,27 @@ namespace boost {
 namespace mysql {
 namespace detail {
 
+enum class connection_status
+{
+    // Never connected or closed
+    not_connected,
+
+    // Connected and ready for a command
+    ready,
+
+    // In the middle of a multi-function operation
+    engaged_in_multi_function,
+};
+
 struct connection_state_data
 {
+    // Are we connected? In the middle of a multi-function operation?
+    connection_status status{connection_status::not_connected};
+
     // Are we currently executing an operation?
     // Prevent the user from running concurrent operations
     bool op_in_progress{false};
 
-    // Is the connection actually connected? Set by handshake
-    bool is_connected{false};
-
     // Are we talking to MySQL or MariaDB?
     db_flavor flavor{db_flavor::mysql};
 
@@ -92,7 +108,7 @@ struct connection_state_data
 
     void reset()
     {
-        is_connected = false;
+        status = connection_status::not_connected;
         flavor = db_flavor::mysql;
         current_capabilities = capabilities();
         // Metadata mode does not get reset on handshake
@@ -128,6 +144,27 @@ struct connection_state_data
         seqnum = res.seqnum;
         return next_action::write({write_buffer, false});
     }
+
+    // Helpers to implement connection status in algorithms
+    error_code check_status_ready() const
+    {
+        switch (status)
+        {
+        case connection_status::not_connected: return client_errc::not_connected;
+        case connection_status::engaged_in_multi_function: return client_errc::engaged_in_multi_function;
+        default: BOOST_ASSERT(status == connection_status::ready); return error_code();
+        }
+    }
+
+    error_code check_status_multi_function() const
+    {
+        switch (status)
+        {
+        case connection_status::not_connected: return client_errc::not_connected;
+        case connection_status::ready: return client_errc::not_engaged_in_multi_function;
+        default: BOOST_ASSERT(status == connection_status::engaged_in_multi_function); return error_code();
+        }
+    }
 };
 
 }  // namespace detail

include/boost/mysql/impl/internal/sansio/execute.hpp

@@ -32,8 +32,10 @@ class read_execute_response_algo
     read_some_rows_algo read_some_rows_st_;
 
 public:
+    // We pass false because they are subordinate algos. This suppresses state checks.
+    // This is always a subordinate algo, so it never performs state checks.
     read_execute_response_algo(execution_processor* proc) noexcept
-        : read_head_st_({proc}), read_some_rows_st_({proc, output_ref()})
+        : read_head_st_({proc}, false), read_some_rows_st_({proc, output_ref()}, false)
     {
     }
 
@@ -81,8 +83,10 @@ class execute_algo
     execution_processor& processor() { return read_response_st_.processor(); }
 
 public:
+    // We pass false to the start execution algo's constructor because it's a subordinate algo.
+    // This disables state checks.
     execute_algo(execute_algo_params params) noexcept
-        : start_execution_st_({params.req, params.proc}), read_response_st_(params.proc)
+        : start_execution_st_({params.req, params.proc}, false), read_response_st_(params.proc)
     {
     }
 
@@ -94,6 +98,11 @@ class execute_algo
         {
         case 0:
 
+            // Check status
+            ec = st.check_status_ready();
+            if (ec)
+                return ec;
+
             // Send request and read the first response
             while (!(act = start_execution_st_.resume(st, diag, ec)).is_done())
                 BOOST_MYSQL_YIELD(resume_point_, 1, act)