diff --git a/include/mqtt/broker/broker.hpp b/include/mqtt/broker/broker.hpp index 42896287d..ac74e47a3 100644 --- a/include/mqtt/broker/broker.hpp +++ b/include/mqtt/broker/broker.hpp @@ -25,6 +25,7 @@ #include #include #include +#include MQTT_BROKER_NS_BEGIN @@ -1032,20 +1033,66 @@ class broker_t { } } - // If the Client supplies a zero-byte ClientId, the Client MUST also set CleanSession to 1 [MQTT-3.1.3-7]. - // If it's a not a clean session, but no client id is provided, we would have no way to map this - // connection's session to a new connection later. So the connection must be rejected. + v5::properties connack_props; + switch (ep.get_protocol_version()) { case protocol_version::v3_1_1: - if (client_id.empty() && !clean_start) { - if (connack_) ep.async_connack(false, connect_return_code::identifier_rejected); - return false; + if (client_id.empty()) { + if (clean_start) { + ep.set_client_id(create_uuid_string()); + } + else { + // https://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc385349242 + // If the Client supplies a zero-byte ClientId, + // the Client MUST also set CleanSession to 1 [MQTT-3.1.3-7]. + // If it's a not a clean session, but no client id is provided, + // we would have no way to map this connection's session to a new connection later. + // So the connection must be rejected. + if (connack_) { + ep.async_connack( + false, + connect_return_code::identifier_rejected, + [&ep, spep = force_move(spep)] + (error_code ec) mutable { + if (ec) { + MQTT_LOG("mqtt_broker", info) + << MQTT_ADD_VALUE(address, spep.get()) + << ec.message(); + } + ep.async_force_disconnect( + [spep = force_move(spep)] + (error_code ec) { + if (ec) { + MQTT_LOG("mqtt_broker", info) + << MQTT_ADD_VALUE(address, spep.get()) + << ec.message(); + } + } + ); + } + ); + } + return false; + } } break; case protocol_version::v5: - if (client_id.empty() && !clean_start) { - if (connack_) ep.async_connack(false, v5::connect_reason_code::client_identifier_not_valid); - return false; + if (client_id.empty()) { + // https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901059 + // A Server MAY allow a Client to supply a ClientID that has a length of zero bytes, + // however if it does so the Server MUST treat this as a special case and assign a + // unique ClientID to that Client [MQTT-3.1.3-6]. It MUST then process the + // CONNECT packet as if the Client had provided that unique ClientID, + // and MUST return the Assigned Client Identifier in the CONNACK packet [MQTT-3.1.3-7]. + // If the Server rejects the ClientID it MAY respond to the CONNECT packet with a CONNACK + // using Reason Code 0x85 (Client Identifier not valid) as described in section 4.13 + // Handling errors, and then it MUST close the Network Connection [MQTT-3.1.3-8]. + // + // mqtt_cpp author's note: On v5.0, no Clean Start restriction is described. + ep.set_client_id(create_uuid_string()); + connack_props.emplace_back( + v5::property::assigned_client_identifier(buffer(string_view(ep.get_client_id()))) + ); } break; default: @@ -1068,14 +1115,15 @@ class broker_t { ); break; case protocol_version::v5: + // connack_props_ member varible is for testing if (connack_props_.empty()) { + // connack_props local variable is is for real case + connack_props.emplace_back(v5::property::topic_alias_maximum{topic_alias_max}); + connack_props.emplace_back(v5::property::receive_maximum{receive_maximum_max}); if (connack_) ep.async_connack( session_present, v5::connect_reason_code::success, - v5::properties{ - v5::property::topic_alias_maximum{topic_alias_max}, - v5::property::receive_maximum{receive_maximum_max} - }, + force_move(connack_props), [finish = force_move(finish)] (error_code ec) { finish(ec); @@ -1083,6 +1131,7 @@ class broker_t { ); } else { + // use connack_props_ for testing if (connack_) ep.async_connack( session_present, v5::connect_reason_code::success, diff --git a/include/mqtt/broker/uuid.hpp b/include/mqtt/broker/uuid.hpp new file mode 100644 index 000000000..5f6ed0da3 --- /dev/null +++ b/include/mqtt/broker/uuid.hpp @@ -0,0 +1,28 @@ +// Copyright Takatoshi Kondo 2021 +// +// 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) + +#if !defined(MQTT_BROKER_UUID_HPP) +#define MQTT_BROKER_UUID_HPP + +#include + +#include +#include +#include + +#include + +MQTT_BROKER_NS_BEGIN + +inline std::string create_uuid_string() { + // See https://github.com/boostorg/uuid/issues/121 + auto gen = boost::uuids::random_generator(); + return boost::uuids::to_string(gen()); +} + +MQTT_BROKER_NS_END + +#endif // MQTT_BROKER_UUID_HPP diff --git a/include/mqtt/client.hpp b/include/mqtt/client.hpp index 525601fe0..8373dad70 100644 --- a/include/mqtt/client.hpp +++ b/include/mqtt/client.hpp @@ -405,26 +405,6 @@ class client : public endpoint { port_ = force_move(port); } - /** - * @brief Set client id. - * @param id client id - * - * This function should be called before calling connect().
- * See https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901059
- * 3.1.3.1 Client Identifier - */ - void set_client_id(std::string id) { - client_id_ = force_move(id); - } - - /** - * @brief Get the client id. - * @return The client id of this client. - */ - std::string const& get_client_id() const { - return client_id_; - } - /** * @brief Set clean session. * @param cs clean session @@ -1173,7 +1153,7 @@ class client : public endpoint { // sync base::connect() refer to parameters only in the function. // So they can be passed as view. base::connect( - buffer(string_view(client_id_)), + buffer(string_view(base::get_client_id())), ( user_name_ ? buffer(string_view(user_name_.value())) : optional() ), ( password_ ? buffer(string_view(password_.value())) @@ -1189,7 +1169,7 @@ class client : public endpoint { // sync base::connect() refer to parameters only in the function. // So they can be passed as view. base::async_connect( - buffer(string_view(client_id_)), + buffer(string_view(base::get_client_id())), ( user_name_ ? buffer(string_view(user_name_.value())) : optional() ), ( password_ ? buffer(string_view(password_.value())) @@ -1563,7 +1543,6 @@ class client : public endpoint { std::string port_; std::uint16_t keep_alive_sec_{0}; std::chrono::steady_clock::duration ping_duration_{std::chrono::steady_clock::duration::zero()}; - std::string client_id_; optional will_; optional user_name_; optional password_; diff --git a/include/mqtt/endpoint.hpp b/include/mqtt/endpoint.hpp index 0f6a12bfc..5a83254fa 100644 --- a/include/mqtt/endpoint.hpp +++ b/include/mqtt/endpoint.hpp @@ -849,6 +849,26 @@ class endpoint : public std::enable_shared_from_this + * See https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901059
+ * 3.1.3.1 Client Identifier + */ + void set_client_id(std::string id) { + client_id_ = force_move(id); + } + /** * @brief get_total_bytes_received * @return The total bytes received on the socket. @@ -5112,6 +5132,10 @@ class endpoint : public std::enable_shared_from_this 2) { + ret = false; + return; + } + set_client_id(std::string(p.val())); + }, [](auto&&) { } ); @@ -7556,7 +7598,7 @@ class endpoint : public std::enable_shared_from_this mqtt_connected_{false}; std::atomic shutdown_requested_{false}; + std::string client_id_; + std::array buf_; std::uint8_t fixed_header_; std::size_t remaining_length_multiplier_; diff --git a/test/system/st_connect.cpp b/test/system/st_connect.cpp index 287882963..c74e70cb3 100644 --- a/test/system/st_connect.cpp +++ b/test/system/st_connect.cpp @@ -487,10 +487,20 @@ BOOST_AUTO_TEST_CASE( nocid ) { case MQTT_NS::protocol_version::v5: c->set_v5_connack_handler( [&chk, &c] - (bool sp, MQTT_NS::v5::connect_reason_code connect_reason_code, MQTT_NS::v5::properties /*props*/) { + (bool sp, MQTT_NS::v5::connect_reason_code connect_reason_code, MQTT_NS::v5::properties props) { MQTT_CHK("h_connack"); BOOST_TEST(sp == false); BOOST_TEST(connect_reason_code == MQTT_NS::v5::connect_reason_code::success); + std::size_t times = 0; + MQTT_NS::v5::visit_props( + props, + [&](MQTT_NS::v5::property::assigned_client_identifier const& p) { + ++times; + BOOST_TEST(p.val() == c->get_client_id()); + }, + [](auto){} + ); + BOOST_TEST(times == 1); c->disconnect(); return true; }); @@ -542,12 +552,27 @@ BOOST_AUTO_TEST_CASE( nocid_noclean ) { }); break; case MQTT_NS::protocol_version::v5: + // On v5, a combination of empty client_id and clean_start:false is accepted. + // Because the client can know the assigned client_id. + // Even if session_expiry_interval != 0 and store the disconnected session, + // the client can access the session using assigned client_id c->set_v5_connack_handler( - [&chk] - (bool sp, MQTT_NS::v5::connect_reason_code connect_reason_code, MQTT_NS::v5::properties /*props*/) { + [&chk, &c] + (bool sp, MQTT_NS::v5::connect_reason_code connect_reason_code, MQTT_NS::v5::properties props) { MQTT_CHK("h_connack"); BOOST_TEST(sp == false); - BOOST_TEST(connect_reason_code == MQTT_NS::v5::connect_reason_code::client_identifier_not_valid); + BOOST_TEST(connect_reason_code == MQTT_NS::v5::connect_reason_code::success); + std::size_t times = 0; + MQTT_NS::v5::visit_props( + props, + [&](MQTT_NS::v5::property::assigned_client_identifier const& p) { + ++times; + BOOST_TEST(p.val() == c->get_client_id()); + }, + [](auto){} + ); + BOOST_TEST(times == 1); + c->force_disconnect(); return true; }); break;