diff --git a/esphome/components/mqtt/mqtt_client.cpp b/esphome/components/mqtt/mqtt_client.cpp index 297a94d2a97..dd376251399 100644 --- a/esphome/components/mqtt/mqtt_client.cpp +++ b/esphome/components/mqtt/mqtt_client.cpp @@ -16,6 +16,7 @@ #include "lwip/dns.h" #include "lwip/err.h" #include "mqtt_component.h" +#include "mqtt_topic_match.h" #ifdef USE_API #include "esphome/components/api/api_server.h" @@ -611,63 +612,6 @@ void MQTTClientComponent::disable() { this->on_shutdown(); } -/** Check if the message topic matches the given subscription topic - * - * INFO: MQTT spec mandates that topics must not be empty and must be valid NULL-terminated UTF-8 strings. - * - * @param message The message topic that was received from the MQTT server. Note: this must not contain - * wildcard characters as mandated by the MQTT spec. - * @param subscription The subscription topic we are matching against. - * @param is_normal Is this a "normal" topic - Does the message topic not begin with a "$". - * @param past_separator Are we past the first '/' topic separator. - * @return true if the subscription topic matches the message topic, false otherwise. - */ -static bool topic_match(const char *message, const char *subscription, bool is_normal, bool past_separator) { - // Reached end of both strings at the same time, this means we have a successful match - if (*message == '\0' && *subscription == '\0') - return true; - - // Either the message or the subscribe are at the end. This means they don't match. - if (*message == '\0' || *subscription == '\0') - return false; - - bool do_wildcards = is_normal || past_separator; - - if (*subscription == '+' && do_wildcards) { - // single level wildcard - // consume + from subscription - subscription++; - // consume everything from message until '/' found or end of string - while (*message != '\0' && *message != '/') { - message++; - } - // after this, both pointers will point to a '/' or to the end of the string - - return topic_match(message, subscription, is_normal, true); - } - - if (*subscription == '#' && do_wildcards) { - // multilevel wildcard - MQTT mandates that this must be at end of subscribe topic - return true; - } - - // this handles '/' and normal characters at the same time. - if (*message != *subscription) - return false; - - past_separator = past_separator || *subscription == '/'; - - // consume characters - subscription++; - message++; - - return topic_match(message, subscription, is_normal, past_separator); -} - -static bool topic_match(const char *message, const char *subscription) { - return topic_match(message, subscription, *message != '\0' && *message != '$', false); -} - void MQTTClientComponent::on_message(const std::string &topic, const std::string &payload) { #ifdef USE_ESP8266 // IMPORTANT: This defer is REQUIRED to prevent stack overflow crashes on ESP8266. diff --git a/esphome/components/mqtt/mqtt_topic_match.h b/esphome/components/mqtt/mqtt_topic_match.h new file mode 100644 index 00000000000..84172bc805e --- /dev/null +++ b/esphome/components/mqtt/mqtt_topic_match.h @@ -0,0 +1,28 @@ +#pragma once + +namespace esphome::mqtt { + +/// True if the message topic matches the subscription, with + and # wildcards. +/// A loop on purpose: recursing per character overflowed the ESP32 loop task's stack on long topics. +inline bool topic_match(const char *message, const char *subscription) { + // First-level wildcards do not match topics that begin with '$' + bool wildcards = *message != '$'; + while (*message != '\0' && *subscription != '\0') { + if (wildcards && *subscription == '#') + return true; + if (wildcards && *subscription == '+') { + subscription++; + while (*message != '\0' && *message != '/') + message++; + continue; + } + if (*message != *subscription) + return false; + wildcards |= *subscription == '/'; + message++; + subscription++; + } + return *message == *subscription; // a match only if both ended +} + +} // namespace esphome::mqtt diff --git a/tests/components/mqtt/test_topic_match.cpp b/tests/components/mqtt/test_topic_match.cpp new file mode 100644 index 00000000000..b51f45ce478 --- /dev/null +++ b/tests/components/mqtt/test_topic_match.cpp @@ -0,0 +1,103 @@ +#include + +#include +#include + +#include "esphome/components/mqtt/mqtt_topic_match.h" + +namespace esphome::mqtt::testing { + +// The previous recursive implementation, kept to check the loop matches it +static bool reference_match(const char *message, const char *subscription, bool is_normal, bool past_separator) { + if (*message == '\0' && *subscription == '\0') + return true; + if (*message == '\0' || *subscription == '\0') + return false; + bool do_wildcards = is_normal || past_separator; + if (*subscription == '+' && do_wildcards) { + subscription++; + while (*message != '\0' && *message != '/') + message++; + return reference_match(message, subscription, is_normal, true); + } + if (*subscription == '#' && do_wildcards) + return true; + if (*message != *subscription) + return false; + past_separator = past_separator || *subscription == '/'; + return reference_match(message + 1, subscription + 1, is_normal, past_separator); +} + +static bool reference_match(const char *message, const char *subscription) { + return reference_match(message, subscription, *message != '\0' && *message != '$', false); +} + +TEST(MQTTTopicMatch, Exact) { + EXPECT_TRUE(topic_match("a/b/c", "a/b/c")); + EXPECT_FALSE(topic_match("a/b/c", "a/b")); + EXPECT_FALSE(topic_match("a/b", "a/b/c")); + EXPECT_FALSE(topic_match("a/b/c", "a/b/d")); +} + +TEST(MQTTTopicMatch, SingleLevelWildcard) { + EXPECT_TRUE(topic_match("a/b/c", "a/+/c")); + EXPECT_TRUE(topic_match("a/b/c", "+/+/+")); + EXPECT_TRUE(topic_match("a//c", "a/+/c")); + EXPECT_FALSE(topic_match("a/b/c", "a/+")); + EXPECT_FALSE(topic_match("a/b/c/d", "a/+/c")); +} + +TEST(MQTTTopicMatch, MultiLevelWildcard) { + EXPECT_TRUE(topic_match("a/b/c", "a/#")); + EXPECT_TRUE(topic_match("a/b/c", "#")); + EXPECT_TRUE(topic_match("a/b/c", "a/+/#")); + EXPECT_FALSE(topic_match("b/c", "a/#")); +} + +TEST(MQTTTopicMatch, DollarTopicsSkipFirstLevelWildcards) { + EXPECT_FALSE(topic_match("$SYS/broker", "#")); + EXPECT_FALSE(topic_match("$SYS/broker", "+/broker")); + EXPECT_TRUE(topic_match("$SYS/broker", "$SYS/#")); + EXPECT_TRUE(topic_match("$SYS/broker", "$SYS/+")); +} + +TEST(MQTTTopicMatch, EdgeCases) { + EXPECT_TRUE(topic_match("", "")); + EXPECT_FALSE(topic_match("a", "")); + // Kept from the previous code; the MQTT spec has "#" match the parent level and "+" an empty last level + EXPECT_FALSE(topic_match("a/b", "a/b/#")); + EXPECT_FALSE(topic_match("a/", "a/+")); + EXPECT_TRUE(topic_match("a/$x", "a/+")); +} + +// Recursion overflowed the ESP32 loop stack at about 250 characters +TEST(MQTTTopicMatch, LongTopic) { + std::string topic(4096, 'x'); + topic[100] = '/'; + EXPECT_TRUE(topic_match(topic.c_str(), topic.c_str())); + std::string other = topic; + other.back() = 'y'; + EXPECT_FALSE(topic_match(topic.c_str(), other.c_str())); +} + +TEST(MQTTTopicMatch, MatchesPreviousImplementation) { + // Small alphabet so wildcards, separators and '$' meet often + static constexpr char MESSAGE_CHARS[] = {'a', 'b', '/', '$'}; + static constexpr char SUBSCRIPTION_CHARS[] = {'a', 'b', '/', '$', '+', '#'}; + std::mt19937 rng(1234); // NOLINT(cert-msc32-c,cert-msc51-cpp,bugprone-random-generator-seed) reproducible + auto random_string = [&rng](const char *chars, size_t count) { + std::string s(rng() % 8, ' '); + for (char &c : s) + c = chars[rng() % count]; + return s; + }; + for (int i = 0; i < 20000; i++) { + std::string message = random_string(MESSAGE_CHARS, sizeof(MESSAGE_CHARS)); + std::string subscription = random_string(SUBSCRIPTION_CHARS, sizeof(SUBSCRIPTION_CHARS)); + ASSERT_EQ(topic_match(message.c_str(), subscription.c_str()), + reference_match(message.c_str(), subscription.c_str())) + << "message='" << message << "' subscription='" << subscription << "'"; + } +} + +} // namespace esphome::mqtt::testing