BC change: DNSCheck fails for absent MX records. Fixes #258

egulias · Oct 17, 2020 · c4f7036 · c4f7036
1 parent af96d07
commit c4f7036
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 27 deletions.
diff --git a/src/Validation/DNSCheckValidation.php b/src/Validation/DNSCheckValidation.php
@@ -95,7 +95,7 @@ public function getWarnings() : array
      *
      * @return bool
      */
-    protected function checkDns($host)
+    protected function checkDns($host) : bool
     {
         $variant = INTL_IDNA_VARIANT_UTS46;
 
@@ -112,7 +112,7 @@ protected function checkDns($host)
      *
      * @return bool True on success.
      */
-    private function validateDnsRecords($host)
+    private function validateDnsRecords($host) : bool
     {
         // Get all MX, A and AAAA DNS records for host
         $dnsRecords = @dns_get_record($host, DNS_MX + DNS_A + DNS_AAAA);
@@ -126,13 +126,16 @@ private function validateDnsRecords($host)
 
         // For each DNS record
         foreach ($dnsRecords as $dnsRecord) {
-            if (!$this->validateMXRecord($dnsRecord)) {
-                // No MX records (fallback to A or AAAA records)
-                if (empty($this->mxRecords)) {
-                    $this->warnings[NoDNSMXRecord::CODE] = new NoDNSMXRecord();
-                }
-                return false;
-            }
+            $this->processRecord($dnsRecord);
+        }
+
+        if (empty($this->mxRecords)) {
+            $this->warnings[NoDNSMXRecord::CODE] = new NoDNSMXRecord();
+            $this->error = new InvalidEmail(new DomainAcceptsNoMail(), "");
+        }
+
+        if ($this->error !== null) {
+            return false;
         }
         return true;
     }
@@ -142,22 +145,16 @@ private function validateDnsRecords($host)
      *
      * @param array $dnsRecord Given DNS record.
      *
-     * @return bool True if valid.
      */
-    private function validateMxRecord($dnsRecord)
+    private function processRecord($dnsRecord)
     {
-        if ($dnsRecord['type'] !== 'MX') {
-            return true;
-        }
-
-        // "Null MX" record indicates the domain accepts no mail (https://tools.ietf.org/html/rfc7505)
-        if (empty($dnsRecord['target']) || $dnsRecord['target'] === '.') {
-            $this->error = new InvalidEmail(new DomainAcceptsNoMail(), "");
-            return false;
+        if ($dnsRecord['type'] === 'MX') {
+            // "Null MX" record indicates the domain accepts no mail (https://tools.ietf.org/html/rfc7505)
+            if (empty($dnsRecord['target']) || $dnsRecord['target'] === '.') {
+                $this->error = new InvalidEmail(new DomainAcceptsNoMail(), "");
+            }
+            $this->mxRecords[] = $dnsRecord;
         }
 
-        $this->mxRecords[] = $dnsRecord;
-
-        return true;
     }
 }
diff --git a/tests/EmailValidator/Validation/DNSCheckValidationTest.php b/tests/EmailValidator/Validation/DNSCheckValidationTest.php
@@ -28,7 +28,7 @@ public function validEmailsProvider()
             ['"Fred\ Bloggs"@ietf.org'],
             ['"Joe.\\Blow"@ietf.org'],
 
-            // unicide
+            // unicode
             ['ñandu.cl'],
         ];
     }
@@ -81,6 +81,9 @@ public function testLocalOrReservedDomainError($localOrReservedEmails)
         $this->assertEquals($expectedError, $validation->getError());
     }
 
+    /**
+     * Empty MX records
+     */
     public function testDomainAcceptsNoMailError()
     {
         $validation = new DNSCheckValidation();
@@ -90,20 +93,22 @@ public function testDomainAcceptsNoMailError()
         $this->assertFalse($isValidResult);
     }
 
-    public function testDNSWarnings()
+    public function testMissingMXisInvalidEmail()
     {
-        $this->markTestSkipped('Need to found a domain with AAAA redords and no MX that fails later in the validations');
+        //$this->markTestSkipped('Need to found a domain with A redords and no MX that fails later in the validations');
         $validation = new DNSCheckValidation();
         $expectedWarnings = [NoDNSMXRecord::CODE => new NoDNSMXRecord()];
-        $validation->isValid("example@invalid.example.com", new EmailLexer());
+        $result = $validation->isValid("example@icluod.com", new EmailLexer());
+        $this->assertFalse($result);
         $this->assertEquals($expectedWarnings, $validation->getWarnings());
     }
 
     public function testNoDNSError()
     {
         $validation = new DNSCheckValidation();
         $expectedError = new InvalidEmail(new NoDNSRecord(), '');
-        $validation->isValid("example@invalid.example.com", new EmailLexer());
+        $result = $validation->isValid("example@invalid.example.com", new EmailLexer());
+        $this->assertFalse($result);
         $this->assertEquals($expectedError, $validation->getError());
     }
 }