Perl-Email-Project / Email-Valid

perl library to validate email addresses
19 stars 19 forks source link

Improved Net::DNS MX Record Checking #18

Closed tmorehouse closed 9 years ago

tmorehouse commented 9 years ago

For some domains (specifically one of my own I was testing against), the mxcheck routine was returning a false positive (pass) when it should have failed the test (i.e. no MX records in the DNS).

This shortened routine appears to get around that issue.

rjbs commented 9 years ago

Thanks! I've squashed these commits and applied them, they'll be in the next release, soon!

tmorehouse commented 9 years ago

Excellent, thanks!

rjbs commented 8 years ago

I'm really baffled, in retrospect, that I applied this.

It's okay for a domain to have no MX. In these cases, mail is delivered to the A record.

Is there a reason not to revert this, based on that?

tmorehouse commented 8 years ago

Any domain that receives email should have an MX record (or two, or more).

I have several domains that cannot receive email, and have no MX records, yet Email-Valid would say that the domain was valid to send email to. and when sending email the message would bounce saying unable to deliver mail. Hence the domain in invalid for receiving email,

tmorehouse commented 8 years ago

Domain Check would be better for validating just if the domain exists and has an A record. MX check should be just for checking if the domain is configured for receiving email (i.e. has MX records, hence the name MX check).

If, in the previous version., if it din't find and MX and fell back to an A record, then you may as well just be using only domain check.

MX Check should check for an MX record, and then test that the MX record content points to an A record (via looking up the A for the MX content name).

tmorehouse commented 8 years ago

If one does want to check for an A record if no MX's are found, one could modify the _net_dns_query function as so:

sub _net_dns_query {
  my $self = shift;
  my $host = shift;

  $Resolver = Net::DNS::Resolver->new unless defined $Resolver;

  # Check for valid MX entries
  my @mx_entries = Net::DNS::mx($Resolver, $host);
  if (@mx_entries) {
    foreach my $mx (@mx_entries) {
      my $mxhost = $mx->exchange;
      my $query  = $Resolver->search($mxhost);
      next unless ($query);
      foreach my $a_rr ($query->answer) {
        return 1 unless $a_rr->type ne 'A';
      }
    }
  }

  # Check for valid $host A records
  my @a_rrs = Net::DNS::rr($Resolver, $host, 'A');
  if (@a_rrs) {
    foreach my $a_rr (@a_rrs) {
      return 1 unless $a_rr->type ne 'A';
    }
  }

  # Else fail MX Check  
  return $self->details('mx');
}