From f98427c3b802fee3c051506de3bd70ee43398287 Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Mon, 4 Mar 2024 18:11:00 +0100 Subject: [PATCH] Connection pools now always use utf8mb4 Fixes error-prone behavior for connection pools, where connections would use the server's default character set. close #187 --- doc/qbk/21_connection_pool.qbk | 39 +++-- include/boost/mysql/any_connection.hpp | 4 + include/boost/mysql/detail/algo_params.hpp | 1 + .../boost/mysql/detail/connection_impl.hpp | 22 ++- .../mysql/detail/connection_pool_fwd.hpp | 3 +- .../connection_pool/connection_node.hpp | 3 +- .../connection_pool/internal_pool_params.hpp | 1 - .../mysql/impl/internal/protocol/protocol.hpp | 2 +- .../impl/internal/sansio/close_statement.hpp | 9 +- .../internal/sansio/connection_state_data.hpp | 7 + .../boost/mysql/impl/internal/sansio/ping.hpp | 7 +- .../impl/internal/sansio/reset_connection.hpp | 82 ++++++++-- .../internal/sansio/set_character_set.hpp | 39 ++--- test/integration/test/connection_pool.cpp | 44 ++++++ .../include/test_unit/create_query_frame.hpp | 50 ++++++ .../connection_pool/connection_pool_impl.cpp | 143 +++++++++--------- test/unit/test/sansio/reset_connection.cpp | 116 +++++++++++++- test/unit/test/sansio/set_character_set.cpp | 27 +--- 18 files changed, 433 insertions(+), 166 deletions(-) create mode 100644 test/unit/include/test_unit/create_query_frame.hpp diff --git a/doc/qbk/21_connection_pool.qbk b/doc/qbk/21_connection_pool.qbk index dc32676f4..159cd440b 100644 --- a/doc/qbk/21_connection_pool.qbk +++ b/doc/qbk/21_connection_pool.qbk @@ -91,9 +91,12 @@ create temporary tables, start transactions, or set session variables. When usin pooled connections, session state can be problematic: if not reset properly, state from a previous operation may affect subsequent ones. -After you return a connection to the pool, it uses [refmem any_connection async_reset_connection] -to wipe session state before the connection can be obtained again. This will deallocate -prepared statements, rollback uncommited transactions and clear variables. +After you return a connection to the pool, the equivalent of +[refmem any_connection async_reset_connection] and [refmemunq any_connection async_set_character_set] +are used to wipe session state before +the connection can be obtained again. This will deallocate +prepared statements, rollback uncommitted transactions, clear variables and restore the connection's +character set to `utf8mb4`. In particular, you don't need to call [refmem any_connection async_close_statement] to deallocate statements. @@ -109,28 +112,20 @@ returned, so it does not affect latency. If you're not sure if an operation affects state or not, assume it does. -[heading Character sets] +[heading Character set] -[warning - The current implementation, together with unintuitive MySQL defaults, - can yield to surprising behavior. -] - -When using pooled connections, [*all connections use the server's default character set]. -This is because [refmem any_connection async_reset_connection] discards the character set -options specified during connection establishment. - -You can obtain this character set by running: +Pooled connections always use `utf8mb4` as its character set. When connections +are reset, the equivalent of [refmem any_connection async_set_character_set] is used +to restore the character set to `utf8mb4` (recall that raw [refmemunq any_connection async_reset_connection] +will wipe character set data). -[!teletype] -``` -"SELECT @@global.character_set_client, @@global.character_set_results;" -``` +Pooled connections always know the character set they're using. +This means that [refmem any_connection format_opts] and [refmemunq any_connection current_character_set] +always succeed. -MySQL v8.0+ defaults to `utf8mb4`, but older MySQL and MariaDB servers default to -`latin1`, which is not usually what you want. Issue a __SET_NAMES__ statement -after you get a connection to change it. [@https://github.com/boostorg/mysql/issues/187 This issue] -tracks solving this limitation. +We recommend to always stick to `utf8mb4`. If you really need to use any other character set, +use [refmemunq any_connection async_set_character_set] on your connection after it's been retrieved +from the pool. [heading Connection lifecycle] diff --git a/include/boost/mysql/any_connection.hpp b/include/boost/mysql/any_connection.hpp index 1a3cb9ec7..e8f6b26a7 100644 --- a/include/boost/mysql/any_connection.hpp +++ b/include/boost/mysql/any_connection.hpp @@ -117,6 +117,10 @@ class any_connection { detail::connection_impl impl_; +#ifndef BOOST_MYSQL_DOXYGEN + friend struct detail::access; +#endif + BOOST_MYSQL_DECL static std::unique_ptr create_stream( asio::any_io_executor ex, diff --git a/include/boost/mysql/detail/algo_params.hpp b/include/boost/mysql/detail/algo_params.hpp index d15dbfd00..3397a0ad7 100644 --- a/include/boost/mysql/detail/algo_params.hpp +++ b/include/boost/mysql/detail/algo_params.hpp @@ -114,6 +114,7 @@ struct ping_algo_params struct reset_connection_algo_params { diagnostics* diag; + character_set charset; // set a non-empty character set to pipeline a SET NAMES with the reset request using result_type = void; }; diff --git a/include/boost/mysql/detail/connection_impl.hpp b/include/boost/mysql/detail/connection_impl.hpp index bfa619578..92a2b8e72 100644 --- a/include/boost/mysql/detail/connection_impl.hpp +++ b/include/boost/mysql/detail/connection_impl.hpp @@ -468,9 +468,27 @@ class connection_impl ping_algo_params make_params_ping(diagnostics& diag) const noexcept { return {&diag}; } // Reset connection - reset_connection_algo_params make_params_reset_connection(diagnostics& diag) const noexcept + reset_connection_algo_params make_params_reset_connection( + diagnostics& diag, + const character_set& charset = {} + ) const noexcept { - return {&diag}; + return {&diag, charset}; + } + + // Reset connection and issue a SET NAMES, using a pipeline. + // Used internally by connection_pool. + template + auto async_reset_with_charset(const character_set& charset, CompletionToken&& token) + BOOST_MYSQL_RETURN_TYPE(decltype(std::declval().async_run( + std::declval(), + std::forward(token) + ))) + { + return async_run( + make_params_reset_connection(shared_diag(), charset), + std::forward(token) + ); } // Quit connection diff --git a/include/boost/mysql/detail/connection_pool_fwd.hpp b/include/boost/mysql/detail/connection_pool_fwd.hpp index 7d41bf034..0e2ea2eea 100644 --- a/include/boost/mysql/detail/connection_pool_fwd.hpp +++ b/include/boost/mysql/detail/connection_pool_fwd.hpp @@ -8,8 +8,6 @@ #ifndef BOOST_MYSQL_DETAIL_CONNECTION_POOL_FWD_HPP #define BOOST_MYSQL_DETAIL_CONNECTION_POOL_FWD_HPP -#include - #include #include @@ -18,6 +16,7 @@ namespace boost { namespace mysql { class pooled_connection; +class any_connection; namespace detail { diff --git a/include/boost/mysql/impl/internal/connection_pool/connection_node.hpp b/include/boost/mysql/impl/internal/connection_pool/connection_node.hpp index a5649714c..9f91d1538 100644 --- a/include/boost/mysql/impl/internal/connection_pool/connection_node.hpp +++ b/include/boost/mysql/impl/internal/connection_pool/connection_node.hpp @@ -9,6 +9,7 @@ #define BOOST_MYSQL_IMPL_INTERNAL_CONNECTION_POOL_CONNECTION_NODE_HPP #include +#include #include #include #include @@ -146,7 +147,7 @@ class basic_connection_node : public intrusive::list_base_hook<>, break; case next_connection_action::reset: run_with_timeout( - node_.conn_.async_reset_connection(asio::deferred), + access::get_impl(node_.conn_).async_reset_with_charset(utf8mb4_charset, asio::deferred), node_.timer_, node_.params_->ping_timeout, std::move(self) diff --git a/include/boost/mysql/impl/internal/connection_pool/internal_pool_params.hpp b/include/boost/mysql/impl/internal/connection_pool/internal_pool_params.hpp index 2de5363ce..399b90f23 100644 --- a/include/boost/mysql/impl/internal/connection_pool/internal_pool_params.hpp +++ b/include/boost/mysql/impl/internal/connection_pool/internal_pool_params.hpp @@ -80,7 +80,6 @@ inline internal_pool_params make_internal_pool_params(pool_params&& params) connect_prms.username = std::move(params.username); connect_prms.password = std::move(params.password); connect_prms.database = std::move(params.database); - connect_prms.connection_collation = 0; // use the server's default collation connect_prms.ssl = params.ssl; connect_prms.multi_queries = params.multi_queries; diff --git a/include/boost/mysql/impl/internal/protocol/protocol.hpp b/include/boost/mysql/impl/internal/protocol/protocol.hpp index 42f561d9a..82ec5ae10 100644 --- a/include/boost/mysql/impl/internal/protocol/protocol.hpp +++ b/include/boost/mysql/impl/internal/protocol/protocol.hpp @@ -100,7 +100,7 @@ struct reset_connection_command }; // Deserializes a response that may be an OK or an error packet. -// Applicable for ping and reset connection. +// Applicable for commands like ping and reset connection. // If the response is an OK packet, sets backslash_escapes according to the // OK packet's server status flags BOOST_ATTRIBUTE_NODISCARD BOOST_MYSQL_DECL error_code deserialize_ok_response( diff --git a/include/boost/mysql/impl/internal/sansio/close_statement.hpp b/include/boost/mysql/impl/internal/sansio/close_statement.hpp index 869f8be59..3b010570b 100644 --- a/include/boost/mysql/impl/internal/sansio/close_statement.hpp +++ b/include/boost/mysql/impl/internal/sansio/close_statement.hpp @@ -58,18 +58,13 @@ class close_statement_algo : public sansio_algorithm, asio::coroutine ping_command{}, ping_seqnum_ ); - BOOST_ASIO_CORO_YIELD return next_action::write(next_action::write_args_t{{}, false}); + BOOST_ASIO_CORO_YIELD return next_action::write({}); // Read ping response BOOST_ASIO_CORO_YIELD return read(ping_seqnum_); // Process the OK packet - return deserialize_ok_response( - st_->reader.message(), - st_->flavor, - *diag_, - st_->backslash_escapes - ); + return st_->deserialize_ok(*diag_); } return next_action(); diff --git a/include/boost/mysql/impl/internal/sansio/connection_state_data.hpp b/include/boost/mysql/impl/internal/sansio/connection_state_data.hpp index be7d321a7..571f21954 100644 --- a/include/boost/mysql/impl/internal/sansio/connection_state_data.hpp +++ b/include/boost/mysql/impl/internal/sansio/connection_state_data.hpp @@ -15,6 +15,7 @@ #include #include +#include #include #include @@ -93,6 +94,12 @@ struct connection_state_data backslash_escapes = true; current_charset = character_set{}; } + + // Reads an OK packet from the reader. This operation is repeated in several places. + error_code deserialize_ok(diagnostics& diag) noexcept + { + return deserialize_ok_response(reader.message(), flavor, diag, backslash_escapes); + } }; } // namespace detail diff --git a/include/boost/mysql/impl/internal/sansio/ping.hpp b/include/boost/mysql/impl/internal/sansio/ping.hpp index a9c89cfb9..21d0d1f67 100644 --- a/include/boost/mysql/impl/internal/sansio/ping.hpp +++ b/include/boost/mysql/impl/internal/sansio/ping.hpp @@ -52,12 +52,7 @@ class ping_algo : public sansio_algorithm, asio::coroutine BOOST_ASIO_CORO_YIELD return read(seqnum_); // Process the OK packet - return deserialize_ok_response( - st_->reader.message(), - st_->flavor, - *diag_, - st_->backslash_escapes - ); + return st_->deserialize_ok(*diag_); } return next_action(); diff --git a/include/boost/mysql/impl/internal/sansio/reset_connection.hpp b/include/boost/mysql/impl/internal/sansio/reset_connection.hpp index 40ea7968f..63643520f 100644 --- a/include/boost/mysql/impl/internal/sansio/reset_connection.hpp +++ b/include/boost/mysql/impl/internal/sansio/reset_connection.hpp @@ -14,8 +14,10 @@ #include +#include #include #include +#include #include @@ -26,11 +28,44 @@ namespace detail { class reset_connection_algo : public sansio_algorithm, asio::coroutine { diagnostics* diag_; - std::uint8_t seqnum_{0}; + character_set charset_; + std::uint8_t reset_seqnum_{0}; + std::uint8_t set_names_seqnum_{0}; + error_code stored_ec_; + + // true if we need to pipeline a SET NAMES with the reset request + bool has_charset() const noexcept { return !charset_.name.empty(); } + + next_action compose_request() + { + if (has_charset()) + { + // Compose the SET NAMES statement + auto query = compose_set_names(charset_); + if (query.has_error()) + return query.error(); + + // Compose the pipeline + st_->writer.prepare_pipelined_write( + reset_connection_command{}, + reset_seqnum_, + query_command{query.value()}, + set_names_seqnum_ + ); + + // Success + return next_action::write({}); + } + else + { + // Just compose the reset connection request + return write(reset_connection_command{}, reset_seqnum_); + } + } public: reset_connection_algo(connection_state_data& st, reset_connection_algo_params params) noexcept - : sansio_algorithm(st), diag_(params.diag) + : sansio_algorithm(st), diag_(params.diag), charset_(params.charset) { } @@ -45,20 +80,41 @@ class reset_connection_algo : public sansio_algorithm, asio::coroutine diag_->clear(); // Send the request - BOOST_ASIO_CORO_YIELD return write(reset_connection_command(), seqnum_); + BOOST_ASIO_CORO_YIELD return compose_request(); - // Read the response - BOOST_ASIO_CORO_YIELD return read(seqnum_); + // Read the reset response + BOOST_ASIO_CORO_YIELD return read(reset_seqnum_); // Verify it's what we expected - ec = deserialize_ok_response(st_->reader.message(), st_->flavor, *diag_, st_->backslash_escapes); - if (ec) - return ec; - - // Reset was successful. Resetting changes the connection's character set - // to the server's default, which is an unknown value that doesn't have to match - // what was specified in handshake. As a safety measure, clear the current charset - st_->current_charset = character_set{}; + stored_ec_ = st_->deserialize_ok(*diag_); + + if (!stored_ec_) + { + // Reset was successful. Resetting changes the connection's character set + // to the server's default, which is an unknown value that doesn't have to match + // what was specified in handshake. As a safety measure, clear the current charset + st_->current_charset = character_set{}; + } + + if (has_charset()) + { + // We issued a SET NAMES too, read its response + BOOST_ASIO_CORO_YIELD return read(set_names_seqnum_); + + // Verify it's what we expected. Don't overwrite diagnostics if reset failed + ec = st_->deserialize_ok(stored_ec_ ? st_->shared_diag : *diag_); + if (!ec) + { + // Set the character set to the new known value + st_->current_charset = charset_; + } + + // Set the return value if there is no error code already stored + if (!stored_ec_) + stored_ec_ = ec; + } + + return stored_ec_; } return next_action(); diff --git a/include/boost/mysql/impl/internal/sansio/set_character_set.hpp b/include/boost/mysql/impl/internal/sansio/set_character_set.hpp index 56fe82b9a..35ce33d9a 100644 --- a/include/boost/mysql/impl/internal/sansio/set_character_set.hpp +++ b/include/boost/mysql/impl/internal/sansio/set_character_set.hpp @@ -29,25 +29,31 @@ namespace boost { namespace mysql { namespace detail { +// Securely compose a SET NAMES statement. This function +// is also used by the pipelined version of reset_connection +inline system::result compose_set_names(const character_set& charset) +{ + // The character set should have a non-empty name + BOOST_ASSERT(!charset.name.empty()); + + // For security, if the character set has non-ascii characters in it name, reject it. + format_context ctx(format_options{ascii_charset, true}); + ctx.append_raw("SET NAMES ").append_value(charset.name); + return std::move(ctx).get(); +} + class set_character_set_algo : public sansio_algorithm, asio::coroutine { diagnostics* diag_; character_set charset_; std::uint8_t seqnum_{0}; - static error_code compose_query(const character_set& charset, std::string& output) + next_action compose_request() { - // The character set should have a non-empty name - BOOST_ASSERT(!charset.name.empty()); - - // For security, if the character set has non-ascii characters in it name, reject it. - format_context ctx(format_options{ascii_charset, true}); - ctx.append_raw("SET NAMES ").append_value(charset.name); - auto res = std::move(ctx).get(); - if (res.has_error()) - return res.error(); - output = std::move(res).value(); - return error_code(); + auto q = compose_set_names(charset_); + if (q.has_error()) + return q.error(); + return write(query_command{q.value()}, seqnum_); } public: @@ -61,26 +67,21 @@ class set_character_set_algo : public sansio_algorithm, asio::coroutine if (ec) return ec; - std::string query; - // SET NAMES never returns rows. Using execute requires us to allocate // a results object, which we can avoid by simply sending the query and reading the OK response. BOOST_ASIO_CORO_REENTER(*this) { // Setup diag_->clear(); - ec = compose_query(charset_, query); - if (ec) - return ec; // Send the execution request - BOOST_ASIO_CORO_YIELD return write(query_command{query}, seqnum_); + BOOST_ASIO_CORO_YIELD return compose_request(); // Read the response BOOST_ASIO_CORO_YIELD return read(seqnum_); // Verify it's what we expected - ec = deserialize_ok_response(st_->reader.message(), st_->flavor, *diag_, st_->backslash_escapes); + ec = st_->deserialize_ok(*diag_); if (ec) return ec; diff --git a/test/integration/test/connection_pool.cpp b/test/integration/test/connection_pool.cpp index d9ee6f5c7..9b186ee42 100644 --- a/test/integration/test/connection_pool.cpp +++ b/test/integration/test/connection_pool.cpp @@ -223,6 +223,50 @@ BOOST_FIXTURE_TEST_CASE(pooled_connection_destructor, fixture) }); } +// Pooled connections use utf8mb4 +static void validate_charset(any_connection& conn, asio::yield_context yield) +{ + // The connection knows its using utf8mb4 + BOOST_TEST(conn.current_character_set()->name == "utf8mb4"); + BOOST_TEST(conn.format_opts()->charset.name == "utf8mb4"); + + // The connection is actually using utf8mb4 + results r; + conn.async_execute( + "SELECT @@character_set_client, @@character_set_connection, @@character_set_results", + r, + yield + ); + const auto rw = r.rows().at(0); + BOOST_TEST(rw.at(0).as_string() == "utf8mb4"); + BOOST_TEST(rw.at(1).as_string() == "utf8mb4"); + BOOST_TEST(rw.at(2).as_string() == "utf8mb4"); +} + +BOOST_FIXTURE_TEST_CASE(charset, fixture) +{ + run_stackful_coro([&](asio::yield_context yield) { + // Create and run the pool + connection_pool pool(yield.get_executor(), create_pool_params(1)); + pool_guard grd(&pool); + pool.async_run(check_err); + + // Get a connection + auto conn = pool.async_get_connection(yield); + validate_charset(conn.get(), yield); + + // Return the connection and retrieve it again + conn = pooled_connection(); + conn = pool.async_get_connection(yield); + validate_charset(conn.get(), yield); + + // Return the connection without reset and retrieve it again + conn.return_without_reset(); + conn = pool.async_get_connection(yield); + validate_charset(conn.get(), yield); + }); +} + BOOST_FIXTURE_TEST_CASE(connections_created_if_required, fixture) { run_stackful_coro([&](asio::yield_context yield) { diff --git a/test/unit/include/test_unit/create_query_frame.hpp b/test/unit/include/test_unit/create_query_frame.hpp new file mode 100644 index 000000000..f1a104e6a --- /dev/null +++ b/test/unit/include/test_unit/create_query_frame.hpp @@ -0,0 +1,50 @@ +// +// Copyright (c) 2019-2023 Ruben Perez Hidalgo (rubenperez038 at gmail dot com) +// +// Distributed under the Boost Software License, Version 1.0. (See accompanying +// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) +// + +#ifndef BOOST_MYSQL_TEST_UNIT_INCLUDE_TEST_UNIT_CREATE_QUERY_FRAME_HPP +#define BOOST_MYSQL_TEST_UNIT_INCLUDE_TEST_UNIT_CREATE_QUERY_FRAME_HPP + +#include + +#include + +#include +#include + +#include "test_common/buffer_concat.hpp" +#include "test_unit/create_frame.hpp" + +namespace boost { +namespace mysql { +namespace test { + +// GCC raises a spurious warning here +#if BOOST_GCC >= 110000 +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wstringop-overread" +#endif +inline std::vector create_query_body(string_view sql) +{ + return buffer_builder() + .add({0x03}) + .add({reinterpret_cast(sql.data()), sql.size()}) + .build(); +} +#if BOOST_GCC >= 110000 +#pragma GCC diagnostic pop +#endif + +inline std::vector create_query_frame(std::uint8_t seqnum, string_view sql) +{ + return create_frame(seqnum, create_query_body(sql)); +} + +} // namespace test +} // namespace mysql +} // namespace boost + +#endif diff --git a/test/unit/test/connection_pool/connection_pool_impl.cpp b/test/unit/test/connection_pool/connection_pool_impl.cpp index 38f9bdbea..5882e1879 100644 --- a/test/unit/test/connection_pool/connection_pool_impl.cpp +++ b/test/unit/test/connection_pool/connection_pool_impl.cpp @@ -6,11 +6,13 @@ // #include +#include #include #include #include #include #include +#include #include #include @@ -47,16 +49,10 @@ // See https://github.com/chriskohlhoff/asio/issues/1398 #ifndef BOOST_ASIO_USE_TS_EXECUTOR_AS_DEFAULT -using namespace boost::mysql::detail; +using namespace boost::mysql; using namespace boost::mysql::test; namespace asio = boost::asio; -using boost::mysql::client_errc; -using boost::mysql::common_server_errc; -using boost::mysql::connect_params; -using boost::mysql::diagnostics; -using boost::mysql::error_code; -using boost::mysql::pool_params; -using boost::mysql::pooled_connection; +using detail::connection_status; using std::chrono::steady_clock; /** @@ -85,51 +81,78 @@ enum fn_type // the passed error_code/diagnostics. All this synchronization uses channels. class mock_connection { - asio::experimental::channel to_test_chan_; - asio::experimental::channel from_test_chan_; - - // Code shared between all mocked ops - struct mocked_op + struct impl_t { - mock_connection& obj; - fn_type op_type; - diagnostics* diag; + asio::experimental::channel to_test_chan_; + asio::experimental::channel from_test_chan_; - template - void operator()(Self& self) + // Code shared between all mocked ops + struct mocked_op { - // Notify the test that we're about to do op_type - obj.to_test_chan_.async_send(error_code(), op_type, std::move(self)); - } + impl_t& obj; + fn_type op_type; + diagnostics* diag; - template - void operator()(Self& self, error_code ec) - { - if (ec) + template + void operator()(Self& self) { - // We were cancelled - self.complete(ec); + // Notify the test that we're about to do op_type + obj.to_test_chan_.async_send(error_code(), op_type, std::move(self)); } - else + + template + void operator()(Self& self, error_code ec) { - // Read from the test what we should return - obj.from_test_chan_.async_receive(std::move(self)); + if (ec) + { + // We were cancelled + self.complete(ec); + } + else + { + // Read from the test what we should return + obj.from_test_chan_.async_receive(std::move(self)); + } } + + template + void operator()(Self& self, error_code ec, diagnostics recv_diag) + { + // Done + if (diag) + *diag = std::move(recv_diag); + self.complete(ec); + } + }; + + template + auto op_impl(fn_type op_type, diagnostics* diag, CompletionToken&& token) + -> decltype(asio::async_compose( + std::declval(), + token, + asio::any_io_executor() + )) + { + return asio::async_compose( + mocked_op{*this, op_type, diag}, + token, + to_test_chan_.get_executor() + ); } - template - void operator()(Self& self, error_code ec, diagnostics recv_diag) + // We use an internal async_reset_connection that allows setting the character set + template + auto async_reset_with_charset(const character_set& charset, CompletionToken&& token) + -> decltype(op_impl(fn_type::reset, nullptr, std::forward(token))) { - // Done - if (diag) - *diag = std::move(recv_diag); - self.complete(ec); + BOOST_TEST(charset.name == "utf8mb4"); // We must always use utf8mb4 + return op_impl(fn_type::reset, nullptr, std::forward(token)); } - }; + } impl_; struct step_op { - mock_connection& obj; + impl_t& obj; fn_type expected_op_type; error_code op_ec; diagnostics op_diag; @@ -161,51 +184,31 @@ class mock_connection } }; - template - auto op_impl(fn_type op_type, diagnostics* diag, CompletionToken&& token) - -> decltype(asio::async_compose( - std::declval(), - token, - asio::any_io_executor() - )) - { - return asio::async_compose( - mocked_op{*this, op_type, diag}, - token, - to_test_chan_.get_executor() - ); - } + friend struct boost::mysql::detail::access; public: boost::mysql::any_connection_params ctor_params; boost::mysql::connect_params last_connect_params; mock_connection(asio::any_io_executor ex, boost::mysql::any_connection_params ctor_params) - : to_test_chan_(ex), from_test_chan_(std::move(ex)), ctor_params(ctor_params) + : impl_{ex, ex}, ctor_params(ctor_params) { } template auto async_connect(const connect_params* params, diagnostics& diag, CompletionToken&& token) - -> decltype(op_impl(fn_type::connect, &diag, std::forward(token))) + -> decltype(impl_.op_impl(fn_type::connect, &diag, std::forward(token))) { BOOST_TEST(params != nullptr); last_connect_params = *params; - return op_impl(fn_type::connect, &diag, std::forward(token)); + return impl_.op_impl(fn_type::connect, &diag, std::forward(token)); } template auto async_ping(CompletionToken&& token) - -> decltype(op_impl(fn_type::ping, nullptr, std::forward(token))) - { - return op_impl(fn_type::ping, nullptr, std::forward(token)); - } - - template - auto async_reset_connection(CompletionToken&& token) - -> decltype(op_impl(fn_type::reset, nullptr, std::forward(token))) + -> decltype(impl_.op_impl(fn_type::ping, nullptr, std::forward(token))) { - return op_impl(fn_type::reset, nullptr, std::forward(token)); + return impl_.op_impl(fn_type::ping, nullptr, std::forward(token)); } void step( @@ -216,9 +219,9 @@ class mock_connection ) { return asio::async_compose, void()>( - step_op{*this, expected_op_type, ec, std::move(diag)}, + step_op{impl_, expected_op_type, ec, std::move(diag)}, handler, - from_test_chan_ + impl_.from_test_chan_ ); } }; @@ -231,8 +234,8 @@ struct mock_io_traits }; struct mock_pooled_connection; -using mock_node = basic_connection_node; -using mock_pool = basic_pool_impl; +using mock_node = detail::basic_connection_node; +using mock_pool = detail::basic_pool_impl; // Mock for pooled_connection struct mock_pooled_connection @@ -1339,7 +1342,7 @@ BOOST_AUTO_TEST_CASE(params_connect_1) // Check params const auto& cparams = node.connection().last_connect_params; - BOOST_TEST(cparams.connection_collation == std::uint16_t(0)); + BOOST_TEST(cparams.connection_collation == mysql_collations::utf8mb4_general_ci); BOOST_TEST(cparams.server_address.hostname() == "myhost"); BOOST_TEST(cparams.server_address.port() == std::uint16_t(1234)); BOOST_TEST(cparams.username == "myuser"); @@ -1379,7 +1382,7 @@ BOOST_AUTO_TEST_CASE(params_connect_2) // Check params const auto& cparams = node.connection().last_connect_params; - BOOST_TEST(cparams.connection_collation == std::uint16_t(0)); + BOOST_TEST(cparams.connection_collation == mysql_collations::utf8mb4_general_ci); BOOST_TEST(cparams.server_address.unix_socket_path() == "/mysock"); BOOST_TEST(cparams.username == "myuser2"); BOOST_TEST(cparams.password == "mypasswd2"); diff --git a/test/unit/test/sansio/reset_connection.cpp b/test/unit/test/sansio/reset_connection.cpp index 2a1dafe11..56a3465ee 100644 --- a/test/unit/test/sansio/reset_connection.cpp +++ b/test/unit/test/sansio/reset_connection.cpp @@ -16,12 +16,14 @@ #include #include "test_common/assert_buffer_equals.hpp" +#include "test_common/buffer_concat.hpp" #include "test_common/create_diagnostics.hpp" #include "test_unit/algo_test.hpp" #include "test_unit/create_err.hpp" #include "test_unit/create_frame.hpp" #include "test_unit/create_ok.hpp" #include "test_unit/create_ok_frame.hpp" +#include "test_unit/create_query_frame.hpp" using namespace boost::mysql::test; using namespace boost::mysql; @@ -30,7 +32,9 @@ BOOST_AUTO_TEST_SUITE(test_reset_connection) struct fixture : algo_fixture_base { - detail::reset_connection_algo algo{st, {&diag}}; + detail::reset_connection_algo algo; + + fixture(character_set charset = {}) : algo(st, {&diag, charset}) {} }; BOOST_AUTO_TEST_CASE(success) @@ -96,4 +100,114 @@ BOOST_AUTO_TEST_CASE(error_response) BOOST_TEST(fix.st.charset_ptr()->name == "utf8mb4"); } +BOOST_AUTO_TEST_CASE(charset_success) +{ + // Setup + fixture fix(ascii_charset); + fix.st.current_charset = utf8mb4_charset; + + // Run the algo + algo_test() + .expect_write(concat_copy(create_frame(0, {0x1f}), create_query_frame(0, "SET NAMES 'ascii'"))) + .expect_read(create_ok_frame(1, ok_builder().build())) + .expect_read(create_ok_frame(1, ok_builder().build())) + .check(fix); + + // The OK packets were processed correctly. The charset is set + BOOST_TEST(fix.st.backslash_escapes); + BOOST_TEST(fix.st.charset_ptr()->name == "ascii"); +} + +BOOST_AUTO_TEST_CASE(charset_error_network) +{ + struct charset_fixture : fixture + { + charset_fixture() : fixture(ascii_charset) {} + }; + + // This covers errors in read and write + algo_test() + .expect_write(concat_copy(create_frame(0, {0x1f}), create_query_frame(0, "SET NAMES 'ascii'"))) + .expect_read(create_ok_frame(1, ok_builder().build())) + .expect_read(create_ok_frame(1, ok_builder().build())) + .check_network_errors(); +} + +BOOST_AUTO_TEST_CASE(charset_reset_error_response) +{ + // Setup + fixture fix(ascii_charset); + fix.st.current_charset = utf8mb4_charset; + + // Run the algo + algo_test() + .expect_write(concat_copy(create_frame(0, {0x1f}), create_query_frame(0, "SET NAMES 'ascii'"))) + .expect_read( + err_builder().seqnum(1).code(common_server_errc::er_cant_create_db).message("abc").build_frame() + ) + .expect_read(create_ok_frame(1, ok_builder().build())) + .check(fix, common_server_errc::er_cant_create_db, create_server_diag("abc")); + + // Both packets are read, even if the first one is an error. The character set is updated because the + // second packet is an OK + BOOST_TEST(fix.st.backslash_escapes); + BOOST_TEST(fix.st.charset_ptr()->name == "ascii"); +} + +BOOST_AUTO_TEST_CASE(charset_set_names_error_response) +{ + // Setup + fixture fix(ascii_charset); + fix.st.current_charset = utf8mb4_charset; + + // Run the algo + algo_test() + .expect_write(concat_copy(create_frame(0, {0x1f}), create_query_frame(0, "SET NAMES 'ascii'"))) + .expect_read(create_ok_frame(1, ok_builder().build())) + .expect_read( + err_builder().seqnum(1).code(common_server_errc::er_cant_create_db).message("abc").build_frame() + ) + .check(fix, common_server_errc::er_cant_create_db, create_server_diag("abc")); + + // Both packets are read. The character set is reset + BOOST_TEST(fix.st.backslash_escapes); + BOOST_TEST(fix.st.charset_ptr() == nullptr); +} + +BOOST_AUTO_TEST_CASE(charset_all_error_responses) +{ + // Setup + fixture fix(ascii_charset); + fix.st.current_charset = utf8mb4_charset; + + // Run the algo + algo_test() + .expect_write(concat_copy(create_frame(0, {0x1f}), create_query_frame(0, "SET NAMES 'ascii'"))) + .expect_read(err_builder() + .seqnum(1) + .code(common_server_errc::er_autoinc_read_failed) + .message("hello") + .build_frame()) + .expect_read( + err_builder().seqnum(1).code(common_server_errc::er_cant_create_db).message("abc").build_frame() + ) + .check(fix, common_server_errc::er_autoinc_read_failed, create_server_diag("hello")); + + // Both packets are read. The character is not reset + BOOST_TEST(fix.st.backslash_escapes); + BOOST_TEST(fix.st.charset_ptr()->name == "utf8mb4"); +} + +BOOST_AUTO_TEST_CASE(charset_error_nonascii_charset_name) +{ + // Setup + fixture fix({"lat\xc3\xadn", ascii_charset.next_char}); + + // Run the algo + algo_test().check(fix, client_errc::invalid_encoding); + + // The current character set was not updated + BOOST_TEST(fix.st.charset_ptr() == nullptr); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/test/unit/test/sansio/set_character_set.cpp b/test/unit/test/sansio/set_character_set.cpp index 8070edfe0..ff39dc0a7 100644 --- a/test/unit/test/sansio/set_character_set.cpp +++ b/test/unit/test/sansio/set_character_set.cpp @@ -25,6 +25,7 @@ #include "test_unit/create_frame.hpp" #include "test_unit/create_ok.hpp" #include "test_unit/create_ok_frame.hpp" +#include "test_unit/create_query_frame.hpp" using namespace boost::mysql::test; using namespace boost::mysql; @@ -38,22 +39,6 @@ struct fixture : algo_fixture_base fixture(const character_set& charset = utf8mb4_charset) : algo(st, {&diag, charset}) {} }; -// GCC raises a spurious warning here -#if BOOST_GCC >= 110000 -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wstringop-overread" -#endif -static std::vector create_query_body(string_view sql) -{ - std::vector res{0x03}; // COM_QUERY command id - const auto* data = reinterpret_cast(sql.data()); - res.insert(res.end(), data, data + sql.size()); - return res; -} -#if BOOST_GCC >= 110000 -#pragma GCC diagnostic pop -#endif - BOOST_AUTO_TEST_CASE(success) { // Setup @@ -61,7 +46,7 @@ BOOST_AUTO_TEST_CASE(success) // Run the algo algo_test() - .expect_write(create_frame(0, create_query_body("SET NAMES 'utf8mb4'"))) + .expect_write(create_query_frame(0, "SET NAMES 'utf8mb4'")) .expect_read(create_ok_frame(1, ok_builder().build())) .check(fix); @@ -77,7 +62,7 @@ BOOST_AUTO_TEST_CASE(success_previous_charset) // Run the algo algo_test() - .expect_write(create_frame(0, create_query_body("SET NAMES 'utf8mb4'"))) + .expect_write(create_query_frame(0, "SET NAMES 'utf8mb4'")) .expect_read(create_ok_frame(1, ok_builder().build())) .check(fix); @@ -89,7 +74,7 @@ BOOST_AUTO_TEST_CASE(error_network) { // This covers errors in read and write algo_test() - .expect_write(create_frame(0, create_query_body("SET NAMES 'utf8mb4'"))) + .expect_write(create_query_frame(0, "SET NAMES 'utf8mb4'")) .expect_read(create_ok_frame(1, ok_builder().build())) .check_network_errors(); } @@ -101,7 +86,7 @@ BOOST_AUTO_TEST_CASE(error_response) // Run the algo algo_test() - .expect_write(create_frame(0, create_query_body("SET NAMES 'utf8mb4'"))) + .expect_write(create_query_frame(0, "SET NAMES 'utf8mb4'")) .expect_read(err_builder() .seqnum(1) .code(common_server_errc::er_unknown_character_set) @@ -124,7 +109,7 @@ BOOST_AUTO_TEST_CASE(charset_name_needs_escaping) // Run the algo algo_test() - .expect_write(create_frame(0, create_query_body("SET NAMES 'lat\\'in\\\\'"))) + .expect_write(create_query_frame(0, "SET NAMES 'lat\\'in\\\\'")) .expect_read(create_ok_frame(1, ok_builder().build())) .check(fix); }