This PR makes the server list a dynamic sorted list of servers. The sort order is [ consecutive failures, system config index ]. The server list can be updated via ares_set_servers_*(). Any queries currently directed to servers that are no longer in the list will be automatically re-queued to a different server.
Also, any time a failure occurs on the server, the sort order of the servers will be updated so that the one with the fewest consecutive failures is chosen for the next query that goes on the wire, this way bad or non-responsive servers are automatically isolated.
Since the server list is now dynamic, the tracking of query failures per server has been removed and instead is relying on the server sort order as previously described. This simplifies the logic while also reducing the amount of memory required per query. However, because of this dynamic nature, it may not be easy to determine the server attempt order for enqueued queries if there have been any failures.
If using the ARES_OPT_ROTATE, this is now implemented to be a random selection of the configured servers. Since the server list is dynamic, its not possible to go to the next server as configuration could have changed between queries or attempts for the same query.
Finally, this PR moved some existing functions into new files to logically separate them.
This should address issues #550 and #440, while also setting the framework to implement #301. #301 needs a little more effort since it configures things other than the servers themselves (domains, search, sortlist, lookups), which need to make sure they can be safely updated.
Fix By: Brad House (@bradh352)
c-ares currently lacks modern data structures that can make coding easier and more efficient. This PR implements a new linked list, skip list (sorted linked list), and hashtable implementation that are easy to use and hard to misuse. Though these implementations use more memory allocations than the prior implementation, the ability to more rapidly iterate on the codebase is a bigger win than any marginal performance difference (which is unlikely to be visible, modern systems are much more powerful than when c-ares was initially created).
The data structure implementation favors readability and audit-ability over performance, however using the algorithmically correct data type for the purpose should offset any perceived losses.
The primary motivation for this PR is to facilitate future implementation for Issues #444, #135, #458, and possibly #301
A couple additional notes:
The ares_timeout() function is now O(1) complexity instead of O(n) due to the use of a skiplist.
Some obscure bugs were uncovered which were actually being incorrectly validated in the test cases. These have been addressed in this PR but are not explicitly discussed.
Fixed some dead code warnings in ares_rand for systems that don't need rc4
Fix By: Brad House (@bradh352)
All files have their licence and copyright information clearly
identifiable. If not in the file header, they are set separately in
.reuse/dep5.
All used license texts are provided in LICENSES/
ai_addrlen was erroneously returning 16 bytes instead of the
sizeof(struct sockaddr_in6). This is a regression introduced
in 1.18.0.
Reported by: James Brown <jbrown@easypost.com>
Fix By: Brad House (@bradh352)
ares_gethostbyname() and ares_getaddrinfo() do a lot of similar things, however ares_getaddrinfo() has some desirable behaviors that should be imported into ares_gethostbyname(). For one, it sorts the address lists for the most likely to succeed based on the current system routes. Next, when AF_UNSPEC is specified, it properly handles search lists instead of first searching all of AF_INET6 then AF_INET, since ares_gethostbyname() searches in parallel. Therefore, this PR should also resolve the issues attempted in #94.
A few things this PR does:
1. ares_parse_a_reply() and ares_parse_aaaa_reply() had very similar code to translate struct ares_addrinfo into a struct hostent as well as into struct ares_addrttl/ares_addr6ttl this has been split out into helper functions of ares__addrinfo2hostent() and ares__addrinfo2addrttl() to prevent this duplicative code.
2. ares_getaddrinfo() was apparently never honoring HOSTALIASES, and this was discovered once ares_gethostbyname() was turned into a wrapper, the affected test cases started failing.
3. A slight API modification to save the query hostname into struct ares_addrinfo as the last element of name. Since this is the last element, and all user-level instances of struct ares_addrinfo are allocated internally by c-ares, this is not an ABI-breaking change nor would it impact any API compatibility. This was needed since struct hostent has an h_name element.
4. Test Framework: MockServer tests via TCP would fail if more than 1 request was received at a time which is common when ares_getaddrinfo() queries for both A and AAAA records simultaneously. Infact, this was a long standing issue in which the ares_getaddrinfo() test were bypassing TCP alltogether. This has been corrected, the message is now processed in a loop.
5. Some tests had to be updated for overall correctness as they were invalid but somehow passing prior to this change.
Change By: Brad House (@bradh352)
EDNS retry should be based on FORMERR returned without an OPT RR record as per https://tools.ietf.org/html/rfc6891#section-7 rather than just treating any unexpected error condition as a reason to disable EDNS on the channel.
Fix By: Erik Lax (@eriklax)
If a query is performed for dynamodb.us-east-1.amazonaws.com. with ndots=5, it was attempting to search the search domains rather than just attempting the FQDN that was passed it. This patch now at least attempts the FQDN first.
We may need to determine if we should abort any further searching, however as is probably intended.
Fix by: Jonathan Maye-Hobbs (@wheelpharoah)
The rc4 function iterates over a buffer of size buffer_len who's maximum
value is INT_MAX with a counter of type short that is not guaranteed to
have maximum size INT_MAX.
In circumstances where short is narrower than int and where buffer_len
is larger than the maximum value of a short, it may be possible to loop
infinitely as counter will overflow and never be greater than or equal
to buffer_len.
The solution is to make the comparison be between types of equal width.
This commit defines counter as an int.
Fix By: Fionn Fitzmaurice (@fionn)
Brad House
Daniel Stenberg
Erik Lax
Jonathan Maye-Hobbs
Fionn Fitzmaurice