Commit Graph

177 Commits

Author SHA1 Message Date
Scott Mitchell
0f1a2ca5ae UnixResolverDnsServerAddressStreamProvider default name server selection and ordering bug
Motivation:
UnixResolverDnsServerAddressStreamProvider allows the default name server address stream to be null, but there should always be a default stream to fall back to ([1] Search Strategy).
UnixResolverDnsServerAddressStreamProvider currently shuffles the names servers are multiple are present, but the defined behavior is to try them sequentially [2].

[1] Search Strategy Section - https://developer.apple.com/legacy/library/documentation/Darwin/Reference/ManPages/man5/resolver.5.html
[2] DESCRIPTION/nameserver Section - https://developer.apple.com/legacy/library/documentation/Darwin/Reference/ManPages/man5/resolver.5.html

Modifications:
- UnixResolverDnsServerAddressStreamProvider should always use the first file provided to derive the default domain server address stream. Currently if there are multiple domain names in the file identified by the first argument of the constructor then one will be selected at random.
- UnixResolverDnsServerAddressStreamProvider should return name servers sequentially.
- Reduce access level on some methods which don't have known use-cases externally.

Result:
Fixes https://github.com/netty/netty/issues/6736
2017-05-18 15:14:58 -07:00
Norman Maurer
f65885fc54 Make DnsNameResolverTest pass on Java7
Motivation:

IDN.toUnicode(...) removes trailing dots when used in Java7 while it not does on java8.

Modifications:

Check if we should test with the trailing dot removed or not.

Result:

Test pass on Java7 as well.
2017-05-05 09:27:08 -07:00
Scott Mitchell
5a2d04684e DNS Resolver visibility into individual queries
Motivation:
A single DNS query may follow many different paths through resolver-dns. The query may fail for various reasons related to the DNS protocol, general IO errors, it may be cancelled due to the query count being exceeded, or other reasons. A query may also result in other queries as we follow the DNS protocol (e.g. redirects, CNAME, etc...). It is currently impossible to collect information about the life cycle of an individual query though resolver-dns. This information may be valuable when considering which DNS servers are preferred over others.

Modifications:
- Introduce an interface which can provide visibility into all the potential outcomes of an individual DNS query

Result:
resolver-dns provides visibility into individual DNS queries which can be used to avoid poorly performing DNS servers.
2017-04-27 15:17:20 -07:00
Nikolay Fedorovskikh
970d310ec9 Regulation of the InternetProtocolFamily usage
Motivation:

1. The use of InternetProtocolFamily is not consistent:
   the DnsNameResolverContext and DnsNameResolver contains switches
   instead of appropriate methods usage.
2. The InternetProtocolFamily class contains redundant switches in the
   constructor.

Modifications:

1. Replacing switches to the use of an appropriate methods.
2. Simplifying the InternetProtocolFamily constructor.

Result:

Code is cleaner and simpler.
2017-04-20 05:22:24 +02:00
Nikolay Fedorovskikh
0692bf1b6a fix the typos 2017-04-20 04:56:09 +02:00
Norman Maurer
a0fcb72e5d Use jndi-dns to obtain default name servers
Motivation:

Using reflection to obtain the default name servers may fail in Java9 and also in previous Java versions if a SecurityManager is present.

Modifications:

Try using jndi-dns to obtain default name servers and only try using reflection if this fails.

Result:

Be able to detect default name servers in all cases. Fixes [#6347].
2017-04-19 12:24:06 +02:00
Scott Mitchell
155983f1a1 DNS move JDK DNS resolution out of DnsServerAddresses static initialization
Motivation:
DnsServerAddresses loads the default DNS servers used for DNS resolution in a static initialization block. This is subject to blocking and may cause unexpected delays. We can move this initialization to DefaultDnsServerAddressStreamProvider where it is more expected to load the JDK's default configuration.

Modifications:
- Move all the static initialization from DnsServerAddresses to DefaultDnsServerAddressStreamProvider
- Deprecate static methods in DnsServerAddresses which have moved to DefaultDnsServerAddressStreamProvider
- Remove usage of deprecated methods in DnsServerAddresses

Result:
Usage of JDK's blocking DNS resolver is not required to use resolver-dns.
2017-04-06 18:09:58 -07:00
Trustin Lee
08646afc1e Do not fail a DNS query promise prematurely
Motivation:

DnsNameResolverContext completes its DNS query promise automatically
when no queries are in progress, which means there's no need to fail the
promise explicitly.

Modifications:

- Do not fail a DNS query promise explicitly but add an informational
  trace

Result:

- Fixes #6600
- Unexpected exception on one question type does not fail the promise
  too soon. If the other question succeeds, the query will succeed,
  making the resolver more robust.
2017-04-06 17:58:54 -07:00
Scott Mitchell
e074df2ae6 DNS Resolve ambiguity in which DNS servers are used during resolution
Motivation:
Recently DnsServerAddressStreamProvider was introduced to allow control for each query as to which DNS server should be used for resolution to respect the local host's default DNS server configuration. However resolver-dns also accepts a stream of DNS servers to use by default, but this stream is not host name aware. This creates an ambiguity as to which method is used to determine the DNS server to user during resolution, and in which order. We can remove this ambiguity and provide a more general API by just supporting DnsServerAddressStreamProvider.

Modifications:
- Remove the fixed DnsServerAddresses and instead only accept a DnsServerAddressStreamProvider.
- Add utility methods to help use DnsServerAddressStreamProvider for a single entry, a list of entries, and get the default for the current machine.

Result:
Fixes https://github.com/netty/netty/issues/6573.
2017-03-31 15:29:49 -07:00
Norman Maurer
fbf0e5f4dd Prefer JDK ThreadLocalRandom implementation over ours.
Motivation:

We have our own ThreadLocalRandom implementation to support older JDKs . That said we should prefer the JDK provided when running on JDK >= 7

Modification:

Using ThreadLocalRandom implementation of the JDK when possible.

Result:

Make use of JDK implementations when possible.
2017-02-16 15:44:00 -08:00
Scott Mitchell
9ce74d46c1 Correct unit test flaw introduced in 54c9ecf682
Motivation:
54c9ecf682 introduced a unit tests which attempted to exclude addresses which resolved to loop back addresses from an assert statement. This was done with a static check for localhost but depending on machine configuration it is possible for other interfaces to be resolved.

Modifications:
- Use InetAddress#isLoopbackAddress() instead of string match on localhost

Result:
DnsNameResolverTest#testNameServerCache is more reliable.
2017-02-13 18:36:06 -08:00
Scott Mitchell
54c9ecf682 DnsNameResolver should respect /etc/resolv.conf and /etc/resolver
Motivation:
The JDK uses gethostbyname for blocking hostname resoltuion. gethostbyname can be configured on Unix systems according to [1][2]. This may impact the name server that is used to resolve particular domains or just override the default fall-back resolver. DnsNameResolver currently ignores these configuration files which means the default resolution behavior is different than the JDK. This may lead to unexpected resolution failures which succeed when using the JDK's resolver.

Modifications:
- Add an interface which can override what DnsServerAddressStream to use for a given hostname
- Provide a Unix specific implementation of this interface and implement [1][2]. Some elements may be ignored sortlist, timeout, etc...

Result:
DnsNameResolver behaves more like the JDK resolver by default.

[1] https://linux.die.net/man/5/resolver
[2] https://developer.apple.com/legacy/library/documentation/Darwin/Reference/ManPages/man5/resolver.5.html
2017-02-13 11:54:09 -08:00
Norman Maurer
667cbe9923 Fix compilation error introduced by 81f9de423c 2017-02-11 09:17:13 +01:00
Stephane Landelle
81f9de423c HostsFileParser should allow both IPv4 and IPv6 for a given host
Motivation:

HostsFileParser only retains the first address for each given hostname.
This is wrong, and it’s allowed to have both an IPv4 and an IPv6.

Modifications:

* Have `HostsFileParser` now return a `HostsFileEntries` that contains IPv4 entries and IPv6 entries
* Introduce `ResolvedAddressTypes` to describe resolved address types preferences
* Add a new `ResolvedAddressTypes` parameter to `HostsFileEntriesResolver::address` to account for address types preferences
* Change `DnsNameResolver` constructor to take a `ResolvedAddressTypes`, allowing for a null value that would use default
* Change `DnsNameResolverBuilder::resolvedAddressTypes` to take a `ResolvedAddressTypes`
* Make `DnsNameResolver::resolvedAddressTypes` return a `ResolvedAddressTypes`
* Add a static `DnsNameResolverBuilder::computeResolvedAddressTypes` to ease converting from `InternetProtocolFamily`

Result:

We now support hosts files that contains IPv4 and IPv6 pairs for a same
hostname.
2017-02-10 20:09:32 -08:00
Scott Mitchell
007048dddd DnsNameResolver empty/null hostname missed by a416b79
Motivation:
a416b79 introduced a check for null or empty host name to be compatible with the JDK resolution. However the doResolve(String, Promise) method, and if the doResolve(String, DnsRecord[], Promise, DnsCache) method was overridden the empty/null hostname would not be correctly resolved.

Modifications:
- Move the empty/null host name check into the lowest level doResolve method in DnsNameResolver
- Remove the duplicate logic in InetNameResolver.java which can be bypassed anyways

Result:
By default (unless behavior is overridden) DnsNameResolver resolves null/empty host names to local host just like the JDK.
2017-02-09 09:22:27 -08:00
Norman Maurer
3462a86a3a Ensure we release the previous retained AddressedEnvelope when we fail to notify the promise.
Motivation:

We need to ensure we release the AddressedEnvelope if we fail to notify the future (as it may be notified before because of an timeout). Otherwise we may leak.

Modifications:

Call release() if we fail to notify the future.

Result:

No more memory leak on notify failure.
2017-02-09 10:11:07 +01:00
Norman Maurer
661ff2538e Implement correct handling of recursive DNS
Motivation:

DnsNameResolver does not handle recursive DNS and so fails if you query a DNS server (for example a ROOT dns server) which provides the correct redirect for a domain.

Modification:

Add support for redirects (a.k.a. handling of AUTHORITY section').

Result:

Its now possible to use a DNS server that redirects.
2017-02-06 20:33:52 +01:00
Norman Maurer
0d5b665fba Automatically decode DNS domain name to unicode
Motivation:

DnsNameResolver will return the domain / host name as ascii code using punycode (https://tools.ietf.org/html/rfc3492). This is different to what the JDK does which always convert it to unicode. We should do the same by default but allow to also not do it.

Modifications:

- Add new builder method on DnsNameResolverBuilder which allow to disable / enable converting. Default is to convert just like the JDK does.
- Add unit tests for it.

Result:

DnsNameResolver and JDK impl behave the same way.
2017-01-31 09:28:57 +01:00
Norman Maurer
a416b79d86 DnsNameResolver.resolve*(...) never notifies the Future when empty hostname is used.
Motivation:

When an empty hostname is used in DnsNameResolver.resolve*(...) it will never notify the future / promise. The root cause is that we not correctly guard against errors of IDN.toASCII(...) which will throw an IllegalArgumentException when it can not parse its input. That said we should also handle an empty hostname the same way as the JDK does and just use "localhost" when this happens.

Modifications:

- If the try to resolve an empty hostname we use localhost
- Correctly guard against errors raised by IDN.toASCII(...) so we will always noify the future / promise
- Add unit test.

Result:

DnsNameResolver.resolve*(...) will always notify the future.
2017-01-24 21:21:49 +01:00
Tim Brooks
3344cd21ac Wrap operations requiring SocketPermission with doPrivileged blocks
Motivation:

Currently Netty does not wrap socket connect, bind, or accept
operations in doPrivileged blocks. Nor does it wrap cases where a dns
lookup might happen.

This prevents an application utilizing the SecurityManager from
isolating SocketPermissions to Netty.

Modifications:

I have introduced a class (SocketUtils) that wraps operations
requiring SocketPermissions in doPrivileged blocks.

Result:

A user of Netty can grant SocketPermissions explicitly to the Netty
jar, without granting it to the rest of their application.
2017-01-19 21:12:52 +01:00
Norman Maurer
ead87b7df8 Respect resolvedAddressTypes when follow CNAME records.
Motivation:

When we follow CNAME records we should respect resolvedAddressTypes and only query A / AAAA depending on which address types are expected.

Modifications:

Check if we should query A / AAAA when follow CNAMEs depending on resolvedAddressTypes.

Result:

Correct behaviour when follow CNAMEs.
2017-01-19 19:36:56 +01:00
Norman Maurer
d7ff71a3d1 Check if DnsCache is null in DnsNameResolver constructor.
Motivation:

We miss checking if DnsCache is null in DnsNameResolver constructor which will later then lead to a NPE. Better fail fast here.

Modifications:

Check for null and if so throw a NPE.

Result:

Fail fast.
2017-01-19 07:54:36 +01:00
Norman Maurer
89e93968ac Remove usage of own Atomic*FieldUpdater in favor of JDKs
Motivation:

In later Java8 versions our Atomic*FieldUpdater are slower then the JDK implementations so we should not use ours anymore. Even worse the JDK implementations provide for example an optimized version of addAndGet(...) using intrinsics which makes it a lot faster for this use-case.

Modifications:

- Remove methods that return our own Atomic*FieldUpdaters.
- Use the JDK implementations everywhere.

Result:

Faster code.
2016-12-15 08:09:06 +00:00
Norman Maurer
705e3f629a Not use InternalThreadLocalMap where access may be done from outside the EventLoop.
Motivation:

We should not use the InternalThreadLocalMap where access may be done from outside the EventLoop as this may create a lot of memory usage while not be reused anyway.

Modifications:

Not use InternalThreadLocalMap in places where the code-path will likely be executed from outside the EventLoop.

Result:

Less memory bloat.
2016-11-10 14:37:16 +01:00
Dmitry Spikhalskiy
eb7f8e4dc5 Expose RoundRobinInetAddressResolver
Motivation:
Make small refactoring for recently merged PR #5867 to make the code more flexible and expose aggressive round robin as a NameResolver too with proper code reuse.

Modifications:
Round robin is a method of hostname resolving - so Round robin related code fully moved to RoundRobinInetAddressResolver implements NameResolver<InetAddress>, RoundRobinInetSocketAddressResolver is deleted as a separate class, instance with the same functionality could be created by calling #asAddressResolver.

Result:
New forced Round Robin code exposed not only as an AddressResolver but as a NameResolver too, more proper code and semantic reusing of InetNameResolver and InetSocketAddressResolver classes.
2016-11-02 06:52:19 +01:00
James Yuzawa
efd118ddec Support aggressive round-robin dns
Motivation:

Suppose the domain `foo.example.com` resolves to the following ip
addresses `10.0.0.1`, `10.0.0.2`, `10.0.0.3`. Round robin DNS works by
having each client probabilistically getting a different ordering of
the set of target IP’s, so connections from different clients (across
the world) would be split up across each of the addresses. Example: In
a `ChannelPool` to manage connections to `foo.example.com`, it may be
desirable for high QPS applications to spread the requests across all
available network addresses. Currently, Netty’s resolver would return
only the first address (`10.0.0.1`) to use. Let say we are making
dozens of connections. The name would be resolved to a single IP and
all of the connections would be made to `10.0.0.1`. The other two
addresses would not see any connections. (they may see it later if new
connections are made and `10.0.0.2` is the first in the list at that
time of a subsequent resolution). In these changes, I add support to
select a random one of the resolved addresses to use on each resolve
call, all while leveraging the existing caching and inflight request
detection. This way in my example, the connections would be make to
random selections of the resolved IP addresses.

Modifications:

I added another method `newAddressResolver` to
`DnsAddressResolverGroup` which can be overriden much like
`newNameResolver`. The current functionality which creates
`InetSocketAddressResolver` is still used. I added
`RoundRobinDnsAddressResolverGroup` which extends
DnsAddressResolverGroup and overrides the `newAddressResolver` method
to return a subclass of the `InetSocketAddressResolver`. This subclass
is called `RoundRobinInetSocketAddressResolver` and it contains logic
that takes a `resolve` request, does a `resolveAll` under the hood, and
returns a single element at random from the result of the `resolveAll`.

Result:

The existing functionality of `DnsAddressResolverGroup` is left
unchanged. All new functionality is in the
`RoundRobinInetSocketAddressResolver` which users will now have the
option to use.
2016-10-10 11:08:44 +02:00
Norman Maurer
dfa3bbbf00 Add support for Client Subnet in DNS Queries (RFC7871)
Motivation:

RFC7871 defines an extension which allows to request responses for a given subset.

Modifications:

- Add DnsOptPseudoRrRecord which can act as base class for extensions based on EDNS(0) as defined in RFC6891
- Add DnsOptEcsRecord to support the Client Subnet in DNS Queries extension
- Add tests

Result:

Client Subnet in DNS Queries extension is now supported.
2016-09-06 07:16:57 +02:00
Trustin Lee
9fef4ba1bf Disable IPv6 address lookups when -Djava.net.preferIPv4Stack=true
Motivation:

According to the Oracle documentation:

> java.net.preferIPv4Stack (default: false)
>
> If IPv6 is available on the operating system, the underlying native
> socket will be an IPv6 socket. This allows Java applications to connect
> to, and accept connections from, both IPv4 and IPv6 hosts.
>
> If an application has a preference to only use IPv4 sockets, then this
> property can be set to true. The implication is that the application
> will not be able to communicate with IPv6 hosts.

which means, if DnsNameResolver returns an IPv6 address, a user (or
Netty) will not be able to connect to it.

Modifications:

- Move the code that retrieves java.net.prefer* properties from
  DnsNameResolver to NetUtil
- Add NetUtil.isIpV6AddressesPreferred()
- Revise the API documentation of NetUtil.isIpV*Preferred()
- Set the default resolveAddressTypes to IPv4 only when
  NetUtil.isIpv4StackPreferred() returns true

Result:

- Fixes #5657
2016-08-10 11:10:34 +02:00
Trustin Lee
b2f1ef57c8 Fix RejectedExecutionException when using DnsAddressResolverGroup
Motivation:

AddressResolverGroup adds a listener to the termination future of an
EventExecutor when a new AddressResolver is created. The listener calls
AddressResolver.close() when the EventExecutor is terminated to give the
AddressResolver a chance to release its resources.

When using DnsAddressResolverGroup, the AddressResolver.close() will
eventually trigger DnsNameResolver.close(), which closes its underlying
DatagramChannel.

DatagramChannel.close() (or any Channel.close()) will travel through
pipeline and trigger EventExecutor.execute() because
DnsNameResolver.close() has been invoked from a non-I/O thread.
(NB: A terminationFuture is always notified from the GlobalEventExecutor
thread.)

However, because we are doing this in the listener of the termination
future of the terminated EventLoop we are trying to execute a task upon,
the attempt to close the channel fails due to RejectedExecutionException.

Modifications:

- Do not call Channel.close() in DnsNameResolver.close() if the Channel
  has been closed by EventLoop already

Result:

No more RejectedExecutionException when shutting down an event loop.
2016-08-01 10:21:10 +02:00
alexlehm
ba80fbbe05 UnknownHostException mentions hostname with search domain added
Motivation:

When a hostname cannot be resolved, the message in the UnknownHostException mentions the hostname with the last attempted search domain appended, which is kind of confusing. I would prefer to see the original hostname supplied to the method in the exception.

Modifications:

Store the pristine hostname in the resolver context and use it to create the exception message instead of the hostname with search domain.
Add unit test to check that the exception does not mention the search domain.

Result:

The exception mentions the unmodified hostname in the message.
2016-08-01 07:20:19 +02:00
Julien Viet
8d4cfd9002 Allow ndots=0 in DnsNameResolver and search domains - fixes #5570
Motivation:

The ndots = 0 is a valid value for ndots, it means that when using a non dotted name, the resolution should first try using a search and if it fails then use subdomains. Currently it is not allowed. Docker compose uses this when wiring up containers as names have usually no dots inside.

Modification:

Modify DnsNameResolver to accept ndots = 0 and handle the case in the resolution procedure. In this case a direct search is done and then a fallback on the search path is performed.

Result:

The ndots = 0 case is implemented.
2016-07-24 20:41:14 +02:00
alexlehm
73c0fb0e23 Construct LOCALHOST4 and LOCALHOST6 object with hostname "localhost"
Motivation:

When resolving localhost on Windows where the hosts file does not contain a localhost entry by default, the resulting InetAddress object returned by the resolver does not have the hostname set so that getHostName returns the ip address 127.0.0.1. This behaviour is inconsistent with Windows where the hosts file does contain a localhost entry and with Linux in any case. It breaks at least some unit tests.

Modifications:

Create the LOCALHOST4 and LOCALHOST6 objects with hostname localhost in addition to the address.
Add unit test domain localhost to DnsNameResolverTest to check the resolution of localhost with ipv4 at least.

Result:

The resolver returns a InetAddress object for localhost with the hostname localhost in all cases.
2016-07-20 05:55:45 +02:00
Julien Viet
79c8ec4d33 DnsNameResolver search domains support
Motivation:

The current DnsNameResolver does not support search domains resolution. Search domains resolution is supported out of the box by the java.net resolver, making the DnsNameResolver not able to be a drop in replacement for io.netty.resolver.DefaultNameResolver.

Modifications:

The DnsNameResolverContext resolution has been modified to resolve a list of search path first when it is configured so. The resolve method now uses the following algorithm:

if (hostname is absolute (start with dot) || no search domains) {
 searchAsIs
} else {
  if (numDots(name) >= ndots) {
    searchAsIs
  }
  if (searchAsIs wasn't performed or failed) {
    searchWithSearchDomainsSequenciallyUntilOneSucceeds
  }
}

The DnsNameResolverBuilder provides configuration for the search domains and the ndots value. The default search domains value is configured with the OS search domains using the same native configuration the java.net resolver uses.

Result:

The DnsNameResolver performs search domains resolution when they are present.
2016-07-08 22:04:01 +02:00
Julien Viet
804e058e27 DnsNameResolver should not bind locally. Fixes #5457 Motivation: Dns resolution failures happen when using the DnsNameResolver and the JVM is not authorized to bind datagram channels. The current DnsNameResolver binds locally a DatagramChannel which is not necessary (and not always permitted).
Modifications:
The DnsNameResolver Bootstrap does not bind anymore, instead it registers and use the channel directly. The localAddress has also been removed from the DnsAddressResolverGroup, DnsNameResolver and DnsNameResolverBuilder as it is not necessary anymore and the API is marked as @UnstableApi.

Result:
Dns resolution does not require anymore to bind locally.
2016-06-28 15:39:42 +02:00
joymufeng
2562ef7cbe Fix a bug of DnsNameResolver while working with NoopDnsCache.
Motivation:

If DnsNameResolver works with NoopDnsCache, IndexOutOfBoundsException will
be thrown.

Modifications:

Test if the result of DnsNameResolver.get(hostname) is empty before
accessing it's elements.
2016-06-27 13:43:06 +02:00
Norman Maurer
a725b97092 [#5386] DnsNameResolver does not resolve localhost on Windows
Motivation:

On Windows localhost is not in hosts file and the DNS server does not resolve this address either, i.e it is handled by the Windows API. So using a Bootstrap (among others) with the resolver based on DnsNameResolver will not resolve localhost.

Modifications:

Workaround behavior of Windows

Result:

Correctly resolve localhost on Windows when using DnsNameResolver
2016-06-22 09:07:39 +02:00
Norman Maurer
c7a0a0f325 [#5391] DnsNameResolver does not resolve property A+CNAME answer
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.
2016-06-20 07:13:00 +02:00
Guido Medina
c3abb9146e Use shaded dependency on JCTools instead of copy and paste
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
2016-06-10 13:19:45 +02:00
Norman Maurer
ee5969edfd Use higher maxQueriesPerResolve and make exception message more clear.
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.
2016-06-06 09:22:11 +02:00
Norman Maurer
9bd94ea021 Make DnsAddressResolverGroup easier to extend
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
2016-06-04 17:40:45 +02:00
Trustin Lee
a517cce92f Do not send duplicate DNS queries when the same query is in progress already
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
2016-05-17 15:07:36 +02:00
Norman Maurer
9229ed98e2 [#5088] Add annotation which marks packages/interfaces/classes as unstable
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.
2016-05-09 15:16:35 +02:00
Norman Maurer
b0242585d7 Cleanup code and so eliminate warnings.
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.
2016-03-23 09:38:58 +01:00
Trustin Lee
ef8dcae9af Fix potential infinite loop when resolving CNAME records
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
2016-03-07 15:12:26 +00:00
Trustin Lee
5ce504070f Remove a unused import 2016-02-24 11:17:35 +09:00
Lukáš Karas
b426566617 DnsNameResolver: makes possible to define additional records in DNS query
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.
2016-02-23 10:36:30 +01:00
Xiaoyan Lin
b7415a3307 Add a reusable ArrayList to InternalThreadLocalMap
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.
2016-02-01 15:49:28 +01:00
Stephane Landelle
391a411264 Init DnsNameResolverBuilder#nameServerAddresses
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.
2016-01-20 13:38:01 +01:00
Xiaoyan Lin
9e76b5319e Fix testResolveIp to make it work in some special environment
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
2016-01-18 09:36:33 +01:00
Stephane Landelle
b4be040f30 Introduce DnsCache API + DnsResolver extensibility
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.
2016-01-08 14:46:47 +01:00
Xiaoyan Lin
a96d52fe66 Fix javadoc links and tags
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.
2015-12-26 08:34:31 +01:00
Stephane Landelle
8d4db050f3 Have hosts file support for DnsNameResolver, close #4074
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.
2015-12-17 15:15:42 +01:00
Stephane Landelle
6393506b97 Extract SocketAdress logic from NameResolver
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>.
2015-12-14 14:03:50 +01:00
Trustin Lee
2fefb2f79c Make DnsNameResolverGroup non-final and overridable
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.
2015-12-07 19:40:01 +09:00
Anuraag Agrawal
a3bdd8c948 Don't cycle DNS servers while cycling DNS record types.
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.
2015-12-07 18:32:39 +09:00
Trustin Lee
120ffaf880 Use separate query ID space for different DNS servers
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
2015-11-09 15:25:13 -08:00
Trustin Lee
07d861c5ff Fix a bug where DnsNameResolver attempts to send extra queries
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.
2015-10-30 08:16:24 +01:00
Norman Maurer
42e6b8fa86 [#4289] Use a mock DNS Server for dns tests.
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.
2015-10-10 20:27:34 +02:00
Trustin Lee
115d3576be Update the public DNS server list
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.
2015-09-25 11:33:33 +09:00
Trustin Lee
e1bf9d6257 Fix unintended timeout in negative DNS lookup cache test
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.
2015-08-29 11:40:32 +09:00
Trustin Lee
d45ad9ec68 Add a bunch of OpenNIC DNS servers for more reliable DNS tests 2015-08-28 21:36:38 +09:00
Trustin Lee
73f472b65d Fix DNS lookup hang / Remove Comodo Secure DNS
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
2015-08-27 12:52:29 +09:00
Trustin Lee
75efe016d6 Add more public DNS servers to DnsNameResolverTest
so that we have more chance of passing the test when some DNS servers
are unavailable or throtlling us.
2015-08-27 12:25:38 +09:00
Trustin Lee
719d1dbad1 Replace infinite Iterable/Iterator with dedicated types
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
2015-08-26 17:38:43 +09:00
Trustin Lee
ad0b7ca56d Add a test case for DNS resolver cache for negative loopups
Related issue: #4065
2015-08-18 18:44:51 +09:00
Trustin Lee
1856ab3a35 Make DnsNameResolverTest.testQueryMx() more robust
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
2015-08-18 18:26:11 +09:00
Trustin Lee
fdfe3149ba Provide more control over DnsNameResolver.query() / Add NameResolver.resolveAll()
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.
2015-08-18 17:40:13 +09:00
Norman Maurer
d1344345bb DnsResolver.resolve(...) fails when ipaddress is used.
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.
2015-07-18 17:14:05 +02:00
Trustin Lee
311532feb0 Fix IllegalReferenceCountException in DnsNameResolver
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.
2015-06-03 19:17:56 +09:00
Trustin Lee
63a02fc04e Revamp DNS codec
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
2015-05-01 11:33:16 +09:00
Trustin Lee
79bb200f59 Remove thepiratebay.se from the test domain list
.. due to its instability
2014-12-22 22:35:17 +09:00
Trustin Lee
23db94f5a1 Fix Java 6 compatibility issue in DnsNameResolver
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.
2014-12-06 22:31:44 +09:00
Jay
2769ad428a Check the bindFuture before writing a DNS query
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
2014-12-01 19:46:18 +09:00
Trustin Lee
c0079840be Improve DnsNameResolverTest.testResolveA()
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.
2014-10-25 17:29:06 +09:00
Trustin Lee
e1787e6876 Fix another resource leak in DnsNameResolver
- Fix a bug in cache expiration task; wrong object was being released
- Added more sanity checks when caching an entry
2014-10-17 11:40:06 +09:00
Trustin Lee
c811d50d61 Fix resource leak in DnsNameResolver 2014-10-16 17:57:32 +09:00
Trustin Lee
e848066cab Name resolver API and DNS-based name resolver
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.
2014-10-16 17:05:20 +09:00