Commit Graph

179 Commits

Author SHA1 Message Date
Shohei Kamimori
73f23c5faa Fix typos in docs.
Motivation:

There are same typos in the docs.

Modifications:

Fix typos. Docs only changing.

Result:

More correct docs.
2018-02-14 08:44:07 +01:00
Norman Maurer
e71fa1e7b6 [maven-release-plugin] prepare for next development iteration 2018-02-05 12:02:35 +00:00
Norman Maurer
41ebb5fcca [maven-release-plugin] prepare release netty-4.1.21.Final 2018-02-05 12:02:19 +00:00
Scott Mitchell
46e66b45bd DnsNameResovlerContext#followCname sending extra queries
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.
2018-02-02 07:46:10 +01:00
Scott Mitchell
5257ec49ec DnsNameResolverContext reuse of DnsServerAddressStream without duplicate
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.
2018-02-02 07:24:11 +01:00
Scott Mitchell
fe5c69bdd9 DnsNameResolverContext#followCname only uses first name server
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.
2018-02-01 09:31:07 +01:00
Norman Maurer
b874edbf65 DefaultDnsCache should expire all records per hostname when one TTL is reached.
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]
2018-01-31 14:39:31 +01:00
Norman Maurer
e305488c5a Fix race-condition when using DnsCache in DnsNameResolver
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.
2018-01-23 08:04:33 +01:00
Norman Maurer
ea58dc7ac7 [maven-release-plugin] prepare for next development iteration 2018-01-21 12:53:51 +00:00
Norman Maurer
96c7132dee [maven-release-plugin] prepare release netty-4.1.20.Final 2018-01-21 12:53:34 +00:00
Norman Maurer
f3c6da32d7 Fix concurrency issue in DnsNameResolver when DefaultDnsCache is used.
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]
2018-01-17 06:08:36 +01:00
Scott Mitchell
83bca87257
Update domains in DnsNameResolverTest
Motivation:
DnsNameResolverTest has not been updated in a while.

Modifications:
- Update the DOMAINS definition in DnsNameResolverTest

Result:
More current domain names.
2018-01-02 19:08:40 -05:00
Norman Maurer
264a5daa41 [maven-release-plugin] prepare for next development iteration 2017-12-15 13:10:54 +00:00
Norman Maurer
0786c4c8d9 [maven-release-plugin] prepare release netty-4.1.19.Final 2017-12-15 13:09:30 +00:00
Norman Maurer
b2bc6407ab [maven-release-plugin] prepare for next development iteration 2017-12-08 09:26:15 +00:00
Norman Maurer
96732f47d8 [maven-release-plugin] prepare release netty-4.1.18.Final 2017-12-08 09:25:56 +00:00
Tomasz Jędrzejewski
e8540c2b7a Adding stable JDK9 module names that follow reverse-DNS style
Automatic-Module-Name entry provides a stable JDK9 module name, when Netty is used in a modular JDK9 applications. More info: http://blog.joda.org/2017/05/java-se-9-jpms-automatic-modules.html

When Netty migrates to JDK9 in the future, the entry can be replaced by actual module-info descriptor.

Modification:

The POM-s are configured to put the correct module names to the manifest.

Result:

Fixes #7218.
2017-11-29 11:50:24 +01:00
Norman Maurer
c1b1d6268a Allow to detect failed query caused by an Timeout / IO error and also not cache these.
Motivation:

At the moment there is not way for the user to know if resolving a domain was failed because the domain was unkown or because of an IO error / timeout. If it was caused by an timeout / IO error the user may want to retry the query. Also if the query was failed because of an IO error / timeout we should not cache it.

Modifications:

- Add DnsNameResolverTimeoutException and include it in the UnkownHostException if the domain could not be resolved because of an timeout. This will allow the user to retry the query when inspecting the cause.
- Do not cache IO errors / timeouts
- Add unit test

Result:

Easier for users to implement retries for DNS querys and not cache IO errors / timeouts.
2017-11-23 10:56:41 +01:00
Norman Maurer
433dbeb149 Revert "Allow to detect failed query caused by an Timeout / IO error and also not cache these."
This reverts commit 12a413bf02 as it needs some more changes due some changes that were merged into 4.1 before.
2017-11-22 22:05:29 +01:00
Norman Maurer
12a413bf02 Allow to detect failed query caused by an Timeout / IO error and also not cache these.
Motivation:

At the moment there is not way for the user to know if resolving a domain was failed because the domain was unkown or because of an IO error / timeout. If it was caused by an timeout / IO error the user may want to retry the query. Also if the query was failed because of an IO error / timeout we should not cache it.

Modifications:

- Add DnsNameResolverTimeoutException and include it in the UnkownHostException if the domain could not be resolved because of an timeout. This will allow the user to retry the query when inspecting the cause.
- Do not cache IO errors / timeouts
- Add unit test

Result:

Easier for users to implement retries for DNS querys and not cache IO errors / timeouts.
2017-11-22 21:43:27 +01:00
Scott Mitchell
907ed79069 Reduce conditionals in DnsNameResovlerContext
Motivation:
Minor cleanup from 844d804 just to reduce the conditional statements and indentation level.

Modifications:
- combine the else + if into an else if statement

Result:
Code cleaned up.
2017-11-20 14:04:17 -08:00
Stanley Shyiko
844d804aba Fix DN resolution when ndots is greater than 1
Motivation:

DN resolution does not fall back to the "original name" lookup after search list is checked. This results in a failure to resolve any name (outside of search list) that has number of dots less than resolv.conf's ndots value (which, for example, is often the case in the context of Kubernetes where kubelet passes on resolv.conf containing "options ndots:5").

It also does not go through the search list in a situation described in resolv.conf man:
"The default for n[dots] is 1, meaning that if there are any dots in a name, the name will be tried first as an absolute name before any search list elements are appended to it."

Modifications:

DnsNameResolverContext::resolve was updated to match Go's https://github.com/golang/go/blob/release-branch.go1.9/src/net/dnsclient_unix.go#L338 logic.

Result:
DnsNameResolverContext::resolve will now try to resolve "original name" if search list yields no results when number of dots in the original name is less than resolv.conf's ndots value. It will also go through the search list in case "origin name" resolution fails and number of dots is equal or larger than resolv.conf's ndots value.
2017-11-20 14:01:16 +01:00
Norman Maurer
0013567cd8 Don't try to use UnixResolverDnsServerAddressStreamProvider when on Windows.
Motivation:

We should not try to use UnixResolverDnsServerAddressStreamProvider when on Windows as it will log some error that will produce noise and may confuse users.

Modifications:

Just use DefaultDnsServerAddressStreamProvider if windows is used.

Result:

Less noise in the logs. This was reported in vert.x: https://github.com/eclipse/vert.x/issues/2204
2017-11-13 20:26:38 +01:00
Norman Maurer
188ea59c9d [maven-release-plugin] prepare for next development iteration 2017-11-08 22:36:53 +00:00
Norman Maurer
812354cf1f [maven-release-plugin] prepare release netty-4.1.17.Final 2017-11-08 22:36:33 +00:00
Scott Mitchell
dcb828f02f DnsResolver CNAME redirect bug
Motviation:
DnsNameResolverContext#followCname attempts to build a query to follow a CNAME, but puts the original hostname in the DnsQuery instead of the CNAME hostname. This will result in not following CNAME redirects correctly.

Result:
- DnsNameResolverContext#followCname should use the CNAME instead of the original hostname when building the DnsQuery

Result:
More correct handling of redirect queries.
2017-10-23 09:37:32 -07:00
Idel Pivnitskiy
50a067a8f7 Make methods 'static' where it possible
Motivation:

Even if it's a super micro-optimization (most JVM could optimize such
 cases in runtime), in theory (and according to some perf tests) it
 may help a bit. It also makes a code more clear and allows you to
 access such methods in the test scope directly, without instance of
 the class.

Modifications:

Add 'static' modifier for all methods, where it possible. Mostly in
test scope.

Result:

Cleaner code with proper 'static' modifiers.
2017-10-21 14:59:26 +02:00
Idel Pivnitskiy
558097449c Add missed 'serialVersionUID' field for Serializable classes
Motivation:

Without a 'serialVersionUID' field, any change to a class will make
previously serialized versions unreadable.

Modifications:

Add missed 'serialVersionUID' field for all Serializable
classes.

Result:

Proper deserialization of previously serialized objects.
2017-10-21 14:41:18 +02:00
Norman Maurer
625a7426cd [maven-release-plugin] prepare for next development iteration 2017-09-25 06:12:32 +02:00
Norman Maurer
f57d8f00e1 [maven-release-plugin] prepare release netty-4.1.16.Final 2017-09-25 06:12:16 +02:00
Norman Maurer
b967805f32 [maven-release-plugin] prepare for next development iteration 2017-08-24 15:38:22 +02:00
Norman Maurer
da8e010a42 [maven-release-plugin] prepare release netty-4.1.15.Final 2017-08-24 15:37:59 +02:00
Scott Mitchell
566566db3a Decouple DnsCache and DnsCacheEntry
Motivation:
DnsCache (an interface) is coupled to DnsCacheEntry (a final class). This means that DnsCache implementations can't implement their own DnsCacheEntry objects if the default behavior isn't appropriate.

Modifications:
- DnsCacheEntry should be moved to DefaultDnsCache as it is an implementation detail
- DnsCache#cache(..) should return a new DnsCacheEntry
- The methods which from DnsCacheEntry that were used outside the scope of DefaultDnsCache should be moved into an interface

Result:
DnsCache is more extensible and not tightly coupled to a default implementation of DnsCacheEntry.
2017-08-21 11:15:27 -07:00
Norman Maurer
bbcab32874 Ensure netty builds with java9 (build 9+181)
Motivation:

To be able to build with latest java9 release we need to adjust commons-lang version and maven-enforcer-plugin.

Modifications:

- Use commons-lang 2.6.0
- Use maven-enforcer-plugin 3.0.0.M1 when building with java9

Result:

Netty builds again with latest java9 release
2017-08-15 20:31:10 +02:00
Norman Maurer
52f384b37f [maven-release-plugin] prepare for next development iteration 2017-08-02 12:55:10 +00:00
Norman Maurer
8cc1071881 [maven-release-plugin] prepare release netty-4.1.14.Final 2017-08-02 12:54:51 +00:00
Norman Maurer
359beff56f Choose ipv4 or ipv6 google dns servers as default fallback based on the settings for this system / jvm
Motivation:

We should not use ipv4 google dns servers if the app is configured to run ipv6.

Modifications:

Use either ipv4 or ipv6 dns servers depending on the system config.

Result:

More correct behaviour
2017-07-26 20:33:52 +02:00
Norman Maurer
34fdc7a33e Skip invalid hostnames when construct default dns servers to use.
Motivation:

When the hostname portion can not be extracted we should just skip the server as otherwise we will produce and exception when trying to create the InetSocketAddress.

This was happing when trying to run the test-suite on a system and using java7:

java.lang.IllegalArgumentException: hostname can't be null
	at java.net.InetSocketAddress.checkHost(InetSocketAddress.java:149)
	at java.net.InetSocketAddress.<init>(InetSocketAddress.java:216)
	at io.netty.util.internal.SocketUtils$10.run(SocketUtils.java:171)
	at io.netty.util.internal.SocketUtils$10.run(SocketUtils.java:168)
	at java.security.AccessController.doPrivileged(Native Method)
	at io.netty.util.internal.SocketUtils.socketAddress(SocketUtils.java:168)
	at io.netty.resolver.dns.DefaultDnsServerAddressStreamProvider.<clinit>(DefaultDnsServerAddressStreamProvider.java:74)
	at io.netty.resolver.dns.DnsServerAddressesTest.testDefaultAddresses(DnsServerAddressesTest.java:39)

Modifications:

Skip if hostname can not be extracted.

Result:

No more java.lang.ExceptionInInitializerError.
2017-07-25 08:43:53 +02:00
Norman Maurer
486f962252 Respect DNS port that is specified via JNDI
Motivation:

JNDI allows to specify an port so we should respect it.

Modifications:

Use the specified port and if none is specifed use 53.

Result:

Correct handling of JNDI configured DNS.
2017-07-25 08:26:59 +02:00
Scott Mitchell
0afe4e0964 Increase timeout for DnsNameResolverTest
Motivation:
DnsNameResolverTest has been observed to timeout on the CI servers. We should increase the timeout from 5 seconds to 30 seconds.

Modifications:
- Increase timeout from 5 to 30 seconds.

Result:
Less false failures due to slower CI machines.
2017-07-19 07:59:56 +02:00
Scott Mitchell
b249714a2d DNS Resovler tests should be more explicit about ndots
Motivation:
The DNS resolver may use default configuration inherited from the environment. This means the ndots value may change and result in test failure if the tests don't explicitly set the assumed value.

Modifications:
- Explicitly set ndots in resolver-dns unit tests so we don't fail if the environment overrides the search domain and ndots

Result:
Unit tests are less dependent upon the enviroment they run in.
Fixes https://github.com/netty/netty/issues/6966.
2017-07-12 15:49:45 -07:00
Norman Maurer
2a376eeb1b [maven-release-plugin] prepare for next development iteration 2017-07-06 13:24:06 +02:00
Norman Maurer
c7f8168324 [maven-release-plugin] prepare release netty-4.1.13.Final 2017-07-06 13:23:51 +02:00
Scott Mitchell
d3581b575e UnixResolverDnsServerAddressStreamProvider should allow for empty /etc/resolver dir
Motivation:
UnixResolverDnsServerAddressStreamProvider currently throws an exception if /etc/resolver exists but it empty. This shouldn't be an exception and can be tolerated as if there is no contribution from /etc/resolver.

Modifications:
- Treat /etc/resolver as present and empty the same as not being present

Result:
UnixResolverDnsServerAddressStreamProvider initialization can tolerate empty /etc/resolver directory.
2017-07-05 20:14:27 -04:00
Scott Mitchell
d040c939e5 UnixResolverDnsServerAddressStreamProviderTest failure
Motivation:
InetSocketAddress#getHostName() may attempt a reverse lookup which may lead to test failures because the expected address will not match.

Modifications:
- Use InetSocketAddress#getHostString() which will not attempt any lookups and instead return the original String

Result:
UnixResolverDnsServerAddressStreamProviderTest is more reliable.
2017-07-05 14:45:04 -04:00
Scott Mitchell
6d80c641e9 DNS Resolver should be more consistent with JDK resolution
Motivation:
If there are multiple DNS servers to query Java's DNS resolver will attempt to resolve A and AAAA records in sequential order and will terminate with a failure once all DNS servers have been exhausted. Netty's DNS server will share the same DnsServerAddressStream for the different record types which may send the A question to the first host and the AAAA question to the second host. Netty's DNS resolution also may not progress to the next DNS server in all situations and doesn't have a means to know when resolution has completed.

Modifications:
- DnsServerAddressStream should support new methods to allow the same stream to be used to issue multiple queries (e.g. A and AAAA) against the same host.
- DnsServerAddressStream should support a method to determine when the stream will start to repeat, and therefore a failure can be returned.
- Introduce SequentialDnsServerAddressStreamProvider for sequential use cases

Result:
Fixes https://github.com/netty/netty/issues/6926.
2017-07-05 09:10:59 -04:00
Scott Mitchell
efe37e0d28 UnknownHostException should mention search domain if used
Motivation:
ba80fbbe05 modified the UnknownHostException to not include the search domain if the DNS query failed, but this masks what DNS query actually failed. Have the full hostname (including the search domain) provides more visibility and may help diagnose a configuration error if queries are unexpectedly failing.

Modifications:
- Remove DnsNameResolverContext#pristineHostname

Result:
UnknownHostException is more accurate and reflect what hostname actually resulted in failure.
2017-06-23 16:44:07 -07:00
Scott Mitchell
1767814a46 Replace DnsNameResolverContext#trace special code with an implementation of DnsQueryLifecycleObserver
Motivation:
DnsQueryLifecycleObserver is designed to capture the life cycle of every query. DnsNameResolverContext has a custom trace mechanism which consists of a StringBuilder and manual calls throughout the class. We can remove some special case code in DnsNameResolverContext and instead use a special implementation of DnsQueryLifecycleObserver when trace is enabled.

Modifications:
- Remove all references to the boolean trace variables in DnsNameResolverContext and DnsNameResolver
- Introduce TraceDnsQueryLifecycleObserver which will be used when trace is enabled and will log similar data as what trace currently provides

Result:
Less special case code in DnsNameResolverContext and instead delegate to TraceDnsQueryLifecycleObserver to capture trace information.
2017-06-23 09:04:59 -07:00
Scott Mitchell
6cd086050f DNS Resolver Search Domain Bugs
Motivation:
The DNS resolver supports search domains. However the ndots are not correctly enforced. The search domain should only be appended under the following scenario [1]:

> Resolver queries having fewer than ndots dots (default is 1) in them will be attempted using each component of the search path in turn until a match is found.

The DNS resolver current appends the search domains if ndots is 0 which should never happen (because no domain can have less than 0 dots).

[1] https://linux.die.net/man/5/resolv.conf

Modifications:
- Parse /etc/resolv.conf to get the default value for ndots on Unix platforms
- The search domain shouldn't be used if ndots is 0
- Avoid failing a promise to trigger the search domain queries in DnsNameResolverContext#resolve

Result:
More correct usage of search domains in the DNS resolver.
Fixes https://github.com/netty/netty/issues/6844.
2017-06-22 00:05:43 -07:00
Stephane Landelle
ca5ed7c114 Let DnsNameResolver constructor use a default value for searchDomains
Motivation:

It’s currently complicated to extend `DnsNameResolver` as the default
value for `searchDomain` is package private.

Modifications:

* let `DnsNameResolver` accept a null `searchDomains` and then default
to `DEFAULT_SEARCH_DOMAINS`, just like it’s being done with
`resolvedAddressTypes`.
* set default `DnsNameResolverBuilder#searchDomains` value to null to
avoid cloning internal `DnsNameResolver.DEFAULT_SEARCH_DOMAINS` in
`DnsNameResolver` constructor.

Result:

More versatile `DnsNameResolver` constructor.
No array copy when using default search domains.
2017-06-13 15:43:08 +02:00