From 7584c93b287d072ab2283b87067388a7b9feae4c Mon Sep 17 00:00:00 2001 From: Brad House Date: Fri, 19 Jul 2024 06:59:24 -0400 Subject: [PATCH] propagate actual error condition on requeue --- src/lib/ares__close_sockets.c | 14 +++++---- src/lib/ares_private.h | 6 ++-- src/lib/ares_process.c | 54 +++++++++++++++++++---------------- test/ares-test-mock-ai.cc | 2 +- test/ares-test-mock-et.cc | 2 +- test/ares-test-mock.cc | 2 +- 6 files changed, 44 insertions(+), 36 deletions(-) diff --git a/src/lib/ares__close_sockets.c b/src/lib/ares__close_sockets.c index 3aba9f64..9d81f593 100644 --- a/src/lib/ares__close_sockets.c +++ b/src/lib/ares__close_sockets.c @@ -31,17 +31,19 @@ #include "ares_private.h" #include -static void ares__requeue_queries(struct server_connection *conn) +static void ares__requeue_queries(struct server_connection *conn, + ares_status_t requeue_status) { struct query *query; struct timeval now = ares__tvnow(); while ((query = ares__llist_first_val(conn->queries_to_conn)) != NULL) { - ares__requeue_query(query, &now); + ares__requeue_query(query, &now, requeue_status); } } -void ares__close_connection(struct server_connection *conn) +void ares__close_connection(struct server_connection *conn, + ares_status_t requeue_status) { struct server_state *server = conn->server; ares_channel_t *channel = server->channel; @@ -59,7 +61,7 @@ void ares__close_connection(struct server_connection *conn) } /* Requeue queries to other connections */ - ares__requeue_queries(conn); + ares__requeue_queries(conn, requeue_status); ares__llist_destroy(conn->queries_to_conn); @@ -75,7 +77,7 @@ void ares__close_sockets(struct server_state *server) while ((node = ares__llist_node_first(server->connections)) != NULL) { struct server_connection *conn = ares__llist_node_val(node); - ares__close_connection(conn); + ares__close_connection(conn, ARES_SUCCESS); } } @@ -107,5 +109,5 @@ void ares__check_cleanup_conn(const ares_channel_t *channel, return; } - ares__close_connection(conn); + ares__close_connection(conn, ARES_SUCCESS); } diff --git a/src/lib/ares_private.h b/src/lib/ares_private.h index c8994632..ac9a7878 100644 --- a/src/lib/ares_private.h +++ b/src/lib/ares_private.h @@ -334,7 +334,8 @@ ares_bool_t ares__timedout(const struct timeval *now, /* Returns one of the normal ares status codes like ARES_SUCCESS */ ares_status_t ares__send_query(struct query *query, struct timeval *now); -ares_status_t ares__requeue_query(struct query *query, struct timeval *now); +ares_status_t ares__requeue_query(struct query *query, struct timeval *now, + ares_status_t status); /*! Retrieve a list of names to use for searching. The first successful * query in the list wins. This function also uses the HOSTSALIASES file @@ -362,7 +363,8 @@ void *ares__dnsrec_convert_arg(ares_callback callback, void *arg); void ares__dnsrec_convert_cb(void *arg, ares_status_t status, size_t timeouts, const ares_dns_record_t *dnsrec); -void ares__close_connection(struct server_connection *conn); +void ares__close_connection(struct server_connection *conn, + ares_status_t requeue_status); void ares__close_sockets(struct server_state *server); void ares__check_cleanup_conn(const ares_channel_t *channel, struct server_connection *conn); diff --git a/src/lib/ares_process.c b/src/lib/ares_process.c index b9705ae8..6c818e39 100644 --- a/src/lib/ares_process.c +++ b/src/lib/ares_process.c @@ -61,7 +61,8 @@ static ares_status_t process_answer(ares_channel_t *channel, struct server_connection *conn, ares_bool_t tcp, struct timeval *now); static void handle_conn_error(struct server_connection *conn, - ares_bool_t critical_failure); + ares_bool_t critical_failure, + ares_status_t failure_status); static ares_bool_t same_questions(const ares_dns_record_t *qrec, const ares_dns_record_t *arec); @@ -252,7 +253,7 @@ static void write_tcp_data(ares_channel_t *channel, fd_set *write_fds, count = ares__socket_write(channel, server->tcp_conn->fd, data, data_len); if (count <= 0) { if (!try_again(SOCKERRNO)) { - handle_conn_error(server->tcp_conn, ARES_TRUE); + handle_conn_error(server->tcp_conn, ARES_TRUE, ARES_ECONNREFUSED); } continue; } @@ -284,7 +285,7 @@ static void read_tcp_data(ares_channel_t *channel, ptr = ares__buf_append_start(server->tcp_parser, &ptr_len); if (ptr == NULL) { - handle_conn_error(conn, ARES_FALSE /* not critical to connection */); + handle_conn_error(conn, ARES_FALSE /* not critical to connection */, ARES_SUCCESS); return; /* bail out on malloc failure. TODO: make this function return error codes */ } @@ -294,7 +295,7 @@ static void read_tcp_data(ares_channel_t *channel, if (count <= 0) { ares__buf_append_finish(server->tcp_parser, 0); if (!(count == -1 && try_again(SOCKERRNO))) { - handle_conn_error(conn, ARES_TRUE); + handle_conn_error(conn, ARES_TRUE, ARES_ECONNREFUSED); } return; } @@ -338,7 +339,7 @@ static void read_tcp_data(ares_channel_t *channel, /* We finished reading this answer; process it */ status = process_answer(channel, data, data_len, conn, ARES_TRUE, now); if (status != ARES_SUCCESS) { - handle_conn_error(conn, ARES_TRUE); + handle_conn_error(conn, ARES_TRUE, status); return; } @@ -453,7 +454,7 @@ static void read_udp_packets_fd(ares_channel_t *channel, break; } - handle_conn_error(conn, ARES_TRUE); + handle_conn_error(conn, ARES_TRUE, ARES_ECONNREFUSED); return; #ifdef HAVE_RECVFROM } else if (!same_address(&from.sa, &conn->server->addr)) { @@ -557,12 +558,11 @@ static void process_timeouts(ares_channel_t *channel, struct timeval *now) break; } - query->error_status = ARES_ETIMEOUT; query->timeouts++; conn = query->conn; server_increment_failures(conn->server); - ares__requeue_query(query, now); + ares__requeue_query(query, now, ARES_ETIMEOUT); ares__check_cleanup_conn(channel, conn); node = next; @@ -704,20 +704,20 @@ static ares_status_t process_answer(ares_channel_t *channel, rcode == ARES_RCODE_REFUSED) { switch (rcode) { case ARES_RCODE_SERVFAIL: - query->error_status = ARES_ESERVFAIL; + status = ARES_ESERVFAIL; break; case ARES_RCODE_NOTIMP: - query->error_status = ARES_ENOTIMP; + status = ARES_ENOTIMP; break; case ARES_RCODE_REFUSED: - query->error_status = ARES_EREFUSED; + status = ARES_EREFUSED; break; default: break; } server_increment_failures(server); - ares__requeue_query(query, now); + ares__requeue_query(query, now, status); /* Should any of these cause a connection termination? * Maybe SERVER_FAILURE? */ @@ -748,7 +748,8 @@ cleanup: } static void handle_conn_error(struct server_connection *conn, - ares_bool_t critical_failure) + ares_bool_t critical_failure, + ares_status_t failure_status) { struct server_state *server = conn->server; @@ -759,16 +760,20 @@ static void handle_conn_error(struct server_connection *conn, } /* This will requeue any connections automatically */ - ares__close_connection(conn); + ares__close_connection(conn, failure_status); } -ares_status_t ares__requeue_query(struct query *query, struct timeval *now) +ares_status_t ares__requeue_query(struct query *query, struct timeval *now, + ares_status_t status) { ares_channel_t *channel = query->channel; size_t max_tries = ares__slist_len(channel->servers) * channel->tries; - query->try_count++; + if (status != ARES_SUCCESS) { + query->error_status = status; + } + query->try_count++; if (query->try_count < max_tries && !query->no_retries) { return ares__send_query(query, now); } @@ -919,8 +924,7 @@ ares_status_t ares__send_query(struct query *query, struct timeval *now) case ARES_ECONNREFUSED: case ARES_EBADFAMILY: server_increment_failures(server); - query->error_status = status; - return ares__requeue_query(query, now); + return ares__requeue_query(query, now, status); /* Anything else is not retryable, likely ENOMEM */ default: @@ -940,7 +944,7 @@ ares_status_t ares__send_query(struct query *query, struct timeval *now) /* Only safe to kill connection if it was new, otherwise it should be * cleaned up by another process later */ if (new_connection) { - ares__close_connection(conn); + ares__close_connection(conn, status); } return status; } @@ -978,8 +982,7 @@ ares_status_t ares__send_query(struct query *query, struct timeval *now) case ARES_ECONNREFUSED: case ARES_EBADFAMILY: server_increment_failures(server); - query->error_status = status; - return ares__requeue_query(query, now); + return ares__requeue_query(query, now, status); /* Anything else is not retryable, likely ENOMEM */ default: @@ -992,13 +995,14 @@ ares_status_t ares__send_query(struct query *query, struct timeval *now) conn = ares__llist_node_val(node); if (ares__socket_write(channel, conn->fd, query->qbuf, query->qlen) == -1) { /* FIXME: Handle EAGAIN here since it likely can happen. */ + status = ARES_ESERVFAIL; server_increment_failures(server); - status = ares__requeue_query(query, now); + status = ares__requeue_query(query, now, status); /* Only safe to kill connection if it was new, otherwise it should be * cleaned up by another process later */ if (new_connection) { - ares__close_connection(conn); + ares__close_connection(conn, status); } return status; @@ -1019,7 +1023,7 @@ ares_status_t ares__send_query(struct query *query, struct timeval *now) /* Only safe to kill connection if it was new, otherwise it should be * cleaned up by another process later */ if (new_connection) { - ares__close_connection(conn); + ares__close_connection(conn, ARES_SUCCESS); } return ARES_ENOMEM; } @@ -1035,7 +1039,7 @@ ares_status_t ares__send_query(struct query *query, struct timeval *now) /* Only safe to kill connection if it was new, otherwise it should be * cleaned up by another process later */ if (new_connection) { - ares__close_connection(conn); + ares__close_connection(conn, ARES_SUCCESS); } return ARES_ENOMEM; } diff --git a/test/ares-test-mock-ai.cc b/test/ares-test-mock-ai.cc index 02f64a48..b996fa4b 100644 --- a/test/ares-test-mock-ai.cc +++ b/test/ares-test-mock-ai.cc @@ -165,7 +165,7 @@ TEST_P(MockTCPChannelTestAI, MalformedResponse) { ares_getaddrinfo(channel_, "www.google.com.", NULL, &hints, AddrInfoCallback, &result); Process(); EXPECT_TRUE(result.done_); - EXPECT_EQ(ARES_ETIMEOUT, result.status_); + EXPECT_EQ(ARES_EBADRESP, result.status_); } TEST_P(MockTCPChannelTestAI, FormErrResponse) { diff --git a/test/ares-test-mock-et.cc b/test/ares-test-mock-et.cc index 2c57c4ae..dd605d84 100644 --- a/test/ares-test-mock-et.cc +++ b/test/ares-test-mock-et.cc @@ -366,7 +366,7 @@ TEST_P(MockTCPEventThreadTest, MalformedResponse) { ares_gethostbyname(channel_, "www.google.com.", AF_INET, HostCallback, &result); Process(); EXPECT_TRUE(result.done_); - EXPECT_EQ(ARES_ETIMEOUT, result.status_); + EXPECT_EQ(ARES_EBADRESP, result.status_); } TEST_P(MockTCPEventThreadTest, FormErrResponse) { diff --git a/test/ares-test-mock.cc b/test/ares-test-mock.cc index e344154e..34641698 100644 --- a/test/ares-test-mock.cc +++ b/test/ares-test-mock.cc @@ -387,7 +387,7 @@ TEST_P(MockTCPChannelTest, MalformedResponse) { ares_gethostbyname(channel_, "www.google.com.", AF_INET, HostCallback, &result); Process(); EXPECT_TRUE(result.done_); - EXPECT_EQ(ARES_ETIMEOUT, result.status_); + EXPECT_EQ(ARES_EBADRESP, result.status_); } TEST_P(MockTCPChannelTest, FormErrResponse) {