andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Do not fail a DNS query promise prematurely

Browse Source 
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.
This commit is contained in:
Trustin Lee 2017-04-04 17:23:34 +09:00 committed by Scott Mitchell
parent 1bc5bc69e3
commit 08646afc1e
1 changed files with 11 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverContext.java
View File

@ -318,8 +318,7 @@ abstract class DnsNameResolverContext<T> {
} }
/** /**
* Handle redirects if needed and returns {@code true} if there was a redirect handled. If {@code true} is returned * Handles a redirect answer if needed and returns {@code true} if a redirect query has been made.
* this method took ownership of the {@code promise} otherwise {@code false} is returned.
*/ */
private boolean handleRedirect( private boolean handleRedirect(
DnsQuestion question, AddressedEnvelope<DnsResponse, InetSocketAddress> envelope, Promise<T> promise) { DnsQuestion question, AddressedEnvelope<DnsResponse, InetSocketAddress> envelope, Promise<T> promise) {
@ -336,7 +335,7 @@ abstract class DnsNameResolverContext<T> {
for (int i = 0; i < additionalCount; i++) { for (int i = 0; i < additionalCount; i++) {
final DnsRecord r = res.recordAt(DnsSection.ADDITIONAL, i); final DnsRecord r = res.recordAt(DnsSection.ADDITIONAL, i);
if ((r.type() == DnsRecordType.A && !parent.supportsARecords()) || if (r.type() == DnsRecordType.A && !parent.supportsARecords() ||
r.type() == DnsRecordType.AAAA && !parent.supportsAAAARecords()) { r.type() == DnsRecordType.AAAA && !parent.supportsAAAARecords()) {
continue; continue;
} }
@ -361,15 +360,17 @@ abstract class DnsNameResolverContext<T> {
} }
if (nameServers.isEmpty()) { if (nameServers.isEmpty()) {
promise.tryFailure( if (traceEnabled) {
new UnknownHostException("Unable to find correct name server for " + hostname)); addTrace(envelope.sender(),
"no matching authoritative name server found in the ADDITIONALS section");
}
} else { } else {
// Shuffle as we want to re-distribute the load across name servers. // Shuffle as we want to re-distribute the load across name servers.
query(DnsServerAddresses.shuffled(nameServers).stream(), question, promise); query(DnsServerAddresses.shuffled(nameServers).stream(), question, promise);
}
return true; return true;
} }
} }
}
return false; return false;
} }
@ -709,7 +710,9 @@ abstract class DnsNameResolverContext<T> {
question = new DefaultDnsQuestion(hostname, type); question = new DefaultDnsQuestion(hostname, type);
} catch (IllegalArgumentException e) { } catch (IllegalArgumentException e) {
// java.net.IDN.toASCII(...) may throw an IllegalArgumentException if it fails to parse the hostname // java.net.IDN.toASCII(...) may throw an IllegalArgumentException if it fails to parse the hostname
promise.tryFailure(e); if (traceEnabled) {
addTrace(e);
}
return false; return false;
} }
query(nextAddr, question, promise); query(nextAddr, question, promise);