Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Implemented client_id assignment when client send empty one. #891

Merged
merged 1 commit into from
Oct 18, 2021
Merged

Implemented client_id assignment when client send empty one. #891

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 62 additions & 13 deletions include/mqtt/broker/broker.hpp
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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