__Motivation__
Upon receiving a DNS answer, we match whether the name in the question matches the name in the record. Some DNS servers we have encountered append a search domain to the record name which fails this match. eg: for question name `netty` and search domains `io` and `com`, we will do 2 queries: `netty.io.` and `netty.com.`, if the answer for `netty.io` contains `netty.com` then we ignore this record.
__Modification__
If the name in the record does not match the name in the question, append configured search domains to the question name to see if it matches the record name.
__Result__
Records names with appended search domains are still returned as valid answers.
Motivation:
The tests must be executed only when there is no hosts file or
there is no entry for localhost in the hosts file. The tested functionality
is relevant only in these use cases.
Modifications:
Skip the windows tests when there is an entry for localhost in the hosts file.
Result:
Fix failing tests on Windows CI when using GitHub Actions
Related to #11384
Motivation:
Sometime in the past we introduced the concept of Void*Promise. As it turned out this was not a good idea at all as basically each handler in the pipeline need to be very careful to correctly handle this. We should better just remove this "optimization".
Modifications:
- Remove Void*Promise and all the related APIs
- Remove tests which were related to Void*Promise
Result:
Less error-prone API
Motivation:
JUnit 5 is more expressive, extensible, and composable in many ways, and it's better able to run tests in parallel.
Modifications:
Use JUnit5 in tests
Result:
Related to https://github.com/netty/netty/issues/10757
Motivation:
DefaultHostsFileEntriesResolver should provide all hosts file's entries for a hostname when
DnsNameResolver#resolveAll as opposed to the current implementation where only the first
entry is taken into consideration
Modification:
- Add DefaultHostsFileEntriesResolver#addresses to provide all hosts file's entries for a hostname
- Add HostsFileEntriesProvider to provide all hosts file's entries for a hostname and to keep
backwards compatibility for HostsFileEntries and HostsFileParser
- DnsNameResolver#resolveAll uses the new DefaultHostsFileEntriesResolver#addresses
- BlockHound configuration: replace HostsFileParser#parse with HostsFileEntriesProvider$ParserImpl#parse
as the latter does the parsing
- Add junit tests
Result:
Fixes#10834
Motivation:
DNS resolver falls back to trying CNAME if no records found, but should
only try this for A/AAAA queries. Does not make sense for other query
types, results in a redundant CNAME query that is just going to fail.
Modification:
Check query type before deciding to try CNAME. Only proceed if type is A
or AAAA.
Added unit test to verify CNAME is only tried after A/AAAA queries.
Result:
Fixes#11214.
Motivation:
NullChecks resulting in a NullPointerException or IllegalArgumentException, numeric ranges (>0, >=0) checks, not empty strings/arrays checks must never be anonymous but with the parameter or variable name which is checked. They must be specific and should not be done with an "OR-Logic" (if a == null || b == null) throw new NullPointerEx.
Modifications:
* import static relevant checks
* Replace manual checks with ObjectUtil methods
Result:
All checks needed are done with ObjectUtil, some exception texts are improved in microbench and resolver-dns
Fixes#11170
Motivation:
On Windows DnsNameResolver is not able to resolve the host(computer) name as it is not in the hosts file and the DNS server is also not able to resolve it.
The exception below is the result of the resolution:
Caused by: java.net.UnknownHostException: failed to resolve 'host(computer)-name' after 2 queries
at io.netty.resolver.dns.DnsResolveContext.finishResolve(DnsResolveContext.java:1013)
at io.netty.resolver.dns.DnsResolveContext.tryToFinishResolve(DnsResolveContext.java:966)
at io.netty.resolver.dns.DnsResolveContext.query(DnsResolveContext.java:414)
at io.netty.resolver.dns.DnsResolveContext.tryToFinishResolve(DnsResolveContext.java:938)
at io.netty.resolver.dns.DnsResolveContext.access$700(DnsResolveContext.java:63)
at io.netty.resolver.dns.DnsResolveContext$2.operationComplete(DnsResolveContext.java:467)
Modifications:
On Windows DnsNameResolver maps host(computer) name to LOCALHOST
Result:
DnsNameResolver is able to resolve the host(computer) name on Windows
Fixes#11142
Motivation:
The DnsResolver default start address listen to "0.0.0.0", which may be not what the user wants.
Modification:
Add localAddress as a param of DnsNameResolver and its builder
Result:
The DnsNameResolver's bind address can be configured.
Motivation:
It is not uncommon to run Netty on OS X without the specific
`MacOSDnsServerAddressStreamProvider`. The current log message is too
verbose because it prints a full stack trace on the console while a
simple logging message would have been enough.
Modifications:
- Print a `WARN` message when `MacOSDnsServerAddressStreamProvider`
class is not found;
- Print a `ERROR` message with a stack trace when the class was found
but could not be loaded due to some other reasons;
Result:
Less noise in logs.
Motivation:
When etcResolver/hosts files are parsed, FileInputStream.read(...) is internally called by
- UnixResolverDnsServerAddressStreamProvider#parseEtcResolverSearchDomains
- UnixResolverDnsServerAddressStreamProvider#parseEtcResolverOptions
- HostsFileParser#parse
This will cause the error below when BlockHound is enabled
reactor.blockhound.BlockingOperationError: Blocking call! java.io.FileInputStream#readBytes
at java.io.FileInputStream.readBytes(FileInputStream.java)
at java.io.FileInputStream.read(FileInputStream.java:255)
Modifications:
- Add whitelist entries to BlockHound configuration
- Fix typos in UnixResolverDnsServerAddressStreamProvider
- Add tests
Result:
Fixes#11004
Motivation:
`DnsServerAddressStreamProviders` tries to load `MacOSDnsServerAddressStreamProvider`
on macOS. However, it doesn't warn users when `MacOSDnsServerAddressStreamProvider`
is not awailable, which may cause incorrect results for DNS resolutions.
Modifications:
- Log at warn level if `MacOSDnsServerAddressStreamProvider` is not found on macOS;
- Log at debug level when `MacOSDnsServerAddressStreamProvider` is loaded and available;
Result:
macOS users are notified when `MacOSDnsServerAddressStreamProvider` is not available.
Motivation:
The DnsNameResolver internally follows CNAME indirects for all records types, and supports caching for CNAME resolution and A* records. For DNS record types that are not cached (e.g. SRV records) the caching of CNAME records may result in failures at incorrect times. For example if a CNAME record has a larger TTL than the entries it resolves this may result in failures which don't occur if the CNAME cache is disabled.
Modifications:
- Don't cache CNAME and also dont use the cache for CNAME when using DnsRecordResolveContext
- Add unit test
Result:
More correct resolving and also not possible to have failures due CNAME still be in the cache while the queried record experied
Motivation:
https in xmlns URIs does not work and will let the maven release plugin fail:
```
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 1.779 s
[INFO] Finished at: 2020-11-10T07:45:21Z
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-release-plugin:2.5.3:prepare (default-cli) on project netty-parent: Execution default-cli of goal org.apache.maven.plugins:maven-release-plugin:2.5.3:prepare failed: The namespace xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" could not be added as a namespace to "project": The namespace prefix "xsi" collides with an additional namespace declared by the element -> [Help 1]
[ERROR]
```
See also https://issues.apache.org/jira/browse/HBASE-24014.
Modifications:
Use http for xmlns
Result:
Be able to use maven release plugin
Motivation:
Since GraalVM version 19.3.0, instances of java.net.InetAddress (and its subclasses Inet4Address and Inet6Address) are not allowed in native image heap (that is, they cannot be stored in static fields of classes initialized at build time or be reachable through static fields of such classes). When building a native image, it makes sense to initialize at build time as many classes as possible.
But some fields of some classes in Netty (for example, NetUtil.LOCALHOST4) contain InetAddress instances. If a program is using code path that makes it possible to reach such fields at build time initialization, it becomes impossible to build a native image initializing core Netty classes initialized at runtime. An example of such a program is a client that uses netty-dns.
Modifications:
- Add netty-testsuite-native-image-client Maven module to test that such an example program can be built after the corresponding fixes
- Add native-image.properties to resolver-dns module to move initialization of some classes to runtime (some of them are parsing configuration during initialization, so it makes no sense to initialize them at build time; for others, it's needed to avoid InetAddress reachability at build time)
- Add substitutions for NetUtil.LOCALHOST4, NetUtil.LOCALHOST6 and NetUtil.LOCALHOST to overcome the InetAddress-related prohibition
- Extract some initialization code from NetUtil to NetUtilInitializations to allow it to be used by the substitutions
Result:
A client program using netty-dns with --initialize-at-build-time=io.netty builds successfully
Motivation:
HTTP is a plaintext protocol which means that someone may be able
to eavesdrop the data. To prevent this, HTTPS should be used whenever
possible. However, maintaining using https:// in all URLs may be
difficult. The nohttp tool can help here. The tool scans all the files
in a repository and reports where http:// is used.
Modifications:
- Added nohttp (via checkstyle) into the build process.
- Suppressed findings for the websites
that don't support HTTPS or that are not reachable
Result:
- Prevent using HTTP in the future.
- Encourage users to use HTTPS when they follow the links they found in
the code.
Motivation:
junit deprecated Assert.assertThat(...)
Modifications:
Use MatcherAssert.assertThat(...) as replacement for deprecated method
Result:
Less deprecation warnings
Motivation:
LGTM reports multiple issues. They need to be triaged,
and real ones should be fixed.
Modifications:
- Fixed multiple issues reported by LGTM, such as redundant conditions,
resource leaks, typos, possible integer overflows.
- Suppressed false-positives.
- Added a few testcases.
Result:
Fixed several possible issues, get rid of false alarms in the LGTM report.
Motivation:
We have a few classes in which we store and reuse static instances of various exceptions. When doing so it is important to also override fillInStacktrace() and so prevent the leak of the ClassLoader in the internal backtrace field.
Modifications:
- Add overrides of fillInStracktrace when needed
- Move ThrowableUtil usage in the static methods
Result:
Fixes https://github.com/netty/netty/pull/10686
Motivation:
Due a change introduced in 68105b257d we incorrectly skipped the usage of nameservers in some cases.
Modifications:
Only fetch a new stream of nameserver if the hostname not matches the original hostname in the query.
Result:
Use all configured nameservers. Fixes https://github.com/netty/netty/issues/10499
Expose a LoggingDnsQueryLifeCycleObserverFactory
Motivation:
There is a use case for having logging in the DnsNameResolver, similar to the LoggingHandler.
Previously, one could set `traceEnabled` on the DnsNameResolverBuilder, but this is not very configurable.
Specifically, the log level and the logger context cannot be changed.
Modification:
Expose a LoggingDnsQueryLifeCycleObserverFactory, that permit changing the log-level
and logger context.
Result:
It is now possible to get logging in the DnsNameResolver at a custom log level and logger,
without very much effort.
Fixes#10485
Motivation:
We need limit the number of queries done to resolve unresolved nameserver as otherwise we may never fail the resolve when there is a missconfigured dns server that lead to a "resolve loop".
Modifications:
- When resolve unresolved nameservers ensure we use the allowedQueries as max upper limit for queries so it will eventually fail
- Add unit tests
Result:
No more possibility to fail in a loop while resolve. This is related to https://github.com/netty/netty/issues/10420
Motivation:
DnsAddressResolverGroup allows to be constructed with a DnsNameResolverBuilder and so should respect its configured EventLoop.
Modifications:
- Correctly respect the configured EventLoop
- Ensure there are no thread-issues by calling copy()
- Add unit tests
Result:
Fixes https://github.com/netty/netty/issues/10460
Motivation:
If we cancel the returned future of resolve query, we may get LEAK. Try to release the ByteBuf if netty can't pass the DnsRawRecord to the caller.
Modification:
Using debug mode I saw there are two places that don't handle trySuccess with release. Try to release there.
Result:
Fixes#10447.
Motivation:
There is a possibility to end up with a StackOverflowError when trying to resolve authorative nameservers because of incorrect wrapping the AuthoritativeDnsServerCache.
Modifications:
Ensure we don't end up with an overflow due wrapping
Result:
Fixes https://github.com/netty/netty/issues/10246
Motivation:
Once a CNAME loop was detected we can just fail fast and so reduce the number of queries.
Modifications:
Fail fast once a CNAME loop is detected
Result:
Fail fast
Motivation:
We did use a HashSet to detect CNAME cache loops which needs allocations. We can use an algorithm that doesnt need any allocations
Modifications:
Use algorithm that doesnt need allocations
Result:
Less allocations on the slow path
Motivation:
The nameserver that should / must be used to resolve a CNAME may be different then the nameserver that was selected for the hostname to resolve. Failing to select the correct nameserver may result in problems during resolution.
Modifications:
Use the correct DnsServerAddressStream for CNAMEs
Result:
Always use the correct DnsServerAddressStream for CNAMEs and so fix resolution failures which could accour when CNAMEs are in the mix that use a different domain then the original hostname that we try to resolve
Motivation:
We should only log with warn level if something really critical happens as otherwise we may spam logs and confuse the user.
Modifications:
- Change log level to debug for most cases
Result:
Less noisy logging
Motivation:
We need to detect CNAME loops during lookup the DnsCnameCache as otherwise we may try to follow cnames forever.
Modifications:
- Correctly detect CNAME loops in the cache
- Add unit test
Result:
Fixes https://github.com/netty/netty/issues/10220
Motivations
-----------
DnsNameResolverBuilder and DnsNameResolver do not auto-configure
themselves uing default options define in /etc/resolv.conf.
In particular, rotate, timeout and attempts options are ignored.
Modifications
-------------
- Modified UnixResolverDnsServerAddressStreamProvider to parse ndots,
attempts and timeout options all at once and use these defaults to
configure DnsNameResolver when values are not provided by the
DnsNameResolverBuilder.
- When rotate option is specified, the DnsServerAddresses returned by
UnixResolverDnsServerAddressStreamProvider is rotational.
- Amend resolv.conf options with the RES_OPTIONS environment variable
when present.
Result:
Fixes https://github.com/netty/netty/issues/10202
Motivation:
A user might want to cancel DNS resolution when it takes too long.
Currently, there's no way to cancel the internal DNS queries especially when there's a lot of search domains.
Modification:
- Stop sending a DNS query if the original `Promise`, which was passed calling `resolve()`, is canceled.
Result:
- You can now stop sending DNS queries by cancelling the `Promise`.
Motivation:
Related https://github.com/line/armeria/issues/2463
Here is an example that an NIC has only link local address for IPv6.
```
$ ipaddr
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue qlen 1000
link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
inet 127.0.0.1/8 scope host lo
valid_lft forever preferred_lft forever
inet6 ::1/128 scope host
valid_lft forever preferred_lft forever
3: eth0@if18692: <BROADCAST,MULTICAST,UP,LOWER_UP,M-DOWN> mtu 1460 qdisc noqueue
link/ether 1a:5e:xx:xx:xx:xx brd ff:ff:ff:ff:ff:ff
inet 10.xxx.xxx.xxx/24 scope global eth0
valid_lft forever preferred_lft forever
inet6 fe80::xxxx:xxxx:xxxx:xxxx/64 scope link
valid_lft forever preferred_lft forever
```
If the NICs have only local or link local addresses, We should not send IPv6 DNS queris.
Modification:
- Ignore link-local IPv6 addresses which may exist even on a machine without IPv6 network.
Result:
- `DnsNameResolver` does not send DNS queries for AAAA when IPv6 is not available.
Motivation:
As we have java8 as a minimum target we can use MethodHandles. We should do so when we expect to have a method called multiple times.
Modifications:
- Replace usage of reflection with MethodHandles where it makes sense
- Remove some code which was there to support java < 8
Result:
Faster code
Motivation:
939e928312 introduced MacOSDnsServerAddressStreamProvider which will ensure the right nameservers are selected when running on MacOS. To ensure this is done automatically on MacOS we should use it by default on these platforms.
Modifications:
Try to use MacOSDnsServerAddressStreamProvider when on MacOS via reflection and fallback if not possible
Result:
Ensure the right nameservers are used on MacOS even when a VPN (for example) is used.
Motivation:
In general, we will close the debug log in a product environment. However, logging without external level check may still affect performance as varargs will need to allocate an array.
Modification:
Add log level check simply before logging.
Result:
Improve performance slightly in a product environment.
Motivation:
In next major version of netty users should use ChannelHandler everywhere. We should ensure we do the same
Modifications:
Replace usage of deprecated classes / interfaces with ChannelHandler
Result:
Use non-deprecated code
Motivation:
The resolver API and implementations should be considered stable by now so we should not mark these with @UnstableApi
Modifications:
Remove @UnstableApi annotation from API and implementation of resolver
Result:
Make it explicit that the API is considered stable
Motivation:
The resolv.conf file may contain inline comments which should be ignored
Modifications:
- Detect if we have a comment after the ipaddress and if so skip it
- Add unit test
Result:
Fixes https://github.com/netty/netty/issues/9889
Motivation:
We should just ignore (and so skip) invalid entries in /etc/resolver.conf.
Modifications:
- Skip invalid entries
- Add unit test
Result:
Fix https://github.com/netty/netty/issues/9684
Motivation
A memory leak related to DNS resolution was reported in #9634,
specifically linked to the TCP retry fallback functionality that was
introduced relatively recently. Upon inspection it's apparent that there
are some error paths where the original UDP response might not be fully
released, and more significantly the TCP response actually leaks every
time on the fallback success path.
It turns out that a bug in the unit test meant that the intended TCP
fallback path was not actually exercised, so it did not expose the main
leak in question.
Modifications
- Fix DnsNameResolverTest#testTruncated0 dummy server fallback logic to
first read transaction id of retried query and use it in replayed
response
- Adjust semantic of internal DnsQueryContext#finish method to always
take refcount ownership of passed in envelope
- Reorder some logic in DnsResponseHandler fallback handling to verify
the context of the response is expected, and ensure that the query
response are either released or propagated in all cases. This also
reduces a number of redundant retain/release pairings
Result
Fixes#9634
Motivation:
Classes `AbstractHttp2StreamChannel.Http2StreamChannelConfig`
and `DnsNameResolver.AddressedEnvelopeAdapter` may be static:
it doesn't reference its enclosing instance.
Modification:
Add `static` modifier.
Result:
Prevents a possible memory leak and uses less memory per class instance.
Motivation:
We currently try to access the the domain search list via reflection on windows which will print a illegal access warning when using Java9 and later.
Modifications:
Add a guard against the used java version.
Result:
Fixes https://github.com/netty/netty/issues/9500.
Motivation:
It is possible that the user uses a too big EDNS0 setting for the MTU and so we may receive a truncated datagram packet. In this case we should try to detect this and retry via TCP if possible
Modifications:
- Fix detecting of incomplete records
- Mark response as truncated if we did not consume the whole packet
- Add unit test
Result:
Fixes https://github.com/netty/netty/issues/9365
Motivation:
We should only ever close the underlying tcp socket once we received the envelope to ensure we never race in the test.
Modifications:
- Only close socket once we received the envelope
- Set REUSE_ADDR
Result:
More robust test
Motivation:
testTruncatedWithTcpFallback was flacky as we may end up closing the socket before we could read all data. We should only close the socket after we succesfully read all data.
Modifications:
Move socket.close() to finally block
Result:
Fix flaky test and so make the CI more stable again.
Motivation:
We should only try to use reflection to access default nameservers when using Java8 and lower as otherwise we will produce an Illegal reflective access warning like:
WARNING: Illegal reflective access by io.netty.resolver.dns.DefaultDnsServerAddressStreamProvider
Modifications:
Add Java version check before try to use reflective access.
Result:
No more warning when Java9+ is used.
