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 should keep the scope of the synchronized block as small as possible.
Modifications:
Reduce scope by copy to an array first before iterate through it
Result:
Smaller scope of synchronized
Motivation:
When an `AddressResolverGroup` closes, it can leave unwanted listeners attached to its `EventExecutor`'s termination future.
Modifications:
- Keep track of listeners attached to termination futures
- Clear listeners if the `AddressResolverGroup` closes before its associated executor(s)
Result:
Unwanted listeners no longer remain in memory after an `AddressResolverGroup` closes before its associated executor(s).
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:
RoundRobinDnsAddressResolverGroup ultimately opens UDP
ports for DNS resolution. Callers likely expect that
RoundRobinDnsAddressResolverGroup#close() will close those
ports, but that is not currently true (see #9212).
Modifications:
Overrode RoundRobinInetAddressResolver#close() to close
the delegate name resolver, which in turn closes any UDP
ports used for name resolution.
Result:
RoundRobinDnsAddressResolverGroup#close() closes UDP ports
as expected. This fixes#9212.
Motivation:
We can just use Objects.requireNonNull(...) as a replacement for ObjectUtil.checkNotNull(....)
Modifications:
- Use Objects.requireNonNull(...)
Result:
Less code to maintain.
Motivation:
We can use lambdas now as we use Java8.
Modification:
use lambda function for all package, #8751 only migrate transport package.
Result:
Code cleanup.
Motivation:
Custom Netty ThreadLocalRandom and ThreadLocalRandomProvider classes are no longer needed and can be removed.
Modification:
Remove own ThreadLocalRandom
Result:
Less code to maintain
Motivation:
We can use the diamond operator these days.
Modification:
Use diamond operator whenever possible.
Result:
More modern code and less boiler-plate.
Motivation:
In windows if the project is in a path that contains whitespace,
resources cannot be accessed and tests fail.
Modifications:
Adds ResourcesUtil.java in netty-common. Tests use ResourcesUtil.java to access a resource.
Result:
Being able to build netty in a path containing whitespace
Motivation:
We should support to parse and read a hosts file which is stored in a different encoding then the system default. Beside this when we are on windows we should just try to parse it with multiple different charset before giving up as there is no real standard what charset to use.
Modifications:
- Add more method overloads to HostsFileParser that take a Charset.
- Try to parse with multiple Charsets in DefaultHostsFileEntriesResolver when windows is used.
- Add unit test
Result:
Fixes https://github.com/netty/netty/issues/8208.
Motivation
There is a cost to concatenating strings and calling methods that will be wasted if the Logger's level is not enabled.
Modifications
Check if Log level is enabled before producing log statement. These are just a few cases found by RegEx'ing in the code.
Result
Tiny bit more efficient code.
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.
Motivation:
The resolver package had some changes late in the 4.1.CR phase and the intention was to mark this package as unstable until these interfaces solidify, but we forgot to mark the package and public classes with the unstable annotation.
Modifications:
- resolver package public interfaces and package-info should be annotated with @UnstableApi
Result:
The unstable nature of the resolver package is more clearly communicated.
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.
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.
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.
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.
Motivation:
Windows 7 hosts file is empty by default (at least on my machine? see
http://serverfault.com/questions/4689/windows-7-localhost-name-resolution-is-handled-within-dns-itself-why
for details and reasoning.
the test relies on the file containing an entry for localhost.
Modifications:
refactor class code to 1st normalize the input host name and then look it up, change the test to verify
that hostnames are normalized in a case-insensitive way before being looked up (which was the intent
of the original test)
Result:
test should pass on vanilla windows 7 (and any other machine with no
localhost in the hosts file). no effect anywhere else or on actual netty
code.
Signed-off-by: radai-rosenblatt <radai.rosenblatt@gmail.com>
Motivation:
Now the ```resolveAll``` method of RoundRobinInetAddressResolver returns results without any rotation and shuffling. As a result, it doesn't force any round-robin for clients that get a result of ```resolveAll``` and use addresses from the result one by one for a connection establishing until success. This commit implements round-robin in RoundRobinInetAddressResolver#resolveAll. These improvements inspired by the discussion here: https://github.com/AsyncHttpClient/async-http-client/issues/1285
Modifications:
Rotate collection from internal ```resolveAll``` call by index, which is incremented every call to RoundRobinInetAddressResolver#resolveAll method.
Random replaced by an incrementing counter, which makes code cheaper and guarantees predictable address order in tests.
Result:
Improved ```RoundRobinInetAddressResolver``` is compatible with clients that use ```resolveAll``` result.
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.
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.
Motivation:
InetSocketAddressResolver.close() must call close() on the wrapped NameResolver.
Modifications:
Correctly call close() on wrapped NameResolver and added test.
Result:
close() is correctly propergated to the wrapped resolver.
Motivation:
Resolving hosts via the /etc/hosts file should be case-insensitive, e.g. localhost and LOCALHOST refer to the same host, this is the same that is applied to dns queries.
Modifications:
Store hosts Map with lowercase keys, lookup the keys as lowercase
Add to unit test for the hosts file parser to use an UPPERCASE file entry
Add unit test for DefaultHostsFileEntriesResolver to resolve both localhost and LOCALHOST
Result:
host resolution for local hosts file should match the rules applied to "getent hosts" or "ping"
Motivation:
When an InetNameResolver resolves a name, it is expected to reserve the
requested host name in the resolved InetAddress.
DefaultHostsFileEntriesResolver does not preserve the host name. For
example, resolving 'localhost' will return an InetAddress whose address
is '127.0.0.1', but its getHostString() will not return 'localhost' but
just '127.0.0.1'.
Modifications:
Fix the construction of parsed InetAddresses in HostsFileParser
Result:
Host name is preserved in the resolved InetAddress
Motivation:
Javadoc reports errors about invalid docs.
Modifications:
Fix some errors reported by javadoc.
Result:
A lot of javadoc errors are fixed by this patch.
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>.
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.
Related: #3478
Motivation:
DefaultNameResolver uses InetSocketAddress.getHostString() instead of
getHostName(). Because Netty uses the DefaultNameResolver by default and
getHostString() is available only since Java 7, a user cannot use Netty
on Java 6 anymore.
Modifications:
Use InetSocketAddress.getHostName() which is practically same and also
is available in Java 6.
Result:
Netty 4.1 runs on Java 6 again.
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.
