Skip to content

Commit 9038bbb

Browse files
BornToBeRootBornToBeRoot
authored
Chore: Refactor/Improve connection check (#3553)
* Chore: Improve connection check * Docs: #3553 * Fix: Handle servfail only for private range
1 parent 161c4eb commit 9038bbb

8 files changed

Lines changed: 461 additions & 865 deletions

File tree

‎Source/NETworkManager.Utilities/DNSClient.cs‎

Lines changed: 56 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,12 @@ public async Task<DNSClientResultIPAddress> ResolveAAsync(string query)
9393
var result = await _client.QueryAsync(query, QueryType.A);
9494

9595
// Pass the error we got from the lookup client (dns server).
96+
// NXDOMAIN is not a real failure like a timeout - flag it via IsNotFound so callers can
97+
// treat it as "no record". SERVFAIL/REFUSED are not included here: for a forward lookup
98+
// they mean the resolver actually failed or refused the query, not "record does not exist".
9699
if (result.HasError)
97-
return new DNSClientResultIPAddress(result.HasError, result.ErrorMessage, $"{result.NameServer}");
100+
return new DNSClientResultIPAddress(result.HasError, result.ErrorMessage, $"{result.NameServer}")
101+
{ IsNotFound = IsNotFoundResponseCode(result.Header.ResponseCode) };
98102

99103
// Validate result because of https://github.com/BornToBeRoot/NETworkManager/issues/1934
100104
var record = result.Answers.ARecords().FirstOrDefault();
@@ -103,7 +107,8 @@ public async Task<DNSClientResultIPAddress> ResolveAAsync(string query)
103107
? new DNSClientResultIPAddress(record.Address, $"{result.NameServer}")
104108
: new DNSClientResultIPAddress(true,
105109
$"IP address for \"{query}\" could not be resolved and the DNS server did not return an error. Try to check your DNS server with: dig @{result.NameServer.Address} {query}",
106-
$"{result.NameServer}");
110+
$"{result.NameServer}")
111+
{ IsNotFound = true };
107112
}
108113
catch (DnsResponseException ex)
109114
{
@@ -131,8 +136,12 @@ public async Task<DNSClientResultIPAddress> ResolveAaaaAsync(string query)
131136
var result = await _client.QueryAsync(query, QueryType.AAAA);
132137

133138
// Pass the error we got from the lookup client (dns server).
139+
// NXDOMAIN is not a real failure like a timeout - flag it via IsNotFound so callers can
140+
// treat it as "no record". SERVFAIL/REFUSED are not included here: for a forward lookup
141+
// they mean the resolver actually failed or refused the query, not "record does not exist".
134142
if (result.HasError)
135-
return new DNSClientResultIPAddress(result.HasError, result.ErrorMessage, $"{result.NameServer}");
143+
return new DNSClientResultIPAddress(result.HasError, result.ErrorMessage, $"{result.NameServer}")
144+
{ IsNotFound = IsNotFoundResponseCode(result.Header.ResponseCode) };
136145

137146
// Validate result because of https://github.com/BornToBeRoot/NETworkManager/issues/1934
138147
var record = result.Answers.AaaaRecords().FirstOrDefault();
@@ -141,7 +150,8 @@ public async Task<DNSClientResultIPAddress> ResolveAaaaAsync(string query)
141150
? new DNSClientResultIPAddress(record.Address, $"{result.NameServer}")
142151
: new DNSClientResultIPAddress(true,
143152
$"IP address for \"{query}\" could not be resolved and the DNS server did not return an error. Try to check your DNS server with: dig @{result.NameServer.Address} {query}",
144-
$"{result.NameServer}");
153+
$"{result.NameServer}")
154+
{ IsNotFound = true };
145155
}
146156
catch (DnsResponseException ex)
147157
{
@@ -169,8 +179,12 @@ public async Task<DNSClientResultString> ResolveCnameAsync(string query)
169179
var result = await _client.QueryAsync(query, QueryType.CNAME);
170180

171181
// Pass the error we got from the lookup client (dns server).
182+
// NXDOMAIN is not a real failure like a timeout - flag it via IsNotFound so callers can
183+
// treat it as "no record". SERVFAIL/REFUSED are not included here: for a forward lookup
184+
// they mean the resolver actually failed or refused the query, not "record does not exist".
172185
if (result.HasError)
173-
return new DNSClientResultString(result.HasError, result.ErrorMessage, $"{result.NameServer}");
186+
return new DNSClientResultString(result.HasError, result.ErrorMessage, $"{result.NameServer}")
187+
{ IsNotFound = IsNotFoundResponseCode(result.Header.ResponseCode) };
174188

175189
// Validate result because of https://github.com/BornToBeRoot/NETworkManager/issues/1934
176190
var record = result.Answers.CnameRecords().FirstOrDefault();
@@ -179,7 +193,8 @@ public async Task<DNSClientResultString> ResolveCnameAsync(string query)
179193
? new DNSClientResultString(record.CanonicalName, $"{result.NameServer}")
180194
: new DNSClientResultString(true,
181195
$"CNAME for \"{query}\" could not be resolved and the DNS server did not return an error. Try to check your DNS server with: dig @{result.NameServer.Address} {query}",
182-
$"{result.NameServer}");
196+
$"{result.NameServer}")
197+
{ IsNotFound = true };
183198
}
184199
catch (DnsResponseException ex)
185200
{
@@ -207,8 +222,17 @@ public async Task<DNSClientResultString> ResolvePtrAsync(IPAddress ipAddress)
207222
var result = await _client.QueryReverseAsync(ipAddress);
208223

209224
// Pass the error we got from the lookup client (dns server).
225+
// NXDOMAIN is always a clean "no record". For private/ULA IP ranges (the common case for
226+
// router/computer PTR lookups) many resolvers also return SERVFAIL/REFUSED instead of a
227+
// clean NXDOMAIN when there is no reverse zone, so those are treated as "not found" too -
228+
// but only for private ranges, since for a public IP a SERVFAIL/REFUSED usually means the
229+
// resolver actually failed or refused the query.
210230
if (result.HasError)
211-
return new DNSClientResultString(result.HasError, result.ErrorMessage, $"{result.NameServer}");
231+
return new DNSClientResultString(result.HasError, result.ErrorMessage, $"{result.NameServer}")
232+
{
233+
IsNotFound = IsNotFoundResponseCode(result.Header.ResponseCode,
234+
IPAddressHelper.IsPrivateIPAddress(ipAddress))
235+
};
212236

213237
// Validate result because of https://github.com/BornToBeRoot/NETworkManager/issues/1934
214238
var record = result.Answers.PtrRecords().FirstOrDefault();
@@ -217,7 +241,8 @@ public async Task<DNSClientResultString> ResolvePtrAsync(IPAddress ipAddress)
217241
? new DNSClientResultString(record.PtrDomainName, $"{result.NameServer}")
218242
: new DNSClientResultString(true,
219243
$"PTR for \"{ipAddress}\" could not be resolved and the DNS server did not return an error. Try to check your DNS server with: dig @{result.NameServer.Address} -x {ipAddress}",
220-
$"{result.NameServer}");
244+
$"{result.NameServer}")
245+
{ IsNotFound = true };
221246
}
222247
catch (DnsResponseException ex)
223248
{
@@ -229,4 +254,27 @@ public async Task<DNSClientResultString> ResolvePtrAsync(IPAddress ipAddress)
229254
return new DNSClientResultString(true, ex.Message);
230255
}
231256
}
257+
258+
/// <summary>
259+
/// Determines whether a DNS response code means "no record" rather than a real failure.
260+
/// NXDOMAIN is always a clean "does not exist". SERVFAIL and REFUSED are only treated as
261+
/// "no record" when <paramref name="treatServerErrorsAsNotFound" /> is set, since outside of
262+
/// that case they usually indicate the resolver actually failed or refused the query rather
263+
/// than a confirmed absence of the record.
264+
/// </summary>
265+
/// <param name="responseCode">The DNS response code to check.</param>
266+
/// <param name="treatServerErrorsAsNotFound">
267+
/// Whether SERVFAIL/REFUSED should also count as "no record" - true for reverse (PTR) lookups
268+
/// on private/RFC1918/ULA IP ranges, where many resolvers return them instead of a clean
269+
/// NXDOMAIN when there is no reverse zone.
270+
/// </param>
271+
private static bool IsNotFoundResponseCode(DnsHeaderResponseCode responseCode,
272+
bool treatServerErrorsAsNotFound = false)
273+
{
274+
if (responseCode is DnsHeaderResponseCode.NotExistentDomain)
275+
return true;
276+
277+
return treatServerErrorsAsNotFound
278+
&& responseCode is DnsHeaderResponseCode.ServerFailure or DnsHeaderResponseCode.Refused;
279+
}
232280
}

‎Source/NETworkManager.Utilities/DNSClientResult.cs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,4 +58,12 @@ public DNSClientResult(bool hasError, string errorMessage, string dnsServer) : t
5858
/// Error message when an error has occurred.
5959
/// </summary>
6060
public string ErrorMessage { get; set; }
61+
62+
/// <summary>
63+
/// Indicates that <see cref="HasError" /> is set because no matching record was found (NXDOMAIN,
64+
/// or SERVFAIL/REFUSED on a reverse lookup for a private/RFC1918/ULA IP range), rather than a real
65+
/// failure like a timeout, an unreachable server, or SERVFAIL/REFUSED for any other query. This is
66+
/// a normal outcome (e.g. no PTR record configured) and not a sign of a broken DNS setup.
67+
/// </summary>
68+
public bool IsNotFound { get; set; }
6169
}

‎Source/NETworkManager.Utilities/DNSClientResultString.cs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,7 @@ public DNSClientResultString(bool hasError, string errorMessage) : base(hasError
3030
/// <param name="hasError">Indicates if an error has occurred.</param>
3131
/// <param name="errorMessage">Error message when an error has occurred.</param>
3232
/// <param name="dnsServer">DNS server which was used for resolving the query.</param>
33-
public DNSClientResultString(bool hasError, string errorMessage, string dnsServer) : base(hasError, errorMessage,
34-
dnsServer)
33+
public DNSClientResultString(bool hasError, string errorMessage, string dnsServer) : base(hasError, errorMessage, dnsServer)
3534
{
3635
}
3736

‎Source/NETworkManager/ViewModels/ConnectionCheckItem.cs‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
using NETworkManager.Models.Network;
2+
using NETworkManager.Utilities;
3+
4+
namespace NETworkManager.ViewModels;
5+
6+
/// <summary>
7+
/// Represents a single "is checking / value / state" row (e.g. Computer IPv4, Router DNS, ...)
8+
/// shown by <see cref="NetworkConnectionWidgetViewModel"/>.
9+
/// </summary>
10+
public class ConnectionCheckItem : PropertyChangedBase
11+
{
12+
/// <summary>
13+
/// Gets or sets a value indicating whether this item is currently being checked.
14+
/// </summary>
15+
public bool IsChecking
16+
{
17+
get;
18+
set
19+
{
20+
if (value == field)
21+
return;
22+
23+
field = value;
24+
OnPropertyChanged();
25+
}
26+
}
27+
28+
/// <summary>
29+
/// Gets or sets the checked value (e.g. an IP address or hostname).
30+
/// </summary>
31+
public string Value
32+
{
33+
get;
34+
set
35+
{
36+
if (value == field)
37+
return;
38+
39+
field = value;
40+
OnPropertyChanged();
41+
}
42+
} = "";
43+
44+
/// <summary>
45+
/// Gets or sets the connection state of this item.
46+
/// </summary>
47+
public ConnectionState State
48+
{
49+
get;
50+
set
51+
{
52+
if (value == field)
53+
return;
54+
55+
field = value;
56+
OnPropertyChanged();
57+
}
58+
} = ConnectionState.None;
59+
60+
/// <summary>
61+
/// Resets the item to its initial "checking" state.
62+
/// </summary>
63+
public void Reset()
64+
{
65+
IsChecking = true;
66+
Value = "";
67+
State = ConnectionState.None;
68+
}
69+
70+
/// <summary>
71+
/// Completes the check with the given value and state.
72+
/// </summary>
73+
public void Complete(string value, ConnectionState state)
74+
{
75+
Value = value;
76+
State = state;
77+
IsChecking = false;
78+
}
79+
}

0 commit comments

Comments
 (0)