From 23c0bbb9045fda365e906504b559a11bad717faa Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Thu, 26 Nov 2020 15:34:56 +0100 Subject: [PATCH] Don't use the cname cache when using DnsRecordResolveContext (#10808) 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 --- .../resolver/dns/DnsRecordResolveContext.java | 6 ++ .../resolver/dns/DnsNameResolverTest.java | 88 +++++++++++++++++++ 2 files changed, 94 insertions(+) diff --git a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsRecordResolveContext.java b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsRecordResolveContext.java index 9fbde0d6a4..d5815980a2 100644 --- a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsRecordResolveContext.java +++ b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsRecordResolveContext.java @@ -84,4 +84,10 @@ final class DnsRecordResolveContext extends DnsResolveContext { // 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; + } } diff --git a/resolver-dns/src/test/java/io/netty/resolver/dns/DnsNameResolverTest.java b/resolver-dns/src/test/java/io/netty/resolver/dns/DnsNameResolverTest.java index 7d2ee0a003..624a6a88a4 100644 --- a/resolver-dns/src/test/java/io/netty/resolver/dns/DnsNameResolverTest.java +++ b/resolver-dns/src/test/java/io/netty/resolver/dns/DnsNameResolverTest.java @@ -111,6 +111,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.SRV; import static io.netty.resolver.dns.DnsServerAddresses.sequential; import static java.util.Collections.singletonList; import static org.hamcrest.MatcherAssert.assertThat; @@ -3134,4 +3135,91 @@ public class DnsNameResolverTest { } } } + + @Test(timeout = 2000) + public void testSrvWithCnameNotCached() throws Exception { + final AtomicBoolean alias = new AtomicBoolean(); + TestDnsServer dnsServer2 = new TestDnsServer(new RecordStore() { + @Override + public Set getRecords(QuestionRecord question) { + String name = question.getDomainName(); + if (name.equals("service.netty.io")) { + Set records = new HashSet(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> recordsFuture) throws Exception { + List records = recordsFuture.get(); + assertFalse(records.isEmpty()); + for (DnsRecord record : records) { + ReferenceCountUtil.release(record); + } + } }