Motivation:
The current DnsNameResolver fails to resolve an A+CNAME answer. For example:
dig moose.rmq.cloudamqp.com
...
;; ANSWER SECTION:
moose.rmq.cloudamqp.com. 1800 IN CNAME ec2-54-152-221-139.compute-1.amazonaws.com.
ec2-54-152-221-139.compute-1.amazonaws.com. 583612 IN A 54.152.221.139
...
The resolver constructs a map of cnames but forgets the trailing "." in the values which lead to not resolve the A record.
Modifications:
Reuse the code of DefaltDnsRecordDecoder which correctly handles the trailing dot.
Result:
Correctly resolve.
Motivation:
JCTools supports both non-unsafe, unsafe versions of queues and JDK6 which allows us to shade the library in netty-common allowing it to stay "zero dependency".
Modifications:
- Remove copy paste JCTools code and shade the library (dependencies that are shaded should be removed from the <dependencies> section of the generated POM).
- Remove usage of OneTimeTask and remove it all together.
Result:
Less code to maintain and easier to update JCTools and less GC pressure as the queue implementation nt creates so much garbage
Motivation:
We use a default of 3 for maxQueriesPerResolve when using the DnsNameResolverBuilder, which is too low if you want to resolve a hostname that uses a lot of CNAME records.
Modifications:
- Use higher default (16)
- Make exception message more clear why it failed.
Result:
Be able to resolve more domains by default and be able to better trouble shoot why a resolver failed.
Motivation:
DnsAddressResolverGroup allows to override the newResolver(...) method to change the settings used by the user. We should better let the user override another method and always apply the InflightNameResolver.
Modifications:
- Mark newResolver(...) method as deprecated, we will make it private soon.
- Add newNameResolver(...) method that user can override.
Result:
Easier to extend DnsAddressResolverGroup
Related issue: #5179
Motivation:
When you attempt to make a lot of connection attempts to the same target
host at the same time and our DNS resolver does not have a record for it
in the cache, the DNS resolver will send as many DNS queries as the
number of connection attempts.
As a result, DNS server will reject or drop the requests, making the
name resolution attempt fail.
Modifications:
- Add InflightNameResolver that keeps the list of name resolution
queries and subscribes to the future of the matching query instead of
sending a duplicate query.
Result:
- The AddressResolvers created by DnsAddressResolverGroup do not send
duplicate DNS queries anymore
Motivation:
Some codecs should be considered unstable as these are relative new. For this purpose we should introduce an annotation which these codecs should us to be marked as unstable in terms of API.
Modifications:
- Add UnstableApi annotation and use it on codecs that are not stable
- Move http2.hpack to http2.internal.hpack as it is internal.
Result:
Better document unstable APIs.
Motivation:
There were some warning in the resolver-dns code base.
Modifications:
- Fix javadocs
- Use the base class to call static method.
Result:
Cleaner code.
Related: #4771
Motivation:
A malicious or misconfigured DNS server can send the CNAME records that
resolve into each other, causing an unexpected infinite loop in
DnsNameResolverContext.onResponseCNAME().
Modifications:
- Remove the dereferenced CNAME from the alias map so that infinite loop
is impossible.
- Fix inspection warnings and typos in DnsNameResolverTest
Result:
Fixes#4771
Motivation:
Current DnsNameResolver api don't allow to define additional records in DNS query.
It can be useful in many cases. For example when we want to query dns server with
real client address (EDNS-CLIENT-SUBNET extension:
http://tools.ietf.org/html/draft-vandergaast-edns-client-subnet-02 )
Modifications:
This change add new query methods with list of additional DnsRecord-s for query.
Result:
It is possible to create dns query with EDNS-CLIENT-SUBNET extension for example.
Motivation:
See #3411. A reusable ArrayList in InternalThreadLocalMap can avoid allocations in the following pattern:
```
List<...> list = new ArrayList<...>();
add something to list but never use InternalThreadLocalMap
return list.toArray(new ...[list.size()]);
```
Modifications:
Add a reusable ArrayList to InternalThreadLocalMap and update codes to use it.
Result:
Reuse a thread local ArrayList to avoid allocations.
Motivation:
DnsNameResolverBuilder#nameServerAddresses isn’t initialized with a
default value. In most cases, user will want
DefaultDnsServerAddresses#defaultAddresses.
Modifications:
Initialize DnsNameResolverBuilder#nameServerAddresses with
DefaultDnsServerAddresses#defaultAddresses
Result:
DnsNameResolverBuilder more convenient usage.
Motivation:
As "getHostName" may do a reverse name lookup and return a host name based on the system configured name lookup service, testResolveIp may fail in some special environment. See #4720
Modifications:
Use getHostAddress instead of getHostName
Result:
testResolveIp works in all environments
Motivation:
Caching is currently nested in DnsResolver.
It should also be possible to extend DnsResolver to ba able to pass a different cache on each resolution attemp.
Modifications:
* Introduce DnsCache, NoopDnsCache and DefaultDnsCache. The latter contains all the current caching logic that was extracted.
* Introduce protected versions of doResolve and doResolveAll that can be used as extension points to build resolvers that bypass the main cache and use a different one on each resolution.
Result:
Isolated caching logic. Better extensibility.
Motivation:
There are some wrong links and tags in javadoc.
Modifications:
Fix the wrong links and tags in javadoc.
Result:
These links will work correctly in javadoc.
Motivation:
On contrary to `DefaultNameResolver`, `DnsNameResolver` doesn't currently honor hosts file.
Modifications:
* Introduce `HostsFileParser` that parses `/etc/hosts` or `C:\Windows\system32\drivers\etc\hosts` depending on the platform
* Introduce `HostsFileEntriesResolver` that uses the former to resolve host names
* Make `DnsNameResolver` check his `HostsFileEntriesResolver` prior to trying to resolve names against the DNS server
* Introduce `DnsNameResolverBuilder` so we now have a builder for `DnsNameResolver`s
* Additionally introduce a `CompositeNameResolver` that takes several `NameResolver`s and tries to resolve names by delegating sequentially
* Change `DnsNameResolver.asAddressResolver` to return a composite and honor hosts file
Result:
Hosts file support when using `DnsNameResolver`.
Consistent behavior with JDK implementation.
Motivation:
As discussed in #4529, NameResolver design shouldn't be resolving SocketAddresses (or String name + port) and return InetSocketAddresses. It should resolve String names and return InetAddresses.
This SocketAddress to InetSocketAddresses resolution is actually a different concern, used by Bootstrap.
Modifications:
Extract SocketAddress to InetSocketAddresses resolution concern to a new class hierarchy named AddressResolver.
These AddressResolvers delegate to NameResolvers.
Result:
Better separation of concerns.
Note that new AddressResolvers generate a bit more allocations because of the intermediate Promise and List<InetAddress>.
Motivation:
There's no way to override the default settings of the DnsNameResolvers
created by DnsNameResolverGroup because DnsNameResolverGroup is final.
Modifications:
- Make DnsNameResolverGroup non-final
- Add a new overridable protected method 'newResolver()' so that a user
can override it to create an alternative DnsNameResolver instance or
set the non-default properties
Result:
A user can configure the DnsNameResolver.
Motivation:
Each server should be checked for every record type. Currently, if there
are only two configured servers and the first is down, it is impossible
to query for IPv4 addresses because the second server is only ever
queried for type AAAA.
Modifications:
Do not cycle DNS servers while cycling DNS record types (A and AAAA)
Result:
Name resolution is less fragile when the number of available DNS servers
is 2.
Related: #3972
Motivation:
DnsNameResolver limits the number of concurrent in-progress DNS queries
to 65536 regardless the number of DNS servers it communicates with. When
the number of available DNS servers are more than just one, we end up
using much less (65536 / numDnsServers) query IDs per DNS server, which
is non-optimal.
Modifications:
- Replace the query ID and context management with
DnsQueryContextManager
- Eash DNS server gets its own query ID space
Result:
Much bigger query ID space, and thus there's less chance of getting the
'query ID space exhaustion' error
Motivation:
When DnsNameResolverContext succeeds to get the address(es), it cancels
the promise of other queries in progress.
Unlike expectation, DnsNameResolverContext.query() attempts to retry
even when the query has failed due to cancellation.
As a result, the resolver sends unnecessary extra queries to a DNS
server and then tries to mark the promised that's been fulfilled
already, leading to unnecessarily verbose 'failed to notify success to a
promise' messages.
Modifications:
Do not perform an extra query when the previous query has failed due to
cancellation
Result:
DnsNameResolver does not send unnecessary extra queries and thus does
not log the 'failed to notify success to a promise' message.
Motivation:
As relaying on external DNS Server can result to test-failures we should better use a mock DNS Server for the dns tests.
Modifications:
- Refactor the DnsNameResolverTest to use a mock DNS Server which is using apacheds.
- Allow to disable adding an opt resources as some servers not support it.
Result:
More stable testsuite.
Motivation:
Some DNS servers in DnsNameResolverTest are outdated and some of them
returns NoError for non-existent domains.
Modifications:
- Update the DNS server list from http://meo.ws/dnsrec.php again
- Update the web-scraper script
Result:
DnsNameResolverTest.testNegativeTtl() should not fail anymore.
Motivation:
DNS lookups in DnsNameResolverTest can take longer than expected due to
retries. The hard limit of 5 seconds is being applied to
testNegativeTtl(), making the first uncached lookup cause a timeout.
Modifications:
Do not use JUnit's Timeout annotation but implement simple timeout
mechanism that apples only to cached lookups.
Result:
testNegativeTtl() should not fail when an initial negative lookup
requires a retry.
Motivation:
- DNS lookup sometimes hang because it does not call
tryToFinishResolve()
- Comodo Secure DNS handles negative lookup incorrectly.
Modifications:
- Add missing tryToFinishResolve()
- Remove Comodo Secure DNS servers from the list
Result
- DNS lookup does not hang on non-existent domain names
- More reliable DnsNameResolverTest
Related: #4065
Motivation:
DnsNameResolver was using a special Iterable/Iterator implementation
that yields an infinite stream of DNS server addresses. However, this
seems to cause confusion.
Modifications:
- Make DnsServerAddresses an abstract class with an abstract stream()
method that returns DnsServerAddressStream
- Add DnsServerAddressStream that yields DNS server address infinitely
- Remove DnsServerResolver(Group) constructors that accept only a single
server address, which wasn't very useful in practice
- Extract the DnsServerAddresses implementations to top level
- DnsServerAddresses.defaultAddresses() now returns DnsServerAddresses.
- Add DnsServerAddresses.defaultAddressList() instead
Result:
Less confusion and more explicitness
Motivation:
DNS servers seem to reply with ServFail(2) response code when it is
busy.
Modifications:
- Retry when response code is ServFail instead of failing the test
- Try all DNS servers instead of retrying twice only
Result:
testQueryMx() is less likely to fail due to public DNS server problems
Related issues:
- #3971
- #3973
- #3976
- #4035
Motivation:
1. Previously, DnsNameResolver.query() retried the request query by its
own. It prevents a user from deciding when to retry or stop. It is also
impossible to get the response object whose code is not NOERROR.
2. NameResolver does not have an operation that resolves a host name
into multiple addresses, like InetAddress.getAllByName()
Modifications:
- Changes related with DnsNameResolver.query()
- Make query() not retry
- Move the retry logic to DnsNameResolver.resolve() instead.
- Make query() fail the promise only when I/O error occurred or it
failed to get a response
- Add DnsNameResolverException and use it when query() fails so that
the resolver can give more information about the failure
- query() does not cache anymore.
- Changes related with NameResolver.resolveAll()
- Add NameResolver.resolveAll()
- Add SimpleNameResolver.doResolveAll()
- Changes related with DnsNameResolver.resolve() and resolveAll()
- Make DnsNameResolveContext abstract so that DnsNameResolver can
decide to get single or multiple addresses from it
- Re-implement cache so that the cache works for resolve() and
resolveAll()
- Add 'traceEnabled' property to enable/disable trace information
- Miscellaneous changes
- Use ObjectUtil.checkNotNull() wherever possible
- Add InternetProtocolFamily.addressType() to remove repetitive
switch-case blocks in DnsNameResolver(Context)
- Do not raise an exception when decoding a truncated DNS response
Result:
- Full control over query()
- A user can now retrieve all addresses via (Dns)NameResolver.resolveAll()
- DNS cache works only for resolve() and resolveAll() now.
Motivation:
DnsResolver.resolve(...) fails when an InetSocketAddress is used that was constructed of an ipaddress string.
Modifications:
Don't try to lookup when the InetSocketAddress was constructed via an ipaddress.
Result:
DnsResolver.resolve(...) works in all cases.
Related: #3797
Motivation:
There is a race condition where DnsNameResolver.query() can attempt to
increase the reference count of the DNS response which was released
already by other thread.
Modifications:
- Make DnsCacheEntry a top-level class for clear access control
- Use 'synchronized' to avoid the race condition
- Add DnsCacheEntry.retainedResponse() to make sure that the response
is never released while it is retained
- Make retainedResponse() return null when the response has been
released already, so that DnsNameResolver.query() knows that the
cached entry has been released
Result:
The forementioned race condition has been fixed.
Motivation:
There are various known issues in netty-codec-dns:
- Message types are not interfaces, which can make it difficult for a
user to implement his/her own message implementation.
- Some class names and field names do not match with the terms in the
RFC.
- The support for decoding a DNS record was limited. A user had to
encode and decode by him/herself.
- The separation of DnsHeader from DnsMessage was unnecessary, although
it is fine conceptually.
- Buffer leak caused by DnsMessage was difficult to analyze, because the
leak detector tracks down the underlying ByteBuf rather than the
DnsMessage itself.
- DnsMessage assumes DNS-over-UDP.
- To send an EDNS message, a user have to create a new DNS record class
instance unnecessarily.
Modifications:
- Make all message types interfaces and add default implementations
- Rename some classes, properties, and constants to match the RFCs
- DnsResource -> DnsRecord
- DnsType -> DnsRecordType
- and many more
- Remove DnsClass and use an integer to support EDNS better
- Add DnsRecordEncoder/DnsRecordDecoder and their default
implementations
- DnsRecord does not require RDATA to be ByteBuf anymore.
- Add DnsRawRecord as the catch-all record type
- Merge DnsHeader into DnsMessage
- Make ResourceLeakDetector track AbstractDnsMessage
- Remove DnsMessage.sender/recipient properties
- Wrap DnsMessage with AddressedEnvelope
- Add DatagramDnsQuest and DatagramDnsResponse for ease of use
- Rename DnsQueryEncoder to DatagramDnsQueryEncoder
- Rename DnsResponseDecoder to DatagramDnsResponseDecoder
- Miscellaneous changes
- Add StringUtil.TAB
Result:
- Cleaner APi
- Can support DNS-over-TCP more easily in the future
- Reduced memory footprint in the default DnsQuery/Response
implementations
- Better leak tracking for DnsMessages
- Possibility to introduce new DnsRecord types in the future and provide
full record encoder/decoder implementation.
- No unnecessary instantiation for an EDNS pseudo resource record
Related: #3173
Motivation:
DnsNameResolver was using InetSocketAddress.getHostString() which is
only available since Java 7.
Modifications:
Use InetSocketAddress.getHostName() in lieu of getHostString() when the
current Java version is less than 7.
Result:
DnsNameResolver runs fine on Java 6.
Related: #3149
Motivation:
DnsQueryContext, using the DatagramChannel bound in DnsNameResolver,
blindly writes to the channel without checking the bind future for
success.
Modifications:
Check the bindFuture before writing a DNS query to a DatagramChannel
Result:
Bug fixed
Motivation:
DnsNameResolver.testResolveA() tests if the cache works as well as the usual DNS protocol test. To ensure the result from the cache is identical to the result without cache, it compares the two Maps which contain the result of cached/uncached resolution. The comparison of two Maps yields an expected behavior, but the output of the comparison on failure is often unreadable due to its long length.
Modifications:
Compare entry-by-entry for more comprehensible test failure output
Result:
When failure occurs, it's easier to see which domain was the cause of the problem.
Motivation:
So far, we relied on the domain name resolution mechanism provided by
JDK. It served its purpose very well, but had the following
shortcomings:
- Domain name resolution is performed in a blocking manner.
This becomes a problem when a user has to connect to thousands of
different hosts. e.g. web crawlers
- It is impossible to employ an alternative cache/retry policy.
e.g. lower/upper bound in TTL, round-robin
- It is impossible to employ an alternative name resolution mechanism.
e.g. Zookeeper-based name resolver
Modification:
- Add the resolver API in the new module: netty-resolver
- Implement the DNS-based resolver: netty-resolver-dns
.. which uses netty-codec-dns
- Make ChannelFactory reusable because it's now used by
io.netty.bootstrap, io.netty.resolver.dns, and potentially by other
modules in the future
- Move ChannelFactory from io.netty.bootstrap to io.netty.channel
- Deprecate the old ChannelFactory
- Add ReflectiveChannelFactory
Result:
It is trivial to resolve a large number of domain names asynchronously.
