Motiviation:
We incorrectly did ignore NS servers during redirect which had no ADDITIONAL record. This could at worse have the affect that we failed the query completely as none of the NS servers had a ADDITIONAL record. Beside this using a DnsCache to cache authoritative nameservers does not work in practise as we we need different features and semantics when cache these servers (for example we also want to cache unresolved nameservers and resolve these on the fly when needed).
Modifications:
- Correctly take NS records into account that have no matching ADDITIONAL record
- Correctly handle multiple ADDITIONAL records for the same NS record
- Introduce AuthoritativeDnsServerCache as a replacement of the DnsCache when caching authoritative nameservers + adding default implementation
- Add an adapter layer to reduce API breakage as much as possible
- Replace DnsNameResolver.uncachedRedirectDnsServerStream(...) with newRedirectDnsServerStream(...)
- Add unit tests
Result:
Our DnsResolver now correctly handle redirects in all cases.
Motivation:
We are currently always remove all entries from the cache for a hostname if the lowest TTL was reached but schedule one for each of the cached entries. This is wasteful.
Modifications:
- Reimplement logic to schedule TTL to only schedule a new removal task if the requested TTL was actual lower then the one for the already scheduled task.
- Ensure we only remove from the internal map if we did not replace the Entries in the meantime.
Result:
Less overhead in terms of scheduled tasks for the DefaultDnsCache
Motivation:
We should ensure we return the same cached entries for the hostname and hostname ending with dot. Beside this we also should use it for the searchdomains as well.
Modifications:
- Internally always use hostname with a dot as a key and so ensure we correctly handle it in the cache.
- Also query the cache for each searchdomain
- Add unit tests
Result:
Use the same cached entries for hostname with and without trailing dot. Query the cache for each searchdomain query as well
Motivation:
55fec94592 fixed a bug where we did not correctly clear all caches when the resolver was closed but did not add a testcase.
Modifications:
Add testcase.
Result:
More tests.
Motivation:
DnsNameResolver manages search domains and will retry the request with the different search domains provided to it. However if the query results in an invalid hostname, the Future corresponding to the resolve request will never be completed.
Modifications:
- If a resolve attempt results in an invalid hostname and the query isn't issued we should fail the associated promise
Result:
No more hang from DnsNameResolver if search domain results in invalid hostname.
Motivation:
At the moment we only clear the resolveCache when the Channel is closed. We should also do the same for the authoritativeDnsServerCache.
Modifications:
Add authoritativeDnsServerCache.clear() to the Channel closeFuture.
Result:
Correctly clear all caches.
Motivation:
We did not handle the case when the query was cancelled which could lead to an exhausted id space. Beside this we did not not cancel the timeout on failed promises.
Modifications:
- Do the removal of the id from the manager in a FutureListener so its handled in all cases.
- Cancel the timeout whenever the original promise was full-filled.
Result:
Fixes https://github.com/netty/netty/issues/8013.
Motivation:
Whenever we fail the query we should also remove the id from the DnsQueryContextManager.
Modifications:
Remove the id from the DnsQueryContextManager if we fail the query because the channel failed to become active.
Result:
More correct code.
Motivation:
At the moment if you do a resolveAll and at least one A / AAAA record is present we will not follow any CNAMEs that are also present. This is different to how the JDK behaves.
Modifications:
- Allows follow CNAMEs.
- Add unit test.
Result:
Fixes https://github.com/netty/netty/issues/7915.
Motivation:
a598c3b69b added a upper limit for ttl but missed to also do the same for minTtl.
Modifications:
- Add upper limit for minTtl
- Add testcase.
Result:
No more IllegalArgumentException possible.
Motivation:
Due a bug we did never store more then one address per hostname in DefaultDnsCache.
Modifications:
- Correctly store multiple entries per hostname
- Add tests
Result:
DefaultDnsCache correctly stores more then one entry. Also fixes https://github.com/netty/netty/issues/7882 .
Motivation:
In b47fb81799 we limited the max supported delay to match what our internal implementat can support. Because of this it was possible that DefaultDnsCache produced an IllegalArgumentException when it tried to schedule a expiration > 3 years.
Modifications:
Limit the max supported TTL to 2 years which is safe for all our EventLoop implementations.
Result:
No more exceptions when adding records to the cache with a huge TTL.
Motivation:
Right now to customize DNS name resolver when using DnsAddressResolverGroup
one should subclass implementation and override newNameResolver method when
in fact it's possible to collect all settings in a DnsNameResolverBuilder
instance. Described in #7749.
Modifications:
- Added new constructor for DnsNameResolverBuilder in order to delay
EventLoop specification
- Added copy() method to DnsNameResolverBuilder to provide an immutable
copy of the builder
- Added new single-argument constructor for DnsAddressResolverGroup and
RoundRobinDnsAddressResolverGroup accepting DnsNameResolverBuilder
instance
- DnsAddressResolverGroup to build a new resolver using DnsNameResolverBuilder
given instead of creating a new one
- Test cases to check that changing channelFactory after the builder was passed
to create a DnsNameResolverGroup would not propagate to the name resolver
Result:
Much easier to customize DNS settings w/o subclassing DnsAddressResolverGroup
Motivation:
Currently, if a DNS server returns a non-preferred address type before the preferred one, then both will be returned as the result, and when only taking a single one, this usually ends up being the non-preferred type. However, the JDK requires lookups to only return the preferred type when possible to allow for backwards compatibility.
To allow a client to be able to resolve the appropriate address when running on a machine that does not support IPv6 but the DNS server returns IPv6 addresses before IPv4 addresses when querying.
Modification:
Filter the returned records to the expected type when both types are present.
Result:
Allows a client to run on a machine with IPv6 disabled even when a server returns both IPv4 and IPv6 results. Netty-based code can be a drop-in replacement for JDK-based code in such circumstances.
This PR filters results before returning them to respect JDK expectations.
* Add DnsNameResolver.resolveAll(DnsQuestion)
Motivation:
A user is currently expected to use DnsNameResolver.query() when he or
she wants to look up the full DNS records rather than just InetAddres.
However, query() only performs a single query. It does not handle
/etc/hosts file, redirection, CNAMEs or multiple name servers.
As a result, such a user has to duplicate all the logic in
DnsNameResolverContext.
Modifications:
- Refactor DnsNameResolverContext so that it can send queries for
arbitrary record types.
- Rename DnsNameResolverContext to DnsResolveContext
- Add DnsAddressResolveContext which extends DnsResolveContext for
A/AAAA lookup
- Add DnsRecordResolveContext which extends DnsResolveContext for
arbitrary lookup
- Add DnsNameResolverContext.resolveAll(DnsQuestion) and its variants
- Change DnsNameResolverContext.resolve() delegates the resolve request
to resolveAll() for simplicity
- Move the code that decodes A/AAAA record content to DnsAddressDecoder
Result:
- Fixes#7795
- A user does not have to duplicate DnsNameResolverContext in his or her
own code to implement the usual DNS resolver behavior.
Motivation:
When we do DNS queries we need to ensure we always release the AddressEnvelope.
Modifications:
Also release the AddressEnvelope if the original resolution was done in the meantime and we did not cancel the extra query yet.
Result:
Should fix [#7713]
Motivation:
When following a CNAME response DnsNameResovlerContext may issue a A and AAAA query. However the DnsNameResolverContext would have already issued a A and AAAA query to get the CNAME response, and this may result in 2 additional A/AAAA queries per CNAME response.
Modifications:
- DnsNameResovlerContext#followCname shouldn't issue 2 queries, but instead just a single query with the same record type as the original query
Result:
No more duplicate queries as a result of CNAME responses.
Motivation:
DnsServerAddressStream provides an iterator like interface but maybe expected to start at a specific point upon each new usage. If a DnsServerAddressStream is re-used in multiple independent iterations the order of iteration maybe incorrect. DnsNameResolverContext has a fallback DnsServerAddressStream reference if the cache doesn't contain a hit, but it is shared across multiple independent iterations. This may lead to undesirable DNS query order.
Modifications:
- DnsNameResolverContext#getNameServers should duplicate the default DnsServerAddressStream
Result:
Consistent iteration over the default DnsServerAddressStream in DnsNameResolverContext.
Motivation:
When following a CNAME it is possible there are multiple name servers to query against. However DnsNameResolverContext#followCname explicitly only uses the first name server address when attempting the query. This may lead to resolution failures because we didn't try all the available name servers.
Modifications:
DnsNameResolverContext#followCname should not just try the first name server, but it should try all name servers
Result:
More complete CNAME resolution.
Motivation:
At the moment DefaultDnsCache will expire each record dependong on its own TTL. This may result in unexpected results for the end-user especially if the user for example uses IPV4_PREFERED but the cached AAAA records has a higher TTL then the A records and so the A record was removed. In this case we would only return the AAAA record and not even try to refresh.
Modifications:
Always expire all records for a hostname when one TTL is reached.
Result:
Fixes [#7329]
Motivation:
The usage of DnsCache in DnsNameResolver was racy in general. First of the isEmpty() was not called in a synchronized block while we depended on synchronized. The other problem was that this whole synchronization only worked if the DefaultDnsCache was used and the returned List was not wrapped by the user.
Modifications:
- Rewrite DefaultDnsCache to not depend on synchronization on the returned List by using a CoW approach.
Result:
Fixes [#7583] and other races.
Motivation:
We need to ensure we only call List.* methods in the synchronized block as the returned List may not be thread-safe.
Modifications:
Do not call isEmpty() outside of the synchronized block.
Result:
Fixes [#7583]
Motivation:
DnsNameResolverTest has not been updated in a while.
Modifications:
- Update the DOMAINS definition in DnsNameResolverTest
Result:
More current domain names.
