Fixed some corner cases for topic_filter::matches()

README.md

@@ -125,7 +125,7 @@ $ sudo apt-get install doxygen graphviz
 
 Unit tests are built using _Catch2_.
 
-_Catch2_ can be found here: [Catch2](https://github.com/catchorg/Catch2).  You must download and install _Catch2_ to build and run the unit tests locally.
+_Catch2_ can be found here: [Catch2](https://github.com/catchorg/Catch2).  You must download and install _Catch2_ to build and run the unit tests locally. Currently _Catch2_ versions v2.x and v3.x are supported.
 
 #### Building the Paho C library
 

include/mqtt/topic.h

@@ -211,6 +211,22 @@ public:
      * 				 '+' and '#'.
      */
     explicit topic_filter(const string& filter);
+	/**
+	 * Determins if the character is a wildcard, '+' or '#'
+	 * @param c The character to check
+	 * @return @em true if `c` is a wildcard, '+' or '#'
+	 */
+	static bool is_wildcard(char c) {
+		return c == '+' || c == '#';
+	}
+	/**
+	 * Determins if the string (field) is a wildcard, "+" or "#"
+	 * @param s The string to check
+	 * @return @em true if `c` is a wildcard, "+" or "#"
+	 */
+	static bool is_wildcard(const string& s) {
+		return s.size() == 1 && is_wildcard(s[0]);
+	}
     /**
      * Determines if the specified topic/filter contains any wildcards.
      *

include/mqtt/topic_matcher.h

@@ -334,6 +334,7 @@ public:
             }
 
             // Topics starting with '$' don't match wildcards in the first field
+			// MQTT v5 Spec, Section 4.7.2:
             // https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901246
 
             if (!snode.first_ || field.empty() || field[0] != '$') {

src/topic.cpp

@@ -97,7 +97,7 @@ bool topic_filter::has_wildcards(const string& filter)
 bool topic_filter::has_wildcards() const
 {
     return std::any_of(fields_.cbegin(), fields_.cend(), [](const auto& f) {
-        return (f == "+" || f == "#");
+        return is_wildcard(f);
     });
 }
 
@@ -108,9 +108,26 @@ bool topic_filter::has_wildcards() const
 bool topic_filter::matches(const string& topic) const
 {
     auto n = fields_.size();
-    auto topic_fields = topic::split(topic);
 
-    if (n > topic_fields.size()) {
+    auto topic_fields = topic::split(topic);
+    auto nt = topic_fields.size();
+
+    // Filter can't match a topic that is shorter
+    if (n > nt) {
+        return false;
+    }
+
+    // Might match a longer topic, but only with '#' wildcard
+    if (n < nt && fields_[n - 1] != "#") {
+        return false;
+    }
+
+    // Topics starting with '$' don't match wildcards in the first field
+    // MQTT v5 Spec, Section 4.7.2:
+    // https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901246
+
+    if (n > 0 && is_wildcard(fields_[0]) && nt > 0 && topic_fields[0].size() > 0 &&
+        topic_fields[0][0] == '$') {
         return false;
     }
 

test/unit/test_topic.cpp

@@ -195,7 +195,7 @@ TEST_CASE("publish full binary", "[topic]")
 //						topic_filter
 /////////////////////////////////////////////////////////////////////////////
 
-TEST_CASE("has_wildcards", "[topic_filter]")
+TEST_CASE("topic has_wildcards", "[topic_filter]")
 {
     REQUIRE(!topic_filter::has_wildcards(TOPIC));
 
@@ -203,13 +203,14 @@ TEST_CASE("has_wildcards", "[topic_filter]")
     REQUIRE(topic_filter::has_wildcards("some/multi/wild/#"));
 }
 
-TEST_CASE("matches", "[topic_filter]")
+TEST_CASE("topic matches", "[topic_filter]")
 {
     SECTION("no_wildcards")
     {
-        topic_filter filt{TOPIC};
+        topic_filter filt{"my/topic/name"};
 
-        REQUIRE(filt.matches(TOPIC));
+        REQUIRE(filt.matches("my/topic/name"));
+        REQUIRE(!filt.matches("my/topic/name/but/longer"));
         REQUIRE(!filt.matches("some/other/topic"));
     }
 
@@ -235,4 +236,42 @@ TEST_CASE("matches", "[topic_filter]")
         REQUIRE(!filt.matches("my/other/name"));
         REQUIRE(!filt.matches("my/other/id"));
     }
+
+    // Th following sections are mostly borrowed from the Paho Python tests.
+    // They have a number of good corner cases that should and should not
+    // match.
+
+    SECTION("should_match")
+    {
+        REQUIRE(topic_filter{"foo/bar"}.matches("foo/bar"));
+        REQUIRE(
+            topic_filter{
+                "foo/+",
+            }
+                .matches("foo/bar")
+        );
+        REQUIRE(topic_filter{"foo/+/baz"}.matches("foo/bar/baz"));
+        REQUIRE(topic_filter{"foo/+/#"}.matches("foo/bar/baz"));
+        REQUIRE(topic_filter{"A/B/+/#"}.matches("A/B/B/C"));
+        REQUIRE(topic_filter{"#"}.matches("foo/bar/baz"));
+        REQUIRE(topic_filter{"#"}.matches("/foo/bar"));
+        REQUIRE(topic_filter{"/#"}.matches("/foo/bar"));
+        REQUIRE(topic_filter{"$SYS/bar"}.matches("$SYS/bar"));
+        REQUIRE(topic_filter{"$SYS/#"}.matches("$SYS/bar"));
+        REQUIRE(topic_filter{"foo/#"}.matches("foo/$bar"));
+        REQUIRE(topic_filter{"foo/+/baz"}.matches("foo/$bar/baz"));
+    }
+
+    SECTION("should_not_match")
+    {
+        REQUIRE(!topic_filter{"test/6/#"}.matches("test/3"));
+        REQUIRE(!topic_filter{"foo/bar"}.matches("foo"));
+        REQUIRE(!topic_filter{"foo/+"}.matches("foo/bar/baz"));
+        REQUIRE(!topic_filter{"foo/+/baz"}.matches("foo/bar/bar"));
+        REQUIRE(!topic_filter{"foo/+/#"}.matches("fo2/bar/baz"));
+        REQUIRE(!topic_filter{"/#"}.matches("foo/bar"));
+        REQUIRE(!topic_filter{"#"}.matches("$SYS/bar"));
+        REQUIRE(!topic_filter{"$BOB/bar"}.matches("$SYS/bar"));
+        REQUIRE(!topic_filter{"+/bar"}.matches("$SYS/bar"));
+    }
 }