Change clean session into MQTT5 symantics

The behavior for MQTT3 clients in the same, but I replaced the term 'clean session' and described the behavior in MQTT5 terms, of 'clean start' and an expiry interval.

Change clean session into MQTT5 symantics
The behavior for MQTT3 clients in the same, but I replaced the term 'clean session' and described the behavior in MQTT5 terms, of 'clean start' and an expiry interval.
Wiebe Cazemier
1 parent a682f1e0
Showing 9 changed files with 53 additions and 57 deletions
FlashMQTests/tst_maintests.cpp
client.cpp
client.h
mainapp.cpp
mqttpacket.cpp
session.cpp
session.h
subscriptionstore.cpp
subscriptionstore.h
@@ -1001,15 +1001,15 @@ void MainTests::testSavingSessions()
         ThreadGlobals::assign(&auth);
         std::shared_ptr<Client> c1(new Client(0, t, nullptr, false, nullptr, settings, false));
-        c1->setClientProperties(ProtocolVersion::Mqtt311, "c1", "user1", true, 60, false);
-        store->registerClientAndKickExistingOne(c1);
+        c1->setClientProperties(ProtocolVersion::Mqtt311, "c1", "user1", true, 60);
+        store->registerClientAndKickExistingOne(c1, false, 512, 120);
         c1->getSession()->touch();
         c1->getSession()->addIncomingQoS2MessageId(2);
         c1->getSession()->addIncomingQoS2MessageId(3);
         std::shared_ptr<Client> c2(new Client(0, t, nullptr, false, nullptr, settings, false));
-        c2->setClientProperties(ProtocolVersion::Mqtt311, "c2", "user2", true, 60, false);
-        store->registerClientAndKickExistingOne(c2);
+        c2->setClientProperties(ProtocolVersion::Mqtt311, "c2", "user2", true, 60);
+        store->registerClientAndKickExistingOne(c2, false, 512, 120);
         c2->getSession()->touch();
         c2->getSession()->addOutgoingQoS2MessageId(55);
         c2->getSession()->addOutgoingQoS2MessageId(66);
@@ -1120,8 +1120,8 @@ void testCopyPacketHelper(const std::string &amp;topic, char from_qos, char to_qos, 
     ThreadGlobals::assign(&auth);
     std::shared_ptr<Client> dummyClient(new Client(0, t, nullptr, false, nullptr, settings, false));
-    dummyClient->setClientProperties(ProtocolVersion::Mqtt311, "qostestclient", "user1", true, 60, false);
-    store->registerClientAndKickExistingOne(dummyClient);
+    dummyClient->setClientProperties(ProtocolVersion::Mqtt311, "qostestclient", "user1", true, 60);
+    store->registerClientAndKickExistingOne(dummyClient, false, 512, 120);
     uint16_t packetid = 66;
     for (int len = 0; len < 150; len++ )
@@ -75,7 +75,7 @@ Client::~Client()
     }
     // MQTT-3.1.2-6
-    if (cleanSession)
+    if (session->getDestroyOnDisconnect())
     {
         store->removeSession(clientid);
     }
@@ -286,9 +286,9 @@ bool Client::writeBufIntoFd()
 std::string Client::repr()
 {
-    std::string s = formatString("[ClientID='%s', username='%s', fd=%d, keepalive=%ds, transport='%s', address='%s', cleanses=%d, prot=%s]",
+    std::string s = formatString("[ClientID='%s', username='%s', fd=%d, keepalive=%ds, transport='%s', address='%s', prot=%s]",
                                  clientid.c_str(), username.c_str(), fd, keepalive, this->transportStr.c_str(), this->address.c_str(),
-                                 cleanSession, protocolVersionString(protocolVersion).c_str());
+                                 protocolVersionString(protocolVersion).c_str());
     return s;
 }
@@ -334,11 +334,6 @@ void Client::resetBuffersIfEligible()
     writebuf.resetSizeIfEligable(initialBufferSize);
 }
-void Client::setCleanSession(bool val)
-{
-    this->cleanSession = val;
-}
-
 #ifndef NDEBUG
 /**
  * @brief IoWrapper::setFakeUpgraded().
@@ -421,24 +416,22 @@ void Client::bufferToMqttPackets(std::vector&lt;MqttPacket&gt; &amp;packetQueueIn, std::sh
     setReadyForReading(readbuf.freeSpace() > 0);
 }
-void Client::setClientProperties(ProtocolVersion protocolVersion, const std::string &clientId, const std::string username, bool connectPacketSeen, uint16_t keepalive, bool cleanSession)
+void Client::setClientProperties(ProtocolVersion protocolVersion, const std::string &clientId, const std::string username, bool connectPacketSeen, uint16_t keepalive)
 {
     const Settings *settings = ThreadGlobals::getSettings();
-    setClientProperties(protocolVersion, clientId, username, connectPacketSeen, keepalive, cleanSession,
-                        settings->maxPacketSize, 0);
+    setClientProperties(protocolVersion, clientId, username, connectPacketSeen, keepalive, settings->maxPacketSize, 0);
 }
 void Client::setClientProperties(ProtocolVersion protocolVersion, const std::string &clientId, const std::string username, bool connectPacketSeen, uint16_t keepalive,
-                                 bool cleanSession, uint32_t maxPacketSize, uint16_t maxTopicAliases)
+                                 uint32_t maxPacketSize, uint16_t maxTopicAliases)
 {
     this->protocolVersion = protocolVersion;
     this->clientid = clientId;
     this->username = username;
     this->connectPacketSeen = connectPacketSeen;
     this->keepalive = keepalive;
-    this->cleanSession = cleanSession;
     this->maxPacketSize = maxPacketSize;
     this->maxTopicAliases = maxTopicAliases;
 }
@@ -74,7 +74,6 @@ class Client
     std::string clientid;
     std::string username;
     uint16_t keepalive = 0;
-    bool cleanSession = false;
     std::string will_topic;
     std::string will_payload;
@@ -108,9 +107,9 @@ public:
     void markAsDisconnecting();
     bool readFdIntoBuffer();
     void bufferToMqttPackets(std::vector<MqttPacket> &packetQueueIn, std::shared_ptr<Client> &sender);
-    void setClientProperties(ProtocolVersion protocolVersion, const std::string &clientId, const std::string username, bool connectPacketSeen, uint16_t keepalive, bool cleanSession);
+    void setClientProperties(ProtocolVersion protocolVersion, const std::string &clientId, const std::string username, bool connectPacketSeen, uint16_t keepalive);
     void setClientProperties(ProtocolVersion protocolVersion, const std::string &clientId, const std::string username, bool connectPacketSeen, uint16_t keepalive,
-                             bool cleanSession, uint32_t maxPacketSize, uint16_t maxTopicAliases);
+                             uint32_t maxPacketSize, uint16_t maxTopicAliases);
     void setWill(const std::string &topic, const std::string &payload, bool retain, char qos);
     void clearWill();
     void setAuthenticated(bool value) { authenticated = value;}
@@ -119,7 +118,6 @@ public:
     std::shared_ptr<ThreadData> getThreadData() { return threadData; }
     std::string &getClientId() { return this->clientid; }
     const std::string &getUsername() const { return this->username; }
-    bool getCleanSession() { return cleanSession; }
     void assignSession(std::shared_ptr<Session> &session);
     std::shared_ptr<Session> getSession();
     void setDisconnectReason(const std::string &reason);
@@ -140,8 +138,6 @@ public:
     std::string getKeepAliveInfoString() const;
     void resetBuffersIfEligible();
-    void setCleanSession(bool val);
-
 #ifndef NDEBUG
     void setFakeUpgraded();
 #endif
@@ -442,11 +442,11 @@ void MainApp::start()
             std::shared_ptr<Client> client = std::make_shared<Client>(fd, threaddata, nullptr, fuzzWebsockets, nullptr, settings, true);
             std::shared_ptr<Client> subscriber = std::make_shared<Client>(fdnull, threaddata, nullptr, fuzzWebsockets, nullptr, settings, true);
-            subscriber->setClientProperties(ProtocolVersion::Mqtt311, "subscriber", "subuser", true, 60, true);
+            subscriber->setClientProperties(ProtocolVersion::Mqtt311, "subscriber", "subuser", true, 60);
             subscriber->setAuthenticated(true);
             std::shared_ptr<Client> websocketsubscriber = std::make_shared<Client>(fdnull2, threaddata, nullptr, true, nullptr, settings, true);
-            websocketsubscriber->setClientProperties(ProtocolVersion::Mqtt311, "websocketsubscriber", "websocksubuser", true, 60, true);
+            websocketsubscriber->setClientProperties(ProtocolVersion::Mqtt311, "websocketsubscriber", "websocksubuser", true, 60);
             websocketsubscriber->setAuthenticated(true);
             websocketsubscriber->setFakeUpgraded();
             subscriptionStore->registerClientAndKickExistingOne(websocketsubscriber);
@@ -363,7 +363,7 @@ void MqttPacket::handleConnect()
         bool will_retain = !!(flagByte & 0b00100000);
         char will_qos = (flagByte & 0b00011000) >> 3;
         bool will_flag = !!(flagByte & 0b00000100);
-        bool clean_session = !!(flagByte & 0b00000010);
+        bool clean_start = !!(flagByte & 0b00000010);
         if (will_qos > 2)
             throw ProtocolError("Invalid QoS for will.");
@@ -467,9 +467,9 @@ void MqttPacket::handleConnect()
             logger->logf(LOG_ERR, "ClientID '%s' has + or # in the id and 'allow_unsafe_clientid_chars' is false.", client_id.c_str());
             validClientId = false;
         }
-        else if (!clean_session && client_id.empty())
+        else if (!clean_start && client_id.empty())
         {
-            logger->logf(LOG_ERR, "ClientID empty and clean session 0, which is incompatible");
+            logger->logf(LOG_ERR, "ClientID empty and clean start 0, which is incompatible");
             validClientId = false;
         }
         else if (protocolVersion < ProtocolVersion::Mqtt311 && client_id.empty())
@@ -493,7 +493,7 @@ void MqttPacket::handleConnect()
             client_id = getSecureRandomString(23);
         }
-        sender->setClientProperties(protocolVersion, client_id, username, true, keep_alive, clean_session, max_packet_size, max_topic_aliases);
+        sender->setClientProperties(protocolVersion, client_id, username, true, keep_alive, max_packet_size, max_topic_aliases);
         sender->setWill(will_topic, will_payload, will_retain, will_qos);
         bool accessGranted = false;
@@ -518,7 +518,7 @@ void MqttPacket::handleConnect()
         if (accessGranted)
         {
-            bool sessionPresent = protocolVersion >= ProtocolVersion::Mqtt311 && !clean_session && subscriptionStore->sessionPresent(client_id);
+            bool sessionPresent = protocolVersion >= ProtocolVersion::Mqtt311 && !clean_start && subscriptionStore->sessionPresent(client_id);
             sender->setAuthenticated(true);
             ConnAck connAck(ConnAckReturnCodes::Accepted, sessionPresent);
@@ -526,7 +526,7 @@ void MqttPacket::handleConnect()
             sender->writeMqttPacket(response);
             logger->logf(LOG_NOTICE, "Client '%s' logged in successfully", sender->repr().c_str());
-            subscriptionStore->registerClientAndKickExistingOne(sender, max_qos_packets, session_expire);
+            subscriptionStore->registerClientAndKickExistingOne(sender, clean_start, max_qos_packets, session_expire);
         }
         else
         {
@@ -76,8 +76,7 @@ bool Session::requiresPacketRetransmission() const
     if (client->getProtocolVersion() < ProtocolVersion::Mqtt311)
         return true;
-    // TODO: for MQTT5, the rules are different.
-    return !client->getCleanSession();
+    return !destroyOnDisconnect;
 }
 void Session::increasePacketId()
@@ -343,18 +342,30 @@ void Session::removeOutgoingQoS2MessageId(u_int16_t packet_id)
         outgoingQoS2MessageIds.erase(it);
 }
-bool Session::getCleanSession() const
+/**
+ * @brief Session::getDestroyOnDisconnect
+ * @return
+ *
+ * MQTT5: Setting Clean Start to 1 and a Session Expiry Interval of 0, is equivalent to setting CleanSession to 1 in the MQTT Specification Version 3.1.1.
+ */
+bool Session::getDestroyOnDisconnect() const
 {
-    auto c = client.lock();
-
-    if (!c)
-        return false;
-
-    return c->getCleanSession();
+    return destroyOnDisconnect;
 }
-void Session::setSessionProperties(uint16_t maxQosPackets, uint32_t sessionExpiryInterval)
+void Session::setSessionProperties(uint16_t maxQosPackets, uint32_t sessionExpiryInterval, bool clean_start, ProtocolVersion protocol_version)
 {
     this->maxQosMsgPending = maxQosPackets;
     this->sessionExpiryInterval = sessionExpiryInterval;
+
+    if (protocol_version <= ProtocolVersion::Mqtt311 && clean_start)
+        destroyOnDisconnect = true;
+    else
+        destroyOnDisconnect = sessionExpiryInterval == 0;
 }
+
+uint32_t Session::getSessionExpiryInterval() const
+{
+    return this->sessionExpiryInterval;
+}
+
@@ -49,6 +49,7 @@ class Session
     uint32_t sessionExpiryInterval = 0;
     uint16_t maxQosMsgPending;
     uint16_t QoSLogPrintedAtId = 0;
+    bool destroyOnDisconnect = false;
     std::chrono::time_point<std::chrono::steady_clock> lastTouched = std::chrono::steady_clock::now();
     Logger *logger = Logger::getInstance();
@@ -86,9 +87,10 @@ public:
     void addOutgoingQoS2MessageId(uint16_t packet_id);
     void removeOutgoingQoS2MessageId(u_int16_t packet_id);
-    bool getCleanSession() const;
+    bool getDestroyOnDisconnect() const;
-    void setSessionProperties(uint16_t maxQosPackets, uint32_t sessionExpiryInterval);
+    void setSessionProperties(uint16_t maxQosPackets, uint32_t sessionExpiryInterval, bool clean_start, ProtocolVersion protocol_version);
+    uint32_t getSessionExpiryInterval() const;
 };
 #endif // SESSION_H
@@ -204,11 +204,11 @@ void SubscriptionStore::removeSubscription(std::shared_ptr&lt;Client&gt; &amp;client, cons
 void SubscriptionStore::registerClientAndKickExistingOne(std::shared_ptr<Client> &client)
 {
     const Settings *settings = ThreadGlobals::getSettings();
-    registerClientAndKickExistingOne(client, settings->maxQosMsgPendingPerClient, settings->expireSessionsAfterSeconds);
+    registerClientAndKickExistingOne(client, true, settings->maxQosMsgPendingPerClient, settings->expireSessionsAfterSeconds);
 }
 // Removes an existing client when it already exists [MQTT-3.1.4-2].
-void SubscriptionStore::registerClientAndKickExistingOne(std::shared_ptr<Client> &client, uint16_t maxQosPackets, uint32_t sessionExpiryInterval)
+void SubscriptionStore::registerClientAndKickExistingOne(std::shared_ptr<Client> &client, bool clean_start, uint16_t maxQosPackets, uint32_t sessionExpiryInterval)
 {
     RWLockGuard lock_guard(&subscriptionsRwlock);
     lock_guard.wrlock();
@@ -216,7 +216,6 @@ void SubscriptionStore::registerClientAndKickExistingOne(std::shared_ptr&lt;Client&gt;
     if (client->getClientId().empty())
         throw ProtocolError("Trying to store client without an ID.");
-    bool originalClientDemandsSessionDestruction = false;
     std::shared_ptr<Session> session;
     auto session_it = sessionsById.find(client->getClientId());
     if (session_it != sessionsById.end())
@@ -232,11 +231,6 @@ void SubscriptionStore::registerClientAndKickExistingOne(std::shared_ptr&lt;Client&gt;
                 logger->logf(LOG_NOTICE, "Disconnecting existing client with id '%s'", cl->getClientId().c_str());
                 cl->setDisconnectReason("Another client with this ID connected");
-                // We have to set session to false, because it's no longer up to the destruction of that client
-                // to destroy the session. We either do it in this function, or not at all.
-                originalClientDemandsSessionDestruction = cl->getCleanSession();
-                cl->setCleanSession(false);
-
                 cl->setReadyForDisconnect();
                 cl->getThreadData()->removeClientQueued(cl);
                 cl->markAsDisconnecting();
@@ -245,7 +239,7 @@ void SubscriptionStore::registerClientAndKickExistingOne(std::shared_ptr&lt;Client&gt;
         }
     }
-    if (!session || client->getCleanSession() || originalClientDemandsSessionDestruction)
+    if (!session || session->getDestroyOnDisconnect())
     {
         session = std::make_shared<Session>();
@@ -254,7 +248,7 @@ void SubscriptionStore::registerClientAndKickExistingOne(std::shared_ptr&lt;Client&gt;
     session->assignActiveConnection(client);
     client->assignSession(session);
-    session->setSessionProperties(maxQosPackets, sessionExpiryInterval);
+    session->setSessionProperties(maxQosPackets, sessionExpiryInterval, clean_start, client->getProtocolVersion());
     uint64_t count = session->sendPendingQosMessages();
     client->getThreadData()->incrementSentMessageCount(count);
 }
@@ -744,7 +738,7 @@ void SubscriptionStore::saveSessionsAndSubscriptions(const std::string &amp;filePath
             const Session &org = *pair.second.get();
             // Sessions created with clean session need to be destroyed when disconnecting, so no point in saving them.
-            if (org.getCleanSession())
+            if (org.getDestroyOnDisconnect())
                 continue;
             sessionCopies.push_back(org.getCopy());
@@ -119,7 +119,7 @@ public:
     void addSubscription(std::shared_ptr<Client> &client, const std::string &topic, const std::vector<std::string> &subtopics, char qos);
     void removeSubscription(std::shared_ptr<Client> &client, const std::string &topic);
     void registerClientAndKickExistingOne(std::shared_ptr<Client> &client);
-    void registerClientAndKickExistingOne(std::shared_ptr<Client> &client, uint16_t maxQosPackets, uint32_t sessionExpiryInterval);
+    void registerClientAndKickExistingOne(std::shared_ptr<Client> &client, bool clean_start, uint16_t maxQosPackets, uint32_t sessionExpiryInterval);
     bool sessionPresent(const std::string &clientid);
     void queuePacketAtSubscribers(const std::vector<std::string> &subtopics, MqttPacket &packet, bool dollar = false);