Count A/AAAA, MX and PTR requests separately, fixes #25 (#28)

Mika56 · Apr 27, 2019 · 0989b3d · 0989b3d
1 parent 88a5285
commit 0989b3d
Show file tree

Hide file tree

Showing 11 changed files with 404 additions and 29 deletions.
diff --git a/src/DNSRecordGetter.php b/src/DNSRecordGetter.php
@@ -13,6 +13,8 @@
 class DNSRecordGetter implements DNSRecordGetterInterface
 {
     protected $requestCount = 0;
+    protected $requestMXCount = 0;
+    protected $requestPTRCount = 0;
 
     /**
      * @param $domain string The domain to get SPF record
@@ -94,7 +96,7 @@ public function resolvePtr($ipAddress)
             return $e['target'];
         }, dns_get_record($revIp, DNS_PTR));
 
-        return array_slice($revs, 0, 10);
+        return $revs;
     }
 
     public function exists($domain)
@@ -106,14 +108,39 @@ public function exists($domain)
         }
     }
 
+    /**
+     * @codeCoverageIgnore
+     */
     public function resetRequestCount()
     {
-        $this->requestCount = 0;
+        trigger_error('DNSRecordGetterInterface::resetRequestCount() is deprecated. Please use resetRequestCounts() instead', E_USER_DEPRECATED);
+        $this->resetRequestCounts();
     }
 
     public function countRequest()
     {
-        if (++$this->requestCount > 10) {
+        if ($this->requestCount++ == 10) {
+            throw new DNSLookupLimitReachedException();
+        }
+    }
+
+    public function resetRequestCounts()
+    {
+        $this->requestCount    = 0;
+        $this->requestMXCount  = 0;
+        $this->requestPTRCount = 0;
+    }
+
+    public function countMxRequest()
+    {
+        if (++$this->requestMXCount > 10) {
+            throw new DNSLookupLimitReachedException();
+        }
+    }
+
+    public function countPtrRequest()
+    {
+        if (++$this->requestPTRCount > 10) {
             throw new DNSLookupLimitReachedException();
         }
     }

diff --git a/src/DNSRecordGetterDirect.php b/src/DNSRecordGetterDirect.php
@@ -15,6 +15,8 @@ class DNSRecordGetterDirect implements DNSRecordGetterInterface
 {
 
     protected $requestCount = 0;
+    protected $requestMXCount = 0;
+    protected $requestPTRCount = 0;
     protected $nameserver = "8.8.8.8";
     protected $port = 53;
     protected $timeout = 30;
@@ -140,7 +142,7 @@ public function resolvePtr($ipAddress)
             return $e['target'];
         }, $this->dns_get_record($revIp, "PTR"));
 
-        return array_slice($revs, 0, 10);
+        return $revs;
     }
 
     public function exists($domain)
@@ -152,18 +154,6 @@ public function exists($domain)
         }
     }
 
-    public function resetRequestCount()
-    {
-        $this->requestCount = 0;
-    }
-
-    public function countRequest()
-    {
-        if (++$this->requestCount > 10) {
-            throw new DNSLookupLimitReachedException();
-        }
-    }
-
     public function dns_get_record($question, $type)
     {
 
@@ -248,4 +238,41 @@ public function dns_get_record($question, $type)
 
         return $response;
     }
+
+    /**
+     * @codeCoverageIgnore
+     */
+    public function resetRequestCount()
+    {
+        trigger_error('DNSRecordGetterInterface::resetRequestCount() is deprecated. Please use resetRequestCounts() instead', E_USER_DEPRECATED);
+        $this->resetRequestCounts();
+    }
+
+    public function countRequest()
+    {
+        if (++$this->requestCount > 10) {
+            throw new DNSLookupLimitReachedException();
+        }
+    }
+
+    public function resetRequestCounts()
+    {
+        $this->requestCount    = 0;
+        $this->requestMXCount  = 0;
+        $this->requestPTRCount = 0;
+    }
+
+    public function countMxRequest()
+    {
+        if (++$this->requestMXCount > 10) {
+            throw new DNSLookupLimitReachedException();
+        }
+    }
+
+    public function countPtrRequest()
+    {
+        if (++$this->requestPTRCount > 10) {
+            throw new DNSLookupLimitReachedException();
+        }
+    }
 }
diff --git a/src/DNSRecordGetterInterface.php b/src/DNSRecordGetterInterface.php
@@ -7,13 +7,62 @@
 namespace Mika56\SPFCheck;
 
 
+use Mika56\SPFCheck\Exception\DNSLookupException;
+use Mika56\SPFCheck\Exception\DNSLookupLimitReachedException;
+
 interface DNSRecordGetterInterface
 {
+    /**
+     * @param $domain
+     * @return string[]
+     * @throws DNSLookupException
+     */
     public function getSPFRecordForDomain($domain);
+
     public function resolveA($domain, $ip4only = false);
+
     public function resolveMx($domain);
+
     public function resolvePtr($ipAddress);
+
+    /**
+     * @param $domain
+     * @return boolean
+     * @throws DNSLookupException
+     */
     public function exists($domain);
+
+    /**
+     * @return void
+     * @deprecated {@see resetRequestCounts}
+     * @codeCoverageIgnore
+     */
     public function resetRequestCount();
+
+    /**
+     * Reset all request counters (A/AAAA, MX, PTR)
+     * @return void
+     */
+    public function resetRequestCounts();
+
+    /**
+     * Count a A/AAAA request
+     * @throws DNSLookupLimitReachedException
+     * @return void
+     */
     public function countRequest();
+
+    /**
+     * Count an MX request
+     * @throws DNSLookupLimitReachedException
+     * @return void
+     */
+    public function countMxRequest();
+
+    /**
+     * Count a PTR request
+     * @throws DNSLookupLimitReachedException
+     * @return void
+     */
+    public function countPtrRequest();
 }
diff --git a/src/SPFCheck.php b/src/SPFCheck.php
@@ -76,7 +76,7 @@ protected function doIsIPAllowed($ipAddress, $domain, $resetRequestCount)
         $this->redirect = null;
         if ($resetRequestCount) {
             $this->voidLookup = 0;
-            $this->DNSRecordGetter->resetRequestCount();
+            $this->DNSRecordGetter->resetRequestCounts();
         }
 
         // Handle IPv4 address in IPv6 format
@@ -92,6 +92,12 @@ protected function doIsIPAllowed($ipAddress, $domain, $resetRequestCount)
         return $result;
     }
 
+    /**
+     * @param $ipAddress
+     * @param $domain
+     * @return bool|string
+     * @throws DNSLookupException
+     */
     private function doCheck($ipAddress, $domain)
     {
         try {
@@ -141,6 +147,14 @@ private function doCheck($ipAddress, $domain)
         return self::RESULT_NEUTRAL;
     }
 
+    /**
+     * @param $ipAddress
+     * @param $part
+     * @param $matchingDomain
+     * @return bool
+     * @throws DNSLookupLimitReachedException
+     * @throws DNSLookupException
+     */
     protected function ipMatchesPart($ipAddress, $part, $matchingDomain)
     {
         $qualifier = substr($part, 0, 1);
@@ -242,10 +256,8 @@ protected function ipMatchesPart($ipAddress, $part, $matchingDomain)
                 $validIpAddresses = [];
                 $this->DNSRecordGetter->countRequest();
                 $mxServers = $this->DNSRecordGetter->resolveMx($domain);
-                if (count($mxServers) > 10) {
-                    return self::RESULT_PERMERROR;
-                }
                 foreach ($mxServers as $mxServer) {
+                    $this->DNSRecordGetter->countMxRequest();
                     if (false !== filter_var($mxServer, FILTER_VALIDATE_IP)) {
                         $validIpAddresses[] = $mxServer;
                     } else {
@@ -277,6 +289,7 @@ protected function ipMatchesPart($ipAddress, $part, $matchingDomain)
                 $ptrRecords                  = $this->DNSRecordGetter->resolvePtr($ipAddress);
                 $validatedSendingDomainNames = array();
                 foreach ($ptrRecords as $ptrRecord) {
+                    $this->DNSRecordGetter->countPtrRequest();
                     $ptrRecord   = strtolower($ptrRecord);
                     $ipAddresses = $this->DNSRecordGetter->resolveA($ptrRecord);
                     if (in_array($ipAddress, $ipAddresses)) {

diff --git a/tests/DNSRecordGetterIssue3.php b/tests/DNSRecordGetterIssue3.php
@@ -13,6 +13,8 @@
 class DNSRecordGetterIssue3 implements DNSRecordGetterInterface
 {
     protected $requestCount = 0;
+    protected $requestMXCount = 0;
+    protected $requestPTRCount = 0;
 
     protected $spfRecords = [
         'domain.com' => 'v=spf1 include:domain.com ~all',
@@ -49,12 +51,34 @@ public function exists($domain)
 
     public function resetRequestCount()
     {
-        $this->requestCount = 0;
+        trigger_error('DNSRecordGetterInterface::resetRequestCount() is deprecated. Please use resetRequestCounts() instead', E_USER_DEPRECATED);
+        $this->resetRequestCounts();
     }
 
     public function countRequest()
     {
-        if (++$this->requestCount == 10) {
+        if (++$this->requestCount > 10) {
+            throw new DNSLookupLimitReachedException();
+        }
+    }
+
+    public function resetRequestCounts()
+    {
+        $this->requestCount    = 0;
+        $this->requestMXCount  = 0;
+        $this->requestPTRCount = 0;
+    }
+
+    public function countMxRequest()
+    {
+        if (++$this->requestMXCount > 10) {
+            throw new DNSLookupLimitReachedException();
+        }
+    }
+
+    public function countPtrRequest()
+    {
+        if (++$this->requestPTRCount > 10) {
             throw new DNSLookupLimitReachedException();
         }
     }

diff --git a/tests/DNSRecordGetterIssue7.php b/tests/DNSRecordGetterIssue7.php
@@ -12,6 +12,8 @@
 class DNSRecordGetterIssue7 implements DNSRecordGetterInterface
 {
     protected $requestCount = 0;
+    protected $requestMXCount = 0;
+    protected $requestPTRCount = 0;
 
     protected $spfRecords = [
     ];
@@ -39,12 +41,34 @@ public function exists($domain)
 
     public function resetRequestCount()
     {
-        $this->requestCount = 0;
+        trigger_error('DNSRecordGetterInterface::resetRequestCount() is deprecated. Please use resetRequestCounts() instead', E_USER_DEPRECATED);
+        $this->resetRequestCounts();
     }
 
     public function countRequest()
     {
-        if (++$this->requestCount == 10) {
+        if (++$this->requestCount > 10) {
+            throw new DNSLookupLimitReachedException();
+        }
+    }
+
+    public function resetRequestCounts()
+    {
+        $this->requestCount    = 0;
+        $this->requestMXCount  = 0;
+        $this->requestPTRCount = 0;
+    }
+
+    public function countMxRequest()
+    {
+        if (++$this->requestMXCount > 10) {
+            throw new DNSLookupLimitReachedException();
+        }
+    }
+
+    public function countPtrRequest()
+    {
+        if (++$this->requestPTRCount > 10) {
             throw new DNSLookupLimitReachedException();
         }
     }

diff --git a/tests/DNSRecordGetterOpenSPF.php b/tests/DNSRecordGetterOpenSPF.php
@@ -16,6 +16,8 @@ class DNSRecordGetterOpenSPF implements DNSRecordGetterInterface
 {
     protected $data;
     protected $requestCount;
+    protected $requestMXCount = 0;
+    protected $requestPTRCount = 0;
 
     public function __construct($data)
     {
@@ -145,12 +147,34 @@ public function exists($domain)
 
     public function resetRequestCount()
     {
-        $this->requestCount = 0;
+        trigger_error('DNSRecordGetterInterface::resetRequestCount() is deprecated. Please use resetRequestCounts() instead', E_USER_DEPRECATED);
+        $this->resetRequestCounts();
     }
 
     public function countRequest()
     {
-        if (++$this->requestCount == 11) {
+        if (++$this->requestCount > 10) {
+            throw new DNSLookupLimitReachedException();
+        }
+    }
+
+    public function resetRequestCounts()
+    {
+        $this->requestCount    = 0;
+        $this->requestMXCount  = 0;
+        $this->requestPTRCount = 0;
+    }
+
+    public function countMxRequest()
+    {
+        if (++$this->requestMXCount > 10) {
+            throw new DNSLookupLimitReachedException();
+        }
+    }
+
+    public function countPtrRequest()
+    {
+        if (++$this->requestPTRCount > 10) {
             throw new DNSLookupLimitReachedException();
         }
     }