Skip to content

Commit

Permalink
Merge pull request #891 from redboltz/add_empty_client_id_support
Browse files Browse the repository at this point in the history 
Implemented client_id assignment when client send empty one.
  • Loading branch information
@redboltz
redboltz authored Oct 18, 2021
2 parents 380a1ac + cbb3fbe commit fff2708
Show file tree
Hide file tree
Showing 5 changed files with 166 additions and 41 deletions.
75 changes: 62 additions & 13 deletions include/mqtt/broker/broker.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include <mqtt/broker/retained_topic_map.hpp>
#include <mqtt/broker/shared_target_impl.hpp>
#include <mqtt/broker/mutex.hpp>
#include <mqtt/broker/uuid.hpp>

MQTT_BROKER_NS_BEGIN

Expand Down Expand Up @@ -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:
Expand All @@ -1068,21 +1115,23 @@ 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);
}
);
}
else {
// use connack_props_ for testing
if (connack_) ep.async_connack(
session_present,
v5::connect_reason_code::success,
Expand Down
28 changes: 28 additions & 0 deletions include/mqtt/broker/uuid.hpp
View file
Original file line number Diff line number Diff line change
@@ -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 <string>

#include <boost/uuid/uuid.hpp>
#include <boost/uuid/uuid_generators.hpp>
#include <boost/uuid/uuid_io.hpp>

#include <mqtt/broker/broker_namespace.hpp>

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
25 changes: 2 additions & 23 deletions include/mqtt/client.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -405,26 +405,6 @@ class client : public endpoint<std::mutex, std::lock_guard, PacketIdBytes> {
port_ = force_move(port);
}

/**
* @brief Set client id.
* @param id client id
*
* This function should be called before calling connect().<BR>
* See https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901059<BR>
* 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
Expand Down Expand Up @@ -1173,7 +1153,7 @@ class client : public endpoint<std::mutex, std::lock_guard, PacketIdBytes> {
// 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<buffer>() ),
( password_ ? buffer(string_view(password_.value()))
Expand All @@ -1189,7 +1169,7 @@ class client : public endpoint<std::mutex, std::lock_guard, PacketIdBytes> {
// 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<buffer>() ),
( password_ ? buffer(string_view(password_.value()))
Expand Down Expand Up @@ -1563,7 +1543,6 @@ class client : public endpoint<std::mutex, std::lock_guard, PacketIdBytes> {
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> will_;
optional<std::string> user_name_;
optional<std::string> password_;
Expand Down
46 changes: 45 additions & 1 deletion include/mqtt/endpoint.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -849,6 +849,26 @@ class endpoint : public std::enable_shared_from_this<endpoint<Mutex, LockGuard,
return clean_start_;
}

/**
* @brief Get the client id.
* @return The client id of this client.
*/
std::string const& get_client_id() const {
return client_id_;
}

/**
* @brief Set client id.
* @param id client id
*
* This function should be called before calling connect().<BR>
* See https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901059<BR>
* 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.
Expand Down Expand Up @@ -5112,6 +5132,10 @@ class endpoint : public std::enable_shared_from_this<endpoint<Mutex, LockGuard,
pid_man_.clear();
}

void set_client_id(protocol_version version) {
version_ = version;
}

private:
enum class connection_type {
client,
Expand Down Expand Up @@ -5208,6 +5232,7 @@ class endpoint : public std::enable_shared_from_this<endpoint<Mutex, LockGuard,
std::size_t topic_alias_maximum_count = 0;
std::size_t maximum_packet_size_count = 0;
std::size_t receive_maximum_count = 0;
std::size_t assigned_client_identifier_count = 0;
v5::visit_props(
props,
[&](v5::property::topic_alias_maximum const& p) {
Expand Down Expand Up @@ -5259,6 +5284,23 @@ class endpoint : public std::enable_shared_from_this<endpoint<Mutex, LockGuard,
}
publish_send_max_ = p.val();
},
[&](v5::property::assigned_client_identifier const& p) {
if (type != connection_type::client) {
MQTT_SEND_ERROR(protocol_error);
ret = false;
return;
}
if (++assigned_client_identifier_count == 2) {
MQTT_SEND_ERROR(protocol_error);
ret = false;
return;
}
if (assigned_client_identifier_count > 2) {
ret = false;
return;
}
set_client_id(std::string(p.val()));
},
[](auto&&) {
}
);
Expand Down Expand Up @@ -7556,7 +7598,7 @@ class endpoint : public std::enable_shared_from_this<endpoint<Mutex, LockGuard,
ep_.publish_send_queue_.clear();
}
}
if (!ep_.set_values_from_props_on_connection(connection_type::server, props_)) return;
if (!ep_.set_values_from_props_on_connection(connection_type::client, props_)) return;
{
auto connack_proc =
[this]
Expand Down Expand Up @@ -11657,6 +11699,8 @@ class endpoint : public std::enable_shared_from_this<endpoint<Mutex, LockGuard,
std::atomic<bool> mqtt_connected_{false};
std::atomic<bool> shutdown_requested_{false};

std::string client_id_;

std::array<char, 10> buf_;
std::uint8_t fixed_header_;
std::size_t remaining_length_multiplier_;
Expand Down
33 changes: 29 additions & 4 deletions test/system/st_connect.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit fff2708

Please sign in to comment.