More robust handling of old and new delimiter

Author: jjxtra

IPBanCore/Core/IPBan/IPBanFilter.cs

@@ -42,11 +42,6 @@ public class IPBanFilter : IIPBanFilter
         /// </summary>
         public const char ItemDelimiter = ',';
 
-        /// <summary>
-        /// Item pieces delimiters including the legacy ? and the current |
-        /// </summary>
-        public static readonly char[] ItemPiecesDelimitersLegacy = ['?', '|'];
-
         /// <summary>
         /// Item pieces delimiter |
         /// </summary>
@@ -57,6 +52,11 @@ public class IPBanFilter : IIPBanFilter
         /// </summary>
         public const char SubEntryDelimiter = ';';
 
+        /// <summary>
+        /// Legacy item pieces delimiter ?
+        /// </summary>
+        private const char itemPiecesDelimiterLegacy = '?';
+
         private static readonly HashSet<string> ignoreListEntries =
         [
             "0.0.0.0",
@@ -146,13 +146,7 @@ public IPBanFilter(string value, string regexValue, IHttpRequestMaker httpReques
                 // | can be used as a sub delimiter instead of ? mark
                 foreach (string entry in value.Split(ItemDelimiter, StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries))
                 {
-                    string entryWithoutComment = entry;
-                    int pos = entryWithoutComment.IndexOfAny(ItemPiecesDelimitersLegacy);
-                    if (pos >= 0)
-                    {
-                        entryWithoutComment = entryWithoutComment[..pos];
-                    }
-                    entryWithoutComment = entryWithoutComment.Trim();
+                    string entryWithoutComment = GetEntryWithoutComment(entry);
 
                     // sub entries (multiple ip addresses) are delimited by semi-colon
                     foreach (string subEntry in entryWithoutComment.Split(SubEntryDelimiter, StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries))
@@ -338,6 +332,63 @@ public bool IsFiltered(IPAddressRange range)
             return false;
         }
 
+        /// <summary>
+        /// Get entry without comment
+        /// </summary>
+        /// <param name="entry">Entry</param>
+        /// <returns>Entry without comment</returns>
+        public static string GetEntryWithoutComment(string entry)
+        {
+            string entryWithoutComment = entry;
+            int pos = entryWithoutComment.IndexOf(ItemPiecesDelimiter);
+
+            // if using new delimiter, remove it and everything after
+            if (pos >= 0)
+            {
+                entryWithoutComment = entryWithoutComment[..pos];
+            }
+            // if using two or more of old delimiter, remove it and everything after
+            else if (entryWithoutComment.Count(e => e == itemPiecesDelimiterLegacy) > 1)
+            {
+                pos = entryWithoutComment.IndexOf(itemPiecesDelimiterLegacy);
+                entryWithoutComment = entryWithoutComment[..pos];
+            }
+            entryWithoutComment = entryWithoutComment.Trim();
+
+            return entryWithoutComment;
+        }
+
+        /// <summary>
+        /// Split entry on new delimiter. If it doesn't exist, legacy delimiter is used.
+        /// </summary>
+        /// <param name="entry">Entry</param>
+        /// <returns>Pieces (always at least 3 items)</returns>
+        public static string[] SplitEntry(string entry)
+        {
+            entry ??= string.Empty;
+            string[] pieces = [];
+
+            // split on new delimiter if we have it
+            if (entry.Contains(ItemPiecesDelimiter))
+            {
+                pieces = entry.Split(ItemPiecesDelimiter, StringSplitOptions.TrimEntries);
+            }
+
+            // split on legacy delimiter if there's two or more
+            else if (entry.Count(e => e == itemPiecesDelimiterLegacy) > 1)
+            {
+                pieces = entry.Split(itemPiecesDelimiterLegacy, StringSplitOptions.TrimEntries);
+            }
+
+            // the split should have at least 3 pieces, if not, return the original entry and two empty ones
+            if (pieces.Length < 3)
+            {
+                return [entry, string.Empty, string.Empty];
+            }
+
+            return pieces;
+        }
+
         /// <summary>
         /// Get all ip address ranges in the filter
         /// </summary>

IPBanTests/IPBanFilterTests.cs

@@ -0,0 +1,41 @@
+using DigitalRuby.IPBanCore;
+
+using NUnit.Framework;
+
+namespace DigitalRuby.IPBanTests
+{
+    /// <summary>
+    /// IPBan filter tests
+    /// </summary>
+    [TestFixture]
+    public sealed class IPBanFilterTests
+    {
+        /// <summary>
+        /// Ensure we can get entry without comment using old and new delimiter properly
+        /// </summary>
+        [TestCase("1.2.3.4?2022-01-01T01:01:01Z?notes", "1.2.3.4")]
+        [TestCase("1.2.3.4|2022-01-01T01:01:01Z|notes", "1.2.3.4")]
+        [TestCase("https://fakeurl.com/?t=123", "https://fakeurl.com/?t=123")]
+        [TestCase("https://fakeurl.com/?t=123|2022-01-01T01:01:01Z|notes", "https://fakeurl.com/?t=123")]
+        public void TestGetEntryWithoutComment(string entry, string expectedResult)
+        {
+            string result = IPBanFilter.GetEntryWithoutComment(entry);
+            Assert.That(result, Is.EqualTo(expectedResult));
+        }
+
+        /// <summary>
+        /// Ensure we can split using old and new delimiters properly
+        /// </summary>
+        /// <param name="entry">Entry</param>
+        /// <param name="expectedResult">Expected result (3 items, | separated)</param>
+        [TestCase("1.2.3.4?2022-01-01T01:01:01Z?notes", "1.2.3.4|2022-01-01T01:01:01Z|notes")]
+        [TestCase("1.2.3.4|2022-01-01T01:01:01Z|notes", "1.2.3.4|2022-01-01T01:01:01Z|notes")]
+        [TestCase("https://fakeurl.com/?t=123", "https://fakeurl.com/?t=123||")]
+        [TestCase("https://fakeurl.com/?t=123|2022-01-01T01:01:01Z|notes", "https://fakeurl.com/?t=123|2022-01-01T01:01:01Z|notes")]
+        public void TestSplitEntry(string entry, string expectedResult)
+        {
+            string[] result = IPBanFilter.SplitEntry(entry);
+            Assert.That(string.Join('|', result), Is.EqualTo(expectedResult));
+        }
+    }
+}