diff --git a/docs/ares_set_servers.3 b/docs/ares_set_servers.3 index ac94416a..e2817e72 100644 --- a/docs/ares_set_servers.3 +++ b/docs/ares_set_servers.3 @@ -42,7 +42,7 @@ The name server linked list pointer argument may be the result of a previous call to \fBares_get_servers(3)\fP or a linked list of \fBares_addr_node\fP structs set up by other means. .PP -The \fBares_set_servers(3)\fP function also allows the specification of UDP and +The \fBares_set_servers_ports(3)\fP function also allows the specification of UDP and TCP ports to be used for communication on a per-server basis. The provided linked list argument may be the result of a previous call to \fBares_get_servers_ports(3)\fP or a linked list of \fBares_addr_port_node\fP structs @@ -51,7 +51,9 @@ set up by other means. This function replaces any potentially previously configured name servers with the ones given in the linked list. So, in order to configure a channel with more than one name server all the desired ones must be specified in a -single list. +single list. Though not recommended, passing NULL will clear all configured +servers and make an inoperable channel, this may be advantageous for test +simulation but unlikely to be useful in production. .PP The function does not take ownership of the linked list argument. The caller is responsible for freeing the linked list when no longer needed. diff --git a/docs/ares_set_servers_csv.3 b/docs/ares_set_servers_csv.3 index 0db15969..44ea72b7 100644 --- a/docs/ares_set_servers_csv.3 +++ b/docs/ares_set_servers_csv.3 @@ -30,7 +30,13 @@ int ares_set_servers_ports_csv(ares_channel_t *\fIchannel\fP, const char* \fIser The \fBares_set_servers_csv\fP and \fBares_set_servers_ports_csv\fPfunctions set the list of DNS servers that ARES will query. As of v1.22.0 this function can be called on an active channel with running queries, previously it would return -ARES_ENOTIMP. The format of the servers option is: +ARES_ENOTIMP. + +Though not recommended, passing NULL for servers will clear all configured +servers and make an inoperable channel, this may be advantageous for test +simulation but unlikely to be useful in production. + +The format of the servers option is: host[:port][,host[:port]]... diff --git a/src/lib/ares_options.c b/src/lib/ares_options.c index 312fa030..de1050c8 100644 --- a/src/lib/ares_options.c +++ b/src/lib/ares_options.c @@ -234,12 +234,12 @@ static ares_status_t ares__init_options_servers(ares_channel_t *channel, const struct in_addr *servers, size_t nservers) { - ares__llist_t *slist; + ares__llist_t *slist = NULL; ares_status_t status; - slist = ares_in_addr_to_server_config_llist(servers, nservers); - if (slist == NULL) { - return ARES_ENOMEM; + status = ares_in_addr_to_server_config_llist(servers, nservers, &slist); + if (status != ARES_SUCCESS) { + return status; } status = ares__servers_update(channel, slist, ARES_TRUE); diff --git a/src/lib/ares_private.h b/src/lib/ares_private.h index 0fcfe886..14931ca5 100644 --- a/src/lib/ares_private.h +++ b/src/lib/ares_private.h @@ -477,9 +477,10 @@ ares_status_t ares__sconfig_append(ares__llist_t **sconfig, unsigned short tcp_port); ares_status_t ares__sconfig_append_fromstr(ares__llist_t **sconfig, const char *str); -ares__llist_t * +ares_status_t ares_in_addr_to_server_config_llist(const struct in_addr *servers, - size_t nservers); + size_t nservers, + ares__llist_t **llist); struct ares_hosts_entry; typedef struct ares_hosts_entry ares_hosts_entry_t; diff --git a/src/lib/ares_process.c b/src/lib/ares_process.c index 2f959b9f..0c34d7f1 100644 --- a/src/lib/ares_process.c +++ b/src/lib/ares_process.c @@ -777,7 +777,7 @@ static struct server_state *ares__random_server(ares_channel_t *channel) /* Silence coverity, not possible */ if (num_servers == 0) - num_servers = 1; + return NULL; ares__rand_bytes(channel->rand_state, &c, 1); @@ -816,9 +816,8 @@ static size_t ares__calc_query_timeout(const struct query *query) size_t rounds; size_t num_servers = ares__slist_len(channel->servers); - /* Silence coverity, not possible */ if (num_servers == 0) - num_servers = 1; + return 0; /* For each trip through the entire server list, we want to double the * retry from the last retry */ @@ -878,6 +877,11 @@ ares_status_t ares__send_query(struct query *query, struct timeval *now) server = ares__slist_first_val(channel->servers); } + if (server == NULL) { + end_query(channel, query, ARES_ESERVFAIL /* ? */, NULL, 0); + return ARES_ECONNREFUSED; + } + if (query->using_tcp) { size_t prior_len = 0; /* Make sure the TCP socket for this server is set up and queue diff --git a/src/lib/ares_update_servers.c b/src/lib/ares_update_servers.c index 2937c727..9075b736 100644 --- a/src/lib/ares_update_servers.c +++ b/src/lib/ares_update_servers.c @@ -531,11 +531,14 @@ ares_status_t ares__servers_update(ares_channel_t *channel, size_t idx = 0; ares_status_t status; - if (channel == NULL || server_list == NULL || - ares__llist_len(server_list) == 0) { + if (channel == NULL) { return ARES_EFORMERR; } + /* NOTE: a NULL or zero entry server list is considered valid due to + * real-world people needing support for this for their test harnesses + */ + /* Add new entries */ for (node = ares__llist_node_first(server_list); node != NULL; node = ares__llist_node_next(node)) { @@ -589,19 +592,18 @@ done: return status; } -static ares__llist_t * - ares_addr_node_to_server_config_llist(const struct ares_addr_node *servers) +static ares_status_t + ares_addr_node_to_server_config_llist(const struct ares_addr_node *servers, + ares__llist_t **llist) { const struct ares_addr_node *node; ares__llist_t *s; - if (servers == NULL) { - return NULL; - } + *llist = NULL; s = ares__llist_create(ares_free); if (s == NULL) { - return NULL; + goto fail; } for (node = servers; node != NULL; node = node->next) { @@ -632,26 +634,26 @@ static ares__llist_t * } } - return s; + *llist = s; + return ARES_SUCCESS; fail: ares__llist_destroy(s); - return NULL; + return ARES_ENOMEM; } -static ares__llist_t *ares_addr_port_node_to_server_config_llist( - const struct ares_addr_port_node *servers) +static ares_status_t ares_addr_port_node_to_server_config_llist( + const struct ares_addr_port_node *servers, + ares__llist_t **llist) { const struct ares_addr_port_node *node; ares__llist_t *s; - if (servers == NULL) { - return NULL; - } + *llist = NULL; s = ares__llist_create(ares_free); if (s == NULL) { - return NULL; + goto fail; } for (node = servers; node != NULL; node = node->next) { @@ -685,30 +687,30 @@ static ares__llist_t *ares_addr_port_node_to_server_config_llist( } } - return s; + *llist = s; + return ARES_SUCCESS; fail: ares__llist_destroy(s); - return NULL; + return ARES_ENOMEM; } -ares__llist_t * +ares_status_t ares_in_addr_to_server_config_llist(const struct in_addr *servers, - size_t nservers) + size_t nservers, + ares__llist_t **llist) { size_t i; ares__llist_t *s; - if (servers == NULL || nservers == 0) { - return NULL; - } + *llist = NULL; s = ares__llist_create(ares_free); if (s == NULL) { - return NULL; + goto fail; } - for (i = 0; i < nservers; i++) { + for (i = 0; servers != NULL && i < nservers; i++) { ares_sconfig_t *sconfig; sconfig = ares_malloc_zero(sizeof(*sconfig)); @@ -725,11 +727,12 @@ ares__llist_t * } } - return s; + *llist = s; + return ARES_SUCCESS; fail: ares__llist_destroy(s); - return NULL; + return ARES_ENOMEM; } int ares_get_servers(ares_channel_t *channel, struct ares_addr_node **servers) @@ -842,13 +845,13 @@ int ares_set_servers(ares_channel_t *channel, ares__llist_t *slist; ares_status_t status; - if (channel == NULL || servers == NULL) { + if (channel == NULL) { return ARES_ENODATA; } - slist = ares_addr_node_to_server_config_llist(servers); - if (slist == NULL) { - return ARES_ENOMEM; + status = ares_addr_node_to_server_config_llist(servers, &slist); + if (status != ARES_SUCCESS) { + return (int)status; } status = ares__servers_update(channel, slist, ARES_TRUE); @@ -864,13 +867,13 @@ int ares_set_servers_ports(ares_channel_t *channel, ares__llist_t *slist; ares_status_t status; - if (channel == NULL || servers == NULL) { + if (channel == NULL) { return ARES_ENODATA; } - slist = ares_addr_port_node_to_server_config_llist(servers); - if (slist == NULL) { - return ARES_ENOMEM; + status = ares_addr_port_node_to_server_config_llist(servers, &slist); + if (status != ARES_SUCCESS) { + return (int)status; } status = ares__servers_update(channel, slist, ARES_TRUE); @@ -904,7 +907,8 @@ static ares_status_t set_servers_csv(ares_channel_t *channel, const char *_csv, i = ares_strlen(_csv); if (i == 0) { - return ARES_SUCCESS; /* blank all servers */ + /* blank all servers */ + return (ares_status_t)ares_set_servers_ports(channel, NULL); } csv = ares_malloc(i + 2); diff --git a/test/ares-test-misc.cc b/test/ares-test-misc.cc index 4016a8e0..28458f21 100644 --- a/test/ares-test-misc.cc +++ b/test/ares-test-misc.cc @@ -45,7 +45,21 @@ TEST_F(DefaultChannelTest, GetServersFailures) { } TEST_F(DefaultChannelTest, SetServers) { - EXPECT_EQ(ARES_ENODATA, ares_set_servers(channel_, nullptr)); + /* NOTE: This test is because we have actual external users doing test case + * simulation and removing all servers to generate various error + * conditions in their own code. It would make more sense to return + * ARES_ENODATA, but due to historical users, we can't break them. + * See: https://github.com/nodejs/node/pull/50800 + */ + EXPECT_EQ(ARES_SUCCESS, ares_set_servers(channel_, nullptr)); + std::vector expected_empty = { }; + EXPECT_EQ(expected_empty, GetNameServers(channel_)); + HostResult result; + ares_gethostbyname(channel_, "www.google.com.", AF_INET, HostCallback, &result); + Process(); + EXPECT_TRUE(result.done_); + EXPECT_EQ(ARES_ESERVFAIL, result.status_); + struct ares_addr_node server1; struct ares_addr_node server2; @@ -63,7 +77,15 @@ TEST_F(DefaultChannelTest, SetServers) { } TEST_F(DefaultChannelTest, SetServersPorts) { - EXPECT_EQ(ARES_ENODATA, ares_set_servers_ports(channel_, nullptr)); + /* NOTE: This test is because we have actual external users doing test case + * simulation and removing all servers to generate various error + * conditions in their own code. It would make more sense to return + * ARES_ENODATA, but due to historical users, we can't break them. + * See: https://github.com/nodejs/node/pull/50800 + */ + EXPECT_EQ(ARES_SUCCESS, ares_set_servers_ports(channel_, nullptr)); + std::vector expected_empty = { }; + EXPECT_EQ(expected_empty, GetNameServers(channel_)); struct ares_addr_port_node server1; struct ares_addr_port_node server2; @@ -91,6 +113,21 @@ TEST_F(DefaultChannelTest, SetServersCSV) { EXPECT_EQ(ARES_ENODATA, ares_set_servers_csv(nullptr, "1.2.3.4.5")); EXPECT_EQ(ARES_ENODATA, ares_set_servers_csv(nullptr, "1:2:3:4:5")); + /* NOTE: This test is because we have actual external users doing test case + * simulation and removing all servers to generate various error + * conditions in their own code. It would make more sense to return + * ARES_ENODATA, but due to historical users, we can't break them. + * See: https://github.com/nodejs/node/pull/50800 + */ + EXPECT_EQ(ARES_SUCCESS, ares_set_servers_csv(channel_, NULL)); + std::vector expected_empty = { }; + EXPECT_EQ(expected_empty, GetNameServers(channel_)); + EXPECT_EQ(ARES_SUCCESS, ares_set_servers_csv(channel_, "")); + EXPECT_EQ(expected_empty, GetNameServers(channel_)); + + + + EXPECT_EQ(ARES_SUCCESS, ares_set_servers_csv(channel_, "1.2.3.4,0102:0304:0506:0708:0910:1112:1314:1516,2.3.4.5")); std::vector expected = {"1.2.3.4:53", "[0102:0304:0506:0708:0910:1112:1314:1516]:53", "2.3.4.5:53"};