[Ignore, gh stacks tests] Implement DnsResolver for MacOS - #131872
Closed
rzikm wants to merge 14 commits into
Closed
[Ignore, gh stacks tests] Implement DnsResolver for MacOS#131872rzikm wants to merge 14 commits into
rzikm wants to merge 14 commits into
Conversation
Implements the DnsResolver PAL for the unix TFM as a managed stub resolver. It builds and parses DNS wire messages and talks to the configured servers over UDP (with TCP fallback on truncation) using System.Net.Sockets. When no servers are configured, the system servers from /etc/resolv.conf are used, falling back to loopback. - Add internal wire primitives: message header, reader, writer, encoded-name (with IDN/ACE support), and per-type record parsers. - Add DnsResolverPal.Managed.cs query engine with shared sync/async paths; the sync path uses blocking socket calls and returns a completed Task, preserving the task.IsCompleted invariant. - Add ResolvConf.cs nameserver discovery plus parser unit tests. - Wire the new sources into the unix ItemGroup; reference the System.Net.Sockets reference assembly to break the project cycle (Sockets references NameResolution); the implementation resolves from the shared framework at runtime. - Generalize the loopback tests to run on Linux by binding an ephemeral port on non-Windows. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds System.Net.NameResolution.Unit.Tests covering the internal DNS wire-format types (DnsEncodedName, DnsMessageHeader/Reader/Writer, and typed RDATA parsing). The project links the production parsing sources so the internal types can be exercised directly. Also adds DnsEncodedName.GetFormattedLength() to expose the decoded dotted-string length (already computed internally) for buffer sizing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The managed Unix DNS stub resolver used System.Net.Sockets.Socket for UDP/TCP queries. Because System.Net.Sockets already depends on System.Net.NameResolution (Socket.Connect(host, port) resolves names through Dns), the emitted NameResolution.dll carried a real metadata reference back to Sockets, closing a dependency cycle that the shared-framework VerifyClosure task rejects. This built locally but failed the CoreCLR/Mono runtime-pack build legs on CI. Access Socket through a new internal DnsSocket reflection wrapper so NameResolution no longer statically references System.Net.Sockets. The wrapper binds the required Socket members to delegates so exceptions (e.g. SocketException) propagate directly instead of being wrapped in TargetInvocationException, and a DynamicDependency attribute preserves the members for trimming/AOT. SocketException, SocketError and AddressFamily live in System.Net.Primitives and continue to be used directly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…uccess - Gate the loopback DNS resolver tests on IsNotBrowser and IsNotWasi in addition to IsNotMobile. The functional test project multi-targets -browser and -wasi where DnsResolverPal is Unsupported, so these socket-based tests would otherwise run and fail there. - Assert success of DnsMessageReader.TryCreate/TryReadQuestion/TryReadRecord in the DnsRecordTypeTests GetAnswerRecord helper so malformed input surfaces clearly instead of failing in less obvious ways. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
- Preserve an earlier failure reason when a retry attempt is rejected without a specific error, so the surfaced exception is not lost. - Normalize the synchronous per-attempt timed-out SocketException to the same TimeoutException produced by the asynchronous cancellation path. - Validate the echoed question CLASS (IN) in addition to type and name, and fold the reader creation into the guard. - Treat an early TCP EOF as a retryable transport failure (IOException) instead of a malformed response. - Widen non-ASCII response label bytes deterministically instead of relying on Ascii.ToUtf16, whose output is undefined for such input. - Add unit tests for excessive/truncated compression pointers and non-ASCII label decoding. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: db7d5bba-0fcb-4a39-bbe2-fc24b2092abf
….conf - SendQuery now treats a SERVFAIL/REFUSED response code as a soft error: the query fails over to the next configured server, retaining the response so it is surfaced only when no server produces a definitive answer. Buffers are released on every exit path. - ParseAddresses now throws InvalidDataException when an answer record matches the queried type but cannot be parsed (e.g. an A record whose RDLENGTH is not 4), instead of silently reporting NODATA. - ResolvConf caches the parsed /etc/resolv.conf and re-parses only when the file's last-write time changes, avoiding a file read per query. - Add loopback tests for server failover, all-servers-fail, and malformed answer records (managed resolver only). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: db7d5bba-0fcb-4a39-bbe2-fc24b2092abf
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f11fb54e-638e-4c9e-ad82-e5d9ff5c20e4
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
|
Tagging subscribers to this area: @karelz, @dotnet/ncl |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a macOS implementation of DnsResolver (via DNSServiceQueryRecord) and introduces a managed stub resolver + DNS wire-format parsing/writing helpers to support Unix/macOS scenarios and loopback testing. It also expands the test matrix to include -osx TFMs and adds substantial unit/functional coverage for DNS message handling and record parsing.
Changes:
- Add
DnsResolverPal.OSXusingInterop.Dnssd(DNSServiceQueryRecord) for system-configured DNS resolution on macOS. - Add a managed stub resolver (
DnsResolverPal.Managed) plus DNS wire-format types (encoded name, header/reader/writer, record parsers) and/etc/resolv.confparsing. - Add new unit tests for DNS wire encoding/parsing, update PAL/functional tests, and add
-osxTFMs to relevant projects.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/System.Net.NameResolution/tests/UnitTests/System.Net.NameResolution.Unit.Tests.csproj | New unit test project linking wire-format production code for focused unit coverage. |
| src/libraries/System.Net.NameResolution/tests/UnitTests/DnsRecordTypeTests.cs | Adds unit tests for typed DNS RDATA parsing (A/AAAA/CNAME/MX/SRV/TXT/PTR/NS/SOA) including malformed cases. |
| src/libraries/System.Net.NameResolution/tests/UnitTests/DnsMessageWriterTests.cs | Adds unit tests validating DNS query writer output and buffer-boundary behavior. |
| src/libraries/System.Net.NameResolution/tests/UnitTests/DnsMessageReaderTests.cs | Adds unit tests for parsing DNS responses, compression pointers, and truncation/malformed handling. |
| src/libraries/System.Net.NameResolution/tests/UnitTests/DnsMessageHeaderTests.cs | Adds unit tests for DNS header encoding/decoding and round-trips. |
| src/libraries/System.Net.NameResolution/tests/UnitTests/DnsEncodedNameTests.cs | Adds unit tests for DNS name encoding/decoding, pointer rules, and IDN behavior. |
| src/libraries/System.Net.NameResolution/tests/PalTests/System.Net.NameResolution.Pal.Tests.csproj | Includes ResolvConf and its tests for Unix PAL coverage. |
| src/libraries/System.Net.NameResolution/tests/PalTests/ResolvConfTests.cs | Adds tests for /etc/resolv.conf “nameserver” parsing behavior. |
| src/libraries/System.Net.NameResolution/tests/FunctionalTests/System.Net.NameResolution.Functional.Tests.csproj | Adds -osx TFM to functional test target frameworks. |
| src/libraries/System.Net.NameResolution/tests/FunctionalTests/DnsResolverTest.cs | Expands platform coverage to Windows+macOS and adds macOS-specific behavior assertions. |
| src/libraries/System.Net.NameResolution/tests/FunctionalTests/DnsResolverLoopbackTest.cs | Broadens loopback coverage beyond Windows and adds macOS managed-resolver verification for custom servers. |
| src/libraries/System.Net.NameResolution/src/System/Net/ResolvConf.cs | Adds /etc/resolv.conf parsing + caching for system DNS servers on Unix-like platforms. |
| src/libraries/System.Net.NameResolution/src/System/Net/DnsWireEnums.cs | Adds internal DNS wire enums (TYPE/CLASS/OPCODE/flags). |
| src/libraries/System.Net.NameResolution/src/System/Net/DnsSocket.cs | Adds reflection-based socket wrapper to avoid a static dependency cycle on System.Net.Sockets. |
| src/libraries/System.Net.NameResolution/src/System/Net/DnsResolverPal.OSX.cs | Adds macOS PAL using DNS-SD APIs for system-policy DNS resolution when no custom servers are configured. |
| src/libraries/System.Net.NameResolution/src/System/Net/DnsResolverPal.Managed.cs | Adds managed stub resolver for Unix/macOS with UDP+TCP fallback, response validation, and record parsing. |
| src/libraries/System.Net.NameResolution/src/System/Net/DnsRecordParsing.cs | Adds typed record parsing helpers over DnsRecord RDATA. |
| src/libraries/System.Net.NameResolution/src/System/Net/DnsMessageWriter.cs | Adds DNS query writer for header + question section. |
| src/libraries/System.Net.NameResolution/src/System/Net/DnsMessageReader.cs | Adds DNS message reader for header/questions/records with compression-aware name parsing. |
| src/libraries/System.Net.NameResolution/src/System/Net/DnsMessageHeader.cs | Adds encoding/decoding for the fixed 12-byte DNS header. |
| src/libraries/System.Net.NameResolution/src/System/Net/DnsEncodedName.cs | Adds DNS wire-format name type with compression-pointer support and IDN handling. |
| src/libraries/System.Net.NameResolution/src/System.Net.NameResolution.csproj | Adds -osx TFM and wires managed + macOS PAL sources/interop into the build. |
| src/libraries/System.Net.NameResolution/src/Resources/Strings.resx | Adds new resource strings used by the managed resolver. |
| src/libraries/Common/src/Interop/OSX/Interop.Dnssd.cs | Adds DNS-SD interop declarations and constants for macOS resolver implementation. |
Suppressed comments (1)
src/libraries/System.Net.NameResolution/src/System/Net/DnsResolverPal.Managed.cs:809
- Throwing a parameterless IOException here makes failures much harder to diagnose. Prefer an IOException with a resource string (similar to the ReceiveExact* path) so callers get an actionable error message.
int sent = socket.Send(buffer.Slice(totalSent));
if (sent == 0)
{
throw new IOException();
}
Comment on lines
+26
to
+31
| lock (s_lock) | ||
| { | ||
| if (s_cachedServers is not null && writeTimeUtc == s_cachedWriteTimeUtc) | ||
| { | ||
| return s_cachedServers; | ||
| } |
Comment on lines
+791
to
+795
| int sent = await socket.SendAsync(buffer[totalSent..], cancellationToken).ConfigureAwait(false); | ||
| if (sent == 0) | ||
| { | ||
| throw new IOException(); | ||
| } |
Member
Author
looks like stacks are not supported across forks yet |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Raising PR in my fork to try out githubs stacked PRs feature