diff --git a/modoboa/admin/lib.py b/modoboa/admin/lib.py index ef5c94dc6..9991d6110 100644 --- a/modoboa/admin/lib.py +++ b/modoboa/admin/lib.py @@ -196,28 +196,25 @@ def get_domain_mx_list(domain): try: mx_domain = dns_answer.exchange.to_unicode( omit_final_dot=True, idna_codec=IDNA_2008_UTS_46) - ip_answers = socket.getaddrinfo( - mx_domain, 25, 0, socket.SOCK_STREAM) - except socket.gaierror as e: - logger.warning( - _("Unable to lookup ip addresses for %(domain)s; " - "%(error)s") - % {"domain": mx_domain, "error": str(e)}, - exc_info=e) + ip_answers = dns.resolver.query(mx_domain) + except dns.resolver.NXDOMAIN as e: + logger.error(_("No DNS records found without MX for %s") % domain, exc_info=e) else: for ip_answer in ip_answers: - try: - mx_ip = ipaddress.ip_address( - smart_text(ip_answer[4][0])) - except ValueError as e: - logger.warning( - _("Invalid IP address format for %(domain)s; " - "%(addr)s") - % {"domain": mx_domain, - "addr": smart_text(ip_answer[4][0])}, - exc_info=e) - else: - result.append((mx_domain, mx_ip)) + for answer_set in ip_answer.response.answer: + for address in answer_set: + try: + mx_ip = ipaddress.ip_address(smart_text(address)) + except ValueError as e: + logger.warning( + _("Invalid IP address format for %(domain)s; " + "%(addr)s") + % {"domain": mx_domain, + "addr": smart_text(address)}, + exc_info=e) + else: + result.append((mx_domain, mx_ip)) + return result diff --git a/modoboa/admin/tests/test_mx.py b/modoboa/admin/tests/test_mx.py index 3eb56bc23..239aa7c43 100644 --- a/modoboa/admin/tests/test_mx.py +++ b/modoboa/admin/tests/test_mx.py @@ -117,11 +117,13 @@ class MXTestCase(ModoTestCase): self.set_global_parameter("use_specific_dns_server", "123.45.67.89") get_domain_mx_list("does-not-exist.example.com") - @mock.patch("socket.getaddrinfo") + #@mock.patch("socket.getaddrinfo") + @mock.patch("ipaddress.ip_address") @mock.patch.object(dns.resolver.Resolver, "query") - def test_get_domain_mx_list_logging(self, mock_query, mock_getaddrinfo): + def test_get_domain_mx_list_logging(self, mock_query, mock_ip_address): mock_query.side_effect = utils.mock_dns_query_result - mock_getaddrinfo.side_effect = utils.mock_ip_query_result + #mock_getaddrinfo.side_effect = utils.mock_ip_query_result + mock_ip_address.side_effect = utils.mock_ip_address_result with LogCapture("modoboa.admin") as log: get_domain_mx_list("does-not-exist.example.com") get_domain_mx_list("no-mx.example.com") @@ -139,12 +141,12 @@ class MXTestCase(ModoTestCase): ("modoboa.admin", "WARNING", _("DNS resolution timeout, unable to query %s at the moment") % "timeout.example.com"), - ("modoboa.admin", "WARNING", - _("Unable to lookup ip addresses for %(domain)s; %(error)s") - % {"domain": "does-not-exist.example.com", "error": ""}), + ("modoboa.admin", "ERROR", + _("No DNS records found without MX for %(domain)s") + % {"domain": "no-lookup.example.com"}), ("modoboa.admin", "WARNING", _("Invalid IP address format for %(domain)s; %(addr)s") - % {"domain": "bad-response.example.com", "addr": "BAD"}), + % {"domain": "bad-response.example.com", "addr": "000.0.0.0"}), ) diff --git a/modoboa/admin/tests/utils.py b/modoboa/admin/tests/utils.py index 797fc6dcb..3e9161d2b 100644 --- a/modoboa/admin/tests/utils.py +++ b/modoboa/admin/tests/utils.py @@ -3,6 +3,7 @@ """Test utilities.""" from __future__ import unicode_literals +from dns.rrset import RRset import socket @@ -10,12 +11,38 @@ from dns.name import Name from dns.rdtypes.ANY.MX import MX from dns.resolver import NXDOMAIN, NoAnswer, NoNameservers, Timeout + + +class RRsetResponse(object): + + def __init__(self): + self.answer = [["192.0.2.1"]] + +class RRset(object): + + def __init__(self): + self.response = RRsetResponse() + +class RRsetResponseInvalid(object): + + def __init__(self): + self.answer = [["000.0.0.0"]] + +class RRsetInvalid(object): + + def __init__(self): + self.response = RRsetResponseInvalid() + + + _MX_RECORD_1 = MX("IN", "MX", 10, Name("mx.example.com".split("."))) _MX_RECORD_2 = MX("IN", "MX", 10, Name("mx2.example.com".split("."))) _BAD_MX_RECORD = MX("IN", "MX", 10, Name("bad-response.example.com".split("."))) _DNE_MX_RECORD = MX("IN", "MX", 10, Name( "does-not-exist.example.com".split("."))) _MX_RECORDS = [_MX_RECORD_1] +_IP_SET_RECORDS = [RRset()] + _IPV4_RECORD_1 = (socket.AF_INET, socket.SOCK_STREAM, 6, "", ("192.0.2.1", 25)) _IPV4_RECORD_2 = (socket.AF_INET, socket.SOCK_STREAM, 6, "", ("192.0.2.2", 25)) @@ -33,6 +60,12 @@ _POSSIBLE_DNS_RESULTS = { "no-lookup.example.com": [_DNE_MX_RECORD], } +_POSSIBLE_DNS_RESULTS_NO_MX = { + "does-not-exist.example.com": NXDOMAIN(), + "mx2.example.com": NXDOMAIN(), + "no-lookup.example.com": NXDOMAIN(), + "bad-response.example.com": [RRsetInvalid()], +} _POSSIBLE_IP_RESULTS = { "test3.com": [_IPV4_RECORD_2], "mx2.example.com": [_IPV4_RECORD_2], @@ -40,18 +73,36 @@ _POSSIBLE_IP_RESULTS = { "does-not-exist.example.com": socket.gaierror(), "192.0.2.254": socket.gaierror(), "2001:db8:254": socket.gaierror(), + "000.0.0.0": ValueError(), } def mock_dns_query_result(qname, *args, **kwargs): - if qname in _POSSIBLE_DNS_RESULTS: - if isinstance(_POSSIBLE_DNS_RESULTS[qname], Exception): - raise _POSSIBLE_DNS_RESULTS[qname] + if "MX" in args: + if qname in _POSSIBLE_DNS_RESULTS: + return get_mock_dns_query_response(_POSSIBLE_DNS_RESULTS, qname) + return _MX_RECORDS + elif qname in _POSSIBLE_DNS_RESULTS_NO_MX: + return get_mock_dns_query_response(_POSSIBLE_DNS_RESULTS_NO_MX, qname) + else: + return _IP_SET_RECORDS + + +def get_mock_dns_query_response(responses, attributeName): + if isinstance(responses[attributeName], Exception): + raise responses[attributeName] + else: + return responses[attributeName] + +def mock_ip_address_result(host, *args, **kwargs): + if host in _POSSIBLE_IP_RESULTS: + if isinstance(_POSSIBLE_IP_RESULTS[host], Exception): + raise _POSSIBLE_IP_RESULTS[host] else: - return _POSSIBLE_DNS_RESULTS[qname] - - return _MX_RECORDS - + return _POSSIBLE_IP_RESULTS[host] + + return _IP_RECORDS + def mock_ip_query_result(host, port, *args, **kwargs): if host in _POSSIBLE_IP_RESULTS: @@ -59,5 +110,5 @@ def mock_ip_query_result(host, port, *args, **kwargs): raise _POSSIBLE_IP_RESULTS[host] else: return _POSSIBLE_IP_RESULTS[host] - + return _IP_RECORDS