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

Fix 435 #439

Merged
merged 4 commits into from
Sep 15, 2019
Merged

Fix 435 #439

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b929e5b
Fixed #435.
redboltz Sep 14, 2019
8d11b53
Fixed tests.
redboltz Sep 14, 2019
88fdbb6
Fixed typos.
redboltz Sep 14, 2019
43521b1
Moved `connect_no_clean()` to `test_util.hpp".
redboltz Sep 14, 2019
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
9 changes: 8 additions & 1 deletion include/mqtt/endpoint.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11863,11 +11863,17 @@ class endpoint : public std::enable_shared_from_this<endpoint<Mutex, LockGuard,
if (h_disconnect_) {
h_disconnect_();
}
if (func) {
func(boost::system::errc::make_error_code(boost::system::errc::success));
}
break;
case protocol_version::v5:
if (h_v5_disconnect_) {
h_v5_disconnect_(info.reason_code, force_move(info.props));
}
if (func) {
func(boost::system::errc::make_error_code(boost::system::errc::success));
}
break;
default:
BOOST_ASSERT(false);
Expand Down Expand Up @@ -12549,7 +12555,8 @@ class endpoint : public std::enable_shared_from_this<endpoint<Mutex, LockGuard,
if (!connected_) return;
if (h_pre_send_) h_pre_send_();
socket_->write(const_buffer_sequence<PacketIdBytes>(std::forward<MessageVariant>(mv)), ec);
if (ec) handle_error(ec);
// If ec is set as error, the error will be handled by async_read.
// If `handle_error(ec);` is called here, error_handler would be called twice.
}

// Non blocking (async) senders
Expand Down
27 changes: 19 additions & 8 deletions test/connect.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -566,13 +566,9 @@ BOOST_AUTO_TEST_CASE( noclean ) {
break;
case 3:
MQTT_CHK("h_connack4");
// If clean session is not provided, than there will be a session present
// if there was ever a previous connection, even if clean session was provided
// on the previous connection.
// This is because MQTTv5 change the semantics of the flag to "clean start"
// such that it only effects the start of the session.
// Post Session cleanup is handled with a timer, not with the clean session flag.
BOOST_TEST(sp == true);
// The previous connection is not set Session Expiry Interval.
// That means session state is cleared on close.
BOOST_TEST(sp == false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that if session expiry interval was not set, that means indefinite lifetime of the session data?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nervermind. I looked it up.

If the Session Expiry Interval is absent the value 0 is used. If it is set to 0, or is absent, the Session ends when the Network Connection is closed.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. no problem.

break;
}
BOOST_TEST(connack_return_code == MQTT_NS::v5::connect_reason_code::success);
Expand Down Expand Up @@ -603,7 +599,22 @@ BOOST_AUTO_TEST_CASE( noclean ) {
case 2:
MQTT_CHK("h_close3");
c->set_clean_session(false);
c->connect();
switch (c->get_protocol_version()) {
case MQTT_NS::protocol_version::v3_1_1:
c->connect();
break;
case MQTT_NS::protocol_version::v5:
// set session_expiry_interval as infinity.
c->connect(
std::vector<MQTT_NS::v5::property_variant>{
MQTT_NS::v5::property::session_expiry_interval(0xFFFFFFFFUL)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this infinity, or just maximum? It's a uint16_t, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, it's actually infinity.

3.1.2.11.2 Session Expiry Interval
17 (0x11) Byte, Identifier of the Session Expiry Interval.

Followed by the Four Byte Integer representing the Session Expiry Interval in seconds. It is a Protocol Error to include the Session Expiry Interval more than once.

If the Session Expiry Interval is absent the value 0 is used. If it is set to 0, or is absent, the Session ends when the Network Connection is closed.

If the Session Expiry Interval is 0xFFFFFFFF (UINT_MAX), the Session does not expire.

The Client and Server MUST store the Session State after the Network Connection is closed if the Session Expiry Interval is greater than 0 [MQTT-3.1.2-23].

}
);
break;
default:
BOOST_CHECK(false);
break;
}
++connect;
break;
case 3:
Expand Down
64 changes: 28 additions & 36 deletions test/offline.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,18 @@

BOOST_AUTO_TEST_SUITE(test_offline)

template <typename Client>
inline void connect_no_clean(Client& c) {
c->set_clean_session(false);
if (c->get_protocol_version() == MQTT_NS::protocol_version::v5) {
// set session_expiry_interval as infinity.
c->connect(std::vector<MQTT_NS::v5::property_variant>{MQTT_NS::v5::property::session_expiry_interval(0xFFFFFFFFUL)});
}
else {
c->connect();
}
}

BOOST_AUTO_TEST_CASE( publish_qos1 ) {
auto test = [](boost::asio::io_context& ioc, auto& c, auto& s, auto& /*b*/) {
using packet_id_t = typename std::remove_reference_t<decltype(*c)>::packet_id_t;
Expand Down Expand Up @@ -79,13 +91,9 @@ BOOST_AUTO_TEST_CASE( publish_qos1 ) {
"h_close1",
[&] {
MQTT_CHK("h_connack2");
// If clean session is not provided, than there will be a session present
// if there was ever a previous connection, even if clean session was provided
// on the previous connection.
// This is because MQTTv5 change the semantics of the flag to "clean start"
// such that it only effects the start of the session.
// Post Session cleanup is handled with a timer, not with the clean session flag.
BOOST_TEST(sp == true);
// The previous connection is not set Session Expiry Interval.
// That means session state is cleared on close.
BOOST_TEST(sp == false);
}
);
BOOST_TEST(ret);
Expand Down Expand Up @@ -114,8 +122,7 @@ BOOST_AUTO_TEST_CASE( publish_qos1 ) {
MQTT_CHK("h_close1");
// offline publish
pid_pub = c->publish("topic1", "topic1_contents", MQTT_NS::qos::at_least_once);
c->set_clean_session(false);
c->connect();
connect_no_clean(c);
},
"h_puback",
[&] {
Expand Down Expand Up @@ -215,13 +222,9 @@ BOOST_AUTO_TEST_CASE( publish_qos2 ) {
"h_close1",
[&] {
MQTT_CHK("h_connack2");
// If clean session is not provided, than there will be a session present
// if there was ever a previous connection, even if clean session was provided
// on the previous connection.
// This is because MQTTv5 change the semantics of the flag to "clean start"
// such that it only effects the start of the session.
// Post Session cleanup is handled with a timer, not with the clean session flag.
BOOST_TEST(sp == true);
// The previous connection is not set Session Expiry Interval.
// That means session state is cleared on close.
BOOST_TEST(sp == false);
}
);
BOOST_TEST(ret);
Expand Down Expand Up @@ -257,8 +260,7 @@ BOOST_AUTO_TEST_CASE( publish_qos2 ) {
MQTT_CHK("h_close1");
// offline publish
pid_pub = c->publish("topic1", "topic1_contents", MQTT_NS::qos::exactly_once);
c->set_clean_session(false);
c->connect();
connect_no_clean(c);
},
"h_pubcomp",
[&] {
Expand Down Expand Up @@ -364,13 +366,9 @@ BOOST_AUTO_TEST_CASE( multi_publish_qos1 ) {
"h_close1",
[&] {
MQTT_CHK("h_connack2");
// If clean session is not provided, than there will be a session present
// if there was ever a previous connection, even if clean session was provided
// on the previous connection.
// This is because MQTTv5 change the semantics of the flag to "clean start"
// such that it only effects the start of the session.
// Post Session cleanup is handled with a timer, not with the clean session flag.
BOOST_TEST(sp == true);
// The previous connection is not set Session Expiry Interval.
// That means session state is cleared on close.
BOOST_TEST(sp == false);
}
);
BOOST_TEST(ret);
Expand Down Expand Up @@ -411,8 +409,7 @@ BOOST_AUTO_TEST_CASE( multi_publish_qos1 ) {
// offline publish
pid_pub1 = c->publish(/*topic_base()*/ + "987/topic1", "topic1_contents1", MQTT_NS::qos::at_least_once);
pid_pub2 = c->publish(/*topic_base()*/ + "987/topic1", "topic1_contents2", MQTT_NS::qos::at_least_once);
c->set_clean_session(false);
c->connect();
connect_no_clean(c);
},
"h_puback2",
[&] {
Expand Down Expand Up @@ -505,13 +502,9 @@ BOOST_AUTO_TEST_CASE( async_publish_qos1 ) {
"h_pub_finish",
[&] {
MQTT_CHK("h_connack2");
// If clean session is not provided, than there will be a session present
// if there was ever a previous connection, even if clean session was provided
// on the previous connection.
// This is because MQTTv5 change the semantics of the flag to "clean start"
// such that it only effects the start of the session.
// Post Session cleanup is handled with a timer, not with the clean session flag.
BOOST_TEST(sp == true);
// The previous connection is not set Session Expiry Interval.
// That means session state is cleared on close.
BOOST_TEST(sp == false);
}
);
BOOST_TEST(ret);
Expand Down Expand Up @@ -549,8 +542,7 @@ BOOST_AUTO_TEST_CASE( async_publish_qos1 ) {
MQTT_CHK("h_pub_finish");
}
);
c->set_clean_session(false);
c->connect();
connect_no_clean(c);
},
"h_puback",
[&] {
Expand Down
Loading