From 0933a93102bf819059807c565f0b086ce0f091e6 Mon Sep 17 00:00:00 2001 From: Aaron Clauson Date: Thu, 30 Jul 2020 14:50:33 +0100 Subject: [PATCH 1/5] Adds a new public method to allow the cache to be queried. The point of this is to allow an application to take different actions depending on whether a network lookup needs to be undertaken or not. See #80. Adds a setting to choose whether lookup failures should be cached and if so for how long. The point of this is to allow applciations to avoid high frequency lookups on failing queries. --- src/DnsClient/DnsQueryExtensions.cs | 4 +- src/DnsClient/DnsQueryOptions.cs | 34 ++++++++++- src/DnsClient/LookupClient.cs | 76 +++++++++++++++++++---- src/DnsClient/ResponseCache.cs | 94 +++++++++++++++++++---------- 4 files changed, 158 insertions(+), 50 deletions(-) diff --git a/src/DnsClient/DnsQueryExtensions.cs b/src/DnsClient/DnsQueryExtensions.cs index 7bfd5112..86770511 100644 --- a/src/DnsClient/DnsQueryExtensions.cs +++ b/src/DnsClient/DnsQueryExtensions.cs @@ -618,14 +618,14 @@ public static async Task ResolveServiceAsync(this IDnsQuery return ResolveServiceProcessResult(result); } - private static string ConcatResolveServiceName(string baseDomain, string serviceName, string tag) + public static string ConcatResolveServiceName(string baseDomain, string serviceName, string tag) { return string.IsNullOrWhiteSpace(tag) ? $"{serviceName}.{baseDomain}." : $"_{serviceName}._{tag}.{baseDomain}."; } - private static ServiceHostEntry[] ResolveServiceProcessResult(IDnsQueryResponse result) + public static ServiceHostEntry[] ResolveServiceProcessResult(IDnsQueryResponse result) { var hosts = new List(); if (result == null || result.HasError) diff --git a/src/DnsClient/DnsQueryOptions.cs b/src/DnsClient/DnsQueryOptions.cs index 3207d467..51e3a94e 100644 --- a/src/DnsClient/DnsQueryOptions.cs +++ b/src/DnsClient/DnsQueryOptions.cs @@ -219,7 +219,20 @@ public int ExtendedDnsBufferSize /// Gets or sets a flag indicating whether EDNS should be enabled and the DO flag should be set. /// Defaults to False. /// - public bool RequestDnsSecRecords { get; set; } = false; + public bool RequestDnsSecRecords { get; set; } = false; + + /// + /// Gets or sets a flag indicating whether the DNS failures are being cached. The purpose of caching + /// failures is to reduce repeated lookup attempts within a short space of time. + /// Defaults to False. + /// + public bool UseCacheForFailures { get; set; } = false; + + /// + /// Gets or sets the duration to cache failed lookups. Does not apply if failed lookups are not being cached. + /// Defaults to 5 seconds. + /// + public TimeSpan CacheFailureDuration { get; set; } = s_defaultTimeout; /// /// Converts the query options into readonly settings. @@ -614,6 +627,19 @@ public class DnsQuerySettings : IEquatable /// public bool RequestDnsSecRecords { get; } + /// + /// Gets a flag indicating whether the DNS failures are being cached. The purpose of caching + /// failures is to reduce repeated lookup attempts within a short space of time. + /// Defaults to False. + /// + public bool UseCacheForFailures { get; } + + /// + /// If failures are being cached this value indicates how long they will be held in the cache for. + /// Defaults to 5 seconds. + /// + public TimeSpan CacheFailureDuration { get; } + /// /// Creates a new instance of . /// @@ -637,6 +663,8 @@ public DnsQuerySettings(DnsQueryOptions options) UseTcpOnly = options.UseTcpOnly; ExtendedDnsBufferSize = options.ExtendedDnsBufferSize; RequestDnsSecRecords = options.RequestDnsSecRecords; + UseCacheForFailures = options.UseCacheForFailures; + CacheFailureDuration = options.CacheFailureDuration; } /// @@ -680,7 +708,9 @@ public bool Equals(DnsQuerySettings other) UseTcpFallback == other.UseTcpFallback && UseTcpOnly == other.UseTcpOnly && ExtendedDnsBufferSize == other.ExtendedDnsBufferSize && - RequestDnsSecRecords == other.RequestDnsSecRecords; + RequestDnsSecRecords == other.RequestDnsSecRecords && + UseCacheForFailures == other.UseCacheForFailures && + CacheFailureDuration.Equals(other.Timeout); } } diff --git a/src/DnsClient/LookupClient.cs b/src/DnsClient/LookupClient.cs index b0ac0dd9..12421b14 100644 --- a/src/DnsClient/LookupClient.cs +++ b/src/DnsClient/LookupClient.cs @@ -376,12 +376,12 @@ internal LookupClient(LookupClientOptions options, DnsMessageHandler udpHandler if (options.AutoResolveNameServers) { _resolvedNameServers = NameServer.ResolveNameServers(skipIPv6SiteLocal: true, fallbackToGooglePublicDns: false); - servers = servers.Concat(_resolvedNameServers).ToArray(); - - // This will periodically get triggered on Query calls and - // will perform the same check as on NetworkAddressChanged. - // The event doesn't seem to get fired on Linux for example... - // TODO: Maybe there is a better way, but this will work for now. + servers = servers.Concat(_resolvedNameServers).ToArray(); + + // This will periodically get triggered on Query calls and + // will perform the same check as on NetworkAddressChanged. + // The event doesn't seem to get fired on Linux for example... + // TODO: Maybe there is a better way, but this will work for now. _skipper = new SkipWorker( () => { @@ -396,7 +396,7 @@ internal LookupClient(LookupClientOptions options, DnsMessageHandler udpHandler } Settings = new LookupClientSettings(options, servers); - Cache = new ResponseCache(true, Settings.MinimumCacheTimeout, Settings.MaximumCacheTimeout); + Cache = new ResponseCache(true, Settings.MinimumCacheTimeout, Settings.MaximumCacheTimeout, Settings.CacheFailureDuration); } private void CheckResolvedNameservers() @@ -482,6 +482,22 @@ public IDnsQueryResponse Query(DnsQuestion question, DnsQueryAndServerOptions qu var settings = GetSettings(queryOptions); return QueryInternal(question, settings, settings.ShuffleNameServers()); + } + + /// + public IDnsQueryResponse QueryCache(string query, QueryType queryType, QueryClass queryClass = QueryClass.IN) + => QueryCache(new DnsQuestion(query, queryType, queryClass)); + + /// + public IDnsQueryResponse QueryCache(DnsQuestion question) + { + if (question is null) + { + throw new ArgumentNullException(nameof(question)); + } + + var settings = GetSettings(); + return QueryCache(question, settings); } /// @@ -875,9 +891,14 @@ private IDnsQueryResponse ResolveQuery( if (lastQueryResponse == null) { throw; + } + + // If its the last server, return. + if (settings.UseCache && settings.UseCacheForFailures) + { + Cache.Add(cacheKey, lastQueryResponse, true); } - // If its the last server, return. return lastQueryResponse; } catch (Exception ex) when ( @@ -1126,9 +1147,14 @@ private async Task ResolveQueryAsync( if (lastQueryResponse == null) { throw; + } + + // If its the last server, return. + if (settings.UseCache && settings.UseCacheForFailures) + { + Cache.Add(cacheKey, lastQueryResponse, true); } - // If its the last server, return. return lastQueryResponse; } catch (Exception ex) when ( @@ -1200,6 +1226,30 @@ ex is TimeoutException timeoutEx { AuditTrail = audit?.Build() }; + } + + private IDnsQueryResponse QueryCache( + DnsQuestion question, + DnsQuerySettings settings) + { + if (question == null) + { + throw new ArgumentNullException(nameof(question)); + } + + var head = new DnsRequestHeader(false, DnsOpCode.Query); + var request = new DnsRequestMessage(head, question); + + var cacheKey = ResponseCache.GetCacheKey(request.Question); + + if (TryGetCachedResult(cacheKey, request, settings, out var cachedResponse)) + { + return cachedResponse; + } + else + { + return null; + } } private enum HandleError @@ -1269,10 +1319,10 @@ private HandleError HandleDnsResponseException(DnsResponseException ex, DnsReque private HandleError HandleDnsResponeParseException(DnsResponseParseException ex, DnsRequestMessage request, DnsMessageHandleType handleType, bool isLastServer) { // Don't try to fallback to TCP if we already are on TCP - if (handleType == DnsMessageHandleType.UDP - // Assuming that if we only got 512 or less bytes, its probably some network issue. - && (ex.ResponseData.Length <= DnsQueryOptions.MinimumBufferSize - // Second assumption: If the parser tried to read outside the provided data, this might also be a network issue. + if (handleType == DnsMessageHandleType.UDP + // Assuming that if we only got 512 or less bytes, its probably some network issue. + && (ex.ResponseData.Length <= DnsQueryOptions.MinimumBufferSize + // Second assumption: If the parser tried to read outside the provided data, this might also be a network issue. || ex.ReadLength + ex.Index > ex.ResponseData.Length)) { // lets assume the response was truncated and retry with TCP. diff --git a/src/DnsClient/ResponseCache.cs b/src/DnsClient/ResponseCache.cs index 7942ff55..5d4ac26a 100644 --- a/src/DnsClient/ResponseCache.cs +++ b/src/DnsClient/ResponseCache.cs @@ -21,6 +21,7 @@ internal class ResponseCache private int _lastCleanup = 0; private TimeSpan? _minimumTimeout; private TimeSpan? _maximumTimeout; + private TimeSpan? _failureEntryTimeout; public int Count => _cache.Count; @@ -54,13 +55,29 @@ public TimeSpan? MaximumTimeout _maximumTimeout = value; } + } + + public TimeSpan? FailureEntryTimeout + { + get { return _failureEntryTimeout; } + set + { + if (value.HasValue && + (value < TimeSpan.Zero || value > s_maxTimeout) && value != s_infiniteTimeout) + { + throw new ArgumentOutOfRangeException(nameof(value)); + } + + _failureEntryTimeout = value; + } } - public ResponseCache(bool enabled = true, TimeSpan? minimumTimout = null, TimeSpan? maximumTimeout = null) + public ResponseCache(bool enabled = true, TimeSpan? minimumTimout = null, TimeSpan? maximumTimeout = null, TimeSpan? failureEntryTimeout = null) { Enabled = enabled; MinimumTimout = minimumTimout; MaximumTimeout = maximumTimeout; + FailureEntryTimeout = failureEntryTimeout; } public static string GetCacheKey(DnsQuestion question) @@ -101,43 +118,54 @@ public IDnsQueryResponse Get(string key, out double? effectiveTtl) return null; } - public bool Add(string key, IDnsQueryResponse response) + public bool Add(string key, IDnsQueryResponse response, bool cacheFailures = false) { if (key == null) throw new ArgumentNullException(key); - if (Enabled && response != null && !response.HasError && response.Answers.Count > 0) + if (Enabled && response != null && (cacheFailures || (!response.HasError && response.Answers.Count > 0))) { - var all = response.AllRecords.Where(p => !(p is Protocol.Options.OptRecord)); - if (all.Any()) - { - // in millis - double minTtl = all.Min(p => p.InitialTimeToLive) * 1000d; - - if (MinimumTimout == Timeout.InfiniteTimeSpan) - { - // TODO: Log warning once? - minTtl = s_maxTimeout.TotalMilliseconds; - } - else if (MinimumTimout.HasValue && minTtl < MinimumTimout.Value.TotalMilliseconds) - { - minTtl = MinimumTimout.Value.TotalMilliseconds; - } - - // max TTL check which can limit the upper boundary - if (MaximumTimeout.HasValue && MaximumTimeout != Timeout.InfiniteTimeSpan && minTtl > MaximumTimeout.Value.TotalMilliseconds) - { - minTtl = MaximumTimeout.Value.TotalMilliseconds; - } - - if (minTtl < 1d) - { - return false; + if (response.Answers.Count == 0 && FailureEntryTimeout.HasValue) + { + // Cache entry for a failure response. + var newEntry = new ResponseEntry(response, FailureEntryTimeout.Value.TotalMilliseconds); + + StartCleanup(); + return _cache.TryAdd(key, newEntry); + } + else + { + var all = response.AllRecords.Where(p => !(p is Protocol.Options.OptRecord)); + if (all.Any()) + { + // in millis + double minTtl = all.Min(p => p.InitialTimeToLive) * 1000d; + + if (MinimumTimout == Timeout.InfiniteTimeSpan) + { + // TODO: Log warning once? + minTtl = s_maxTimeout.TotalMilliseconds; + } + else if (MinimumTimout.HasValue && minTtl < MinimumTimout.Value.TotalMilliseconds) + { + minTtl = MinimumTimout.Value.TotalMilliseconds; + } + + // max TTL check which can limit the upper boundary + if (MaximumTimeout.HasValue && MaximumTimeout != Timeout.InfiniteTimeSpan && minTtl > MaximumTimeout.Value.TotalMilliseconds) + { + minTtl = MaximumTimeout.Value.TotalMilliseconds; + } + + if (minTtl < 1d) + { + return false; + } + + var newEntry = new ResponseEntry(response, minTtl); + + StartCleanup(); + return _cache.TryAdd(key, newEntry); } - - var newEntry = new ResponseEntry(response, minTtl); - - StartCleanup(); - return _cache.TryAdd(key, newEntry); } } From 1fbc7eaa2366c5555d2369138da832be7b2b676a Mon Sep 17 00:00:00 2001 From: Aaron Clauson Date: Thu, 30 Jul 2020 21:41:17 +0100 Subject: [PATCH 2/5] Added query cache methods to the IDnsQuery interface. --- src/DnsClient/IDnsQuery.cs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/DnsClient/IDnsQuery.cs b/src/DnsClient/IDnsQuery.cs index 506a4e15..d7018600 100644 --- a/src/DnsClient/IDnsQuery.cs +++ b/src/DnsClient/IDnsQuery.cs @@ -47,6 +47,29 @@ public interface IDnsQuery /// After retries and fallbacks, if none of the servers were accessible, timed out or (if is enabled) returned error results. IDnsQueryResponse Query(DnsQuestion question, DnsQueryAndServerOptions queryOptions); + /// + /// Performs a lookup for the given against the in-memory cache. + /// + /// The domain name query. + /// + /// The which contains the cached response headers and lists of resource records. + /// If no matching cache entry is found null is returned. + /// + IDnsQueryResponse QueryCache(DnsQuestion question); + + /// + /// Performs a DNS lookup for the given , and + /// against the in-memory cache. + /// + /// The domain name query. + /// The . + /// The . + /// + /// The which contains the cached response headers and lists of resource records. + /// If no matching cache entry is found null is returned. + /// + IDnsQueryResponse QueryCache(string query, QueryType queryType, QueryClass queryClass = QueryClass.IN); + /// /// Performs a DNS lookup for the given , and /// From 6d13d107e9c4808fa1803d2502e81fb0ad266aef Mon Sep 17 00:00:00 2001 From: Aaron Clauson Date: Thu, 30 Jul 2020 22:14:08 +0100 Subject: [PATCH 3/5] Added unit tests for new cache settings. --- src/DnsClient/DnsQueryOptions.cs | 19 +++++++-- .../LookupConfigurationTest.cs | 42 ++++++++++++++++++- 2 files changed, 56 insertions(+), 5 deletions(-) diff --git a/src/DnsClient/DnsQueryOptions.cs b/src/DnsClient/DnsQueryOptions.cs index 51e3a94e..fcb657a3 100644 --- a/src/DnsClient/DnsQueryOptions.cs +++ b/src/DnsClient/DnsQueryOptions.cs @@ -27,6 +27,7 @@ public class DnsQueryOptions private static readonly TimeSpan s_maxTimeout = TimeSpan.FromMilliseconds(int.MaxValue); private TimeSpan _timeout = s_defaultTimeout; private int _ednsBufferSize = MaximumBufferSize; + private TimeSpan _cacheFailureDuration = s_defaultTimeout; /// /// Gets or sets a flag indicating whether each will contain a full documentation of the response(s). @@ -232,8 +233,20 @@ public int ExtendedDnsBufferSize /// Gets or sets the duration to cache failed lookups. Does not apply if failed lookups are not being cached. /// Defaults to 5 seconds. /// - public TimeSpan CacheFailureDuration { get; set; } = s_defaultTimeout; - + public TimeSpan CacheFailureDuration + { + get { return _cacheFailureDuration; } + set + { + if ((value <= TimeSpan.Zero || value > s_maxTimeout) && value != s_infiniteTimeout) + { + throw new ArgumentOutOfRangeException(nameof(value)); + } + + _cacheFailureDuration = value; + } + } + /// /// Converts the query options into readonly settings. /// @@ -710,7 +723,7 @@ public bool Equals(DnsQuerySettings other) ExtendedDnsBufferSize == other.ExtendedDnsBufferSize && RequestDnsSecRecords == other.RequestDnsSecRecords && UseCacheForFailures == other.UseCacheForFailures && - CacheFailureDuration.Equals(other.Timeout); + CacheFailureDuration.Equals(other.CacheFailureDuration); } } diff --git a/test/DnsClient.Tests/LookupConfigurationTest.cs b/test/DnsClient.Tests/LookupConfigurationTest.cs index 0224b42f..e96f75d8 100644 --- a/test/DnsClient.Tests/LookupConfigurationTest.cs +++ b/test/DnsClient.Tests/LookupConfigurationTest.cs @@ -287,6 +287,8 @@ public void LookupClientOptions_Defaults() Assert.True(options.UseRandomNameServer); Assert.Equal(DnsQueryOptions.MaximumBufferSize, options.ExtendedDnsBufferSize); Assert.False(options.RequestDnsSecRecords); + Assert.False(options.UseCacheForFailures); + Assert.Equal(options.CacheFailureDuration, TimeSpan.FromSeconds(5)); } [Fact] @@ -310,7 +312,9 @@ public void LookupClientOptions_DefaultsNoResolve() Assert.True(options.ContinueOnEmptyResponse); Assert.True(options.UseRandomNameServer); Assert.Equal(DnsQueryOptions.MaximumBufferSize, options.ExtendedDnsBufferSize); - Assert.False(options.RequestDnsSecRecords); + Assert.False(options.RequestDnsSecRecords); + Assert.False(options.UseCacheForFailures); + Assert.Equal(options.CacheFailureDuration, TimeSpan.FromSeconds(5)); } [Fact] @@ -335,7 +339,9 @@ public void LookupClient_SettingsValid() UseTcpFallback = !defaultOptions.UseTcpFallback, UseTcpOnly = !defaultOptions.UseTcpOnly, ExtendedDnsBufferSize = 1234, - RequestDnsSecRecords = true + RequestDnsSecRecords = true, + UseCacheForFailures = true, + CacheFailureDuration = TimeSpan.FromSeconds(10) }; var client = new LookupClient(options); @@ -357,6 +363,8 @@ public void LookupClient_SettingsValid() Assert.Equal(!defaultOptions.UseTcpOnly, client.Settings.UseTcpOnly); Assert.Equal(1234, client.Settings.ExtendedDnsBufferSize); Assert.Equal(!defaultOptions.RequestDnsSecRecords, client.Settings.RequestDnsSecRecords); + Assert.Equal(!defaultOptions.UseCacheForFailures, client.Settings.UseCacheForFailures); + Assert.Equal(TimeSpan.FromSeconds(10), client.Settings.CacheFailureDuration); Assert.Equal(new LookupClientSettings(options), client.Settings); } @@ -561,6 +569,36 @@ public void LookupClientOptions_InvalidMaximumCacheTimeout2() Action act = () => options.MaximumCacheTimeout = TimeSpan.FromDays(25); + Assert.ThrowsAny(act); + } + + [Fact] + public void LookupClientOptions_InvalidCacheFailureDuration() + { + var options = new LookupClientOptions(); + + Action act = () => options.CacheFailureDuration = TimeSpan.FromMilliseconds(0); + + Assert.ThrowsAny(act); + } + + [Fact] + public void LookupClientOptions_InvalidCacheFailureDuration2() + { + var options = new LookupClientOptions(); + + Action act = () => options.CacheFailureDuration = TimeSpan.FromMilliseconds(-23); + + Assert.ThrowsAny(act); + } + + [Fact] + public void LookupClientOptions_InvalidCacheFailureDuration3() + { + var options = new LookupClientOptions(); + + Action act = () => options.CacheFailureDuration = TimeSpan.FromDays(25); + Assert.ThrowsAny(act); } From 4ea1c879942417dcb637785b35eba8a9014df4de Mon Sep 17 00:00:00 2001 From: Aaron Clauson Date: Thu, 30 Jul 2020 22:44:43 +0100 Subject: [PATCH 4/5] Added unit tests for caching failure responses. --- src/DnsClient/ResponseCache.cs | 19 ++++++---- test/DnsClient.Tests/ResponseCacheTest.cs | 44 +++++++++++++++++++++++ 2 files changed, 56 insertions(+), 7 deletions(-) diff --git a/src/DnsClient/ResponseCache.cs b/src/DnsClient/ResponseCache.cs index 5d4ac26a..b214d498 100644 --- a/src/DnsClient/ResponseCache.cs +++ b/src/DnsClient/ResponseCache.cs @@ -14,6 +14,8 @@ internal class ResponseCache // max is 24 days private static readonly TimeSpan s_maxTimeout = TimeSpan.FromMilliseconds(int.MaxValue); + private static readonly TimeSpan s_defaultFailureTimeout = TimeSpan.FromSeconds(5); + private static readonly int s_cleanupInterval = (int)TimeSpan.FromMinutes(10).TotalMilliseconds; private readonly ConcurrentDictionary _cache = new ConcurrentDictionary(); private readonly object _cleanupLock = new object(); @@ -21,7 +23,7 @@ internal class ResponseCache private int _lastCleanup = 0; private TimeSpan? _minimumTimeout; private TimeSpan? _maximumTimeout; - private TimeSpan? _failureEntryTimeout; + private TimeSpan _failureEntryTimeout = s_defaultFailureTimeout; public int Count => _cache.Count; @@ -57,13 +59,12 @@ public TimeSpan? MaximumTimeout } } - public TimeSpan? FailureEntryTimeout + public TimeSpan FailureEntryTimeout { get { return _failureEntryTimeout; } set { - if (value.HasValue && - (value < TimeSpan.Zero || value > s_maxTimeout) && value != s_infiniteTimeout) + if ((value < TimeSpan.Zero || value > s_maxTimeout) && value != s_infiniteTimeout) { throw new ArgumentOutOfRangeException(nameof(value)); } @@ -77,7 +78,11 @@ public ResponseCache(bool enabled = true, TimeSpan? minimumTimout = null, TimeSp Enabled = enabled; MinimumTimout = minimumTimout; MaximumTimeout = maximumTimeout; - FailureEntryTimeout = failureEntryTimeout; + + if (failureEntryTimeout.HasValue) + { + FailureEntryTimeout = failureEntryTimeout.Value; + } } public static string GetCacheKey(DnsQuestion question) @@ -124,10 +129,10 @@ public bool Add(string key, IDnsQueryResponse response, bool cacheFailures = fal if (Enabled && response != null && (cacheFailures || (!response.HasError && response.Answers.Count > 0))) { - if (response.Answers.Count == 0 && FailureEntryTimeout.HasValue) + if (response.Answers.Count == 0) { // Cache entry for a failure response. - var newEntry = new ResponseEntry(response, FailureEntryTimeout.Value.TotalMilliseconds); + var newEntry = new ResponseEntry(response, FailureEntryTimeout.TotalMilliseconds); StartCleanup(); return _cache.TryAdd(key, newEntry); diff --git a/test/DnsClient.Tests/ResponseCacheTest.cs b/test/DnsClient.Tests/ResponseCacheTest.cs index f0e75d49..9b9e5c11 100644 --- a/test/DnsClient.Tests/ResponseCacheTest.cs +++ b/test/DnsClient.Tests/ResponseCacheTest.cs @@ -207,6 +207,50 @@ public void Cache_GetOrAddExists() var fail = cache.Add("key", response.AsQueryResponse(new NameServer(IPAddress.Any), null)); Assert.False(fail); + } + + [Fact] + public void Cache_DoesNotCacheFailureIfDisabled() + { + var cache = new ResponseCache(true); + var failureStatus = DnsResponseCode.NotExistentDomain; + var response = new DnsResponseMessage(new DnsResponseHeader(1, (ushort)failureStatus, 0, 0, 0, 0), 0); + + cache.Add("key", response.AsQueryResponse(new NameServer(IPAddress.Any), null)); + var item = cache.Get("key", out _); + + // Should be null because cache does not accept failure responses by default. + Assert.Null(item); + } + + [Fact] + public void Cache_DoesCacheFailureIfEnabled() + { + var cache = new ResponseCache(true); + var failureStatus = DnsResponseCode.NotExistentDomain; + var response = new DnsResponseMessage(new DnsResponseHeader(1, (ushort)failureStatus, 0, 0, 0, 0), 0); + + cache.Add("key", response.AsQueryResponse(new NameServer(IPAddress.Any), null), true); + var item = cache.Get("key", out _); + + Assert.NotNull(item); + } + + [Fact] + public async Task Cache_DoesCacheFailureExpire() + { + var cache = new ResponseCache(true, null, null, TimeSpan.FromMilliseconds(1)); + var failureStatus = DnsResponseCode.NotExistentDomain; + var response = new DnsResponseMessage(new DnsResponseHeader(1, (ushort)failureStatus, 0, 0, 0, 0), 0); + + cache.Add("key", response.AsQueryResponse(new NameServer(IPAddress.Any), null), true); + + await Task.Delay(10); + + var item = cache.Get("key", out _); + + // Should be null because failed response expires after 1 millisecond. + Assert.Null(item); } } } \ No newline at end of file From bd39ab897e0772493c7bfcace19e3584a8a2b7a2 Mon Sep 17 00:00:00 2001 From: Aaron Clauson Date: Fri, 31 Jul 2020 08:53:00 +0100 Subject: [PATCH 5/5] Renamed settings for caching failure responses. Fixed up comments. --- src/DnsClient/DnsQueryExtensions.cs | 18 ++++++++++++--- src/DnsClient/DnsQueryOptions.cs | 22 +++++++++---------- src/DnsClient/IDnsQuery.cs | 20 +++++++++++------ src/DnsClient/LookupClient.cs | 6 ++--- .../LookupConfigurationTest.cs | 22 +++++++++---------- 5 files changed, 53 insertions(+), 35 deletions(-) diff --git a/src/DnsClient/DnsQueryExtensions.cs b/src/DnsClient/DnsQueryExtensions.cs index 86770511..3a0086b6 100644 --- a/src/DnsClient/DnsQueryExtensions.cs +++ b/src/DnsClient/DnsQueryExtensions.cs @@ -571,7 +571,7 @@ public static ServiceHostEntry[] ResolveService(this IDnsQuery query, string bas throw new ArgumentNullException(nameof(serviceName)); } - var queryString = ConcatResolveServiceName(baseDomain, serviceName, tag); + var queryString = ConcatServiceName(baseDomain, serviceName, tag); var result = query.Query(queryString, QueryType.SRV); @@ -611,20 +611,32 @@ public static async Task ResolveServiceAsync(this IDnsQuery throw new ArgumentNullException(nameof(serviceName)); } - var queryString = ConcatResolveServiceName(baseDomain, serviceName, tag); + var queryString = ConcatServiceName(baseDomain, serviceName, tag); var result = await query.QueryAsync(queryString, QueryType.SRV).ConfigureAwait(false); return ResolveServiceProcessResult(result); } - public static string ConcatResolveServiceName(string baseDomain, string serviceName, string tag) + /// + /// Constructs a DNS query string from the constituent parts. + /// + /// The base domain, which will be appended to the end of the query string. + /// The name of the service to look for. Must not have any _ prefix. + /// An optional tag. Must not have any _ prefix. + /// A service string that can be used in a DNS service query. + public static string ConcatServiceName(string baseDomain, string serviceName, string tag = null) { return string.IsNullOrWhiteSpace(tag) ? $"{serviceName}.{baseDomain}." : $"_{serviceName}._{tag}.{baseDomain}."; } + /// + /// Transforms a DNS query result into a collection of objects. + /// + /// The DNS + /// A collection of s. public static ServiceHostEntry[] ResolveServiceProcessResult(IDnsQueryResponse result) { var hosts = new List(); diff --git a/src/DnsClient/DnsQueryOptions.cs b/src/DnsClient/DnsQueryOptions.cs index fcb657a3..ee2d49e2 100644 --- a/src/DnsClient/DnsQueryOptions.cs +++ b/src/DnsClient/DnsQueryOptions.cs @@ -27,7 +27,7 @@ public class DnsQueryOptions private static readonly TimeSpan s_maxTimeout = TimeSpan.FromMilliseconds(int.MaxValue); private TimeSpan _timeout = s_defaultTimeout; private int _ednsBufferSize = MaximumBufferSize; - private TimeSpan _cacheFailureDuration = s_defaultTimeout; + private TimeSpan _failedResultsCacheDuration = s_defaultTimeout; /// /// Gets or sets a flag indicating whether each will contain a full documentation of the response(s). @@ -227,15 +227,15 @@ public int ExtendedDnsBufferSize /// failures is to reduce repeated lookup attempts within a short space of time. /// Defaults to False. /// - public bool UseCacheForFailures { get; set; } = false; + public bool CacheFailedResults { get; set; } = false; /// /// Gets or sets the duration to cache failed lookups. Does not apply if failed lookups are not being cached. /// Defaults to 5 seconds. /// - public TimeSpan CacheFailureDuration + public TimeSpan FailedResultsCacheDuration { - get { return _cacheFailureDuration; } + get { return _failedResultsCacheDuration; } set { if ((value <= TimeSpan.Zero || value > s_maxTimeout) && value != s_infiniteTimeout) @@ -243,7 +243,7 @@ public TimeSpan CacheFailureDuration throw new ArgumentOutOfRangeException(nameof(value)); } - _cacheFailureDuration = value; + _failedResultsCacheDuration = value; } } @@ -645,13 +645,13 @@ public class DnsQuerySettings : IEquatable /// failures is to reduce repeated lookup attempts within a short space of time. /// Defaults to False. /// - public bool UseCacheForFailures { get; } + public bool CacheFailedResults { get; } /// /// If failures are being cached this value indicates how long they will be held in the cache for. /// Defaults to 5 seconds. /// - public TimeSpan CacheFailureDuration { get; } + public TimeSpan FailedResultsCacheDuration { get; } /// /// Creates a new instance of . @@ -676,8 +676,8 @@ public DnsQuerySettings(DnsQueryOptions options) UseTcpOnly = options.UseTcpOnly; ExtendedDnsBufferSize = options.ExtendedDnsBufferSize; RequestDnsSecRecords = options.RequestDnsSecRecords; - UseCacheForFailures = options.UseCacheForFailures; - CacheFailureDuration = options.CacheFailureDuration; + CacheFailedResults = options.CacheFailedResults; + FailedResultsCacheDuration = options.FailedResultsCacheDuration; } /// @@ -722,8 +722,8 @@ public bool Equals(DnsQuerySettings other) UseTcpOnly == other.UseTcpOnly && ExtendedDnsBufferSize == other.ExtendedDnsBufferSize && RequestDnsSecRecords == other.RequestDnsSecRecords && - UseCacheForFailures == other.UseCacheForFailures && - CacheFailureDuration.Equals(other.CacheFailureDuration); + CacheFailedResults == other.CacheFailedResults && + FailedResultsCacheDuration.Equals(other.FailedResultsCacheDuration); } } diff --git a/src/DnsClient/IDnsQuery.cs b/src/DnsClient/IDnsQuery.cs index d7018600..ed5577cc 100644 --- a/src/DnsClient/IDnsQuery.cs +++ b/src/DnsClient/IDnsQuery.cs @@ -48,25 +48,31 @@ public interface IDnsQuery IDnsQueryResponse Query(DnsQuestion question, DnsQueryAndServerOptions queryOptions); /// - /// Performs a lookup for the given against the in-memory cache. + /// Returns cached results for the given from the in-memory cache, if available, or Null otherwise. /// + /// + /// This method will not perform a full lookup if there is nothing found in cache or the cache is disabled! + /// /// The domain name query. /// /// The which contains the cached response headers and lists of resource records. - /// If no matching cache entry is found null is returned. + /// If no matching cache entry is found Null is returned. /// - IDnsQueryResponse QueryCache(DnsQuestion question); - + IDnsQueryResponse QueryCache(DnsQuestion question); + /// - /// Performs a DNS lookup for the given , and - /// against the in-memory cache. + /// Returns cached results for the given , and + /// against the in-memory cache, if available, or Null otherwise. /// + /// + /// This method will not perform a full lookup if there is nothing found in cache or the cache is disabled! + /// /// The domain name query. /// The . /// The . /// /// The which contains the cached response headers and lists of resource records. - /// If no matching cache entry is found null is returned. + /// If no matching cache entry is found Null is returned. /// IDnsQueryResponse QueryCache(string query, QueryType queryType, QueryClass queryClass = QueryClass.IN); diff --git a/src/DnsClient/LookupClient.cs b/src/DnsClient/LookupClient.cs index 12421b14..5a5fff78 100644 --- a/src/DnsClient/LookupClient.cs +++ b/src/DnsClient/LookupClient.cs @@ -396,7 +396,7 @@ internal LookupClient(LookupClientOptions options, DnsMessageHandler udpHandler } Settings = new LookupClientSettings(options, servers); - Cache = new ResponseCache(true, Settings.MinimumCacheTimeout, Settings.MaximumCacheTimeout, Settings.CacheFailureDuration); + Cache = new ResponseCache(true, Settings.MinimumCacheTimeout, Settings.MaximumCacheTimeout, Settings.FailedResultsCacheDuration); } private void CheckResolvedNameservers() @@ -894,7 +894,7 @@ private IDnsQueryResponse ResolveQuery( } // If its the last server, return. - if (settings.UseCache && settings.UseCacheForFailures) + if (settings.UseCache && settings.CacheFailedResults) { Cache.Add(cacheKey, lastQueryResponse, true); } @@ -1150,7 +1150,7 @@ private async Task ResolveQueryAsync( } // If its the last server, return. - if (settings.UseCache && settings.UseCacheForFailures) + if (settings.UseCache && settings.CacheFailedResults) { Cache.Add(cacheKey, lastQueryResponse, true); } diff --git a/test/DnsClient.Tests/LookupConfigurationTest.cs b/test/DnsClient.Tests/LookupConfigurationTest.cs index e96f75d8..114a16fc 100644 --- a/test/DnsClient.Tests/LookupConfigurationTest.cs +++ b/test/DnsClient.Tests/LookupConfigurationTest.cs @@ -287,8 +287,8 @@ public void LookupClientOptions_Defaults() Assert.True(options.UseRandomNameServer); Assert.Equal(DnsQueryOptions.MaximumBufferSize, options.ExtendedDnsBufferSize); Assert.False(options.RequestDnsSecRecords); - Assert.False(options.UseCacheForFailures); - Assert.Equal(options.CacheFailureDuration, TimeSpan.FromSeconds(5)); + Assert.False(options.CacheFailedResults); + Assert.Equal(options.FailedResultsCacheDuration, TimeSpan.FromSeconds(5)); } [Fact] @@ -313,8 +313,8 @@ public void LookupClientOptions_DefaultsNoResolve() Assert.True(options.UseRandomNameServer); Assert.Equal(DnsQueryOptions.MaximumBufferSize, options.ExtendedDnsBufferSize); Assert.False(options.RequestDnsSecRecords); - Assert.False(options.UseCacheForFailures); - Assert.Equal(options.CacheFailureDuration, TimeSpan.FromSeconds(5)); + Assert.False(options.CacheFailedResults); + Assert.Equal(options.FailedResultsCacheDuration, TimeSpan.FromSeconds(5)); } [Fact] @@ -340,8 +340,8 @@ public void LookupClient_SettingsValid() UseTcpOnly = !defaultOptions.UseTcpOnly, ExtendedDnsBufferSize = 1234, RequestDnsSecRecords = true, - UseCacheForFailures = true, - CacheFailureDuration = TimeSpan.FromSeconds(10) + CacheFailedResults = true, + FailedResultsCacheDuration = TimeSpan.FromSeconds(10) }; var client = new LookupClient(options); @@ -363,8 +363,8 @@ public void LookupClient_SettingsValid() Assert.Equal(!defaultOptions.UseTcpOnly, client.Settings.UseTcpOnly); Assert.Equal(1234, client.Settings.ExtendedDnsBufferSize); Assert.Equal(!defaultOptions.RequestDnsSecRecords, client.Settings.RequestDnsSecRecords); - Assert.Equal(!defaultOptions.UseCacheForFailures, client.Settings.UseCacheForFailures); - Assert.Equal(TimeSpan.FromSeconds(10), client.Settings.CacheFailureDuration); + Assert.Equal(!defaultOptions.CacheFailedResults, client.Settings.CacheFailedResults); + Assert.Equal(TimeSpan.FromSeconds(10), client.Settings.FailedResultsCacheDuration); Assert.Equal(new LookupClientSettings(options), client.Settings); } @@ -577,7 +577,7 @@ public void LookupClientOptions_InvalidCacheFailureDuration() { var options = new LookupClientOptions(); - Action act = () => options.CacheFailureDuration = TimeSpan.FromMilliseconds(0); + Action act = () => options.FailedResultsCacheDuration = TimeSpan.FromMilliseconds(0); Assert.ThrowsAny(act); } @@ -587,7 +587,7 @@ public void LookupClientOptions_InvalidCacheFailureDuration2() { var options = new LookupClientOptions(); - Action act = () => options.CacheFailureDuration = TimeSpan.FromMilliseconds(-23); + Action act = () => options.FailedResultsCacheDuration = TimeSpan.FromMilliseconds(-23); Assert.ThrowsAny(act); } @@ -597,7 +597,7 @@ public void LookupClientOptions_InvalidCacheFailureDuration3() { var options = new LookupClientOptions(); - Action act = () => options.CacheFailureDuration = TimeSpan.FromDays(25); + Action act = () => options.FailedResultsCacheDuration = TimeSpan.FromDays(25); Assert.ThrowsAny(act); }