Move dnshelper.c into cython and fix some test discrepencies for localhost.

_setupares.py

@@ -95,7 +95,6 @@ ARES = Extension(
     libraries=list(LIBRARIES),
     define_macros=list(DEFINE_MACROS),
     depends=glob_many(
-        'src/gevent/resolver/dnshelper.c',
         'src/gevent/resolver/cares_*.[ch]')
 )
 

docs/changes/1012.bugfix

+As part of this, certain parts of the c-ares extension were adapted to
+use modern Cython idioms.
+
+A few minor errors and discrepancies were fixed as well, such as
+``gethostbyaddr(''localhost')`` working on Python 3 and failing on
+Python 2.

src/gevent/resolver/ares.py

@@ -83,6 +83,10 @@ class Resolver(AbstractResolver):
       results, and c-ares may report more ips on a multi-homed
       host.
 
+    - The system implementation may return some names fully qualified, where
+      this implementation returns only the host name. This appears to be
+      the case only with entries found in ``/etc/hosts``.
+
     .. caution::
 
         This module is considered extremely experimental on PyPy, and
@@ -175,11 +179,16 @@ class Resolver(AbstractResolver):
         # pylint:disable=too-many-locals,too-many-branches
         if isinstance(host, text_type):
             host = host.encode('idna')
-        elif not isinstance(host, str) or (flags & AI_NUMERICHOST):
+        if not isinstance(host, bytes) or (flags & AI_NUMERICHOST) or host in (
+                b'localhost', b'ip6-localhost'):
             # this handles cases which do not require network access
             # 1) host is None
             # 2) host is of an invalid type
             # 3) AI_NUMERICHOST flag is set
+            # 4) It's a well-known alias. TODO: This is special casing that we don't
+            #    really want to do. It's here because it resolves a discrepancy with the system
+            #    resolvers caught by test cases. In gevent 20.4.0, this only worked correctly on
+            #    Python 3 and not Python 2, by accident.
             return getaddrinfo(host, port, family, socktype, proto, flags)
             # we also call _socket.getaddrinfo below if family is not one of AF_*
 

src/gevent/resolver/cares.pyx

@@ -3,14 +3,17 @@
 # seems to be buggy (at least for the `result` class) and produces code that
 # can't compile ("local variable 'result' referenced before assignment").
 # See https://github.com/cython/cython/issues/1786
-# cython: auto_pickle=False
+# cython: auto_pickle=False,language_level=3str
 cimport libcares as cares
 import sys
 
+from cpython.tuple cimport PyTuple_Check
+from cpython.getargs cimport PyArg_ParseTuple
 from cpython.ref cimport Py_INCREF
 from cpython.ref cimport Py_DECREF
 from cpython.mem cimport PyMem_Malloc
 from cpython.mem cimport PyMem_Free
+from libc.string cimport memset
 
 from _socket import gaierror
 
@@ -32,14 +35,27 @@ TIMEOUT = 1
 DEF EV_READ = 1
 DEF EV_WRITE = 2
 
+cdef extern from *:
+    """
+#ifdef CARES_EMBED
+#include "ares_setup.h"
+#endif
 
-cdef extern from "dnshelper.c":
+#ifdef HAVE_NETDB_H
+#include <netdb.h>
+#endif
+    """
+
+cdef extern from "ares.h":
     int AF_INET
     int AF_INET6
+    int INET6_ADDRSTRLEN
 
     struct hostent:
         char* h_name
         int h_addrtype
+        char** h_aliases
+        char** h_addr_list
 
     struct sockaddr_t "sockaddr":
         pass
@@ -47,21 +63,27 @@ cdef extern from "dnshelper.c":
     struct ares_channeldata:
         pass
 
-    object parse_h_name(hostent*)
-    object parse_h_aliases(hostent*)
-    object parse_h_addr_list(hostent*)
-    void* create_object_from_hostent(void*)
+    struct in_addr:
+        unsigned int s_addr
+
+    struct sockaddr_in:
+        int sin_family
+        int sin_port
+        in_addr sin_addr
+
+    struct in6_addr:
+        char s6_addr[16]
 
-    # this imports _socket lazily
-    object PyUnicode_FromString(char*)
-    int PyTuple_Check(object)
-    int PyArg_ParseTuple(object, char*, ...) except 0
     struct sockaddr_in6:
-        pass
-    int gevent_make_sockaddr(char* hostp, int port, int flowinfo, int scope_id, sockaddr_in6* sa6)
+        int sin6_family
+        int sin6_port
+        unsigned int sin6_flowinfo
+        in6_addr sin6_addr
+        unsigned int sin6_scope_id
 
 
-    void memset(void*, int, int)
+    unsigned int htons(unsigned int hostshort)
+
 
 
 ARES_SUCCESS = cares.ARES_SUCCESS
@@ -207,6 +229,43 @@ class ares_host_result(tuple):
         return (self.family, tuple(self))
 
 
+cdef list _parse_h_aliases(hostent* host):
+    cdef list result = []
+    cdef char** aliases = host.h_aliases
+
+    if not aliases or not aliases[0]:
+        return result
+
+    while aliases[0]: # *aliases
+        # The old C version of this excluded an alias if
+        # it matched the host name. I don't think the stdlib does that?
+        result.append(_as_str(aliases[0]))
+        aliases += 1
+    return result
+
+
+cdef list _parse_h_addr_list(hostent* host):
+    cdef list result = []
+    cdef char** addr_list = host.h_addr_list
+    cdef int addr_type = host.h_addrtype
+    # INET6_ADDRSTRLEN is 46, but we can't use that named constant
+    # here; cython doesn't like it.
+    cdef char tmpbuf[46]
+
+    if not addr_list or not addr_list[0]:
+        return result
+
+    while addr_list[0]:
+        if not cares.ares_inet_ntop(host.h_addrtype, addr_list[0], tmpbuf, INET6_ADDRSTRLEN):
+            import _socket
+            raise _socket.error("Failed in ares_inet_ntop")
+
+        result.append(_as_str(tmpbuf))
+        addr_list += 1
+
+    return result
+
+
 cdef void gevent_ares_host_callback(void *arg, int status, int timeouts, hostent* host):
     cdef channel channel
     cdef object callback
@@ -218,7 +277,10 @@ cdef void gevent_ares_host_callback(void *arg, int status, int timeouts, hostent
             callback(result(None, gaierror(status, strerror(status))))
         else:
             try:
-                host_result = ares_host_result(host.h_addrtype, (parse_h_name(host), parse_h_aliases(host), parse_h_addr_list(host)))
+                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:
@@ -226,6 +288,16 @@ cdef void gevent_ares_host_callback(void *arg, int status, int timeouts, hostent
     except:
         channel.loop.handle_error(callback, *sys.exc_info())
 
+from cpython.version cimport PY_MAJOR_VERSION
+
+cdef object _as_str(const char* val):
+    if not val:
+        return None
+
+    if PY_MAJOR_VERSION < 3:
+        return <bytes>val
+    return val.decode('utf-8')
+
 
 cdef void gevent_ares_nameinfo_callback(void *arg, int status, int timeouts, char *c_node, char *c_service):
     cdef channel channel
@@ -238,19 +310,30 @@ cdef void gevent_ares_nameinfo_callback(void *arg, int status, int timeouts, cha
         if status:
             callback(result(None, gaierror(status, strerror(status))))
         else:
-            if c_node:
-                node = PyUnicode_FromString(c_node)
-            else:
-                node = None
-            if c_service:
-                service = PyUnicode_FromString(c_service)
-            else:
-                service = None
+            node = _as_str(c_node)
+            service = _as_str(c_service)
             callback(result((node, service)))
     except:
         channel.loop.handle_error(callback, *sys.exc_info())
 
 
+
+cdef int _make_sockaddr(const char* hostp, int port, int flowinfo, int scope_id, sockaddr_in6* sa6):
+    if cares.ares_inet_pton(AF_INET, hostp, &(<sockaddr_in*>sa6).sin_addr.s_addr) > 0:
+        (<sockaddr_in*>sa6).sin_family = AF_INET
+        (<sockaddr_in*>sa6).sin_port = htons(port)
+        return sizeof(sockaddr_in)
+
+    if cares.ares_inet_pton(AF_INET6, hostp, &(sa6.sin6_addr).s6_addr) > 0:
+        sa6.sin6_family = AF_INET6
+        sa6.sin6_port = htons(port)
+        sa6.sin6_flowinfo = flowinfo
+        sa6.sin6_scope_id = scope_id
+        return sizeof(sockaddr_in6);
+
+    return -1;
+
+
 cdef class channel:
 
     cdef public object loop
@@ -447,7 +530,7 @@ cdef class channel:
         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)
-        cdef int length = gevent_make_sockaddr(hostp, port, flowinfo, scope_id, &sa6)
+        cdef int length = _make_sockaddr(hostp, port, flowinfo, scope_id, &sa6)
         if length <= 0:
             raise InvalidIP(repr(hostp))
         cdef object arg = (self, callback)

src/gevent/resolver/dnshelper.c

-/* Copyright (c) 2011 Denis Bilenko. See LICENSE for details. */
-#include "Python.h"
-#ifdef CARES_EMBED
-#include "ares_setup.h"
-#endif
-
-#ifdef HAVE_NETDB_H
-#include <netdb.h>
-#endif
-
-#include "ares.h"
-
-
-#if PY_MAJOR_VERSION >= 3
-#define PY3K
-#define GPyNative_FromString PyUnicode_FromString
-#else
-#define GPyNative_FromString PyString_FromString
-#endif
-
-
-static PyObject* _socket_error = 0;
-
-static PyObject*
-get_socket_object(PyObject** pobject, const char* name)
-{
-    if (!*pobject) {
-        PyObject* _socket;
-        _socket = PyImport_ImportModule("_socket");
-        if (_socket) {
-            *pobject = PyObject_GetAttrString(_socket, name);
-            if (!*pobject) {
-                PyErr_WriteUnraisable(Py_None);
-            }
-            Py_DECREF(_socket);
-        }
-        else {
-            PyErr_WriteUnraisable(Py_None);
-        }
-        if (!*pobject) {
-            *pobject = PyExc_IOError;
-        }
-    }
-    return *pobject;
-}
-
-
-static int
-gevent_append_addr(PyObject* list, int family, void* src, char* tmpbuf, size_t tmpsize) {
-    int status = -1;
-    PyObject* tmp;
-    if (ares_inet_ntop(family, src, tmpbuf, tmpsize)) {
-        tmp = GPyNative_FromString(tmpbuf);
-        if (tmp) {
-            status = PyList_Append(list, tmp);
-            Py_DECREF(tmp);
-        }
-    }
-    return status;
-}
-
-
-static PyObject*
-parse_h_name(struct hostent *h)
-{
-    return GPyNative_FromString(h->h_name);
-}
-
-
-static PyObject*
-parse_h_aliases(struct hostent *h)
-{
-    char **pch;
-    PyObject *result = NULL;
-    PyObject *tmp;
-
-    result = PyList_New(0);
-
-    if (result && h->h_aliases) {
-        for (pch = h->h_aliases; *pch != NULL; pch++) {
-            if (*pch != h->h_name && strcmp(*pch, h->h_name)) {
-                int status;
-                tmp = GPyNative_FromString(*pch);
-                if (tmp == NULL) {
-                    break;
-                }
-
-                status = PyList_Append(result, tmp);
-                Py_DECREF(tmp);
-
-                if (status) {
-                    break;
-                }
-            }
-        }
-    }
-
-    return result;
-}
-
-
-static PyObject *
-parse_h_addr_list(struct hostent *h)
-{
-    char **pch;
-    PyObject *result = NULL;
-
-    result = PyList_New(0);
-
-    if (result) {
-        switch (h->h_addrtype) {
-            case AF_INET:
-            {
-                char tmpbuf[sizeof "255.255.255.255"];
-                for (pch = h->h_addr_list; *pch != NULL; pch++) {
-                    if (gevent_append_addr(result, AF_INET, *pch, tmpbuf, sizeof(tmpbuf))) {
-                        break;
-                    }
-                }
-                break;
-            }
-            case AF_INET6:
-            {
-                char tmpbuf[sizeof("ffff:ffff:ffff:ffff:ffff:ffff:255.255.255.255")];
-                for (pch = h->h_addr_list; *pch != NULL; pch++) {
-                    if (gevent_append_addr(result, AF_INET6, *pch, tmpbuf, sizeof(tmpbuf))) {
-                        break;
-                    }
-                }
-                break;
-            }
-            default:
-                PyErr_SetString(get_socket_object(&_socket_error, "error"), "unsupported address family");
-                Py_DECREF(result);
-                result = NULL;
-        }
-    }
-
-    return result;
-}
-
-
-static int
-gevent_make_sockaddr(char* hostp, int port, int flowinfo, int scope_id, struct sockaddr_in6* sa6) {
-    if ( ares_inet_pton(AF_INET, hostp, &((struct sockaddr_in*)sa6)->sin_addr.s_addr) > 0 ) {
-        ((struct sockaddr_in*)sa6)->sin_family = AF_INET;
-        ((struct sockaddr_in*)sa6)->sin_port = htons(port);
-        return sizeof(struct sockaddr_in);
-    }
-    else if ( ares_inet_pton(AF_INET6, hostp, &sa6->sin6_addr.s6_addr) > 0 ) {
-        sa6->sin6_family = AF_INET6;
-        sa6->sin6_port = htons(port);
-        sa6->sin6_flowinfo = flowinfo;
-        sa6->sin6_scope_id = scope_id;
-        return sizeof(struct sockaddr_in6);
-    }
-    return -1;
-}

src/gevent/resolver/libcares.pxd

@@ -139,11 +139,6 @@ cdef extern from "ares.h":
         ares_addrinfo_cname *cnames
         ares_addrinfo_node  *nodes
 
-    # typedef void (*ares_addrinfo_callback)(
-    #     void *arg, int status,
-    #     int timeouts,
-    #     ares_addrinfo *result)
-
     void ares_getaddrinfo(
         void* channel,
         const char *name,
@@ -156,3 +151,4 @@ cdef extern from "ares.h":
     void ares_freeaddrinfo(ares_addrinfo *ai)
 
     int ares_inet_pton(int af, const char *src, void *dst)
+    const char* ares_inet_ntop(int af, const void *src, char *dst, ares_socklen_t size);

src/gevent/tests/test__socket_dns.py

@@ -327,13 +327,18 @@ class TestCase(greentest.TestCase):
             result.sort()
         return result
 
+    def _normalize_result_getnameinfo(self, result):
+        return result
+
+    NORMALIZE_GHBA_IGNORE_ALIAS = False
     def _normalize_result_gethostbyaddr(self, result):
         if not RESOLVER_NOT_SYSTEM:
             return result
 
-        if isinstance(result, tuple):
+        if self.NORMALIZE_GHBA_IGNORE_ALIAS and isinstance(result, tuple):
             # On some systems, a random alias is found in the aliaslist
-            # by the system resolver, but not by cares and vice versa. We deem the aliaslist
+            # by the system resolver, but not by cares and vice versa. This is *probably* only the
+            # case for localhost or things otherwise in /etc/hosts. We deem the aliaslist
             # unimportant and discard it.
             return (result[0], [], result[2])
         return result
@@ -381,9 +386,9 @@ add(TestTypeError, 25)
 
 
 class TestHostname(TestCase):
+    NORMALIZE_GHBA_IGNORE_ALIAS = True
 
-    def _normalize_result_gethostbyaddr(self, result):
-        result = TestCase._normalize_result_gethostbyaddr(self, result)
+    def _ares_normalize_name(self, result):
         if RESOLVER_ARES and isinstance(result, tuple):
             # The system resolver can return the FQDN, in the first result,
             # when given certain configurations. But c-ares
@@ -392,6 +397,16 @@ class TestHostname(TestCase):
             name = name.split('.', 1)[0]
             result = (name,) + result[1:]
         return result
+    def _normalize_result_gethostbyaddr(self, result):
+        result = TestCase._normalize_result_gethostbyaddr(self, result)
+        return self._ares_normalize_name(result)
+
+    def _normalize_result_getnameinfo(self, result):
+        result = TestCase._normalize_result_getnameinfo(self, result)
+        if PY2:
+            # Not sure why we only saw this on Python 2
+            result = self._ares_normalize_name(result)
+        return result
 
 add(
     TestHostname,
@@ -417,6 +432,7 @@ class TestLocalhost(TestCase):
             return ()
         return super(TestLocalhost, self)._normalize_result_getaddrinfo(result)
 
+    NORMALIZE_GHBA_IGNORE_ALIAS = True
     if greentest.RUNNING_ON_TRAVIS and greentest.PY2 and RESOLVER_NOT_SYSTEM:
         def _normalize_result_gethostbyaddr(self, result):
             # Beginning in November 2017 after an upgrade to Travis,
@@ -455,7 +471,7 @@ add(Test1234, '1.2.3.4')
 
 
 class Test127001(TestCase):
-    pass
+    NORMALIZE_GHBA_IGNORE_ALIAS = True
 
 add(
     Test127001, '127.0.0.1',

src/gevent/tests/test__socket_dns6.py

@@ -36,7 +36,7 @@ from gevent.testing.sysinfo import RESOLVER_DNSPYTHON
 # by default we skip the tests everywhere else.
 
 class Test6(TestCase):
-
+    NORMALIZE_GHBA_IGNORE_ALIAS = True
     # host that only has AAAA record
     host = 'aaaa.test-ipv6.com'