From 4d2dd86647fb15fc49066ef46085def1154bac8f Mon Sep 17 00:00:00 2001 From: Jakub Hrozek Date: Tue, 27 Mar 2012 18:00:18 -0400 Subject: [PATCH] Return correct resolv_status on resolver timeout https://fedorahosted.org/sssd/ticket/1274 --- src/providers/fail_over.c | 22 +++++----- src/providers/ipa/ipa_dyndns.c | 7 +++- src/resolv/async_resolv.c | 7 +++- src/tests/resolv-tests.c | 83 ++++++++++++++++++++++++++++++++++----- 4 files changed, 95 insertions(+), 24 deletions(-) diff --git a/src/providers/fail_over.c b/src/providers/fail_over.c index b83106840d0ca3af45cf28a19dd1df8f66da455f..deb7d394cede1802f627aa3f09e755a5a6fdaf6d 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 089f8fba8450e211b089b584d9fa67d59c33a40b..71fc27bcb23b20e444ba9d3d7fcdc1c181ed7875 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 1fbd671a99a7d4c8151681f06f56f63a56145bbf..ff19050b01291fd4251bc4490a3d955840a285e5 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 b9a33ca75661d2582fff0c6c125c547756395aa6..b56dc9e872364cdb56a4191d204e5d6c5ba5ccf2 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); } -- 1.7.7.6