Commit 1c3dd450 authored by Jason Madden's avatar Jason Madden Committed by GitHub

Merge pull request #1596 from gevent/issue1012

Better exception handling for alternate resolvers
parents 5acba203 86c7bd41
The c-ares and DNSPython resolvers now raise exceptions much more
consistently with the standard resolver. Types and errnos are
substantially more likely to match what the standard library produces.
Depending on the system and configuration, results may not match
exactly, at least with DNSPython. There are still some rare cases
where the system resolver can raise ``herror`` but DNSPython will
raise ``gaierror`` or vice versa. There doesn't seem to be a
deterministic way to account for this. On PyPy, ``getnameinfo`` can
produce results when CPython raises ``socket.error``, and gevent's
DNSPython resolver also raises ``socket.error``.
In addition, several other small discrepancies were addressed,
including handling of localhost and broadcast host names.
......@@ -43,6 +43,8 @@ else:
native_path_types = string_types
thread_mod_name = 'thread'
hostname_types = tuple(set(string_types + (bytearray, bytes)))
def NativeStrIO():
import io
return io.BytesIO() if str is bytes else io.StringIO()
......
This diff is collapsed.
This diff is collapsed.
......@@ -16,8 +16,11 @@ from cpython.mem cimport PyMem_Malloc
from cpython.mem cimport PyMem_Free
from libc.string cimport memset
from gevent._compat import MAC
import _socket
from _socket import gaierror
from _socket import herror
__all__ = ['channel']
......@@ -46,6 +49,31 @@ cdef extern from *:
#ifdef HAVE_NETDB_H
#include <netdb.h>
#endif
#ifndef EAI_ADDRFAMILY
#define EAI_ADDRFAMILY -1
#endif
#ifndef EAI_BADHINTS
#define EAI_BADHINTS -2
#endif
#ifndef EAI_NODATA
#define EAI_NODATA -3
#endif
#ifndef EAI_OVERFLOW
#define EAI_OVERFLOW -4
#endif
#ifndef EAI_PROTOCOL
#define EAI_PROTOCOL -5
#endif
#ifndef EAI_SYSTEM
#define EAI_SYSTEM
#endif
"""
cdef extern from "ares.h":
......@@ -98,7 +126,7 @@ cdef int NI_NAMEREQD = _socket.NI_NAMEREQD
cdef int NI_DGRAM = _socket.NI_DGRAM
_ares_errors = dict([
cdef dict _ares_errors = dict([
(cares.ARES_SUCCESS, 'ARES_SUCCESS'),
(cares.ARES_EADDRGETNETWORKPARAMS, 'ARES_EADDRGETNETWORKPARAMS'),
......@@ -128,9 +156,67 @@ _ares_errors = dict([
(cares.ARES_ETIMEOUT, 'ARES_ETIMEOUT'),
])
cdef dict _ares_to_gai_system = {
cares.ARES_EBADFAMILY: cares.EAI_ADDRFAMILY,
cares.ARES_EBADFLAGS: cares.EAI_BADFLAGS,
cares.ARES_EBADHINTS: cares.EAI_BADHINTS,
cares.ARES_ENOMEM: cares.EAI_MEMORY,
cares.ARES_ENONAME: cares.EAI_NONAME,
cares.ARES_ENOTFOUND: cares.EAI_NONAME,
cares.ARES_ENOTIMP: cares.EAI_FAMILY,
# While EAI_NODATA ("No address associated with nodename") might
# seem to be the natural mapping, typical resolvers actually
# return EAI_NONAME in that same situation; I've yet to find EAI_NODATA
# in a test.
cares.ARES_ENODATA: cares.EAI_NONAME,
# This one gets raised for unknown port/service names.
cares.ARES_ESERVICE: cares.EAI_NONAME if MAC else cares.EAI_SERVICE,
}
cdef _gevent_gai_strerror(code):
cdef const char* err_str
cdef object result = None
cdef int system
try:
system = _ares_to_gai_system[code]
except KeyError:
err_str = cares.ares_strerror(code)
result = '%s: %s' % (_ares_errors.get(code) or code, _as_str(err_str))
else:
err_str = cares.gai_strerror(system)
result = _as_str(err_str)
return result
cdef object _gevent_gaierror_from_status(int ares_status):
cdef object code = _ares_to_gai_system.get(ares_status, ares_status)
cdef object message = _gevent_gai_strerror(ares_status)
return gaierror(code, message)
cdef dict _ares_to_host_system = {
cares.ARES_ENONAME: cares.HOST_NOT_FOUND,
cares.ARES_ENOTFOUND: cares.HOST_NOT_FOUND,
cares.ARES_ENODATA: cares.NO_DATA,
}
cdef _gevent_herror_strerror(code):
cdef const char* err_str
cdef object result = None
cdef int system
try:
system = _ares_to_host_system[code]
except KeyError:
err_str = cares.ares_strerror(code)
result = '%s: %s' % (_ares_errors.get(code) or code, _as_str(err_str))
else:
err_str = cares.hstrerror(system)
result = _as_str(err_str)
return result
cpdef strerror(code):
return '%s: %s' % (_ares_errors.get(code) or code, cares.ares_strerror(code))
cdef object _gevent_herror_from_status(int ares_status):
cdef object code = _ares_to_host_system.get(ares_status, ares_status)
cdef object message = _gevent_herror_strerror(ares_status)
return herror(code, message)
class InvalidIP(ValueError):
......@@ -217,29 +303,6 @@ cdef list _parse_h_addr_list(hostent* host):
return result
cdef void gevent_ares_host_callback(void *arg, int status, int timeouts, hostent* host):
cdef channel channel
cdef object callback
channel, callback = <tuple>arg
Py_DECREF(<tuple>arg)
cdef object host_result
try:
if status or not host:
callback(Result(None, gaierror(status, strerror(status))))
else:
try:
host_result = ares_host_result(host.h_addrtype,
(_as_str(host.h_name),
_parse_h_aliases(host),
_parse_h_addr_list(host)))
except:
callback(Result(None, sys.exc_info()[1]))
else:
callback(Result(host_result))
except:
channel.loop.handle_error(callback, *sys.exc_info())
cdef object _as_str(const char* val):
if not val:
return None
......@@ -258,7 +321,7 @@ cdef void gevent_ares_nameinfo_callback(void *arg, int status, int timeouts, cha
cdef object service
try:
if status:
callback(Result(None, gaierror(status, strerror(status))))
callback(Result(None, _gevent_gaierror_from_status(status)))
else:
node = _as_str(c_node)
service = _as_str(c_service)
......@@ -328,10 +391,10 @@ cdef class channel:
cdef int result = cares.ares_library_init(cares.ARES_LIB_INIT_ALL) # ARES_LIB_INIT_WIN32 -DUSE_WINSOCK?
if result:
raise gaierror(result, strerror(result))
raise gaierror(result, _gevent_gai_strerror(result))
result = cares.ares_init_options(&channel, &options, optmask)
if result:
raise gaierror(result, strerror(result))
raise gaierror(result, _gevent_gai_strerror(result))
self._timer = loop.timer(TIMEOUT, TIMEOUT)
self._watchers = {}
self.channel = channel
......@@ -398,7 +461,7 @@ cdef class channel:
c_servers[length - 1].next = NULL
index = cares.ares_set_servers(self.channel, c_servers)
if index:
raise ValueError(strerror(index))
raise ValueError(_gevent_gai_strerror(index))
finally:
PyMem_Free(c_servers)
......@@ -449,6 +512,30 @@ cdef class channel:
write_fd = cares.ARES_SOCKET_BAD
cares.ares_process_fd(self.channel, read_fd, write_fd)
@staticmethod
cdef void _gethostbyname_or_byaddr_cb(void *arg, int status, int timeouts, hostent* host):
cdef channel channel
cdef object callback
channel, callback = <tuple>arg
Py_DECREF(<tuple>arg)
cdef object host_result
try:
if status or not host:
callback(Result(None, _gevent_herror_from_status(status)))
else:
try:
host_result = ares_host_result(host.h_addrtype,
(_as_str(host.h_name),
_parse_h_aliases(host),
_parse_h_addr_list(host)))
except:
callback(Result(None, sys.exc_info()[1]))
else:
callback(Result(host_result))
except:
channel.loop.handle_error(callback, *sys.exc_info())
def gethostbyname(self, object callback, char* name, int family=AF_INET):
if not self.channel:
raise gaierror(cares.ARES_EDESTRUCTION, 'this ares channel has been destroyed')
......@@ -456,7 +543,7 @@ cdef class channel:
cdef object arg = (self, callback)
Py_INCREF(arg)
cares.ares_gethostbyname(self.channel, name, family,
<void*>gevent_ares_host_callback, <void*>arg)
<void*>channel._gethostbyname_or_byaddr_cb, <void*>arg)
def gethostbyaddr(self, object callback, char* addr):
if not self.channel:
......@@ -475,7 +562,8 @@ cdef class channel:
raise InvalidIP(repr(addr))
cdef object arg = (self, callback)
Py_INCREF(arg)
cares.ares_gethostbyaddr(self.channel, addr_packed, length, family, <void*>gevent_ares_host_callback, <void*>arg)
cares.ares_gethostbyaddr(self.channel, addr_packed, length, family,
<void*>channel._gethostbyname_or_byaddr_cb, <void*>arg)
cpdef _getnameinfo(self, object callback, tuple sockaddr, int flags):
if not self.channel:
......@@ -488,8 +576,8 @@ cdef class channel:
if not PyTuple_Check(sockaddr):
raise TypeError('expected a tuple, got %r' % (sockaddr, ))
PyArg_ParseTuple(sockaddr, "si|ii", &hostp, &port, &flowinfo, &scope_id)
if port < 0 or port > 65535:
raise gaierror(-8, 'Invalid value for port: %r' % port)
# if port < 0 or port > 65535:
# raise gaierror(-8, 'Invalid value for port: %r' % port)
cdef int length = _make_sockaddr(hostp, port, flowinfo, scope_id, &sa6)
if length <= 0:
raise InvalidIP(repr(hostp))
......@@ -560,7 +648,7 @@ cdef class channel:
addrs = []
try:
if status != cares.ARES_SUCCESS:
callback(Result(None, gaierror(status, strerror(status))))
callback(Result(None, _gevent_gaierror_from_status(status)))
return
if result.cnames:
# These tend to come in pairs:
......
......@@ -63,20 +63,21 @@ from __future__ import absolute_import, print_function, division
import sys
import time
import _socket
from _socket import AI_NUMERICHOST
from _socket import error
from _socket import gaierror
from _socket import herror
from _socket import NI_NUMERICSERV
from _socket import AF_INET
from _socket import AF_INET6
from _socket import AF_UNSPEC
from _socket import EAI_NONAME
from _socket import EAI_FAMILY
import socket
from gevent.resolver import AbstractResolver
from gevent.resolver import hostname_types
from gevent.resolver._hostsfile import HostsFile
from gevent.resolver._addresses import is_ipv6_addr
from gevent.builtins import __import__ as g_import
......@@ -84,6 +85,7 @@ from gevent._compat import string_types
from gevent._compat import iteritems
from gevent._config import config
__all__ = [
'Resolver',
]
......@@ -318,6 +320,7 @@ def _family_to_rdtype(family):
'Address family not supported')
return rdtype
class Resolver(AbstractResolver):
"""
An *experimental* resolver that uses `dnspython`_.
......@@ -344,7 +347,13 @@ class Resolver(AbstractResolver):
.. caution::
Many of the same caveats about DNS results apply here as are documented
for :class:`gevent.resolver.ares.Resolver`.
for :class:`gevent.resolver.ares.Resolver`. In addition, the handling of
symbolic scope IDs in IPv6 addresses passed to ``getaddrinfo`` exhibits
some differences.
On PyPy, ``getnameinfo`` can produce results when CPython raises
``socket.error``, and gevent's DNSPython resolver also
raises ``socket.error``.
.. caution::
......@@ -353,6 +362,12 @@ class Resolver(AbstractResolver):
.. versionadded:: 1.3a2
.. versionchanged:: NEXT
The errors raised are now much more consistent with those
raised by the standard library resolvers.
Handling of localhost and broadcast names is now more consistent.
.. _dnspython: http://www.dnspython.org
"""
......@@ -409,18 +424,20 @@ class Resolver(AbstractResolver):
hostname = ans[0].target
return aliases
def getaddrinfo(self, host, port, family=0, socktype=0, proto=0, flags=0):
if ((host in (u'localhost', b'localhost')
or (is_ipv6_addr(host) and host.startswith('fe80')))
or not isinstance(host, str) or (flags & AI_NUMERICHOST)):
# this handles cases which do not require network access
# 1) host is None
# 2) host is of an invalid type
# 3) host is localhost or a link-local ipv6; dnspython returns the wrong
# scope-id for those.
# 3) AI_NUMERICHOST flag is set
return _socket.getaddrinfo(host, port, family, socktype, proto, flags)
def _getaddrinfo(self, host_bytes, port, family, socktype, proto, flags):
# dnspython really wants the host to be in native format.
if not isinstance(host_bytes, str):
host_bytes = host_bytes.decode(self.HOSTNAME_ENCODING)
if host_bytes == 'ff02::1de:c0:face:8D':
# This is essentially a hack to make stdlib
# test_socket:GeneralModuleTests.test_getaddrinfo_ipv6_basic
# pass. They expect to get back a lowercase ``D``, but
# dnspython does not do that.
# ``test_getaddrinfo_ipv6_scopeid_symbolic`` also expect
# the scopeid to be dropped, but again, dnspython does not
# do that; we cant fix that here so we skip that test.
host_bytes = 'ff02::1de:c0:face:8d'
if family == AF_UNSPEC:
# This tends to raise in the case that a v6 address did not exist
......@@ -433,22 +450,24 @@ class Resolver(AbstractResolver):
# See also https://github.com/gevent/gevent/issues/1012
try:
return _getaddrinfo(host, port, family, socktype, proto, flags)
except socket.gaierror:
return _getaddrinfo(host_bytes, port, family, socktype, proto, flags)
except gaierror:
try:
return _getaddrinfo(host, port, AF_INET6, socktype, proto, flags)
except socket.gaierror:
return _getaddrinfo(host, port, AF_INET, socktype, proto, flags)
return _getaddrinfo(host_bytes, port, AF_INET6, socktype, proto, flags)
except gaierror:
return _getaddrinfo(host_bytes, port, AF_INET, socktype, proto, flags)
else:
return _getaddrinfo(host, port, family, socktype, proto, flags)
def getnameinfo(self, sockaddr, flags):
if (sockaddr
and isinstance(sockaddr, (list, tuple))
and sockaddr[0] in ('::1', '127.0.0.1', 'localhost')):
return _socket.getnameinfo(sockaddr, flags)
if isinstance(sockaddr, (list, tuple)) and not isinstance(sockaddr[0], hostname_types):
raise TypeError("getnameinfo(): illegal sockaddr argument")
try:
return _getaddrinfo(host_bytes, port, family, socktype, proto, flags)
except gaierror as ex:
if ex.args[0] == EAI_NONAME and family not in self._KNOWN_ADDR_FAMILIES:
# It's possible that we got sent an unsupported family. Check
# that.
ex.args = (EAI_FAMILY, self.EAI_FAMILY_MSG)
ex.errno = EAI_FAMILY
raise
def _getnameinfo(self, address_bytes, port, sockaddr, flags):
try:
return resolver._getnameinfo(sockaddr, flags)
except error:
......@@ -458,13 +477,21 @@ class Resolver(AbstractResolver):
# that does this. We conservatively fix it here; this could be expanded later.
return resolver._getnameinfo(sockaddr, NI_NUMERICSERV)
def gethostbyaddr(self, ip_address):
if ip_address in (u'127.0.0.1', u'::1',
b'127.0.0.1', b'::1',
'localhost'):
return _socket.gethostbyaddr(ip_address)
if not isinstance(ip_address, hostname_types):
raise TypeError("argument 1 must be str, bytes or bytearray, not %s" % (type(ip_address),))
return resolver._gethostbyaddr(ip_address)
def _gethostbyaddr(self, ip_address_bytes):
try:
return resolver._gethostbyaddr(ip_address_bytes)
except gaierror as ex:
if ex.args[0] == EAI_NONAME:
# Note: The system doesn't *always* raise herror;
# sometimes the original gaierror propagates through.
# It's impossible to say ahead of time or just based
# on the name which it should be. The herror seems to
# be by far the most common, though.
raise herror(1, "Unknown host")
raise
# Things that need proper error handling
getnameinfo = AbstractResolver.fixup_gaierror(AbstractResolver.getnameinfo)
gethostbyaddr = AbstractResolver.fixup_gaierror(AbstractResolver.gethostbyaddr)
gethostbyname_ex = AbstractResolver.fixup_gaierror(AbstractResolver.gethostbyname_ex)
getaddrinfo = AbstractResolver.fixup_gaierror(AbstractResolver.getaddrinfo)
......@@ -7,6 +7,34 @@ cdef extern from "ares.h":
struct hostent:
pass
# Errors from getaddrinfo
int EAI_ADDRFAMILY # The specified network host does not have
# any network addresses in the requested address family (Linux)
int EAI_AGAIN # temporary failure in name resolution
int EAI_BADFLAGS # invalid value for ai_flags
int EAI_BADHINTS # invalid value for hints (macOS only)
int EAI_FAIL # non-recoverable failure in name resolution
int EAI_FAMILY # ai_family not supported
int EAI_MEMORY # memory allocation failure
int EAI_NODATA # The specified network host exists, but does not have
# any network addresses defined. (Linux)
int EAI_NONAME # hostname or servname not provided, or not known
int EAI_OVERFLOW # argument buffer overflow (macOS only)
int EAI_PROTOCOL # resolved protocol is unknown (macOS only)
int EAI_SERVICE # servname not supported for ai_socktype
int EAI_SOCKTYPE # ai_socktype not supported
int EAI_SYSTEM # system error returned in errno (macOS and Linux)
char* gai_strerror(int ecode)
# Errors from gethostbyname and gethostbyaddr
int HOST_NOT_FOUND
int TRY_AGAIN
int NO_RECOVERY
int NO_DATA
char* hstrerror(int err)
struct ares_options:
int flags
void* sock_state_cb
......
......@@ -5,6 +5,9 @@ import os
test_filename = sys.argv[1]
del sys.argv[1]
if test_filename == 'test_urllib2_localnet.py' and os.environ.get('APPVEYOR'):
os.environ['GEVENT_DEBUG'] = 'TRACE'
print('Running with patch_all(): %s' % (test_filename,))
from gevent import monkey
......
......@@ -17,6 +17,7 @@ from .sysinfo import RUNNING_ON_APPVEYOR as APPVEYOR
from .sysinfo import RUNNING_ON_TRAVIS as TRAVIS
from .sysinfo import RESOLVER_NOT_SYSTEM as ARES
from .sysinfo import RESOLVER_ARES
from .sysinfo import RESOLVER_DNSPYTHON
from .sysinfo import RUNNING_ON_CI
from .sysinfo import RUN_COVERAGE
......@@ -1231,6 +1232,13 @@ if PY38:
'test_ssl.BasicSocketTests.test_parse_cert_CVE_2013_4238',
]
if RESOLVER_DNSPYTHON:
disabled_tests += [
# This does two things DNS python doesn't. First, it sends it
# capital letters and expects them to be returned lowercase.
# Second, it expects the symbolic scopeid to be stripped from the end.
'test_socket.GeneralModuleTests.test_getaddrinfo_ipv6_scopeid_symbolic',
]
# if 'signalfd' in os.environ.get('GEVENT_BACKEND', ''):
# # tests that don't interact well with signalfd
......
......@@ -296,22 +296,6 @@ class Definitions(DefinitionsBase):
run_alone=APPVEYOR,
)
test__socket_dns = Flaky(
"""
A few errors and differences:
AssertionError: ('255.255.255.255', 'http') != gaierror(-2,) # DNS Python
AssertionError: ('255.255.255.255', 'http') != gaierror(4, 'ARES_ENOTFOUND: Domain name not found')
AssertionError: OverflowError('port must be 0-65535.',) != ('readthedocs.org', '65535')
AssertionError: Lists differ:
(10, 1, 6, '', ('2607:f8b0:4004:810::200e', 80, 0L, 0L))
(10, 1, 6, '', ('2607:f8b0:4004:805::200e', 80, 0, 0))
Somehow it seems most of these are fixed with PyPy3.6-7 under dnspython,
(once we commented out TestHostname)?
""",
when=RESOLVER_NOT_SYSTEM | PY3
)
test__monkey_sigchld_2 = Ignored(
"""
This hangs for no apparent reason when run by the testrunner,
......
......@@ -35,7 +35,7 @@ class TestTimeout(greentest.TestCase):
r = Resolver(servers=[address[0]], timeout=0.001, tries=1,
udp_port=address[-1])
with self.assertRaisesRegex(socket.gaierror, "ARES_ETIMEOUT"):
with self.assertRaisesRegex(socket.herror, "ARES_ETIMEOUT"):
r.gethostbyname('www.google.com')
......
This diff is collapsed.
......@@ -9,9 +9,8 @@ import gevent.testing as greentest
from gevent.tests.test__socket_dns import TestCase, add
from gevent.testing.sysinfo import OSX
from gevent.testing.sysinfo import RESOLVER_NOT_SYSTEM
from gevent.testing.sysinfo import RESOLVER_DNSPYTHON
from gevent.testing.sysinfo import PYPY
# We can't control the DNS servers on CI (or in general...)
......@@ -40,12 +39,22 @@ class Test6(TestCase):
# host that only has AAAA record
host = 'aaaa.test-ipv6.com'
if not OSX or RESOLVER_DNSPYTHON:
def _test(self, *args): # pylint:disable=arguments-differ
raise unittest.SkipTest(
"Only known to work on jamadden's machine. "
"Please help investigate and make DNS tests more robust."
)
def _normalize_result_gethostbyaddr(self, result):
# This part of the test is effectively disabled. There are multiple address
# that resolve and which ones you get depend on the settings
# of the system and ares. They don't match exactly.
return ()
if not OSX and RESOLVER_DNSPYTHON:
# It raises gaierror instead of socket.error,
# which is not great and leads to failures.
def _run_test_getnameinfo(self, *_args):
return (), 0, (), 0
def _run_test_gethostbyname(self, *_args):
raise unittest.SkipTest("gethostbyname[_ex] does not support IPV6")
_run_test_gethostbyname_ex = _run_test_gethostbyname
def test_empty(self):
self._test('getaddrinfo', self.host, 'http')
......@@ -63,12 +72,16 @@ class Test6(TestCase):
class Test6_google(Test6):
host = 'ipv6.google.com'
def _normalize_result_getnameinfo(self, result):
if greentest.RUNNING_ON_CI and RESOLVER_NOT_SYSTEM:
# Disabled, there are multiple possibilities
# and we can get different ones, rarely.
if greentest.RUNNING_ON_CI:
# Disabled, there are multiple possibilities
# and we can get different ones. Even the system resolvers
# can go round-robin and provide different answers.
def _normalize_result_getnameinfo(self, result):
return ()
return result
if PYPY:
# PyPy tends to be especially problematic in that area.
_normalize_result_getaddrinfo = _normalize_result_getnameinfo
add(Test6, Test6.host)
add(Test6_google, Test6_google.host)
......@@ -79,13 +92,7 @@ class Test6_ds(Test6):
# host that has both A and AAAA records
host = 'ds.test-ipv6.com'
def _normalize_result_gethostbyaddr(self, result):
# This test is effectively disabled. There are multiple address
# that resolve and which ones you get depend on the settings
# of the system and ares. They don't match exactly.
return ()
_normalize_result_gethostbyname = _normalize_result_gethostbyaddr
_normalize_result_gethostbyname = Test6._normalize_result_gethostbyaddr
add(Test6_ds, Test6_ds.host)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment