Return correct resolv_status on resolver timeout

https://fedorahosted.org/sssd/ticket/1274

4 files changed, 95 insertions, 24 deletions

diff --git a/src/providers/fail_over.c b/src/providers/fail_over.c
index b8310684..deb7d394 100644
--- a/src/providers/fail_over.c
+++ b/src/providers/fail_over.c
@@ -946,12 +946,8 @@ fo_resolve_service_done(struct tevent_req *subreq)
     /* Take care of all requests for this server. */
     while ((request = common->request_list) != NULL) {
         DLIST_REMOVE(common->request_list, request);
-        if (resolv_status) {
-            /* FIXME FIXME: resolv_status is an ARES error.
-             * but any caller will expect classic error codes.
-             * also the send() function may return ENOENT, so this mix
-             * IS explosive (ENOENT = 2 = ARES_EFORMER) */
-            tevent_req_error(request->req, resolv_status);
+        if (ret) {
+            tevent_req_error(request->req, ret);
         } else {
             tevent_req_done(request->req);
         }
@@ -1285,16 +1281,20 @@ static void resolve_get_domain_done(struct tevent_req *subreq)
                                                       struct resolve_get_domain_state);
     struct resolv_hostent *rhostent;
     int ret;
+    int resolv_status;
 
-    ret = resolv_gethostbyname_recv(subreq, req, NULL, NULL, &rhostent);
+    ret = resolv_gethostbyname_recv(subreq, req, &resolv_status,
+                                    NULL, &rhostent);
     talloc_zfree(subreq);
     if (ret) {
-        DEBUG(2, ("Could not get fully qualified name for host name %s "
-                  "resolver returned: [%d]: %s\n",
-                  state->hostname, ret, strerror(ret)));
+        DEBUG(SSSDBG_OP_FAILURE,
+              ("Could not get fully qualified name for host name %s "
+               "error [%d]: %s, resolver returned: [%d]: %s\n",
+               state->hostname, ret, strerror(ret), resolv_status,
+               resolv_strerror(resolv_status)));
         /* We'll proceed with hostname in this case */
     } else {
-        DEBUG(7, ("The full FQDN is: %s\n", rhostent->name));
+        DEBUG(SSSDBG_TRACE_LIBS, ("The full FQDN is: %s\n", rhostent->name));
         state->fqdn = rhostent->name;
     }
     tevent_req_done(req);
diff --git a/src/providers/ipa/ipa_dyndns.c b/src/providers/ipa/ipa_dyndns.c
index 089f8fba..71fc27bc 100644
--- a/src/providers/ipa/ipa_dyndns.c
+++ b/src/providers/ipa/ipa_dyndns.c
@@ -639,8 +639,9 @@ ipa_dyndns_update_get_addrs_done(struct tevent_req *subreq)
                                      struct ipa_dyndns_update_get_addrs_state);
     struct resolv_hostent *rhostent;
     int i;
+    int resolv_status;
 
-    ret = resolv_gethostbyname_recv(subreq, state, NULL, NULL,
+    ret = resolv_gethostbyname_recv(subreq, state, &resolv_status, NULL,
                                     &rhostent);
     talloc_zfree(subreq);
 
@@ -666,6 +667,10 @@ ipa_dyndns_update_get_addrs_done(struct tevent_req *subreq)
         tevent_req_done(req);
         return;
     } else if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE,
+              ("Could not resolve address for this machine, error [%d]: %s, "
+               "resolver returned: [%d]: %s\n", ret, strerror(ret),
+               resolv_status, resolv_strerror(resolv_status)));
         tevent_req_error(req, ret);
         return;
     }
diff --git a/src/resolv/async_resolv.c b/src/resolv/async_resolv.c
index 1fbd671a..ff19050b 100644
--- a/src/resolv/async_resolv.c
+++ b/src/resolv/async_resolv.c
@@ -511,7 +511,9 @@ resolv_init(TALLOC_CTX *mem_ctx, struct tevent_context *ev_ctx,
     struct resolv_ctx *ctx;
 
     if (timeout < 1) {
-        return EINVAL;
+        DEBUG(SSSDBG_MINOR_FAILURE,
+              ("The timeout is too short, DNS operations are going to fail. "
+               "This is a bug outside unit tests\n"));
     }
 
     ctx = talloc_zero(mem_ctx, struct resolv_ctx);
@@ -1389,6 +1391,9 @@ resolv_gethostbyname_done(struct tevent_req *subreq)
         /* No more databases and/or address families */
         tevent_req_error(req, ENOENT);
         return;
+    } else if (ret == ETIMEDOUT) {
+        /* In case we killed the request before c-ares answered */
+        state->status = ARES_ETIMEOUT;
     }
 
     if (ret != EOK) {
diff --git a/src/tests/resolv-tests.c b/src/tests/resolv-tests.c
index b9a33ca7..b56dc9e8 100644
--- a/src/tests/resolv-tests.c
+++ b/src/tests/resolv-tests.c
@@ -37,6 +37,8 @@
 /* Interface under test */
 #include "resolv/async_resolv.h"
 
+#define RESOLV_DEFAULT_TIMEOUT 5
+
 static int use_net_test;
 static char *txt_host;
 static char *srv_host;
@@ -55,7 +57,7 @@ struct resolv_test_ctx {
     bool done;
 };
 
-static int setup_resolv_test(struct resolv_test_ctx **ctx)
+static int setup_resolv_test(int timeout, struct resolv_test_ctx **ctx)
 {
     struct resolv_test_ctx *test_ctx;
     int ret;
@@ -73,7 +75,7 @@ static int setup_resolv_test(struct resolv_test_ctx **ctx)
         return EFAULT;
     }
 
-    ret = resolv_init(test_ctx, test_ctx->ev, 5, &test_ctx->resolv);
+    ret = resolv_init(test_ctx, test_ctx->ev, timeout, &test_ctx->resolv);
     if (ret != EOK) {
         fail("Could not init resolv context");
         talloc_free(test_ctx);
@@ -194,7 +196,7 @@ START_TEST(test_resolv_ip_addr)
     struct tevent_req *req;
     const char *hostname = "127.0.0.1";
 
-    ret = setup_resolv_test(&test_ctx);
+    ret = setup_resolv_test(RESOLV_DEFAULT_TIMEOUT, &test_ctx);
     if (ret != EOK) {
         fail("Could not set up test");
         return;
@@ -263,7 +265,7 @@ START_TEST(test_resolv_localhost)
     struct tevent_req *req;
     const char *hostname = "localhost.localdomain";
 
-    ret = setup_resolv_test(&test_ctx);
+    ret = setup_resolv_test(RESOLV_DEFAULT_TIMEOUT, &test_ctx);
     if (ret != EOK) {
         fail("Could not set up test");
         return;
@@ -319,7 +321,7 @@ START_TEST(test_resolv_negative)
     const char *hostname = "sssd.foo";
     struct resolv_test_ctx *test_ctx;
 
-    ret = setup_resolv_test(&test_ctx);
+    ret = setup_resolv_test(RESOLV_DEFAULT_TIMEOUT, &test_ctx);
     if (ret != EOK) {
         fail("Could not set up test");
         return;
@@ -424,7 +426,7 @@ START_TEST(test_resolv_internet)
     const char *hostname = "redhat.com";
     struct resolv_test_ctx *test_ctx;
 
-    ret = setup_resolv_test(&test_ctx);
+    ret = setup_resolv_test(RESOLV_DEFAULT_TIMEOUT, &test_ctx);
     if (ret != EOK) {
         fail("Could not set up test");
         return;
@@ -457,7 +459,7 @@ START_TEST(test_resolv_internet_txt)
     struct tevent_req *req;
     struct resolv_test_ctx *test_ctx;
 
-    ret = setup_resolv_test(&test_ctx);
+    ret = setup_resolv_test(RESOLV_DEFAULT_TIMEOUT, &test_ctx);
     fail_if(ret != EOK, "Could not set up test");
     test_ctx->tested_function = TESTING_TXT;
 
@@ -482,7 +484,7 @@ START_TEST(test_resolv_internet_srv)
     struct tevent_req *req;
     struct resolv_test_ctx *test_ctx;
 
-    ret = setup_resolv_test(&test_ctx);
+    ret = setup_resolv_test(RESOLV_DEFAULT_TIMEOUT, &test_ctx);
     fail_if(ret != EOK, "Could not set up test");
     test_ctx->tested_function = TESTING_SRV;
 
@@ -531,7 +533,7 @@ START_TEST(test_resolv_free_context)
     struct tevent_timer *free_timer, *terminate_timer;
     struct timeval free_tv, terminate_tv;
 
-    ret = setup_resolv_test(&test_ctx);
+    ret = setup_resolv_test(RESOLV_DEFAULT_TIMEOUT, &test_ctx);
     if (ret != EOK) {
         fail("Could not set up test");
         return;
@@ -591,7 +593,7 @@ START_TEST(test_resolv_sort_srv_reply)
     int num_replies = 3;
     int i;
 
-    ret = setup_resolv_test(&test_ctx);
+    ret = setup_resolv_test(RESOLV_DEFAULT_TIMEOUT, &test_ctx);
     if (ret != EOK) {
         fail("Could not set up test");
         return;
@@ -678,7 +680,7 @@ START_TEST(test_resolv_free_req)
     struct tevent_timer *free_timer, *terminate_timer;
     struct timeval free_tv, terminate_tv;
 
-    ret = setup_resolv_test(&test_ctx);
+    ret = setup_resolv_test(RESOLV_DEFAULT_TIMEOUT, &test_ctx);
     if (ret != EOK) {
         fail("Could not set up test");
         return;
@@ -721,6 +723,64 @@ done:
 }
 END_TEST
 
+static void test_timeout(struct tevent_req *req)
+{
+    int recv_status;
+    int status;
+    struct resolv_test_ctx *test_ctx;
+    TALLOC_CTX *tmp_ctx;
+    struct resolv_hostent *rhostent = NULL;
+
+    test_ctx = tevent_req_callback_data(req, struct resolv_test_ctx);
+
+    test_ctx->done = true;
+
+    tmp_ctx = talloc_new(test_ctx);
+    check_leaks_push(tmp_ctx);
+
+    fail_unless(test_ctx->tested_function == TESTING_HOSTNAME);
+    recv_status = resolv_gethostbyname_recv(req, tmp_ctx,
+                                            &status, NULL, &rhostent);
+    talloc_zfree(req);
+    fail_unless(recv_status == ETIMEDOUT);
+    fail_unless(status == ARES_ETIMEOUT);
+    check_leaks_pop(tmp_ctx);
+    talloc_free(tmp_ctx);
+}
+
+START_TEST(test_resolv_timeout)
+{
+    struct resolv_test_ctx *test_ctx;
+    errno_t ret;
+    struct tevent_req *req;
+    const char *hostname = "redhat.com";
+
+    ret = setup_resolv_test(0, &test_ctx);
+    if (ret != EOK) {
+        fail("Could not set up test");
+        return;
+    }
+
+    test_ctx->tested_function = TESTING_HOSTNAME;
+
+    req = resolv_gethostbyname_send(test_ctx, test_ctx->ev,
+                                    test_ctx->resolv, hostname, IPV4_FIRST,
+                                    default_host_dbs);
+    DEBUG(7, ("Sent resolv_gethostbyname\n"));
+    if (req == NULL) {
+        ret = ENOMEM;
+    }
+
+    if (ret == EOK) {
+        tevent_req_set_callback(req, test_timeout, test_ctx);
+        ret = test_loop(test_ctx);
+    }
+
+    fail_unless(ret == EOK);
+    talloc_zfree(test_ctx);
+}
+END_TEST
+
 Suite *create_resolv_suite(void)
 {
     Suite *s = suite_create("resolv");
@@ -736,6 +796,7 @@ Suite *create_resolv_suite(void)
         tcase_add_test(tc_resolv, test_resolv_internet);
         tcase_add_test(tc_resolv, test_resolv_negative);
         tcase_add_test(tc_resolv, test_resolv_localhost);
+        tcase_add_test(tc_resolv, test_resolv_timeout);
         if (txt_host != NULL) {
             tcase_add_test(tc_resolv, test_resolv_internet_txt);
         }