Fix number of lookups being counted wrong, fixes #3

markvantilburg · Nov 6, 2023 · 3c83314 · 3c83314
1 parent e5dd801
commit 3c83314
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 14 deletions.
diff --git a/BusinessMonitor.MailTools.Test/SpfTests.cs b/BusinessMonitor.MailTools.Test/SpfTests.cs
@@ -213,7 +213,7 @@ public void TestMX()
 
             // Check the number of lookups
             var lookups = (int)typeof(SpfCheck).GetField("_lookups", BindingFlags.NonPublic | BindingFlags.Instance).GetValue(check);
-            Assert.AreEqual(3, lookups);
+            Assert.AreEqual(1, lookups);
         }
 
         [Test]
@@ -259,6 +259,24 @@ public void TestMaxLookups()
             });
         }
 
+        [Test]
+        public void TestMaxMX()
+        {
+            var resolver = new DummyResolver("businessmonitor.nl", "v=spf1 mx");
+
+            for (var i = 0; i < 11; i++)
+            {
+                resolver.AddMail("businessmonitor.nl", $"mx{i}.businessmonitor.nl");
+            }
+
+            var check = new SpfCheck(resolver);
+
+            Assert.Throws<SpfException>(() =>
+            {
+                check.GetSpfRecord("businessmonitor.nl");
+            });
+        }
+
         [Test]
         public void TestIncludeFail()
         {

diff --git a/BusinessMonitor.MailTools/Spf/SpfCheck.cs b/BusinessMonitor.MailTools/Spf/SpfCheck.cs
@@ -64,13 +64,12 @@ public SpfRecord GetSpfRecord(string domain)
 
             _lookups = 0;
 
-            return GetRecord(domain, false);
+            return GetRecord(domain);
         }
 
-        private SpfRecord GetRecord(string domain, bool lookup = true)
+        private SpfRecord GetRecord(string domain)
         {
             var records = _resolver.GetTextRecords(domain);
-            if (lookup) _lookups++;
 
             // Find the SPF record
             var record = records.FirstOrDefault(x => x.StartsWith("v=spf1", StringComparison.InvariantCultureIgnoreCase));
@@ -87,6 +86,8 @@ private SpfRecord GetRecord(string domain, bool lookup = true)
             {
                 if (directive.Mechanism == SpfMechanism.Include && directive.Include != null)
                 {
+                    _lookups++;
+
                     if (_lookups > MaxLookups)
                     {
                         throw new SpfLookupException("SPF record exceeds max lookups of 10");
@@ -106,6 +107,8 @@ private SpfRecord GetRecord(string domain, bool lookup = true)
 
                 if (directive.Mechanism == SpfMechanism.A || directive.Mechanism == SpfMechanism.MX)
                 {
+                    _lookups++;
+
                     if (_lookups > MaxLookups)
                     {
                         throw new SpfLookupException("SPF record exceeds max lookups of 10");
@@ -269,8 +272,6 @@ private static SpfModifier ParseModifier(string term)
 
         private IPAddress[] ResolveDirective(SpfDirective directive)
         {
-            _lookups++;
-
             // If a mechanism lookup the addresses and return
             if (directive.Mechanism == SpfMechanism.A)
             {
@@ -279,17 +280,15 @@ private IPAddress[] ResolveDirective(SpfDirective directive)
 
             // Lookup all MX records and do a lookup on those
             var records = _resolver.GetMailRecords(directive.Domain);
-            var addresses = new List<IPAddress>();
 
-            foreach (var record in records)
+            if (records.Length > 10)
             {
-                if (_lookups > MaxLookups)
-                {
-                    throw new SpfLookupException("SPF record exceeds max lookups of 10");
-                }
-
-                _lookups++;
+                throw new SpfException("MX mechanism exceeds max MX records of 10");
+            }
 
+            var addresses = new List<IPAddress>();
+            foreach (var record in records)
+            {
                 addresses.AddRange(_resolver.GetAddressRecords(record));
             }