Skip to content

Commit 84a2413

Browse files
committed
Check and fail domains with null MX records
mock.patch("dns.resolver.LRUCache.get") breaks the DNS check in a way that didn't fail the deliverability check before, but it does now, so the mock is replaced with something else.
1 parent b08d0d3 commit 84a2413

File tree

3 files changed

+46
-26
lines changed

3 files changed

+46
-26
lines changed

README.md

+5-4
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ addresses are allowed when passing a `bytes`) and:
100100

101101
When an email address is not valid, `validate_email` raises either an
102102
`EmailSyntaxError` if the form of the address is invalid or an
103-
`EmailUndeliverableError` if the domain name does not resolve. Both
103+
`EmailUndeliverableError` if the domain name fails the DNS check. Both
104104
exception classes are subclasses of `EmailNotValidError`, which in turn
105105
is a subclass of `ValueError`.
106106

@@ -113,14 +113,15 @@ one uses anymore even though they are still valid and deliverable, since
113113
they will probably give you grief if you're using email for login. (See
114114
later in the document about that.)
115115

116-
The validator checks that the domain name in the email address resolves.
116+
The validator checks that the domain name in the email address has a
117+
(non-null) MX DNS record indicating that it is configured for email.
117118
There is nothing to be gained by trying to actually contact an SMTP
118119
server, so that's not done here. For privacy, security, and practicality
119120
reasons servers are good at not giving away whether an address is
120121
deliverable or not: email addresses that appear to accept mail at first
121122
can bounce mail after a delay, and bounced mail may indicate a temporary
122123
failure of a good email address (sometimes an intentional failure, like
123-
greylisting).
124+
greylisting). (A/AAAA-record fallback is also checked.)
124125

125126
The function also accepts the following keyword arguments (default as
126127
shown):
@@ -129,7 +130,7 @@ shown):
129130
require the
130131
[SMTPUTF8](https://tools.ietf.org/html/rfc6531) extension.
131132

132-
`check_deliverability=True`: Set to `False` to skip the domain name resolution check.
133+
`check_deliverability=True`: Set to `False` to skip the domain name MX DNS record check.
133134

134135
`allow_empty_local=False`: Set to `True` to allow an empty local part (i.e.
135136
`@example.com`), e.g. for validating Postfix aliases.

email_validator/__init__.py

+10-1
Original file line numberDiff line numberDiff line change
@@ -531,10 +531,19 @@ def dns_resolver_resolve_shim(domain, record):
531531
raise dns.exception.Timeout()
532532

533533
try:
534-
# Try resolving for MX records and get them in sorted priority order.
534+
# Try resolving for MX records and get them in sorted priority order
535+
# as (priority, qname) pairs.
535536
response = dns_resolver_resolve_shim(domain, "MX")
536537
mtas = sorted([(r.preference, str(r.exchange).rstrip('.')) for r in response])
537538
mx_fallback = None
539+
540+
# Do not permit delivery if there is only a "null MX" record (whose value is
541+
# (0, ".") but we've stripped trailing dots, so the 'exchange' is just "").
542+
mtas = [(preference, exchange) for preference, exchange in mtas
543+
if exchange != ""]
544+
if len(mtas) == 0:
545+
raise EmailUndeliverableError("The domain name %s does not accept email." % domain_i18n)
546+
538547
except (dns.resolver.NoNameservers, dns.resolver.NXDOMAIN, dns.resolver.NoAnswer):
539548

540549
# If there was no MX record, fall back to an A record.

tests/test_main.py

+31-21
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
from unittest import mock
21
import dns.resolver
32
import pytest
43
from email_validator import EmailSyntaxError, EmailUndeliverableError, \
@@ -292,10 +291,6 @@ def test_dict_accessor():
292291
assert valid_email.as_dict()["original_email"] == input_email
293292

294293

295-
def test_deliverability_no_records():
296-
assert validate_email_deliverability('example.com', 'example.com') == {'mx': [(0, '')], 'mx-fallback': None}
297-
298-
299294
def test_deliverability_found():
300295
response = validate_email_deliverability('gmail.com', 'gmail.com')
301296
assert response.keys() == {'mx', 'mx-fallback'}
@@ -307,10 +302,16 @@ def test_deliverability_found():
307302

308303

309304
def test_deliverability_fails():
305+
# No MX record.
310306
domain = 'xkxufoekjvjfjeodlfmdfjcu.com'
311307
with pytest.raises(EmailUndeliverableError, match='The domain name {} does not exist'.format(domain)):
312308
validate_email_deliverability(domain, domain)
313309

310+
# Null MX record.
311+
domain = 'example.com'
312+
with pytest.raises(EmailUndeliverableError, match='The domain name {} does not accept email'.format(domain)):
313+
validate_email_deliverability(domain, domain)
314+
314315

315316
def test_deliverability_dns_timeout():
316317
validate_email_deliverability.TEST_CHECK_TIMEOUT = True
@@ -379,25 +380,34 @@ def test_main_output_shim(monkeypatch, capsys):
379380
assert stdout == "b'An email address cannot have a period immediately after the @-sign.'\n"
380381

381382

382-
@mock.patch("dns.resolver.LRUCache.put")
383-
def test_validate_email__with_caching_resolver(mocked_put):
384-
dns_resolver = caching_resolver()
385-
validate_email("[email protected]", dns_resolver=dns_resolver)
386-
assert mocked_put.called
383+
def test_validate_email__with_caching_resolver():
384+
# unittest.mock.patch("dns.resolver.LRUCache.get") doesn't
385+
# work --- it causes get to always return an empty list.
386+
# So we'll mock our own way.
387+
class MockedCache:
388+
get_called = False
389+
put_called = False
387390

388-
with mock.patch("dns.resolver.LRUCache.get") as mocked_get:
389-
validate_email("[email protected]", dns_resolver=dns_resolver)
390-
assert mocked_get.called
391+
def get(self, key):
392+
self.get_called = True
393+
return None
391394

395+
def put(self, key, value):
396+
self.put_called = True
392397

393-
@mock.patch("dns.resolver.LRUCache.put")
394-
def test_validate_email__with_configured_resolver(mocked_put):
398+
# Test with caching_resolver helper method.
399+
mocked_cache = MockedCache()
400+
dns_resolver = caching_resolver(cache=mocked_cache)
401+
validate_email("[email protected]", dns_resolver=dns_resolver)
402+
assert mocked_cache.put_called
403+
validate_email("[email protected]", dns_resolver=dns_resolver)
404+
assert mocked_cache.get_called
405+
406+
# Test with dns.resolver.Resolver instance.
395407
dns_resolver = dns.resolver.Resolver()
396408
dns_resolver.lifetime = 10
397-
dns_resolver.cache = dns.resolver.LRUCache(max_size=1000)
409+
dns_resolver.cache = MockedCache()
398410
validate_email("[email protected]", dns_resolver=dns_resolver)
399-
assert mocked_put.called
400-
401-
with mock.patch("dns.resolver.LRUCache.get") as mocked_get:
402-
validate_email("[email protected]", dns_resolver=dns_resolver)
403-
assert mocked_get.called
411+
assert mocked_cache.put_called
412+
validate_email("[email protected]", dns_resolver=dns_resolver)
413+
assert mocked_cache.get_called

0 commit comments

Comments
 (0)