Merge branch 'fix/pgroen/topic_length_limitation' into 'master'

fix/pgroen/topic length limitation hasWildcard had an invalid check on the last character. When the topic size is 36 and we check the size>() -1 (35) it is interpreted as an '#' which in turn is a wildcard in the topic making sure it is not published. We now check the actual character of the last entry in the string. See merge request !21

Merge branch 'fix/pgroen/topic_length_limitation' into 'master'
fix/pgroen/topic length limitation hasWildcard had an invalid check on the last character. When the topic size is 36 and we check the size>() -1 (35) it is interpreted as an '#' which in turn is a wildcard in the topic making sure it is not published. We now check the actual character of the last entry in the string. See merge request !21
Peter M. Groen
2 parents 2569446f 60ac1cde
Showing 6 changed files with 146 additions and 4 deletions
CMakeLists.txt
src/CMakeLists.txt
src/mqttclient.cpp
src/mqttutil.cpp
test/CMakeLists.txt
test/TopicLengthTest.cpp
@@ -13,5 +13,7 @@ add_subdirectory(examples/pub)
 add_subdirectory(examples/sub)
 add_subdirectory(examples/subunsub)
  
+add_subdirectory(test)
+
 include(packaging)
 package_component()
@@ -13,8 +13,8 @@ include_directories(
 )
  
 set(SRC_LIST
-	${CMAKE_CURRENT_SOURCE_DIR}/log.cpp
-	${CMAKE_CURRENT_SOURCE_DIR}/threadcontext.cpp
+        ${CMAKE_CURRENT_SOURCE_DIR}/log.cpp
+        ${CMAKE_CURRENT_SOURCE_DIR}/threadcontext.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/mqttpublisherbase.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/mqttsubscriberbase.cpp
         ${CMAKE_CURRENT_SOURCE_DIR}/clientpaho.cpp
@@ -215,9 +215,17 @@ void MqttClient::disconnect()
  
 Token MqttClient::publish(const MqttMessage& message, int qos)
 {
-    if (hasWildcard(message.topic()) || !isValidTopic(message.topic())) {
+    if (hasWildcard(message.topic()))
+    {
+        LogDebug("[MqttClient::publish]","Topic has wildcard : " + message.topic());
         return Token(m_clientId, -1);
     }
+    else if(!isValidTopic(message.topic()))
+    {
+        LogDebug("[MqttClient::publish]","Topic is invalid : " + message.topic());
+        return Token(m_clientId, -1);
+    }
+
     OSDEV_COMPONENTS_LOCKGUARD(m_interfaceMutex);
     IMqttClientImpl* client(nullptr);
     {
@@ -239,6 +247,13 @@ Token MqttClient::publish(const MqttMessage&amp; message, int qos)
         }
         client = m_principalClient.get();
     }
+
+    if(!client)
+    {
+        LogDebug("[MqttClient::publish]", "Invalid pointer to IMqttClient retrieved.");
+        return Token(m_clientId, -1);
+    }
+
     return Token(client->clientId(), client->publish(message, qos));
 }
  
@@ -55,7 +55,12 @@ bool isValidTopic( const std::string &amp;topic )
  
 bool hasWildcard( const std::string &topic )
 {
-    return ( topic.size() > 0 && (topic.find( '+' ) != std::string::npos || topic.size() - 1 == '#' ) );
+    return ( topic.size() > 0 &&
+            (
+                topic.find( '+' ) != std::string::npos ||
+                topic.back() == '#'
+            )
+           );
 }
  
 bool testForOverlap( const std::string &existingTopic, const std::string &newTopic )
+#****************************************************************************
+#* Copyright (c) 2023 Open Systems Development B.V.
+#*****************************************************************************
+#
+# Don't call this file directly from cmake.
+# This file is included from the upper directory.
+#
+# Build rules for the MQTT Library
+
+add_executable(topictest
+    TopicLengthTest.cpp
+)
+
+target_include_directories(topictest PRIVATE
+    ${CMAKE_CIRRENT_SOURECE_DIR}
+    ../include
+)
+
+target_link_libraries(topictest PRIVATE
+    gmock_main
+    gmock
+    gtest
+    mqtt-cpp
+)
+
+add_test(NAME topictest COMMAND topictest)
+
+set_tests_properties(topictest PROPERTIES
+    WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}"
+)
+/****************************************************************************
+ * COpyright (c) 2023 Open Systems Development B.V.
+ ****************************************************************************/
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+#include <string>
+#include <memory>
+
+#include "mqttclient.h"
+
+using namespace osdev::components::mqtt;
+using namespace osdev::components::log;
+
+static const std::string main_topic = "test/";
+
+/****************************************************************************
+ *                      H E L P E R   C L A S S E S
+ ****************************************************************************/
+class Publisher
+{
+public:
+    Publisher() : m_mqtt_client("TopicTester"){}
+    virtual ~Publisher() {}
+
+    void    connect(const std::string &hostname,
+                 int portnumber = 1883,
+                 const std::string &username = std::string(),
+                 const std::string &password = std::string(),
+                 const std::string &lwt_topic = std::string(),
+                 const std::string &lwt_message = std::string()
+                 )
+    {
+        m_mqtt_client.connect(hostname, portnumber,
+                              osdev::components::mqtt::Credentials(username, password),
+                              osdev::components::mqtt::mqtt_LWT(lwt_topic, lwt_message),
+                              true,
+                              osdev::components::log::LogSettings
+                              {
+                                  osdev::components::log::LogLevel::Debug,
+                                  osdev::components::log::LogMask::None
+                              });
+    }
+
+    void    publish(const std::string &message_topic, const std::string &message_payload)
+    {
+        osdev::components::mqtt::MqttMessage message(message_topic, true, false, message_payload);
+        osdev::components::mqtt::Token t_result = m_mqtt_client.publish(message, 0);
+    }
+
+private:
+    osdev::components::mqtt::MqttClient     m_mqtt_client;
+};
+
+/// @brief  class to generate a cumulative topic..
+class TopicTester
+{
+    public:
+        TopicTester(std::shared_ptr<Publisher> publisher) : m_publisher(publisher){}
+        virtual ~TopicTester(){}
+
+        void    RunTopicTester(int max_number_of_chars)
+        {
+            for(int nCount = 1; nCount < max_number_of_chars; nCount++)
+            {
+                std::string subtopic(nCount, 'a');
+                std::string topic = std::string(main_topic + subtopic);
+                std::string message(std::to_string(topic.size()) + " (" + std::to_string(subtopic.size()) + ")");
+
+                m_publisher->publish(topic, message);
+            }
+        }
+
+    private:
+        std::shared_ptr<Publisher>  m_publisher;
+};
+
+/*****************************************************************************
+ *                      T H E   A C T U A L   T E S T S
+ *****************************************************************************/
+/// TopicTester
+TEST(topictest, TopicLengthTest)
+{
+    std::shared_ptr<Publisher> pPublisher = std::make_shared<Publisher>();
+    pPublisher->connect("127.0.0.1", 1883);
+
+    TopicTester oTester(pPublisher);
+
+    oTester.RunTopicTester(101);
+}