Merged
Merge Request #21 · created by Peter M. Groen


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.


From fix/pgroen/topic_length_limitation into master

Merged by Peter M. Groen

Source branch has been removed
1 participants



CMakeLists.txt
@@ -13,5 +13,7 @@ add_subdirectory(examples/pub) @@ -13,5 +13,7 @@ add_subdirectory(examples/pub)
13 add_subdirectory(examples/sub) 13 add_subdirectory(examples/sub)
14 add_subdirectory(examples/subunsub) 14 add_subdirectory(examples/subunsub)
15 15
  16 +add_subdirectory(test)
  17 +
16 include(packaging) 18 include(packaging)
17 package_component() 19 package_component()
src/CMakeLists.txt
@@ -13,8 +13,8 @@ include_directories( @@ -13,8 +13,8 @@ include_directories(
13 ) 13 )
14 14
15 set(SRC_LIST 15 set(SRC_LIST
16 - ${CMAKE_CURRENT_SOURCE_DIR}/log.cpp  
17 - ${CMAKE_CURRENT_SOURCE_DIR}/threadcontext.cpp 16 + ${CMAKE_CURRENT_SOURCE_DIR}/log.cpp
  17 + ${CMAKE_CURRENT_SOURCE_DIR}/threadcontext.cpp
18 ${CMAKE_CURRENT_SOURCE_DIR}/mqttpublisherbase.cpp 18 ${CMAKE_CURRENT_SOURCE_DIR}/mqttpublisherbase.cpp
19 ${CMAKE_CURRENT_SOURCE_DIR}/mqttsubscriberbase.cpp 19 ${CMAKE_CURRENT_SOURCE_DIR}/mqttsubscriberbase.cpp
20 ${CMAKE_CURRENT_SOURCE_DIR}/clientpaho.cpp 20 ${CMAKE_CURRENT_SOURCE_DIR}/clientpaho.cpp
src/mqttclient.cpp
@@ -215,9 +215,17 @@ void MqttClient::disconnect() @@ -215,9 +215,17 @@ void MqttClient::disconnect()
215 215
216 Token MqttClient::publish(const MqttMessage& message, int qos) 216 Token MqttClient::publish(const MqttMessage& message, int qos)
217 { 217 {
218 - if (hasWildcard(message.topic()) || !isValidTopic(message.topic())) { 218 + if (hasWildcard(message.topic()))
  219 + {
  220 + LogDebug("[MqttClient::publish]","Topic has wildcard : " + message.topic());
219 return Token(m_clientId, -1); 221 return Token(m_clientId, -1);
220 } 222 }
  223 + else if(!isValidTopic(message.topic()))
  224 + {
  225 + LogDebug("[MqttClient::publish]","Topic is invalid : " + message.topic());
  226 + return Token(m_clientId, -1);
  227 + }
  228 +
221 OSDEV_COMPONENTS_LOCKGUARD(m_interfaceMutex); 229 OSDEV_COMPONENTS_LOCKGUARD(m_interfaceMutex);
222 IMqttClientImpl* client(nullptr); 230 IMqttClientImpl* client(nullptr);
223 { 231 {
@@ -239,6 +247,13 @@ Token MqttClient::publish(const MqttMessage& message, int qos) @@ -239,6 +247,13 @@ Token MqttClient::publish(const MqttMessage& message, int qos)
239 } 247 }
240 client = m_principalClient.get(); 248 client = m_principalClient.get();
241 } 249 }
  250 +
  251 + if(!client)
  252 + {
  253 + LogDebug("[MqttClient::publish]", "Invalid pointer to IMqttClient retrieved.");
  254 + return Token(m_clientId, -1);
  255 + }
  256 +
242 return Token(client->clientId(), client->publish(message, qos)); 257 return Token(client->clientId(), client->publish(message, qos));
243 } 258 }
244 259
src/mqttutil.cpp
@@ -55,7 +55,12 @@ bool isValidTopic( const std::string &topic ) @@ -55,7 +55,12 @@ bool isValidTopic( const std::string &topic )
55 55
56 bool hasWildcard( const std::string &topic ) 56 bool hasWildcard( const std::string &topic )
57 { 57 {
58 - return ( topic.size() > 0 && (topic.find( '+' ) != std::string::npos || topic.size() - 1 == '#' ) ); 58 + return ( topic.size() > 0 &&
  59 + (
  60 + topic.find( '+' ) != std::string::npos ||
  61 + topic.back() == '#'
  62 + )
  63 + );
59 } 64 }
60 65
61 bool testForOverlap( const std::string &existingTopic, const std::string &newTopic ) 66 bool testForOverlap( const std::string &existingTopic, const std::string &newTopic )
test/CMakeLists.txt 0 → 100644
  1 +#****************************************************************************
  2 +#* Copyright (c) 2023 Open Systems Development B.V.
  3 +#*****************************************************************************
  4 +#
  5 +# Don't call this file directly from cmake.
  6 +# This file is included from the upper directory.
  7 +#
  8 +# Build rules for the MQTT Library
  9 +
  10 +add_executable(topictest
  11 + TopicLengthTest.cpp
  12 +)
  13 +
  14 +target_include_directories(topictest PRIVATE
  15 + ${CMAKE_CIRRENT_SOURECE_DIR}
  16 + ../include
  17 +)
  18 +
  19 +target_link_libraries(topictest PRIVATE
  20 + gmock_main
  21 + gmock
  22 + gtest
  23 + mqtt-cpp
  24 +)
  25 +
  26 +add_test(NAME topictest COMMAND topictest)
  27 +
  28 +set_tests_properties(topictest PROPERTIES
  29 + WORKING_DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}"
  30 +)
test/TopicLengthTest.cpp 0 → 100644
  1 +/****************************************************************************
  2 + * COpyright (c) 2023 Open Systems Development B.V.
  3 + ****************************************************************************/
  4 +
  5 +#include <gmock/gmock.h>
  6 +#include <gtest/gtest.h>
  7 +#include <string>
  8 +#include <memory>
  9 +
  10 +#include "mqttclient.h"
  11 +
  12 +using namespace osdev::components::mqtt;
  13 +using namespace osdev::components::log;
  14 +
  15 +static const std::string main_topic = "test/";
  16 +
  17 +/****************************************************************************
  18 + * H E L P E R C L A S S E S
  19 + ****************************************************************************/
  20 +class Publisher
  21 +{
  22 +public:
  23 + Publisher() : m_mqtt_client("TopicTester"){}
  24 + virtual ~Publisher() {}
  25 +
  26 + void connect(const std::string &hostname,
  27 + int portnumber = 1883,
  28 + const std::string &username = std::string(),
  29 + const std::string &password = std::string(),
  30 + const std::string &lwt_topic = std::string(),
  31 + const std::string &lwt_message = std::string()
  32 + )
  33 + {
  34 + m_mqtt_client.connect(hostname, portnumber,
  35 + osdev::components::mqtt::Credentials(username, password),
  36 + osdev::components::mqtt::mqtt_LWT(lwt_topic, lwt_message),
  37 + true,
  38 + osdev::components::log::LogSettings
  39 + {
  40 + osdev::components::log::LogLevel::Debug,
  41 + osdev::components::log::LogMask::None
  42 + });
  43 + }
  44 +
  45 + void publish(const std::string &message_topic, const std::string &message_payload)
  46 + {
  47 + osdev::components::mqtt::MqttMessage message(message_topic, true, false, message_payload);
  48 + osdev::components::mqtt::Token t_result = m_mqtt_client.publish(message, 0);
  49 + }
  50 +
  51 +private:
  52 + osdev::components::mqtt::MqttClient m_mqtt_client;
  53 +};
  54 +
  55 +/// @brief class to generate a cumulative topic..
  56 +class TopicTester
  57 +{
  58 + public:
  59 + TopicTester(std::shared_ptr<Publisher> publisher) : m_publisher(publisher){}
  60 + virtual ~TopicTester(){}
  61 +
  62 + void RunTopicTester(int max_number_of_chars)
  63 + {
  64 + for(int nCount = 1; nCount < max_number_of_chars; nCount++)
  65 + {
  66 + std::string subtopic(nCount, 'a');
  67 + std::string topic = std::string(main_topic + subtopic);
  68 + std::string message(std::to_string(topic.size()) + " (" + std::to_string(subtopic.size()) + ")");
  69 +
  70 + m_publisher->publish(topic, message);
  71 + }
  72 + }
  73 +
  74 + private:
  75 + std::shared_ptr<Publisher> m_publisher;
  76 +};
  77 +
  78 +/*****************************************************************************
  79 + * T H E A C T U A L T E S T S
  80 + *****************************************************************************/
  81 +/// TopicTester
  82 +TEST(topictest, TopicLengthTest)
  83 +{
  84 + std::shared_ptr<Publisher> pPublisher = std::make_shared<Publisher>();
  85 + pPublisher->connect("127.0.0.1", 1883);
  86 +
  87 + TopicTester oTester(pPublisher);
  88 +
  89 + oTester.RunTopicTester(101);
  90 +}