Only fall back to CNAME on A/AAAA queries (#11216)

Motivation:

DNS resolver falls back to trying CNAME if no records found, but should
only try this for A/AAAA queries. Does not make sense for other query
types, results in a redundant CNAME query that is just going to fail.

Modification:

Check query type before deciding to try CNAME. Only proceed if type is A
or AAAA.

Added unit test to verify CNAME is only tried after A/AAAA queries.

Result:

Fixes #11214.
This commit is contained in:
Ben Evans 2021-05-04 17:38:07 +12:00 committed by GitHub
parent d2643ed835
commit 4fabd803c2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 62 additions and 1 deletions

View File

@ -930,7 +930,8 @@ abstract class DnsResolveContext<T> {
// If cause != null we know this was caused by a timeout / cancel / transport exception. In this case we
// won't try to resolve the CNAME as we only should do this if we could not get the expected records
// because they do not exist and the DNS server did probably signal it.
if (cause == null && !triedCNAME) {
if (cause == null && !triedCNAME &&
(question.type() == DnsRecordType.A || question.type() == DnsRecordType.AAAA)) {
// As the last resort, try to query CNAME, just in case the name server has it.
triedCNAME = true;

View File

@ -112,6 +112,7 @@ import java.util.concurrent.atomic.AtomicReference;
import static io.netty.handler.codec.dns.DnsRecordType.A;
import static io.netty.handler.codec.dns.DnsRecordType.AAAA;
import static io.netty.handler.codec.dns.DnsRecordType.CNAME;
import static io.netty.handler.codec.dns.DnsRecordType.NAPTR;
import static io.netty.handler.codec.dns.DnsRecordType.SRV;
import static io.netty.resolver.dns.DnsNameResolver.DEFAULT_RESOLVE_ADDRESS_TYPES;
import static io.netty.resolver.dns.DnsServerAddresses.sequential;
@ -3286,6 +3287,65 @@ public class DnsNameResolverTest {
}
}
@Test
public void testCNAMEOnlyTriedOnAddressLookups() throws Exception {
final AtomicInteger cnameQueries = new AtomicInteger();
TestDnsServer dnsServer2 = new TestDnsServer(new RecordStore() {
@Override
public Set<ResourceRecord> getRecords(QuestionRecord questionRecord) {
if (questionRecord.getRecordType() == RecordType.CNAME) {
cnameQueries.incrementAndGet();
}
return Collections.emptySet();
}
});
dnsServer2.start();
DnsNameResolver resolver = null;
try {
resolver = newNonCachedResolver(ResolvedAddressTypes.IPV4_PREFERRED)
.maxQueriesPerResolve(4)
.searchDomains(Collections.emptyList())
.nameServerProvider(new SingletonDnsServerAddressStreamProvider(dnsServer2.localAddress()))
.build();
// We expect these resolves to fail with UnknownHostException,
// and then check that no unexpected CNAME queries were performed.
assertThat(resolver.resolveAll(new DefaultDnsQuestion("lookup-srv.netty.io", SRV)).await().cause(),
instanceOf(UnknownHostException.class));
assertEquals(0, cnameQueries.get());
assertThat(resolver.resolveAll(new DefaultDnsQuestion("lookup-naptr.netty.io", NAPTR)).await().cause(),
instanceOf(UnknownHostException.class));
assertEquals(0, cnameQueries.get());
assertThat(resolver.resolveAll(new DefaultDnsQuestion("lookup-cname.netty.io", CNAME)).await().cause(),
instanceOf(UnknownHostException.class));
assertEquals(1, cnameQueries.getAndSet(0));
assertThat(resolver.resolveAll(new DefaultDnsQuestion("lookup-a.netty.io", A)).await().cause(),
instanceOf(UnknownHostException.class));
assertEquals(1, cnameQueries.getAndSet(0));
assertThat(resolver.resolveAll(new DefaultDnsQuestion("lookup-aaaa.netty.io", AAAA)).await().cause(),
instanceOf(UnknownHostException.class));
assertEquals(1, cnameQueries.getAndSet(0));
assertThat(resolver.resolveAll("lookup-address.netty.io").await().cause(),
instanceOf(UnknownHostException.class));
assertEquals(1, cnameQueries.getAndSet(0));
} finally {
dnsServer2.stop();
if (resolver != null) {
resolver.close();
}
}
}
private static void assertNotEmptyAndRelease(Future<List<DnsRecord>> recordsFuture) throws Exception {
List<DnsRecord> records = recordsFuture.get();
assertFalse(records.isEmpty());