Merged
Merge Request #21
·
created by
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
-
mentioned in commit 078653bde8059dc973fca91f91f5c4bb660046ca
-
Status changed to merged
Showing
6 changed files
CMakeLists.txt
src/CMakeLists.txt
... | ... | @@ -13,8 +13,8 @@ include_directories( |
13 | 13 | ) |
14 | 14 | |
15 | 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 | 18 | ${CMAKE_CURRENT_SOURCE_DIR}/mqttpublisherbase.cpp |
19 | 19 | ${CMAKE_CURRENT_SOURCE_DIR}/mqttsubscriberbase.cpp |
20 | 20 | ${CMAKE_CURRENT_SOURCE_DIR}/clientpaho.cpp | ... | ... |
src/mqttclient.cpp
... | ... | @@ -215,9 +215,17 @@ void MqttClient::disconnect() |
215 | 215 | |
216 | 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 | 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 | 229 | OSDEV_COMPONENTS_LOCKGUARD(m_interfaceMutex); |
222 | 230 | IMqttClientImpl* client(nullptr); |
223 | 231 | { |
... | ... | @@ -239,6 +247,13 @@ Token MqttClient::publish(const MqttMessage& message, int qos) |
239 | 247 | } |
240 | 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 | 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 | 55 | |
56 | 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 | 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 | +} | ... | ... |