Use snapshot of maxIncomingTopicAliasValue from settings in client

This fixes clients being disconnected after reducing the max value and reloading the settings.

Use snapshot of maxIncomingTopicAliasValue from settings in client
This fixes clients being disconnected after reducing the max value and reloading the settings.
Wiebe Cazemier
1 parent 0c1a5dc4
Showing 3 changed files with 15 additions and 5 deletions
client.cpp
client.h
mqttpacket.cpp
@@ -41,6 +41,7 @@ Client::Client(int fd, std::shared_ptr&lt;ThreadData&gt; threadData, SSL *ssl, bool we
     initialBufferSize(settings->clientInitialBufferSize), // The client is constructed in the main thread, so we need to use its settings copy
     maxOutgoingPacketSize(settings->maxPacketSize), // Same as initialBufferSize comment.
     maxIncomingPacketSize(settings->maxPacketSize),
+    maxIncomingTopicAliasValue(settings->maxIncomingTopicAliasValue), // Retaining snapshot of current setting, to not confuse clients when the setting changes.
     ioWrapper(ssl, websocket, initialBufferSize, this),
     readbuf(initialBufferSize),
     writebuf(initialBufferSize),
@@ -377,11 +378,9 @@ void Client::setTopicAlias(const uint16_t alias_id, const std::string &amp;topic)
     if (topic.empty())
         return;
-    const Settings *settings = ThreadGlobals::getSettings();
-
     // The specs actually say "The Client MUST NOT send a Topic Alias [...] to the Server greater than this value [Topic Alias Maximum]". So, it's not about count.
-    if (alias_id > settings->maxIncomingTopicAliasValue)
-        throw ProtocolError(formatString("Client tried to set more topic aliases than the server max of %d per client", settings->maxIncomingTopicAliasValue),
+    if (alias_id > this->maxIncomingTopicAliasValue)
+        throw ProtocolError(formatString("Client tried to set more topic aliases than the server max of %d per client", this->maxIncomingTopicAliasValue),
                             ReasonCodes::TopicAliasInvalid);
     this->incomingTopicAliases[alias_id] = topic;
@@ -402,6 +401,15 @@ uint32_t Client::getMaxIncomingPacketSize() const
     return this->maxIncomingPacketSize;
 }
+/**
+ * @brief We use this to send back in the connack, so we know we don't race with the value from settings, which may change during the connection handshake.
+ * @return
+ */
+uint16_t Client::getMaxIncomingTopicAliasValue() const
+{
+    return this->maxIncomingTopicAliasValue;
+}
+
 void Client::sendOrQueueWill()
 {
     if (!this->threadData)
@@ -68,6 +68,7 @@ class Client
     const uint32_t maxIncomingPacketSize;
     uint16_t maxOutgoingTopicAliasValue = 0;
+    const uint16_t maxIncomingTopicAliasValue;
     IoWrapper ioWrapper;
     std::string transportStr;
@@ -169,6 +170,7 @@ public:
     const std::string &getTopicAlias(const uint16_t id);
     uint32_t getMaxIncomingPacketSize() const;
+    uint16_t getMaxIncomingTopicAliasValue() const;
     void sendOrQueueWill();
     void serverInitiatedDisconnect(ReasonCodes reason);
@@ -622,7 +622,7 @@ void MqttPacket::handleConnect()
             connAck->propertyBuilder->writeMaxPacketSize(sender->getMaxIncomingPacketSize());
             if (clientIdGenerated)
                 connAck->propertyBuilder->writeAssignedClientId(client_id);
-            connAck->propertyBuilder->writeMaxTopicAliases(settings.maxIncomingTopicAliasValue);
+            connAck->propertyBuilder->writeMaxTopicAliases(sender->getMaxIncomingTopicAliasValue());
             connAck->propertyBuilder->writeWildcardSubscriptionAvailable(1);
             connAck->propertyBuilder->writeSubscriptionIdentifiersAvailable(0);
             connAck->propertyBuilder->writeSharedSubscriptionAvailable(0);