Skip to content

Fix connection timeout issue when try combine MX + A records for specfic DNS when A record not exists #410

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
alexander-schranz wants to merge 3 commits into
base: 4.x
Choose a base branch
from
Open

Fix connection timeout issue when try combine MX + A records for specfic DNS when A record not exists #410

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7a7bb3d
Fix connection timeout issue when try combine MX + A records for spec…
alexander-schranz Mar 6, 2025
39fb12b
Move validate MxRecords in own method
alexander-schranz Mar 6, 2025
1b0892a
Add some phpdoc
alexander-schranz Jun 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 20 additions & 6 deletions src/Validation/DNSCheckValidation.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class DNSCheckValidation implements EmailValidation
/**
* Reserved Top Level DNS Names (https://tools.ietf.org/html/rfc2606#section-2),
* mDNS and private DNS Namespaces (https://tools.ietf.org/html/rfc6762#appendix-G)
*
*
* @var string[]
*/
public const RESERVED_DNS_TOP_LEVEL_NAMES = [
Expand Down Expand Up @@ -145,14 +145,20 @@ protected function checkDns($host)
*/
private function validateDnsRecords($host): bool
{
$dnsRecordsResult = $this->dnsGetRecord->getRecords($host, DNS_A + DNS_MX);
$dnsRecords = [];

if ($dnsRecordsResult->withError()) {
$this->error = new InvalidEmail(new UnableToGetDNSRecord(), '');
return false;
$mxRecordsResult = $this->dnsGetRecord->getRecords($host, DNS_MX);

if (! $mxRecordsResult->withError()) {
$dnsRecords = $mxRecordsResult->getRecords();
}

$dnsRecords = $dnsRecordsResult->getRecords();
// Combined check for A+MX can fail with connection timed out, even in the presence of valid MX record
$aRecordsResult = $this->dnsGetRecord->getRecords($host, DNS_A);

if (! $aRecordsResult->withError()) {
$dnsRecords = array_merge($dnsRecords, $aRecordsResult->getRecords());
}

// Combined check for A+MX+AAAA can fail with SERVFAIL, even in the presence of valid A/MX records
$aaaaRecordsResult = $this->dnsGetRecord->getRecords($host, DNS_AAAA);
Expand All @@ -163,6 +169,14 @@ private function validateDnsRecords($host): bool

// No MX, A or AAAA DNS records
if ($dnsRecords === []) {
if ($mxRecordsResult->withError()
&& $aRecordsResult->withError()
&& $aaaaRecordsResult->withError()
) {
$this->error = new InvalidEmail(new UnableToGetDNSRecord(), '');
return false;
}
Comment on lines +172 to +178
Copy link
Author

@alexander-schranz alexander-schranz Mar 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unsure when we should return UnableToGetDNSRecord if one DNS lookup failed or when all failed?


$this->error = new InvalidEmail(new ReasonNoDNSRecord(), '');
return false;
}
Expand Down