diff --git a/include/boost/redis/connection.hpp b/include/boost/redis/connection.hpp index 57df0915..47e7c5b6 100644 --- a/include/boost/redis/connection.hpp +++ b/include/boost/redis/connection.hpp @@ -13,16 +13,17 @@ #include #include #include +#include #include #include #include #include -#include #include #include #include #include #include +#include #include #include @@ -67,7 +68,6 @@ struct connection_impl { Executor, void(system::error_code, std::size_t)>; using health_checker_type = detail::health_checker; - using resp3_handshaker_type = detail::resp3_handshaker; using exec_notifier_type = asio::experimental::channel< Executor, void(system::error_code, std::size_t)>; @@ -81,12 +81,13 @@ struct connection_impl { timer_type reconnect_timer_; // to wait the reconnection period receive_channel_type receive_channel_; health_checker_type health_checker_; - resp3_handshaker_type handshaker_; config cfg_; multiplexer mpx_; connection_logger logger_; read_buffer read_buffer_; + request hello_req_; + generic_response hello_resp_; using executor_type = Executor; @@ -323,6 +324,27 @@ private: using order_t = std::array; + static system::error_code on_hello(connection_impl& conn, system::error_code ec) + { + ec = check_hello_response(ec, conn.hello_resp_); + conn.logger_.on_hello(ec, conn.hello_resp_); + if (ec) { + conn.cancel(operation::run); + } + return ec; + } + + template + auto handshaker(CompletionToken&& token) + { + return conn_->async_exec( + conn_->hello_req_, + any_adapter(conn_->hello_resp_), + asio::deferred([&conn = *this->conn_](system::error_code hello_ec, std::size_t) { + return asio::deferred.values(on_hello(conn, hello_ec)); + }))(std::forward(token)); + } + template auto reader(CompletionToken&& token) { @@ -389,6 +411,9 @@ public: return; } + // Set up the hello request, as it only depends on the config + setup_hello_request(conn_->cfg_, conn_->hello_req_); + for (;;) { // Try to connect BOOST_ASIO_CORO_YIELD @@ -398,6 +423,7 @@ public: if (!ec) { conn_->read_buffer_.clear(); conn_->mpx_.reset(); + clear_response(conn_->hello_resp_); // Note: Order is important here because the writer might // trigger an async_write before the async_hello thereby @@ -405,7 +431,7 @@ public: BOOST_ASIO_CORO_YIELD asio::experimental::make_parallel_group( [this](auto token) { - return conn_->handshaker_.async_hello(*conn_, token); + return this->handshaker(token); }, [this](auto token) { return conn_->health_checker_.async_ping(*conn_, token); @@ -602,7 +628,6 @@ public: { impl_->cfg_ = cfg; impl_->health_checker_.set_config(cfg); - impl_->handshaker_.set_config(cfg); impl_->read_buffer_.set_config({cfg.read_buffer_append_size, cfg.max_read_size}); return asio::async_compose( @@ -907,7 +932,6 @@ private: executor_type, void(system::error_code, std::size_t)>; using health_checker_type = detail::health_checker; - using resp3_handshaker_type = detail::resp3_handshaker; auto use_ssl() const noexcept { return impl_->cfg_.use_ssl; } diff --git a/include/boost/redis/detail/health_checker.hpp b/include/boost/redis/detail/health_checker.hpp index c4a7eccb..0351ad5a 100644 --- a/include/boost/redis/detail/health_checker.hpp +++ b/include/boost/redis/detail/health_checker.hpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -17,6 +18,7 @@ #include #include #include +#include #include #include @@ -72,6 +74,12 @@ public: self.complete(ec); return; } + + if (is_cancelled(self)) { + conn_->logger_.trace("ping_op (5): cancelled"); + self.complete(asio::error::operation_aborted); + return; + } } } }; diff --git a/include/boost/redis/detail/hello_utils.hpp b/include/boost/redis/detail/hello_utils.hpp new file mode 100644 index 00000000..aa313249 --- /dev/null +++ b/include/boost/redis/detail/hello_utils.hpp @@ -0,0 +1,22 @@ +/* Copyright (c) 2018-2024 Marcelo Zimbres Silva (mzimbres@gmail.com) + * + * Distributed under the Boost Software License, Version 1.0. (See + * accompanying file LICENSE.txt) + */ + +#ifndef BOOST_REDIS_HELLO_UTILS_HPP +#define BOOST_REDIS_HELLO_UTILS_HPP + +#include +#include +#include + +namespace boost::redis::detail { + +void setup_hello_request(config const& cfg, request& req); +void clear_response(generic_response& res); +system::error_code check_hello_response(system::error_code io_ec, const generic_response&); + +} // namespace boost::redis::detail + +#endif // BOOST_REDIS_RUNNER_HPP diff --git a/include/boost/redis/detail/resp3_handshaker.hpp b/include/boost/redis/detail/resp3_handshaker.hpp deleted file mode 100644 index 06eef7ae..00000000 --- a/include/boost/redis/detail/resp3_handshaker.hpp +++ /dev/null @@ -1,115 +0,0 @@ -/* Copyright (c) 2018-2024 Marcelo Zimbres Silva (mzimbres@gmail.com) - * - * Distributed under the Boost Software License, Version 1.0. (See - * accompanying file LICENSE.txt) - */ - -#ifndef BOOST_REDIS_RUNNER_HPP -#define BOOST_REDIS_RUNNER_HPP - -#include -#include -#include -#include -#include -#include - -#include -#include - -#include - -namespace boost::redis::detail { - -void push_hello(config const& cfg, request& req); - -// TODO: Can we avoid this whole function whose only purpose is to -// check for an error in the hello response and complete with an error -// so that the parallel group that starts it can exit? -template -struct hello_op { - Handshaker* handshaker_ = nullptr; - ConnectionImpl* conn_ = nullptr; - asio::coroutine coro_{}; - - template - void operator()(Self& self, system::error_code ec = {}, std::size_t = 0) - { - BOOST_ASIO_CORO_REENTER(coro_) - { - handshaker_->add_hello(); - - BOOST_ASIO_CORO_YIELD - conn_->async_exec( - handshaker_->hello_req_, - any_adapter{handshaker_->hello_resp_}, - std::move(self)); - - conn_->logger_.on_hello(ec, handshaker_->hello_resp_); - - if (ec) { - conn_->cancel(operation::run); - self.complete(ec); - return; - } - - if (handshaker_->has_error_in_response()) { - conn_->cancel(operation::run); - self.complete(error::resp3_hello); - return; - } - - self.complete({}); - } - } -}; - -template -class resp3_handshaker { -public: - void set_config(config const& cfg) { cfg_ = cfg; } - - template - auto async_hello(ConnectionImpl& conn, CompletionToken token) - { - return asio::async_compose( - hello_op{this, &conn}, - token, - conn); - } - -private: - template friend struct hello_op; - - void add_hello() - { - hello_req_.clear(); - if (hello_resp_.has_value()) - hello_resp_.value().clear(); - push_hello(cfg_, hello_req_); - } - - bool has_error_in_response() const noexcept - { - if (!hello_resp_.has_value()) - return true; - - auto f = [](auto const& e) { - switch (e.data_type) { - case resp3::type::simple_error: - case resp3::type::blob_error: return true; - default: return false; - } - }; - - return std::any_of(std::cbegin(hello_resp_.value()), std::cend(hello_resp_.value()), f); - } - - request hello_req_; - generic_response hello_resp_; - config cfg_; -}; - -} // namespace boost::redis::detail - -#endif // BOOST_REDIS_RUNNER_HPP diff --git a/include/boost/redis/impl/hello_utils.ipp b/include/boost/redis/impl/hello_utils.ipp new file mode 100644 index 00000000..b1376b14 --- /dev/null +++ b/include/boost/redis/impl/hello_utils.ipp @@ -0,0 +1,53 @@ +/* Copyright (c) 2018-2024 Marcelo Zimbres Silva (mzimbres@gmail.com) + * + * Distributed under the Boost Software License, Version 1.0. (See + * accompanying file LICENSE.txt) + */ + +#include + +namespace boost::redis::detail { + +void setup_hello_request(config const& cfg, request& req) +{ + // Which parts of the command should we send? + // Don't send AUTH if the user is the default and the password is empty. + // Other users may have empty passwords. + // Note that this is just an optimization. + bool send_auth = !(cfg.username.empty() || (cfg.username == "default" && cfg.password.empty())); + bool send_setname = !cfg.clientname.empty(); + + req.clear(); + if (send_auth && send_setname) + req.push("HELLO", "3", "AUTH", cfg.username, cfg.password, "SETNAME", cfg.clientname); + else if (send_auth) + req.push("HELLO", "3", "AUTH", cfg.username, cfg.password); + else if (send_setname) + req.push("HELLO", "3", "SETNAME", cfg.clientname); + else + req.push("HELLO", "3"); + + if (cfg.database_index && cfg.database_index.value() != 0) + req.push("SELECT", cfg.database_index.value()); +} + +void clear_response(generic_response& res) +{ + if (res.has_value()) + res->clear(); + else + res.emplace(); +} + +system::error_code check_hello_response(system::error_code io_ec, const generic_response& resp) +{ + if (io_ec) + return io_ec; + + if (resp.has_error()) + return error::resp3_hello; + + return system::error_code(); +} + +} // namespace boost::redis::detail diff --git a/include/boost/redis/impl/resp3_handshaker.ipp b/include/boost/redis/impl/resp3_handshaker.ipp deleted file mode 100644 index db516419..00000000 --- a/include/boost/redis/impl/resp3_handshaker.ipp +++ /dev/null @@ -1,26 +0,0 @@ -/* Copyright (c) 2018-2024 Marcelo Zimbres Silva (mzimbres@gmail.com) - * - * Distributed under the Boost Software License, Version 1.0. (See - * accompanying file LICENSE.txt) - */ - -#include - -namespace boost::redis::detail { - -void push_hello(config const& cfg, request& req) -{ - if (!cfg.username.empty() && !cfg.password.empty() && !cfg.clientname.empty()) - req.push("HELLO", "3", "AUTH", cfg.username, cfg.password, "SETNAME", cfg.clientname); - else if (cfg.password.empty() && cfg.clientname.empty()) - req.push("HELLO", "3"); - else if (cfg.clientname.empty()) - req.push("HELLO", "3", "AUTH", cfg.username, cfg.password); - else - req.push("HELLO", "3", "SETNAME", cfg.clientname); - - if (cfg.database_index && cfg.database_index.value() != 0) - req.push("SELECT", cfg.database_index.value()); -} - -} // namespace boost::redis::detail diff --git a/include/boost/redis/src.hpp b/include/boost/redis/src.hpp index 144b3a4a..31069f8f 100644 --- a/include/boost/redis/src.hpp +++ b/include/boost/redis/src.hpp @@ -14,7 +14,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index e9525e62..1c7f7c3a 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -41,6 +41,7 @@ make_test(test_exec_fsm) make_test(test_log_to_file) make_test(test_conn_logging) make_test(test_reader_fsm) +make_test(test_hello_utils) # Tests that require a real Redis server make_test(test_conn_quit) @@ -55,6 +56,7 @@ make_test(test_conn_exec_cancel) make_test(test_conn_exec_cancel2) make_test(test_conn_echo_stress) make_test(test_conn_move) +make_test(test_conn_auth) make_test(test_issue_50) make_test(test_issue_181) make_test(test_conversions) diff --git a/test/Jamfile b/test/Jamfile index f5425573..68f3fa79 100644 --- a/test/Jamfile +++ b/test/Jamfile @@ -57,6 +57,7 @@ local tests = test_log_to_file test_conn_logging test_reader_fsm + test_hello_utils ; # Build and run the tests diff --git a/test/test_conn_auth.cpp b/test/test_conn_auth.cpp new file mode 100644 index 00000000..d305e284 --- /dev/null +++ b/test/test_conn_auth.cpp @@ -0,0 +1,143 @@ +// +// Copyright (c) 2025 Marcelo Zimbres Silva (mzimbres@gmail.com), +// 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) +// + +#include +#include +#include +#include + +#include +#include +#include + +#include "common.hpp" + +#include +#include +#include + +namespace asio = boost::asio; +namespace redis = boost::redis; +using namespace std::chrono_literals; +using boost::system::error_code; + +namespace { + +// Creates a user with a known password. Harmless if the user already exists +void setup_password() +{ + // Setup + asio::io_context ioc; + redis::connection conn{ioc}; + + // Enable the user and grant them permissions on everything + redis::request req; + req.push("ACL", "SETUSER", "myuser", "on", ">mypass", "~*", "&*", "+@all"); + redis::generic_response resp; + + bool run_finished = false, exec_finished = false; + conn.async_run(make_test_config(), [&](error_code ec) { + run_finished = true; + BOOST_TEST_EQ(ec, asio::error::operation_aborted); + }); + + conn.async_exec(req, resp, [&](error_code ec, std::size_t) { + exec_finished = true; + BOOST_TEST_EQ(ec, error_code()); + conn.cancel(); + }); + + ioc.run_for(test_timeout); + + BOOST_TEST(run_finished); + BOOST_TEST(exec_finished); + BOOST_TEST(resp.has_value()); +} + +void test_auth_success() +{ + // Setup + asio::io_context ioc; + redis::connection conn{ioc}; + + // This request should return the username we're logged in as + redis::request req; + req.push("ACL", "WHOAMI"); + redis::response resp; + + // These credentials are set up in main, before tests are run + auto cfg = make_test_config(); + cfg.username = "myuser"; + cfg.password = "mypass"; + + bool exec_finished = false, run_finished = false; + + conn.async_exec(req, resp, [&](error_code ec, std::size_t) { + exec_finished = true; + BOOST_TEST_EQ(ec, error_code()); + conn.cancel(); + }); + + conn.async_run(cfg, [&](error_code ec) { + run_finished = true; + BOOST_TEST_EQ(ec, asio::error::operation_aborted); + }); + + ioc.run_for(test_timeout); + + BOOST_TEST(exec_finished); + BOOST_TEST(run_finished); + BOOST_TEST_EQ(std::get<0>(resp).value(), "myuser"); +} + +void test_auth_failure() +{ + // Verify that we log appropriately (see https://github.com/boostorg/redis/issues/297) + std::ostringstream oss; + redis::logger lgr(redis::logger::level::info, [&](redis::logger::level, std::string_view msg) { + oss << msg << '\n'; + }); + + // Setup + asio::io_context ioc; + redis::connection conn{ioc, std::move(lgr)}; + + // Disable reconnection so the hello error causes the connection to exit + auto cfg = make_test_config(); + cfg.username = "myuser"; + cfg.password = "wrongpass"; // wrong + cfg.reconnect_wait_interval = 0s; + + bool run_finished = false; + + conn.async_run(cfg, [&](error_code ec) { + run_finished = true; + BOOST_TEST_EQ(ec, redis::error::resp3_hello); + }); + + ioc.run_for(test_timeout); + + BOOST_TEST(run_finished); + + // Check the log + auto log = oss.str(); + if (!BOOST_TEST_NE(log.find("WRONGPASS"), std::string::npos)) { + std::cerr << "Log was: " << log << std::endl; + } +} + +} // namespace + +int main() +{ + setup_password(); + test_auth_success(); + test_auth_failure(); + + return boost::report_errors(); +} diff --git a/test/test_hello_utils.cpp b/test/test_hello_utils.cpp new file mode 100644 index 00000000..11abd1b3 --- /dev/null +++ b/test/test_hello_utils.cpp @@ -0,0 +1,189 @@ +// +// Copyright (c) 2025 Marcelo Zimbres Silva (mzimbres@gmail.com), +// 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) +// + +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include + +namespace asio = boost::asio; +namespace redis = boost::redis; +using redis::detail::setup_hello_request; +using redis::detail::clear_response; +using redis::detail::check_hello_response; +using boost::system::error_code; + +namespace { + +void test_setup_hello_request() +{ + redis::config cfg; + cfg.clientname = ""; + redis::request req; + + setup_hello_request(cfg, req); + + std::string_view const expected = "*2\r\n$5\r\nHELLO\r\n$1\r\n3\r\n"; + BOOST_TEST_EQ(req.payload(), expected); +} + +void test_setup_hello_request_select() +{ + redis::config cfg; + cfg.clientname = ""; + cfg.database_index = 10; + redis::request req; + + setup_hello_request(cfg, req); + + std::string_view const expected = + "*2\r\n$5\r\nHELLO\r\n$1\r\n3\r\n" + "*2\r\n$6\r\nSELECT\r\n$2\r\n10\r\n"; + BOOST_TEST_EQ(req.payload(), expected); +} + +void test_setup_hello_request_clientname() +{ + redis::config cfg; + redis::request req; + + setup_hello_request(cfg, req); + + std::string_view const + expected = "*4\r\n$5\r\nHELLO\r\n$1\r\n3\r\n$7\r\nSETNAME\r\n$11\r\nBoost.Redis\r\n"; + BOOST_TEST_EQ(req.payload(), expected); +} + +void test_setup_hello_request_auth() +{ + redis::config cfg; + cfg.clientname = ""; + cfg.username = "foo"; + cfg.password = "bar"; + redis::request req; + + setup_hello_request(cfg, req); + + std::string_view const + expected = "*5\r\n$5\r\nHELLO\r\n$1\r\n3\r\n$4\r\nAUTH\r\n$3\r\nfoo\r\n$3\r\nbar\r\n"; + BOOST_TEST_EQ(req.payload(), expected); +} + +void test_setup_hello_request_auth_empty_password() +{ + redis::config cfg; + cfg.clientname = ""; + cfg.username = "foo"; + redis::request req; + + setup_hello_request(cfg, req); + + std::string_view const + expected = "*5\r\n$5\r\nHELLO\r\n$1\r\n3\r\n$4\r\nAUTH\r\n$3\r\nfoo\r\n$0\r\n\r\n"; + BOOST_TEST_EQ(req.payload(), expected); +} + +void test_setup_hello_request_auth_setname() +{ + redis::config cfg; + cfg.clientname = "mytest"; + cfg.username = "foo"; + cfg.password = "bar"; + redis::request req; + + setup_hello_request(cfg, req); + + std::string_view const expected = + "*7\r\n$5\r\nHELLO\r\n$1\r\n3\r\n$4\r\nAUTH\r\n$3\r\nfoo\r\n$3\r\nbar\r\n$7\r\nSETNAME\r\n$" + "6\r\nmytest\r\n"; + BOOST_TEST_EQ(req.payload(), expected); +} + +// clear response +void test_clear_response_empty() +{ + redis::generic_response resp; + clear_response(resp); + BOOST_TEST(resp.has_value()); + BOOST_TEST_EQ(resp.value().size(), 0u); +} + +void test_clear_response_nonempty() +{ + redis::generic_response resp; + resp->push_back({}); + clear_response(resp); + BOOST_TEST(resp.has_value()); + BOOST_TEST_EQ(resp.value().size(), 0u); +} + +void test_clear_response_error() +{ + redis::generic_response resp{ + boost::system::in_place_error, + redis::adapter::error{redis::resp3::type::blob_error, "message"} + }; + clear_response(resp); + BOOST_TEST(resp.has_value()); + BOOST_TEST_EQ(resp.value().size(), 0u); +} + +// check response +void test_check_hello_response_success() +{ + redis::generic_response resp; + resp->push_back({}); + auto ec = check_hello_response(error_code(), resp); + BOOST_TEST_EQ(ec, error_code()); +} + +void test_check_hello_response_io_error() +{ + redis::generic_response resp; + auto ec = check_hello_response(asio::error::already_open, resp); + BOOST_TEST_EQ(ec, asio::error::already_open); +} + +void test_check_hello_response_server_error() +{ + redis::generic_response resp{ + boost::system::in_place_error, + redis::adapter::error{redis::resp3::type::simple_error, "wrong password"} + }; + auto ec = check_hello_response(error_code(), resp); + BOOST_TEST_EQ(ec, redis::error::resp3_hello); +} + +} // namespace + +int main() +{ + test_setup_hello_request(); + test_setup_hello_request_select(); + test_setup_hello_request_clientname(); + test_setup_hello_request_auth(); + test_setup_hello_request_auth_empty_password(); + test_setup_hello_request_auth_setname(); + + test_clear_response_empty(); + test_clear_response_nonempty(); + test_clear_response_error(); + + test_check_hello_response_success(); + test_check_hello_response_io_error(); + test_check_hello_response_server_error(); + + return boost::report_errors(); +} \ No newline at end of file diff --git a/test/test_low_level_sync_sans_io.cpp b/test/test_low_level_sync_sans_io.cpp index 9b198c4b..4e3ff88b 100644 --- a/test/test_low_level_sync_sans_io.cpp +++ b/test/test_low_level_sync_sans_io.cpp @@ -8,26 +8,24 @@ #include #include #include -#include +#include #include #include #include -#define BOOST_TEST_MODULE conn_quit -#include +#include -#include "common.hpp" +#define BOOST_TEST_MODULE low_level_sync_sans_io +#include #include #include +using boost::redis::request; using boost::redis::adapter::adapt2; using boost::redis::adapter::result; -using boost::redis::config; using boost::redis::detail::multiplexer; -using boost::redis::detail::push_hello; using boost::redis::generic_response; using boost::redis::ignore_t; -using boost::redis::request; using boost::redis::resp3::detail::deserialize; using boost::redis::resp3::node; using boost::redis::resp3::to_string; @@ -57,61 +55,6 @@ BOOST_AUTO_TEST_CASE(low_level_sync_sans_io) } } -BOOST_AUTO_TEST_CASE(config_to_hello) -{ - config cfg; - cfg.clientname = ""; - request req; - - push_hello(cfg, req); - - std::string_view const expected = "*2\r\n$5\r\nHELLO\r\n$1\r\n3\r\n"; - BOOST_CHECK_EQUAL(req.payload(), expected); -} - -BOOST_AUTO_TEST_CASE(config_to_hello_with_select) -{ - config cfg; - cfg.clientname = ""; - cfg.database_index = 10; - request req; - - push_hello(cfg, req); - - std::string_view const expected = - "*2\r\n$5\r\nHELLO\r\n$1\r\n3\r\n" - "*2\r\n$6\r\nSELECT\r\n$2\r\n10\r\n"; - - BOOST_CHECK_EQUAL(req.payload(), expected); -} - -BOOST_AUTO_TEST_CASE(config_to_hello_cmd_clientname) -{ - config cfg; - request req; - - push_hello(cfg, req); - - std::string_view const - expected = "*4\r\n$5\r\nHELLO\r\n$1\r\n3\r\n$7\r\nSETNAME\r\n$11\r\nBoost.Redis\r\n"; - BOOST_CHECK_EQUAL(req.payload(), expected); -} - -BOOST_AUTO_TEST_CASE(config_to_hello_cmd_auth) -{ - config cfg; - cfg.clientname = ""; - cfg.username = "foo"; - cfg.password = "bar"; - request req; - - push_hello(cfg, req); - - std::string_view const - expected = "*5\r\n$5\r\nHELLO\r\n$1\r\n3\r\n$4\r\nAUTH\r\n$3\r\nfoo\r\n$3\r\nbar\r\n"; - BOOST_CHECK_EQUAL(req.payload(), expected); -} - BOOST_AUTO_TEST_CASE(issue_210_empty_set) { try {