Working on test__socket_dns stability.

The c-ares resolver comes even closer to the system resolver.

Tests pass locally on a manylinux docker image. So take it out of the known flaky list for now, see how that does.

src/gevent/resolver/__init__.py

@@ -2,8 +2,8 @@
 
 from _socket import gaierror
 from _socket import error
-from _socket import getservbyname
-from _socket import getaddrinfo
+from _socket import getservbyname as native_getservbyname
+from _socket import getaddrinfo as native_getaddrinfo
 from _socket import SOCK_STREAM
 from _socket import SOCK_DGRAM
 from _socket import SOL_TCP
@@ -34,21 +34,21 @@ def _lookup_port(port, socktype):
                 if socktype == 0:
                     origport = port
                     try:
-                        port = getservbyname(port, 'tcp')
+                        port = native_getservbyname(port, 'tcp')
                         socktypes.append(SOCK_STREAM)
                     except error:
-                        port = getservbyname(port, 'udp')
+                        port = native_getservbyname(port, 'udp')
                         socktypes.append(SOCK_DGRAM)
                     else:
                         try:
-                            if port == getservbyname(origport, 'udp'):
+                            if port == native_getservbyname(origport, 'udp'):
                                 socktypes.append(SOCK_DGRAM)
                         except error:
                             pass
                 elif socktype == SOCK_STREAM:
-                    port = getservbyname(port, 'tcp')
+                    port = native_getservbyname(port, 'tcp')
                 elif socktype == SOCK_DGRAM:
-                    port = getservbyname(port, 'udp')
+                    port = native_getservbyname(port, 'udp')
                 else:
                     raise gaierror(EAI_SERVICE, 'Servname not supported for ai_socktype')
             except error as ex:
@@ -74,8 +74,8 @@ def _resolve_special(hostname, family):
     if not isinstance(hostname, hostname_types):
         raise TypeError("argument 1 must be str, bytes or bytearray, not %s" % (type(hostname),))
 
-    if hostname == '':
-        result = getaddrinfo(None, 0, family, SOCK_DGRAM, 0, AI_PASSIVE)
+    if hostname in (u'', b''):
+        result = native_getaddrinfo(None, 0, family, SOCK_DGRAM, 0, AI_PASSIVE)
         if len(result) != 1:
             raise error('wildcard resolved to multiple address')
         return result[0][4][0]

src/gevent/resolver/ares.py

@@ -6,6 +6,7 @@ from __future__ import absolute_import, print_function, division
 import os
 
 from _socket import getaddrinfo as native_getaddrinfo
+from _socket import gethostbyname_ex as native_gethostbyname_ex
 from _socket import gaierror
 from _socket import error
 
@@ -151,17 +152,29 @@ class Resolver(AbstractResolver):
         hostname = _resolve_special(hostname, family)
         return self.gethostbyname_ex(hostname, family)[-1][0]
 
+    HOSTNAME_ENCODING = 'idna' if PY3 else 'ascii'
+    _LOCAL_HOSTNAMES = (
+        b'localhost',
+        b'ip6-localhost',
+    )
+
+    _LOCAL_AND_BROADCAST_HOSTNAMES = _LOCAL_HOSTNAMES + (
+        b'255.255.255.255',
+    )
+
+    def _hostname_to_bytes(self, hostname):
+        if isinstance(hostname, text_type):
+            hostname = hostname.encode(self.HOSTNAME_ENCODING)
+        elif not isinstance(hostname, (bytes, bytearray)):
+            raise TypeError('Expected str, bytes or bytearray, not %s' % type(hostname).__name__)
+
+        return bytes(hostname)
+
     def gethostbyname_ex(self, hostname, family=AF_INET):
-        if PY3:
-            if isinstance(hostname, str):
-                hostname = hostname.encode('idna')
-            elif not isinstance(hostname, (bytes, bytearray)):
-                raise TypeError('Expected es(idna), not %s' % type(hostname).__name__)
-        else:
-            if isinstance(hostname, text_type):
-                hostname = hostname.encode('ascii')
-            elif not isinstance(hostname, str):
-                raise TypeError('Expected string, not %s' % type(hostname).__name__)
+        hostname = self._hostname_to_bytes(hostname)
+
+        if hostname in self._LOCAL_AND_BROADCAST_HOSTNAMES:
+            return native_gethostbyname_ex(hostname)
 
         while True:
             ares = self.cares
@@ -174,11 +187,6 @@ class Resolver(AbstractResolver):
                 return result
             except gaierror:
                 if ares is self.cares:
-                    if hostname == b'255.255.255.255':
-                        # The stdlib handles this case in 2.7 and 3.x, but ares does not.
-                        # It is tested by test_socket.py in 3.4.
-                        # HACK: So hardcode the expected return.
-                        return ('255.255.255.255', [], ['255.255.255.255'])
                     raise
                 # "self.cares is not ares" means channel was destroyed (because we were forked)
 
@@ -198,8 +206,8 @@ class Resolver(AbstractResolver):
         # pylint:disable=too-many-locals,too-many-branches
         if isinstance(host, text_type):
             host = host.encode('idna')
-        if not isinstance(host, bytes) or (flags & AI_NUMERICHOST) or host in (
-                b'localhost', b'ip6-localhost'):
+
+        if not isinstance(host, bytes) or (flags & AI_NUMERICHOST) or host in self._LOCAL_HOSTNAMES:
             # XXX: Now that we're using ares_getaddrinfo, how much of this is still
             # necessary?
             # this handles cases which do not require network access
@@ -247,6 +255,7 @@ class Resolver(AbstractResolver):
             # ever. It's at least supposed to do that if they were given as
             # hints, but it doesn't (https://github.com/c-ares/c-ares/issues/317)
             # Sigh.
+            # The SOL_* constants are another (older?) name for IPPROTO_*
             if socktype:
                 hard_type_proto = [
                     (socktype, SOL_TCP if socktype == SOCK_STREAM else SOL_UDP),
@@ -284,16 +293,7 @@ class Resolver(AbstractResolver):
                     raise
 
     def _gethostbyaddr(self, ip_address):
-        if PY3:
-            if isinstance(ip_address, str):
-                ip_address = ip_address.encode('idna')
-            elif not isinstance(ip_address, (bytes, bytearray)):
-                raise TypeError('Expected es(idna), not %s' % type(ip_address).__name__)
-        else:
-            if isinstance(ip_address, text_type):
-                ip_address = ip_address.encode('ascii')
-            elif not isinstance(ip_address, str):
-                raise TypeError('Expected string, not %s' % type(ip_address).__name__)
+        ip_address = self._hostname_to_bytes(ip_address)
 
         waiter = Waiter(self.hub)
         try:
@@ -322,30 +322,9 @@ class Resolver(AbstractResolver):
                 if ares is self.cares:
                     raise
 
-    def _getnameinfo(self, sockaddr, flags):
-        if not isinstance(flags, integer_types):
-            raise TypeError('an integer is required')
-        if not isinstance(sockaddr, tuple):
-            raise TypeError('getnameinfo() argument 1 must be a tuple')
+    def _getnameinfo(self, hostname, port, sockaddr, flags):
 
-        address = sockaddr[0]
-        if not PY3 and isinstance(address, text_type):
-            address = address.encode('ascii')
-
-        if not isinstance(address, string_types):
-            raise TypeError('sockaddr[0] must be a string, not %s' % type(address).__name__)
-
-        port = sockaddr[1]
-        if not isinstance(port, integer_types):
-            raise TypeError('port must be an integer, not %s' % type(port))
-
-        if len(sockaddr) > 2:
-            # Must be IPv6: (host, port, [flowinfo, [scopeid]])
-            flowinfo = sockaddr[2]
-            if flowinfo > 0xfffff:
-                raise OverflowError("getnameinfo(): flowinfo must be 0-1048575.")
-
-        result = self._getaddrinfo(address, port,
+        result = self._getaddrinfo(hostname, port,
                                    family=AF_UNSPEC, socktype=SOCK_DGRAM, fill_in_type_proto=False)
         if len(result) != 1:
             raise error('sockaddr resolved to multiple addresses')
@@ -375,10 +354,39 @@ class Resolver(AbstractResolver):
         return node, service or '0'
 
     def getnameinfo(self, sockaddr, flags):
+        if not isinstance(flags, integer_types):
+            raise TypeError('an integer is required')
+        if not isinstance(sockaddr, tuple):
+            raise TypeError('getnameinfo() argument 1 must be a tuple')
+
+        address = sockaddr[0]
+        address = self._hostname_to_bytes(sockaddr[0])
+
+        port = sockaddr[1]
+        if not isinstance(port, integer_types):
+            raise TypeError('port must be an integer, not %s' % type(port))
+
+        if port >= 65536:
+            # System resolvers do different things with an
+            # out-of-bound port; macOS CPython 3.8 raises ``gaierror: [Errno 8]
+            # nodename nor servname provided, or not known``, while
+            # manylinux CPython 2.7 appears to ignore it and raises ``error:
+            # sockaddr resolved to multiple addresses``. TravisCI, at least ot
+            # one point, successfully resolved www.gevent.org to ``(readthedocs.org, '0')``.
+            # But c-ares 1.16 would raise ``gaierror(25, 'ARES_ESERVICE: unknown')``.
+            # Doing this appears to get the expected results.
+            port = 0
+
+        if len(sockaddr) > 2:
+            # Must be IPv6: (host, port, [flowinfo, [scopeid]])
+            flowinfo = sockaddr[2]
+            if flowinfo > 0xfffff:
+                raise OverflowError("getnameinfo(): flowinfo must be 0-1048575.")
+
         while True:
             ares = self.cares
             try:
-                return self._getnameinfo(sockaddr, flags)
+                return self._getnameinfo(address, port, sockaddr, flags)
             except gaierror:
                 if ares is self.cares:
                     raise

src/gevent/tests/known_failures.py

@@ -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,

src/gevent/tests/test__socket_dns.py

@@ -265,6 +265,11 @@ class TestCase(greentest.TestCase):
         return repr(result1) != repr(result2)
 
     def _test(self, func_name, *args):
+        """
+        Runs the function *func_name* with *args* and compares gevent and the system.
+
+        Returns the gevent result.
+        """
         gevent_func = getattr(gevent_socket, func_name)
         real_func = monkey.get_original('socket', func_name)
 
@@ -330,22 +335,38 @@ class TestCase(greentest.TestCase):
         # On some systems, the hostname can get caps
         return (result[0].lower(), [], ips)
 
-    IGNORE_CANONICAL_NAME = RESOLVER_ARES # It tends to return them even when not asked for
+    NORMALIZE_GAI_IGNORE_CANONICAL_NAME = RESOLVER_ARES # It tends to return them even when not asked for
     if not RESOLVER_NOT_SYSTEM:
         def _normalize_result_getaddrinfo(self, result):
             return result
     else:
         def _normalize_result_getaddrinfo(self, result):
+            # Result is a list
+            # (family, socktype, proto, canonname, sockaddr)
+            # e.g.,
+            # (AF_INET, SOCK_STREAM, IPPROTO_TCP, 'readthedocs.io', (127.0.0.1, 80))
+
             # On Python 3, the builtin resolver can return SOCK_RAW results, but
             # c-ares doesn't do that. So we remove those if we find them.
-            if hasattr(socket, 'SOCK_RAW') and isinstance(result, list):
-                result = [x for x in result if x[1] != socket.SOCK_RAW]
-            if self.IGNORE_CANONICAL_NAME:
+            # Likewise, on certain Linux systems, even on Python 2, IPPROTO_SCTP (132)
+            # results may be returned --- but that may not even have a constant in the
+            # socket module! So to be safe, we strip out anything that's not
+            # SOCK_STREAM or SOCK_DGRAM
+            if isinstance(result, list):
+                result = [
+                    x
+                    for x in result
+                    if x[1] in (socket.SOCK_STREAM, socket.SOCK_DGRAM)
+                    and x[2] in (socket.IPPROTO_TCP, socket.IPPROTO_UDP)
+                ]
+
+            if self.NORMALIZE_GAI_IGNORE_CANONICAL_NAME:
                 result = [
                     (family, kind, proto, '', addr)
                     for family, kind, proto, _, addr
                     in result
                 ]
+
             if isinstance(result, list):
                 result.sort()
             return result
@@ -593,33 +614,38 @@ class TestGeventOrg(TestCase):
             return result
 
     def test_AI_CANONNAME(self):
-        self.IGNORE_CANONICAL_NAME = False
-        result = self._test('getaddrinfo',
-                            # host
-                            TestGeventOrg.HOSTNAME,
-                            # port
-                            None,
-                            # family
-                            socket.AF_INET,
-                            # type
-                            0,
-                            # proto
-                            0,
-                            # flags
-                            socket.AI_CANONNAME)
-        self.assertEqual(result[0][3], 'readthedocs.io')
+        # Not all systems support AI_CANONNAME; notably tha manylinux
+        # resolvers *sometimes* do not. Specifically, sometimes they
+        # provide the canonical name *only* on the first result.
+
+        args = (
+            # host
+            TestGeventOrg.HOSTNAME,
+            # port
+            None,
+            # family
+            socket.AF_INET,
+            # type
+            0,
+            # proto
+            0,
+            # flags
+            socket.AI_CANONNAME
+        )
+        gevent_result = gevent_socket.getaddrinfo(*args)
+        self.assertEqual(gevent_result[0][3], 'readthedocs.io')
+        real_result = socket.getaddrinfo(*args)
+
+        self.NORMALIZE_GAI_IGNORE_CANONICAL_NAME = not all(r[3] for r in real_result)
+        try:
+            self.assertEqualResults(real_result, gevent_result, 'getaddrinfo')
+        finally:
+            del self.NORMALIZE_GAI_IGNORE_CANONICAL_NAME
 
 add(TestGeventOrg, TestGeventOrg.HOSTNAME)
 
 
 class TestFamily(TestCase):
-    maxDiff = None
-    @classmethod
-    def getresult(cls):
-        if not hasattr(cls, '_result'):
-            cls._result = socket.getaddrinfo(TestGeventOrg.HOSTNAME, None)
-        return cls._result
-
     def test_inet(self):
         self._test('getaddrinfo', TestGeventOrg.HOSTNAME, None, socket.AF_INET)
 
@@ -840,10 +866,6 @@ class TestInvalidPort(TestCase):
     def test_typeerror_str(self):
         self._test('getnameinfo', ('www.gevent.org', 'x'), 0)
 
-    @unittest.skipIf(RESOLVER_DNSPYTHON,
-                     "System resolvers do funny things with this: macOS raises gaierror, "
-                     "Travis CI returns (readthedocs.org, '0'). It's hard to match that exactly. "
-                     "dnspython raises OverflowError.")
     def test_overflow_port_too_large(self):
         self._test('getnameinfo', ('www.gevent.org', 65536), 0)
 

src/gevent/tests/test__socket_dns6.py

@@ -3,7 +3,6 @@
 from __future__ import print_function, absolute_import, division
 
 import socket
-import unittest
 
 import gevent.testing as greentest
 from gevent.tests.test__socket_dns import TestCase, add
@@ -40,12 +39,17 @@ 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 test_empty(self):
         self._test('getaddrinfo', self.host, 'http')
@@ -79,13 +83,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)