Skip to content

Commit

Permalink
Don't use the cname cache when using DnsRecordResolveContext (#10808)
Browse files Browse the repository at this point in the history
Motivation:

The DnsNameResolver internally follows CNAME indirects for all records types, and supports caching for CNAME resolution and A* records. For DNS record types that are not cached (e.g. SRV records) the caching of CNAME records may result in failures at incorrect times. For example if a CNAME record has a larger TTL than the entries it resolves this may result in failures which don't occur if the CNAME cache is disabled.

Modifications:

- Don't cache CNAME and also dont use the cache for CNAME when using DnsRecordResolveContext
- Add unit test

Result:

More correct resolving and also not possible to have failures due CNAME still be in the cache while the queried record experied
  • Loading branch information
normanmaurer committed Nov 26, 2020
1 parent 221c1a1 commit 567b46f
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -84,4 +84,10 @@ void cache(String hostname, DnsRecord[] additionals, UnknownHostException cause)
// Do not cache.
// XXX: When we implement cache, we would need to retain the reference count of the result record.
}

@Override
DnsCnameCache cnameCache() {
// We don't use a cache here at all as we also don't cache if we end up using the DnsRecordResolverContext.
return NoopDnsCnameCache.INSTANCE;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@
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.SRV;
import static io.netty.resolver.dns.DnsServerAddresses.sequential;
import static java.util.Collections.singletonList;
import static org.hamcrest.MatcherAssert.assertThat;
Expand Down Expand Up @@ -3178,4 +3179,91 @@ public DnsServerAddressStream nameServerAddressStream(String hostname) {
}
}
}

@Test(timeout = 2000)
public void testSrvWithCnameNotCached() throws Exception {
final AtomicBoolean alias = new AtomicBoolean();
TestDnsServer dnsServer2 = new TestDnsServer(new RecordStore() {
@Override
public Set<ResourceRecord> getRecords(QuestionRecord question) {
String name = question.getDomainName();
if (name.equals("service.netty.io")) {
Set<ResourceRecord> records = new HashSet<ResourceRecord>(2);

ResourceRecordModifier rm = new ResourceRecordModifier();
rm.setDnsClass(RecordClass.IN);
rm.setDnsName(name);
rm.setDnsTtl(10);
rm.setDnsType(RecordType.CNAME);
rm.put(DnsAttribute.DOMAIN_NAME, "alias.service.netty.io");
records.add(rm.getEntry());

rm = new ResourceRecordModifier();
rm.setDnsClass(RecordClass.IN);
rm.setDnsName(name);
rm.setDnsTtl(10);
rm.setDnsType(RecordType.SRV);
rm.put(DnsAttribute.DOMAIN_NAME, "foo.service.netty.io");
rm.put(DnsAttribute.SERVICE_PORT, "8080");
rm.put(DnsAttribute.SERVICE_PRIORITY, "10");
rm.put(DnsAttribute.SERVICE_WEIGHT, "1");
records.add(rm.getEntry());
return records;
}
if (name.equals("foo.service.netty.io")) {
ResourceRecordModifier rm = new ResourceRecordModifier();
rm.setDnsClass(RecordClass.IN);
rm.setDnsName(name);
rm.setDnsTtl(10);
rm.setDnsType(RecordType.A);
rm.put(DnsAttribute.IP_ADDRESS, "10.0.0.1");
return Collections.singleton(rm.getEntry());
}
if (alias.get()) {
ResourceRecordModifier rm = new ResourceRecordModifier();
rm.setDnsClass(RecordClass.IN);
rm.setDnsName(name);
rm.setDnsTtl(10);
rm.setDnsType(RecordType.SRV);
rm.put(DnsAttribute.DOMAIN_NAME, "foo.service.netty.io");
rm.put(DnsAttribute.SERVICE_PORT, "8080");
rm.put(DnsAttribute.SERVICE_PRIORITY, "10");
rm.put(DnsAttribute.SERVICE_WEIGHT, "1");
return Collections.singleton(rm.getEntry());
}
return null;
}
});
dnsServer2.start();
DnsNameResolver resolver = null;
try {
DnsNameResolverBuilder builder = newResolver()
.recursionDesired(false)
.queryTimeoutMillis(10000)
.resolvedAddressTypes(ResolvedAddressTypes.IPV4_PREFERRED)
.completeOncePreferredResolved(true)
.maxQueriesPerResolve(16)
.nameServerProvider(new SingletonDnsServerAddressStreamProvider(dnsServer2.localAddress()));

resolver = builder.build();
assertNotEmptyAndRelease(resolver.resolveAll(new DefaultDnsQuestion("service.netty.io", SRV)));
alias.set(true);
assertNotEmptyAndRelease(resolver.resolveAll(new DefaultDnsQuestion("service.netty.io", SRV)));
alias.set(false);
assertNotEmptyAndRelease(resolver.resolveAll(new DefaultDnsQuestion("service.netty.io", SRV)));
} 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());
for (DnsRecord record : records) {
ReferenceCountUtil.release(record);
}
}
}

0 comments on commit 567b46f

Please sign in to comment.