Merge remote-tracking branch 'origin/dev' into mqtt-async-client-topic-length

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.

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

tests/components/mqtt/test_topic_match.cpp

@@ -0,0 +1,103 @@
+#include <gtest/gtest.h>
+
+#include <random>
+#include <string>
+
+#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