Handle RFC 6761 special-use domain names in Dns resolution#123076
Handle RFC 6761 special-use domain names in Dns resolution#123076laveeshb wants to merge 7 commits intodotnet:mainfrom
Conversation
dotnet-policy-service
bot
added
the
community-contribution
|
Tagging subscribers to this area: @dotnet/ncl |
There was a problem hiding this comment.
Pull request overview
This PR implements RFC 6761 compliant handling for special-use domain names in System.Net.Dns. The implementation adds logic to intercept DNS queries for invalid/*.invalid domains (returning NXDOMAIN) and *.localhost subdomains (returning loopback addresses) before they reach the OS resolver. Plain localhost continues to use the OS resolver to preserve /etc/hosts customizations.
Changes:
- Added RFC 6761 special-use domain name handling to DNS resolution with proper telemetry integration
- Implemented helper methods for efficient string matching and loopback address allocation
- Added comprehensive test coverage for invalid domains, localhost subdomains, and address family filtering
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/libraries/System.Net.NameResolution/src/System/Net/Dns.cs | Added helper methods (IsReservedName, GetLoopbackAddresses, TryHandleRfc6761SpecialName) and integrated RFC 6761 handling into both sync and async resolution paths |
| src/libraries/System.Net.NameResolution/tests/FunctionalTests/GetHostEntryTest.cs | Added tests for invalid domain rejection, localhost subdomain resolution, address family filtering, and edge cases |
| src/libraries/System.Net.NameResolution/tests/FunctionalTests/GetHostAddressesTest.cs | Added parallel tests for GetHostAddresses API covering invalid domains and localhost subdomains |
| } | ||
|
|
||
| // RFC 6761 Section 6.3: "*.localhost" subdomains must resolve to loopback. | ||
| // Note: Plain "localhost" is handled by the OS resolver (preserves /etc/hosts customizations). |
There was a problem hiding this comment.
It feels weird to allow customizations for "localhost" name itself, but not for subdomains.
It also means that without any customization, names "localhost" and "a.localhost" may return different results (e.g. IPv4 and IPv6 addresses in different order)
There was a problem hiding this comment.
You're right that this creates an inconsistency where localhost and a.localhost may return different results.
The current approach defers plain \localhost\ to the OS resolver intentionally — handling it in code would be a breaking change for users with custom hosts file entries (e.g., mapping localhost to a specific IP).
Handling both identically would be more RFC 6761 compliant and provide consistent behavior, but at the cost of backward compatibility.
My preference would be to avoid the breaking change, but I'm not sure what the general practice is in dotnet for situations like this. Is there guidance on when RFC compliance should take precedence over backward compatibility?
There was a problem hiding this comment.
what if we strip the subdomains and return the same result as simply querying "localhost"? that would preserve the existing behavior and in majority of the cases the subdomains would be resolved to loopback addresses anyway (and we no longer need to care about whether IPv4 or IPv6 is enabled as that would be handled by the native layer)
There was a problem hiding this comment.
Done - this is exactly what the updated implementation does.
laveeshb
force-pushed
the
fix/rfc6761-special-domains
branch
from
84b6b50 to
8fbc05e
Compare
|
Correction to formatting above: Thanks for the feedback @rzikm! I've updated the implementation based on your suggestion: Changes made:
Benefits of this approach:
The All 133 tests pass, including new tests that verify localhost subdomains return the same addresses as plain |
| bool isLocalhostSubdomain = IsLocalhostSubdomain(hostName); | ||
| Task? t; | ||
| if (NameResolutionTelemetry.AnyDiagnosticsEnabled()) | ||
| if (NameResolutionTelemetry.AnyDiagnosticsEnabled() || isLocalhostSubdomain) |
There was a problem hiding this comment.
Why do we need the telemetry-enabled path for localhost subdomains? What is preventing us from using the non-telemetry path?
There was a problem hiding this comment.
The telemetry-enabled path (GetAddrInfoWithTelemetryAsync) contains the localhost subdomain fallback logic - if the OS resolver returns an empty address list or throws a SocketException, it falls back to resolving plain localhost instead. This is the RFC 6761 Section 6.3 compliant behavior for *.localhost subdomains.
The non-telemetry path (NameResolutionPal.GetAddrInfoAsync) doesn't have this fallback handling - it just returns whatever the OS resolver returns. So we need to route localhost subdomains through the telemetry-enabled path to ensure the fallback mechanism is invoked when needed.
The condition could perhaps be renamed to be clearer, e.g., GetAddrInfoWithFallbackAsync or add a comment explaining that the method handles both telemetry and RFC 6761 fallback. Would that help clarify the intent?
|
Some of the test failures seems relevant @laveeshb @rzikm is out this week but we should figure out the test failures. |
@wfurt I'll take a look at the unit test failures |
|
@laveeshb any idea when you will get time to get to it? |
|
Investigating the test failure in The failing test shows: After investigation, I believe this is happening because:
This is actually consistent with the agreed behavior from @rzikm's comment:
The OS resolver succeeds with valid loopback addresses, so no fallback occurs - the behavior is working as designed. The test was incorrectly assuming that the OS resolver would always fail for Proposed fix: Update the test to verify that localhost subdomains return loopback addresses (RFC 6761 requirement), rather than requiring exact equality with plain // Verify both return loopback addresses (the key RFC 6761 requirement)
Assert.All(localhostAddresses, addr => Assert.True(IPAddress.IsLoopback(addr)));
Assert.All(subdomainAddresses, addr => Assert.True(IPAddress.IsLoopback(addr)));This makes the test consistent with @rzikm @wfurt - Does this analysis look correct? Should I proceed with the test fix, or is there a preference for different behavior? |
laveeshb
force-pushed
the
fix/rfc6761-special-domains
branch
from
b86c5db to
b58b6c1
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
src/libraries/System.Net.NameResolution/tests/FunctionalTests/GetHostAddressesTest.cs:295
- This equality assertion will fail on machines where the OS resolver returns a configured address set for
foo.localhost/bar.localhost(the new code returns OS results when non-empty). To keep the test deterministic, use a unique subdomain per run to ensure fallback tolocalhost, or change the assertion to allow OS-customized results.
IPAddress[] localhostAddresses = Dns.GetHostAddresses("localhost");
IPAddress[] subdomainAddresses = Dns.GetHostAddresses("foo.localhost");
// Both should return loopback addresses (the subdomain falls back to localhost resolution)
Assert.True(localhostAddresses.Length >= 1);
Assert.True(subdomainAddresses.Length >= 1);
// The addresses should be equivalent (same loopback addresses)
Assert.Equal(
localhostAddresses.OrderBy(a => a.ToString()).ToArray(),
subdomainAddresses.OrderBy(a => a.ToString()).ToArray());
// Async version
localhostAddresses = await Dns.GetHostAddressesAsync("localhost");
subdomainAddresses = await Dns.GetHostAddressesAsync("bar.localhost");
Assert.Equal(
localhostAddresses.OrderBy(a => a.ToString()).ToArray(),
subdomainAddresses.OrderBy(a => a.ToString()).ToArray());
}
src/libraries/System.Net.NameResolution/tests/FunctionalTests/GetHostEntryTest.cs:366
- These assertions require that any
*.localhostresolution returns only loopback addresses. But the implementation explicitly tries the OS resolver first to preserve/etc/hosts/ local DNS customizations, which may legitimately return non-loopback (or different loopback) addresses. This makes the test configuration-dependent. Consider using a per-run unique subdomain to reliably exercise the fallback path (and then assert loopback), or relax the assertion to accept non-empty OS-provided results.
// The subdomain goes to OS resolver first. If it fails (likely on most systems),
// it falls back to resolving plain "localhost", which should return loopback addresses.
IPHostEntry entry = Dns.GetHostEntry(hostName);
Assert.True(entry.AddressList.Length >= 1, "Expected at least one loopback address");
Assert.All(entry.AddressList, addr => Assert.True(IPAddress.IsLoopback(addr), $"Expected loopback address but got: {addr}"));
entry = await Dns.GetHostEntryAsync(hostName);
Assert.True(entry.AddressList.Length >= 1, "Expected at least one loopback address");
Assert.All(entry.AddressList, addr => Assert.True(IPAddress.IsLoopback(addr), $"Expected loopback address but got: {addr}"));
src/libraries/System.Net.NameResolution/tests/FunctionalTests/GetHostEntryTest.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.NameResolution/tests/FunctionalTests/GetHostAddressesTest.cs
Show resolved
Hide resolved
LGTM, proceed with your proposed fix |
laveeshb
force-pushed
the
fix/rfc6761-special-domains
branch
from
45d43ea to
4b6ce99
Compare
src/libraries/System.Net.NameResolution/tests/FunctionalTests/GetHostEntryTest.cs
Show resolved
Hide resolved
src/libraries/System.Net.NameResolution/tests/FunctionalTests/GetHostAddressesTest.cs
Outdated
Show resolved
Hide resolved
Implement RFC 6761 compliant handling for special-use domain names: - 'invalid' and '*.invalid' domains return SocketError.HostNotFound (NXDOMAIN) - '*.localhost' subdomains resolve to loopback addresses (127.0.0.1 and/or ::1) - Plain 'localhost' continues to use OS resolver to preserve /etc/hosts customizations Implementation details: - Pre-allocated loopback address arrays to avoid allocations on hot path - Respects AddressFamily parameter for filtered results - Includes telemetry for diagnostics consistency - Case-insensitive matching per RFC specification Added tests for invalid domains, localhost subdomains, AddressFamily filtering, and edge cases like 'notlocalhost' to ensure similar names aren't incorrectly matched.
- Fix IPv6 address order: use [IPv6Loopback, Loopback] to match Windows resolver behavior - Add validation to reject malformed hostnames (starting with '.' or containing '..') in IsReservedName, letting them fall through to OS resolver - Add tests for malformed hostname edge cases (.localhost, foo..localhost, .invalid, test..invalid)
- RFC 6761 Section 6.4: 'invalid' and '*.invalid' domains immediately return NXDOMAIN - RFC 6761 Section 6.3: '*.localhost' subdomains try OS resolver first, fall back to plain 'localhost' resolution if OS returns failure or empty results - Handles trailing dots (DNS root notation) correctly - Rejects malformed hostnames (starting with '.' or containing '..') by passing to OS resolver - Comprehensive test coverage for all edge cases
The DnsGetHostAddresses_LocalhostSubdomain_ReturnsSameAsLocalhost test was failing because on systems with systemd-resolved, the OS resolver handles *.localhost natively and may return different addresses (e.g., both IPv6+IPv4) than plain localhost (IPv4 only from /etc/hosts). Update the test to verify the RFC 6761 requirement: localhost subdomains must return loopback addresses. This is consistent with the agreed design where the OS resolver is tried first and fallback only occurs if empty.
laveeshb
force-pushed
the
fix/rfc6761-special-domains
branch
from
4b6ce99 to
a054182
Compare
- Fix TryHandleRfc6761InvalidDomain setting HResult to 0 (success) by using SocketException constructor directly instead of CreateException with nativeError=0, which overwrote the failure HResult. - Rename ReturnsSameAsLocalhost tests to ReturnsLoopback to match what they actually verify. - Fix GetHostEntryTest to verify loopback addresses instead of exact equality (same fix as GetHostAddressesTest).
src/libraries/System.Net.NameResolution/tests/FunctionalTests/GetHostEntryTest.cs
Show resolved
Hide resolved
src/libraries/System.Net.NameResolution/tests/FunctionalTests/GetHostAddressesTest.cs
Show resolved
Hide resolved
- Move localhost fallback recursive call outside try-catch in sync path to prevent LogFailure from calling AfterResolution a second time when the fallback itself throws. - Add !fallbackOccurred guard to async catch filter to prevent re-catching exceptions from an already-initiated fallback. - Rename [Fact] ReturnsLoopback() methods to BothReturnLoopback() to avoid name collision with [Theory] ReturnsLoopback(string) overloads. - Add case-insensitive InlineData to GetHostAddresses localhost tests. - Add SimilarButNotReserved, MalformedReservedName, and LocalhostWithTrailingDot negative tests to GetHostAddressesTest for parity with GetHostEntryTest.
| throw CreateException(errorCode, nativeErrorCode); | ||
| } | ||
| } | ||
| else if (isLocalhostSubdomain && addresses.Length == 0) |
There was a problem hiding this comment.
| else if (isLocalhostSubdomain && addresses.Length == 0) | |
| else if (addresses.Length == 0 && IsLocalhostSubdomain(hostName)) |
Presumably if the common case is that DNS is giving us some addresses, we can avoid the (admittedly small) overhead of IsLocalhostSubdomain
There was a problem hiding this comment.
Done — swapped to addresses.Length == 0 && isLocalhostSubdomain.
There was a problem hiding this comment.
I meant that we can run the IsLocalhostSubdomain(hostName) checks lazily instead of lifting it to a local. In the common case where we do get a result, we don't have to call it.
| fallbackOccurred = true; | ||
|
|
||
| // Resolve plain "localhost" instead | ||
| return await ((Task<T>)(justAddresses |
There was a problem hiding this comment.
Isn't it guaranteed that justAddresses = true here?
There was a problem hiding this comment.
No — GetAddrInfoWithTelemetryAsync<T> is called with both T = IPAddress[] (justAddresses=true) and T = IPHostEntry (justAddresses=false) from the caller at line ~724. The CompleteAsync function handles both cases, which is why it has separate result is IPAddress[] and result is IPHostEntry checks for the empty-result fallback.
There was a problem hiding this comment.
The method is, but the result is IPAddress[] check implies that justAddresses == true in this branch, no?
- Use EndsWith('.') API instead of manual length-1 char checks
- Restore ValidateAddressFamily before IP/hostname split in async path
- Swap condition order: addresses.Length == 0 first for micro-optimization
- Use 'object? result = null' with Debug.Assert instead of null!
- Extract 'localhost'/'invalid' string literals into shared consts
| Assert.True(addresses.Length >= 1, "Expected at least one address"); | ||
| Assert.All(addresses, addr => Assert.Equal(addressFamily, addr.AddressFamily)); | ||
|
|
||
| addresses = await Dns.GetHostAddressesAsync(hostName, addressFamily); | ||
| Assert.True(addresses.Length >= 1, "Expected at least one address"); |
There was a problem hiding this comment.
In the AddressFamily-specific localhost-subdomain test, the InterNetworkV6 case asserts addresses.Length >= 1. This is likely to be flaky on Linux CI: TestSettings indicates Linux CI may not have IPv6 localhost configured, so Dns.GetHostAddresses("localhost", InterNetworkV6) (and the *.localhost fallback) can legitimately return an empty array even when Socket.OSSupportsIPv6 is true. Consider conditioning the non-empty assertion on actually having at least one IPv6 localhost address, or removing the Length >= 1 requirement and only validating the address families for any returned addresses.
| Assert.True(addresses.Length >= 1, "Expected at least one address"); | |
| Assert.All(addresses, addr => Assert.Equal(addressFamily, addr.AddressFamily)); | |
| addresses = await Dns.GetHostAddressesAsync(hostName, addressFamily); | |
| Assert.True(addresses.Length >= 1, "Expected at least one address"); | |
| Assert.All(addresses, addr => Assert.Equal(addressFamily, addr.AddressFamily)); | |
| addresses = await Dns.GetHostAddressesAsync(hostName, addressFamily); |
5 participants
Handle RFC 6761 special-use domain names in Dns resolution
Description
This PR implements RFC 6761 compliant handling for special-use domain names in
System.Net.Dns.RFC 6761 compliance:
invalidand*.invalid- ReturnSocketError.HostNotFound(NXDOMAIN) per Section 6.4*.localhostsubdomains - Try OS resolver first, fall back to plainlocalhostresolution if OS returns failure or empty (per Section 6.3)localhost- Continues to use OS resolver to preserve/etc/hostscustomizationsBehavior Change
Before this PR:
Dns.GetHostAddresses("test.invalid")- Would query DNS servers and eventually failDns.GetHostAddresses("foo.localhost")- Would fail with HostNotFound (most systems don't have subdomains configured)After this PR:
Dns.GetHostAddresses("test.invalid")- Immediately returnsSocketError.HostNotFoundwithout network I/ODns.GetHostAddresses("foo.localhost")- Tries OS resolver first, falls back to returning loopback addresses if OS fails/returns emptyThis is an intentional breaking change for RFC 6761 compliance. Applications that previously caught
SocketExceptionfor*.localhostsubdomains will now get successful results (loopback addresses). This enables local development scenarios where services use*.localhostsubdomains.Implementation Notes
Updated based on team feedback from @rzikm:
The previous implementation returned pre-allocated loopback arrays immediately for
*.localhostsubdomains. This was changed to:/etc/hostsor local DNS) to work correctlylocalhostresolution - If OS resolver fails or returns empty results, resolve plainlocalhostinstead to guarantee loopback addressesThis approach:
localhoston typical systemsAddressFamilyfilteringKey implementation details:
GetHostEntryOrAddressesCore) are moved outside thetry/catchto preventLogFailurefrom double-reporting telemetry when the fallback itself throwsCompleteAsync) guard thecatchfilter with!fallbackOccurredto prevent catching exceptions from an already-initiated fallbackTryHandleRfc6761InvalidDomainusesnew SocketException((int)SocketError.HostNotFound)directly instead ofCreateException(SocketError.HostNotFound, 0)which would overwrite the HResult with 0 (S_OK)Testing
Added comprehensive tests covering both
GetHostAddressesandGetHostEntry:invalid,test.invalid,foo.bar.invalid,invalid.,test.invalid.)foo.localhost,bar.foo.localhost,foo.localhost.)INVALID,Test.INVALID,FOO.LOCALHOST,Test.LocalHost)AddressFamilyfiltering for localhost subdomainsnotlocalhost,localhostfoo,invalidname,testinvalid).localhost,foo..localhost,.invalid,test..invalid) passed through to OS resolverlocalhost.(with trailing dot) not treated as a subdomainFixes #118569