Commit Graph

272 Commits

Author SHA1 Message Date
Nick Hill
2e5dd28800 Fix DnsNameResolver TCP fallback test and message leaks (#9647)
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
2019-10-14 16:10:15 +02:00
Nikolay Fedorovskikh
4dc1eccf60 Make some inner classes static (#9624)
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.
2019-10-07 08:14:02 +02:00
root
92941cdcac [maven-release-plugin] prepare for next development iteration 2019-09-25 06:15:31 +00:00
root
bd907c3b3a [maven-release-plugin] prepare release netty-4.1.42.Final 2019-09-25 06:14:31 +00:00
root
01d805bb76 [maven-release-plugin] prepare for next development iteration 2019-09-12 16:09:55 +00:00
root
7cf69022d4 [maven-release-plugin] prepare release netty-4.1.41.Final 2019-09-12 16:09:00 +00:00
root
aef47bec7f [maven-release-plugin] prepare for next development iteration 2019-09-12 05:38:11 +00:00
root
267e5da481 [maven-release-plugin] prepare release netty-4.1.40.Final 2019-09-12 05:37:30 +00:00
Norman Maurer
68673b652e
Do not try to retrieve domain search list via reflection hack on windows when using Java9 and later (#9511)
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.
2019-08-28 08:08:01 +02:00
Norman Maurer
d0bfbdde8e
Detect truncated responses caused by EDNS0 and MTU miss-match (#9468)
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
2019-08-17 09:58:22 +02:00
Norman Maurer
04d71b3773
DnsNameResolverTest.testTruncated0(...) should only close socket once envelope is received (#9469)
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
2019-08-15 16:28:22 +02:00
root
d45a4ce01b [maven-release-plugin] prepare for next development iteration 2019-08-13 17:16:42 +00:00
root
88c2a4cab5 [maven-release-plugin] prepare release netty-4.1.39.Final 2019-08-13 17:15:20 +00:00
root
718b7626e6 [maven-release-plugin] prepare for next development iteration 2019-07-24 09:05:57 +00:00
root
465c900c04 [maven-release-plugin] prepare release netty-4.1.38.Final 2019-07-24 09:05:23 +00:00
root
5b58b8e6b5 [maven-release-plugin] prepare for next development iteration 2019-06-28 05:57:21 +00:00
root
35e0843376 [maven-release-plugin] prepare release netty-4.1.37.Final 2019-06-28 05:56:28 +00:00
jimin
9621a5b981 remove unused imports (#9287)
Motivation:

Some imports are not used

Modification:

remove unused imports

Result:

Code cleanup
2019-06-26 21:08:31 +02:00
Norman Maurer
9dd1aab482
Fix flaky DnsNameResolverTest.testTruncatedWithTcpFallback (#9262)
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.
2019-06-21 09:28:51 +02:00
Norman Maurer
9c6365ee95
Only try to use reflection to access default nameservers when using Java8 and lower (#9157)
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.
2019-05-18 08:21:33 +02:00
Norman Maurer
f17bfd0f64
Only use static Exception instances when we can ensure addSuppressed … (#9152)
Motivation:

OOME is occurred by increasing suppressedExceptions because other libraries call Throwable#addSuppressed. As we have no control over what other libraries do we need to ensure this can not lead to OOME.

Modifications:

Only use static instances of the Exceptions if we can either dissable addSuppressed or we run on java6.

Result:

Not possible to OOME because of addSuppressed. Fixes https://github.com/netty/netty/issues/9151.
2019-05-17 22:23:02 +02:00
Norman Maurer
1672b6d12c
Add support for TCP fallback when we receive a truncated DnsResponse (#9139)
Motivation:

Sometimes DNS responses can be very large which mean they will not fit in a UDP packet. When this is happening the DNS server will set the TC flag (truncated flag) to tell the resolver that the response was truncated. When a truncated response was received we should allow to retry via TCP and use the received response (if possible) as a replacement for the truncated one.

See https://tools.ietf.org/html/rfc7766.

Modifications:

- Add support for TCP fallback by allow to specify a socketChannelFactory / socketChannelType on the DnsNameResolverBuilder. If this is set to something different then null we will try to fallback to TCP.
- Add decoder / encoder for TCP
- Add unit tests

Result:

Support for TCP fallback as defined by https://tools.ietf.org/html/rfc7766 when using DnsNameResolver.
2019-05-17 14:37:11 +02:00
Norman Maurer
6ee8b651e6
DnsNameResolver.resolveAll(DnsQuestion) should not try to filter duplicates (#9141)
Motivation:

https://github.com/netty/netty/pull/9021 did apply some changes to filter out duplicates InetAddress when calling resolveAll(...) to mimic JDK behaviour. Unfortunally this also introduced a regression as we should not filter duplicates when the user explicit calls resolveAll(DnsQuestion).

Modifications:

- Only filter duplicates if resolveAll(String) is used
- Add unit test

Result:

Fixes regressions introduces by https://github.com/netty/netty/pull/9021
2019-05-13 06:59:06 +02:00
Norman Maurer
df20a125aa
Allow to have DnsNameResolver.resolveAll(...) notify as soon as the preferred records were resolved (#9136)
Motivation:

075cf8c02e introduced a change to allow resolve(...) to notify as soon as the preferred record was resolved. This works great but we should also allow the user to configure that we want to do the same for resolveAll(...), which means we should be able to notify as soon as all records for a preferred record were resolved.

Modifications:

- Add a new DnsNameResolverBuilder method to allow configure this (use false as default to not change default behaviour)
- Add unit test

Result:

Be able to speed up resolving.
2019-05-09 08:06:52 +02:00
root
ba06eafa1c [maven-release-plugin] prepare for next development iteration 2019-04-30 16:42:29 +00:00
root
49a451101c [maven-release-plugin] prepare release netty-4.1.36.Final 2019-04-30 16:41:28 +00:00
root
baab215f66 [maven-release-plugin] prepare for next development iteration 2019-04-17 07:26:24 +00:00
root
dfe657e2d4 [maven-release-plugin] prepare release netty-4.1.35.Final 2019-04-17 07:25:40 +00:00
Norman Maurer
075cf8c02e
DnsNameResolver.resolve(...) should notify future as soon as one preferred record was resolved (#9050)
Motivation:

At the moment resolve(...) does just delegate to resolveAll(...) and so will only notify the future once all records were resolved. This is wasteful as we are only interested in the first record anyway. We should notify the promise as soon as one record that matches the preferred record type is resolved.

Modifications:

- Introduce DnsResolveContext.isCompleteEarly(...) to be able to detect once we should early notify the promise.
- Make use of this early detecting if resolve(...) is called
- Remove FutureListener which could lead to IllegalReferenceCountException due double releases
- add unit test

Result:

Be able to notify about resolved host more quickly.
2019-04-15 21:42:04 +02:00
Norman Maurer
4b36a5b08b
Correctly calculate ttl for AuthoritativeNameServer when update existing records (#9051)
Motivation:

We did not correctly calculate the new ttl as we did forget to add `this.`

Modifications:

Add .this and so correctly calculate the TTL

Result:

Use correct TTL for authoritative nameservers when updating these.
2019-04-15 21:41:04 +02:00
Norman Maurer
dde3f561bc
Use ResolvedAddressTypes.IPV4_ONLY in DnsNameResolver by default if n… (#9048)
Motivation:

To closely mimic what the JDK does we should not try to resolve AAAA records if the system itself does not support IPv6 at all as it is impossible to connect to this addresses later on. In this case we need to use ResolvedAddressTypes.IPV4_ONLY.

Modifications:

Add static method to detect if IPv6 is supported and if not use ResolvedAddressTypes.IPV4_ONLY.

Result:

More consistent behaviour between JDK and our resolver implementation.
2019-04-15 13:07:05 +02:00
Norman Maurer
26cd59c328
DnsNameResolver.resolveAll(...) should also contain non preferred addresses (#9044)
Motivation:

At the moment we basically drop all non prefered addresses when calling DnsNameResolver.resolveAll(...). This is just incorrect and was introduced by 4cd39cc4b3. More correct is to still retain these but sort the returned List to have the prefered addresses on the beginning of the List. This also ensures resolve(...) will return the correct return type.

Modifications:

- Introduce PreferredAddressTypeComperator which we use to sort the List so it will contain the preferred address type first.
- Add unit test to verify behaviour

Result:

Include not only preferred addresses in the List that is returned by resolveAll(...)
2019-04-15 10:19:54 +02:00
Norman Maurer
c0d3444f6d
DnsNameResolver should log in trace level if notification of the promise fails (#9022)
Motivation:

During investigating some other bug I noticed that we log with warn level if we fail to notify the promise due the fact that it is already full-filled. This is not correct and missleading as there is nothing wrong with it in general. A promise may already been fullfilled because we did multiple queries and one of these was successful.

Modifications:

- Change log level to trace
- Add unit test which before did log with warn level but now does with trace level.

Result:

Less missleading noise in the log.
2019-04-10 07:13:53 +02:00
Norman Maurer
e63c596f24
DnsNameResolver.resolveAll(...) should not include duplicates (#9021)
Motivation:

DnsNameResolver#resolveAll(String) may return duplicate results in the event that the original hostname DNS response includes an IP address X and a CNAME that ends up resolving the same IP address X. This behavior is inconsistent with the JDK’s resolver and is unexpected to retrun a List with duplicate entries from a resolveAll(..) call.

Modifications:

- Filter out duplicates
- Add unit test

Result:

More consistent and less suprising behavior
2019-04-09 09:44:23 +02:00
Norman Maurer
0ee067082b
Add unit test for query TXT records. (#8923)
Motivation:

We did not have any unit tests that queries for TXT records.

Modifications:

Add unit test to query TXT records.

Result:

More test-coverage.
2019-03-09 21:41:28 +01:00
root
92b19cfedd [maven-release-plugin] prepare for next development iteration 2019-03-08 08:55:45 +00:00
root
ff7a9fa091 [maven-release-plugin] prepare release netty-4.1.34.Final 2019-03-08 08:51:34 +00:00
root
cf03ed0478 [maven-release-plugin] prepare for next development iteration 2019-01-21 12:26:44 +00:00
root
37484635cb [maven-release-plugin] prepare release netty-4.1.33.Final 2019-01-21 12:26:12 +00:00
Norman Maurer
df5eb060f7
Only handle NXDOMAIN as failure when nameserver is authoritive or no other nameservers are left. (#8731)
Motivation:

When using multiple nameservers and a nameserver respond with NXDOMAIN we should only fail the query if the nameserver in question is authoritive or no nameservers are left to try.

Modifications:

- Try next nameserver if NXDOMAIN was returned but the nameserver is not authoritive
- Adjust testcase to respect correct behaviour.

Result:

Fixes https://github.com/netty/netty/issues/8261
2019-01-18 21:06:44 +01:00
Norman Maurer
82ec6ba815
Correctly detect and handle CNAME loops. (#8691)
Motivation:

We do not correctly detect loops when follow CNAMEs and so may try to follow it without any success.

Modifications:

- Correctly detect CNAME loops
- Do not cache CNAME entries which point to itself
- Add unit test.

Result:

Fixes https://github.com/netty/netty/issues/8687.
2019-01-14 08:17:44 +01:00
kashike
6fdd7fcddb Fix minor spelling issues in javadocs (#8701)
Motivation:

Javadocs contained some spelling errors, we should fix these.

Modification:

Fix spelling

Result:

Javadoc cleanup.
2019-01-14 07:24:34 +01:00
Norman Maurer
a3844da10b
NoClassDefFoundError on Android platform when try to use DefaultDnsServerAddressStreamProvider. (#8656)
Motivation:

Andoid does not contain javax.naming.* so we should not try to use it to prevent a NoClassDefFoundError on init.

Modifications:

Only try to use javax.naming.* to retrieve nameservers when not using Android.

Result:

Fixes https://github.com/netty/netty/issues/8654.
2018-12-14 21:31:21 +01:00
Feri73
5df235c083 Correcting Maven Dependencies (#8622)
Motivation:

Most of the maven modules do not explicitly declare their
dependencies and rely on transitivity, which is not always correct.

Modifications:

For all maven modules, add all of their dependencies to pom.xml

Result:

All of the (essentially non-transitive) depepdencies of the modules are explicitly declared in pom.xml
2018-12-06 09:01:14 +01:00
root
8eb313072e [maven-release-plugin] prepare for next development iteration 2018-11-29 11:15:09 +00:00
root
afcb4a37d3 [maven-release-plugin] prepare release netty-4.1.32.Final 2018-11-29 11:14:20 +00:00
Norman Maurer
d728a72e74
Combine flushes in DnsNameResolver to allow usage of sendmmsg to reduce syscall costs (#8470)
Motivation:

Some of transports support gathering writes when using datagrams. For example this is the case for EpollDatagramChannel. We should minimize the calls to flush() to allow making efficient usage of sendmmsg in this case.

Modifications:

- minimize flush() operations when we query for multiple address types.
- reduce GC by always directly schedule doResolveAll0(...) on the EventLoop.

Result:

Be able to use sendmmsg internally in the DnsNameResolver.
2018-11-21 06:42:40 +01:00
Norman Maurer
cb0d23923f
Refresh DNS configuration each 5 minutes. (#8468)
Motivation:

We should refresh the DNS configuration each 5 minutes to be able to detect changes done by the user. This is inline with what OpenJDK is doing

Modifications:

Refresh config every 5 minutes.

Result:

Be able to consume changes made by the user.
2018-11-16 10:37:29 +01:00
Stephane Landelle
f4cf674f01 Fix NPE when trying to build a DnsNameResolver with a null resolvedAddressTypes (#8445)
Motivation:

It should be possible to build a DnsNameResolver with a null resolvedAddressTypes, defaulting then to DEFAULT_RESOLVE_ADDRESS_TYPES (see line 309).

Sadly, `preferredAddressType` is then called on line 377 with the original parameter instead of the instance attribute, causing an NPE when it's null.

Modification:

Call preferredAddressType with instance attribuet instead of constructor parameter.

Result:

No more NPE
2018-10-30 13:15:16 +01:00
root
3e7ddb36c7 [maven-release-plugin] prepare for next development iteration 2018-10-29 15:38:51 +00:00