Fixes and clean the new topic matcher from israellot

Frameworks/MQTTnet.NetStandard/Server/MqttTopicFilterComparer.cs

@@ -4,46 +4,116 @@ namespace MQTTnet.Server
 {
     public static class MqttTopicFilterComparer
     {
-        private static readonly char[] TopicLevelSeparator = { '/' };
+        private const char LEVEL_SEPARATOR = '/';
+        private const char WILDCARD_MULTI_LEVEL = '#';
+        private const char WILDCARD_SINGLE_LEVEL = '+';
 
         public static bool IsMatch(string topic, string filter)
         {
-            if (topic == null) throw new ArgumentNullException(nameof(topic));
-            if (filter == null) throw new ArgumentNullException(nameof(filter));
+            if (string.IsNullOrEmpty(topic)) throw new ArgumentNullException(nameof(topic));
+            if (string.IsNullOrEmpty(filter)) throw new ArgumentNullException(nameof(filter));
 
-            if (string.Equals(topic, filter, StringComparison.Ordinal))
-            {
-                return true;
-            }
-
-            var fragmentsTopic = topic.Split(TopicLevelSeparator, StringSplitOptions.None);
-            var fragmentsFilter = filter.Split(TopicLevelSeparator, StringSplitOptions.None);
+            int spos = 0;
+            int slen = filter.Length;
+            int tpos = 0;
+            int tlen = topic.Length;
 
-            // # > In either case it MUST be the last character specified in the Topic Filter [MQTT-4.7.1-2].
-            for (var i = 0; i < fragmentsFilter.Length; i++)
+            while (spos < slen && tpos < tlen)
             {
-                if (fragmentsFilter[i] == "+")
-                {
-                    continue;
-                }
-
-                if (fragmentsFilter[i] == "#")
+                if (filter[spos] == topic[tpos])
                 {
-                    return true;
+                    if (tpos == tlen - 1)
+                    {
+                        /* Check for e.g. foo matching foo/# */
+                        if (spos == slen - 3
+                                && filter[spos + 1] == LEVEL_SEPARATOR
+                                && filter[spos + 2] == WILDCARD_MULTI_LEVEL)
+                        {
+                            return true;
+                        }
+                    }
+                    spos++;
+                    tpos++;
+                    if (spos == slen && tpos == tlen)
+                    {
+                        return true;
+                    }
+                    else if (tpos == tlen && spos == slen - 1 && filter[spos] == WILDCARD_SINGLE_LEVEL)
+                    {
+                        if (spos > 0 && filter[spos - 1] != LEVEL_SEPARATOR)
+                        {
+                            // Invalid filter string
+                            return false;
+                        }
+                        spos++;
+                        return true;
+                    }
                 }
-
-                if (i >= fragmentsTopic.Length)
-                {
-                    return false;
-                }
-
-                if (!string.Equals(fragmentsFilter[i], fragmentsTopic[i], StringComparison.Ordinal))
+                else
                 {
-                    return false;
+                    if (filter[spos] == WILDCARD_SINGLE_LEVEL)
+                    {
+                        /* Check for bad "+foo" or "a/+foo" subscription */
+                        if (spos > 0 && filter[spos - 1] != LEVEL_SEPARATOR)
+                        {
+                            // Invalid filter string
+                            return false;
+                        }
+                        /* Check for bad "foo+" or "foo+/a" subscription */
+                        if (spos < slen - 1 && filter[spos + 1] != LEVEL_SEPARATOR)
+                        {
+                            // Invalid filter string
+                            return false;
+                        }
+                        spos++;
+                        while (tpos < tlen && topic[tpos] != LEVEL_SEPARATOR)
+                        {
+                            tpos++;
+                        }
+                        if (tpos == tlen && spos == slen)
+                        {
+                            return true;
+                        }
+                    }
+                    else if (filter[spos] == WILDCARD_MULTI_LEVEL)
+                    {
+                        if (spos > 0 && filter[spos - 1] != LEVEL_SEPARATOR)
+                        {
+                            // Invalid filter string
+                            return false;
+                        }
+                        if (spos + 1 != slen)
+                        {
+                            // Invalid filter string
+                            return false;
+                        }
+                        else
+                        {
+                            return true;
+                        }
+                    }
+                    else
+                    {
+                        /* Check for e.g. foo/bar matching foo/+/# */
+                        if (spos > 0
+                                && spos + 2 == slen
+                                && tpos == tlen
+                                && filter[spos - 1] == WILDCARD_SINGLE_LEVEL
+                                && filter[spos] == LEVEL_SEPARATOR
+                                && filter[spos + 1] == WILDCARD_MULTI_LEVEL)
+                        {
+                            return true;
+                        }
+                        return false;
+                    }
                 }
             }
+            if (tpos < tlen || spos < slen)
+            {
+                return false;
+            }
 
-            return fragmentsTopic.Length == fragmentsFilter.Length;
+            return false;
         }
     }
 }

Tests/MQTTnet.Benchmarks/MQTTnet.Benchmarks.csproj

@@ -147,6 +147,7 @@
     </Reference>
   </ItemGroup>
   <ItemGroup>
+    <Compile Include="TopicFilterComparerBenchmark.cs" />
     <Compile Include="LoggerBenchmark.cs" />
     <Compile Include="MessageProcessingBenchmark.cs" />
     <Compile Include="Program.cs" />

Tests/MQTTnet.Benchmarks/Program.cs

@@ -12,6 +12,7 @@ namespace MQTTnet.Benchmarks
             Console.WriteLine("1 = MessageProcessingBenchmark");
             Console.WriteLine("2 = SerializerBenchmark");
             Console.WriteLine("3 = LoggerBenchmark");
+            Console.WriteLine("4 = TopicFilterComparerBenchmark");
 
             var pressedKey = Console.ReadKey(true);
             switch (pressedKey.KeyChar)
@@ -25,6 +26,9 @@ namespace MQTTnet.Benchmarks
                 case '3':
                     BenchmarkRunner.Run<LoggerBenchmark>();
                     break;
+                case '4':
+                    BenchmarkRunner.Run<TopicFilterComparerBenchmark>();
+                    break;
             }
 
             Console.ReadLine();

Tests/MQTTnet.Benchmarks/TopicFilterComparerBenchmark.cs

@@ -0,0 +1,91 @@
+using BenchmarkDotNet.Attributes;
+using BenchmarkDotNet.Attributes.Exporters;
+using BenchmarkDotNet.Attributes.Jobs;
+using MQTTnet.Server;
+using System;
+
+namespace MQTTnet.Benchmarks
+{
+    [ClrJob]
+    [RPlotExporter]
+    [MemoryDiagnoser]
+    public class TopicFilterComparerBenchmark
+    {
+        private static readonly char[] TopicLevelSeparator = { '/' };
+
+        [GlobalSetup]
+        public void Setup()
+        {
+        }
+
+        [Benchmark]
+        public void MqttTopicFilterComparer_10000_StringSplitMethod()
+        {
+            for (var i = 0; i < 10000; i++)
+            {
+                LegacyMethodByStringSplit("sport/tennis/player1", "sport/#");
+                LegacyMethodByStringSplit("sport/tennis/player1/ranking", "sport/#/ranking");
+                LegacyMethodByStringSplit("sport/tennis/player1/score/wimbledon", "sport/+/player1/#");
+                LegacyMethodByStringSplit("sport/tennis/player1", "sport/tennis/+");
+                LegacyMethodByStringSplit("/finance", "+/+");
+                LegacyMethodByStringSplit("/finance", "/+");
+                LegacyMethodByStringSplit("/finance", "+");
+            }
+        }
+
+        [Benchmark]
+        public void MqttTopicFilterComparer_10000_LoopMethod()
+        {
+            for (var i = 0; i < 10000; i++)
+            {
+                MqttTopicFilterComparer.IsMatch("sport/tennis/player1", "sport/#");
+                MqttTopicFilterComparer.IsMatch("sport/tennis/player1/ranking", "sport/#/ranking");
+                MqttTopicFilterComparer.IsMatch("sport/tennis/player1/score/wimbledon", "sport/+/player1/#");
+                MqttTopicFilterComparer.IsMatch("sport/tennis/player1", "sport/tennis/+");
+                MqttTopicFilterComparer.IsMatch("/finance", "+/+");
+                MqttTopicFilterComparer.IsMatch("/finance", "/+");
+                MqttTopicFilterComparer.IsMatch("/finance", "+");
+            }
+        }
+
+        private static bool LegacyMethodByStringSplit(string topic, string filter)
+        {
+            if (topic == null) throw new ArgumentNullException(nameof(topic));
+            if (filter == null) throw new ArgumentNullException(nameof(filter));
+
+            if (string.Equals(topic, filter, StringComparison.Ordinal))
+            {
+                return true;
+            }
+
+            var fragmentsTopic = topic.Split(TopicLevelSeparator, StringSplitOptions.None);
+            var fragmentsFilter = filter.Split(TopicLevelSeparator, StringSplitOptions.None);
+
+            // # > In either case it MUST be the last character specified in the Topic Filter [MQTT-4.7.1-2].
+            for (var i = 0; i < fragmentsFilter.Length; i++)
+            {
+                if (fragmentsFilter[i] == "+")
+                {
+                    continue;
+                }
+
+                if (fragmentsFilter[i] == "#")
+                {
+                    return true;
+                }
+
+                if (i >= fragmentsTopic.Length)
+                {
+                    return false;
+                }
+
+                if (!string.Equals(fragmentsFilter[i], fragmentsTopic[i], StringComparison.Ordinal))
+                {
+                    return false;
+                }
+            }
+
+            return fragmentsTopic.Length == fragmentsFilter.Length;
+        }
+    }
+}