Skip to content

Implement DnsResolver for Windows#129302

Open
rzikm wants to merge 16 commits into
dotnet:mainfrom
rzikm:dns-resolver
Open

Implement DnsResolver for Windows#129302
rzikm wants to merge 16 commits into
dotnet:mainfrom
rzikm:dns-resolver

Conversation

@rzikm

@rzikm rzikm commented Jun 11, 2026

Copy link
Copy Markdown
Member

Implements the API approved in #19443:

  • New Dns.Resolve*[Async] static methods for A/AAAA/SRV/MX/TXT/CNAME/PTR/NS records.
  • New DnsResolver / DnsResolverOptions for instance-based resolution with optional custom DNS servers.
  • Record types AddressRecord, SrvRecord, MxRecord, TxtRecord, CNameRecord, PtrRecord, NsRecord.
  • DnsResponseCode enum and DnsResult envelope carrying ResponseCode, Records, and NegativeCacheTtl.

Windows implementation uses DnsQueryEx. Non-Windows platforms get PlatformNotSupportedException stubs pending follow-up implementations (splitting to multiple stacked PRs for easier review).

rzikm and others added 10 commits June 3, 2026 15:17
Implements the API approved in dotnet#19443:
* New Dns.Resolve*[Async] static methods for A/AAAA/SRV/MX/TXT/CNAME/PTR/NS records.
* New DnsResolver / DnsResolverOptions for instance-based resolution with optional custom DNS servers.
* Record types AddressRecord, SrvRecord, MxRecord, TxtRecord, CNameRecord, PtrRecord, NsRecord.
* DnsResponseCode enum and DnsResult<T> envelope carrying ResponseCode, Records, and NegativeCacheTtl.

Windows implementation uses DnsQueryEx (DNS_QUERY_REQUEST v1 by default; DNS_QUERY_REQUEST3 when the caller supplies non-default ports on Windows 11 build 22000+, throwing PlatformNotSupportedException otherwise). Non-Windows platforms get PlatformNotSupportedException stubs pending follow-up implementations.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds argument-validation tests and Windows-only OuterLoop network tests for the new DnsResolver / Dns.Resolve* APIs.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… port 53

DnsQueryEx only contacts custom DNS servers on the standard port 53 and
requires the sockaddr port field to be 0; any non-zero port (even 53) is
rejected with ERROR_INVALID_PARAMETER. Simplify the Windows resolver to
always use the v1 DNS_QUERY_REQUEST path, write sockaddr port 0, and throw
PlatformNotSupportedException for server endpoints requesting a non-default
port. Remove the now-unused v3 custom-server code path.

Add an in-process loopback DNS server (bound to 127.0.0.1:53, skipped when
the port is unavailable) and a comprehensive behavioral test suite covering
address/SRV/MX/TXT/CNAME/PTR/NS resolution, NXDOMAIN vs NODATA, TTLs,
SRV additional-address glue, port-0 acceptance, and in-flight cancellation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The synchronous Resolve* methods previously blocked on the async path via
GetAwaiter().GetResult(). DnsQueryEx executes synchronously when no completion
callback is supplied, so call it directly instead of going through the async
state machine. Record-list parsing is factored into shared helpers reused by
both the sync and async paths, and the loopback behavioral tests are
parameterized over both APIs.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move all platform-specific logic from DnsResolver.Windows.cs and
DnsResolver.WindowsAsync.cs into a new DnsResolverPal.Windows.cs static
PAL class, mirroring the existing NameResolutionPal pattern. The
cross-platform Resolve*Core methods remain on DnsResolver and delegate to
the PAL, providing a seam for future instrumentation and telemetry.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Instrument the Resolve*Core seam in DnsResolver with the existing
NameResolutionTelemetry infrastructure (EventSource counters, the
DnsLookup Activity span, and the dns.lookup.duration metric), matching
the static Dns class.

When no diagnostics consumer is enabled the PAL task is returned
directly, keeping the common path allocation-free and preserving the
synchronous-completion invariant the sync Resolve* methods depend on.

Extend NameResolutionActivity.Stop to accept a string[] answer so the
new record types can populate the dns.answers tag.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The Resolve*Core methods invoked the PAL eagerly and only then wrapped
the resulting task with telemetry. On the synchronous path the Windows
PAL executes the query while creating the task, so BeforeResolution ran
after most of the work was done and the recorded duration excluded the
query time. Defer the PAL invocation behind a Func so telemetry brackets
the entire query for both sync and async paths.

Adds a regression test asserting the recorded dns.lookup.duration covers
a delayed server response on both sync and async paths.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 11, 2026 16:57
@rzikm rzikm requested a review from a team June 11, 2026 16:58
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @karelz, @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a new DNS query API surface area to System.Net.NameResolution (new Dns.Resolve* static entry points plus an instance-based DnsResolver), and implements the Windows PAL using DnsQueryEx. It also introduces functional tests, including deterministic loopback-server-driven tests for record parsing/handling.

Changes:

  • Adds new public APIs: Dns.Resolve* (A/AAAA/SRV/MX/TXT/CNAME/PTR/NS), DnsResolver, DnsResolverOptions, record structs, DnsResult<T>, and DnsResponseCode.
  • Implements Windows DNS querying/parsing via DnsQueryEx (sync + async paths), plus non-Windows PNSE stubs.
  • Adds functional tests (outer-loop network tests + loopback DNS server tests) and extends NameResolution telemetry to handle string[] answers.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 18 comments.

Show a summary per file
File Description
src/libraries/System.Net.NameResolution/tests/FunctionalTests/System.Net.NameResolution.Functional.Tests.csproj Adds new functional test sources to the project
src/libraries/System.Net.NameResolution/tests/FunctionalTests/LoopbackDnsServer.cs Adds an in-process UDP/TCP loopback DNS server for deterministic tests
src/libraries/System.Net.NameResolution/tests/FunctionalTests/DnsResponseBuilder.cs Adds a fluent builder for DNS response bytes used by loopback tests
src/libraries/System.Net.NameResolution/tests/FunctionalTests/DnsResolverTest.cs Adds outer-loop functional tests for the new APIs (real DNS)
src/libraries/System.Net.NameResolution/tests/FunctionalTests/DnsResolverLoopbackTest.cs Adds deterministic loopback-server-driven tests for parsing/behavior/metrics
src/libraries/System.Net.NameResolution/src/System/Net/NameResolutionTelemetry.cs Extends telemetry answer handling to support string[]
src/libraries/System.Net.NameResolution/src/System/Net/DnsResult.cs Introduces DnsResult<T> envelope type
src/libraries/System.Net.NameResolution/src/System/Net/DnsResponseCode.cs Introduces DnsResponseCode enum
src/libraries/System.Net.NameResolution/src/System/Net/DnsResolverPal.Windows.cs Implements Windows PAL via DnsQueryEx, record parsing, cancellation, glue, TTL extraction
src/libraries/System.Net.NameResolution/src/System/Net/DnsResolverPal.Unsupported.cs Adds non-Windows PNSE PAL stubs
src/libraries/System.Net.NameResolution/src/System/Net/DnsResolverOptions.cs Adds options type for custom server selection
src/libraries/System.Net.NameResolution/src/System/Net/DnsResolver.cs Adds instance-based resolver API + telemetry wrapping
src/libraries/System.Net.NameResolution/src/System/Net/DnsRecords.cs Adds record structs (Address/Srv/Mx/Txt/CName/Ptr/Ns)
src/libraries/System.Net.NameResolution/src/System/Net/Dns.Resolve.cs Adds new static Dns.Resolve* APIs backed by a default resolver
src/libraries/System.Net.NameResolution/src/System/Net/Dns.cs Makes Dns partial to host the new Dns.Resolve* partial
src/libraries/System.Net.NameResolution/src/System.Net.NameResolution.csproj Wires new sources and Windows Dnsapi interop into the build
src/libraries/System.Net.NameResolution/src/Resources/Strings.resx Adds SR string for unsupported custom DNS ports
src/libraries/System.Net.NameResolution/ref/System.Net.NameResolution.cs Adds new public API contract entries
src/libraries/Common/src/Interop/Windows/Interop.Libraries.cs Adds Libraries.Dnsapi constant
src/libraries/Common/src/Interop/Windows/Dnsapi/Interop.DnsTypes.cs Adds DnsQueryEx/DNS_RECORD-related structs/constants
src/libraries/Common/src/Interop/Windows/Dnsapi/Interop.DnsApi.cs Adds LibraryImport declarations for DnsQueryEx/DnsCancelQuery/DnsFree

Comment thread src/libraries/System.Net.NameResolution/src/System/Net/DnsResolverPal.Windows.cs Outdated
Comment thread src/libraries/System.Net.NameResolution/src/System/Net/DnsResolverPal.Windows.cs Outdated
Comment thread src/libraries/System.Net.NameResolution/src/System/Net/DnsResolverPal.Windows.cs Outdated
Comment thread src/libraries/System.Net.NameResolution/src/System/Net/DnsResolverPal.Windows.cs Outdated
Comment thread src/libraries/Common/src/Interop/Windows/Dnsapi/Interop.DnsApi.cs Outdated

@MihaZupan MihaZupan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First pass through.

Just curious, will there be a way to opt-in to using the managed implementation even on Windows, or would there be no reason to want to do that anyway?

Comment on lines +75 to +82
public System.Net.DnsResult<System.Net.AddressRecord> ResolveAddresses(string name) { throw null; }
public System.Net.DnsResult<System.Net.AddressRecord> ResolveAddresses(string name, System.Net.Sockets.AddressFamily addressFamily) { throw null; }
public System.Net.DnsResult<System.Net.SrvRecord> ResolveSrv(string name) { throw null; }
public System.Net.DnsResult<System.Net.MxRecord> ResolveMx(string name) { throw null; }
public System.Net.DnsResult<System.Net.TxtRecord> ResolveTxt(string name) { throw null; }
public System.Net.DnsResult<System.Net.CNameRecord> ResolveCName(string name) { throw null; }
public System.Net.DnsResult<System.Net.PtrRecord> ResolvePtr(string name) { throw null; }
public System.Net.DnsResult<System.Net.NsRecord> ResolveNs(string name) { throw null; }

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public System.Net.DnsResult<System.Net.AddressRecord> ResolveAddresses(string name) { throw null; }
public System.Net.DnsResult<System.Net.AddressRecord> ResolveAddresses(string name, System.Net.Sockets.AddressFamily addressFamily) { throw null; }
public System.Net.DnsResult<System.Net.SrvRecord> ResolveSrv(string name) { throw null; }
public System.Net.DnsResult<System.Net.MxRecord> ResolveMx(string name) { throw null; }
public System.Net.DnsResult<System.Net.TxtRecord> ResolveTxt(string name) { throw null; }
public System.Net.DnsResult<System.Net.CNameRecord> ResolveCName(string name) { throw null; }
public System.Net.DnsResult<System.Net.PtrRecord> ResolvePtr(string name) { throw null; }
public System.Net.DnsResult<System.Net.NsRecord> ResolveNs(string name) { throw null; }

I think we should talk about this with the team :)

My stance continues to be that exposing sync APIs here is a mistake, especially if we have real plans to eventually add support for resolution over HTTPS/QUIC.
The fact that e.g. Resolve silently has 2x the latency compared to the async variant makes the argument that we need sync APIs for perf wrong IMO.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am also not entirely happy with having synchronous versions as well, we can bring it in a team discussion and push back if more of us feel strongly about this :)

Comment thread src/libraries/System.Net.NameResolution/src/System/Net/DnsResolver.cs Outdated
Comment thread src/libraries/System.Net.NameResolution/src/System/Net/DnsResolver.cs Outdated
Comment thread src/libraries/System.Net.NameResolution/src/System/Net/DnsResolver.cs Outdated
Comment thread src/libraries/System.Net.NameResolution/src/System/Net/DnsResolverPal.Windows.cs Outdated
Comment thread src/libraries/System.Net.NameResolution/src/System/Net/DnsResolverPal.Windows.cs Outdated
Comment thread src/libraries/System.Net.NameResolution/src/System/Net/DnsResolverPal.Windows.cs Outdated
Comment thread src/libraries/System.Net.NameResolution/src/System/Net/DnsResolverPal.Windows.cs Outdated
Comment thread src/libraries/System.Net.NameResolution/src/System/Net/DnsResolver.cs Outdated
rzikm and others added 3 commits June 12, 2026 13:25
- Conform instance DnsResolver to approved API: remove ResolvePtr(IPAddress)
  overloads (static Dns keeps them, delegating via BuildArpaName).
- Validate DnsResolverOptions.Servers setter is non-null; resolver takes a
  defensive snapshot of the configured servers.
- Fix negative-cache TTL: always extract from authority SOA and surface it on
  the success path (covers NODATA), plus merge max TTL when combining A/AAAA.
- PAL cleanups: typed GCHandle<T>, bool completion flag, span-based address
  parsing, mixed-address-family validation, checked size arithmetic, simplified
  using-based query helpers.
- Allocation-free telemetry path via TState + static delegates.
- Simplify reverse-arpa name building (plain interpolation / string.Concat).
- Add XML docs across the new public surface; reword/extend SR strings.
- Tests: assert NegativeCacheTtl for NODATA/NXDOMAIN; skip custom-server test
  when no system DNS server is configured; move IPAddress PTR tests to static API.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The approved API shape was corrected to include the IPAddress reverse-lookup
overloads on the instance resolver, so restore ResolvePtr(IPAddress) and
ResolvePtrAsync(IPAddress) on DnsResolver. The static Dns entry points now
delegate to them.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 17, 2026 12:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 4 comments.

Comment thread src/libraries/System.Net.NameResolution/src/System/Net/Dns.Resolve.cs Outdated
rzikm and others added 2 commits June 17, 2026 16:05
Replace pointer-based record and sockaddr handling with safe Span<byte> and
Marshal-based reads where possible:

- TryParseAddress now reads DNS_A_DATA/DNS_AAAA_DATA via Marshal.PtrToStructure
  instead of byte* spans; DNS_AAAA_DATA uses an InlineArray field, removing the
  fixed buffer (and unsafe) from the interop struct.
- BuildAddrArray populates the DNS_ADDR_ARRAY through a Span<byte> using
  BinaryPrimitives; WriteSockAddr takes Span<byte> instead of byte*.
- Drop the unnecessary 'unsafe' modifier from PtrToString.

Also fix a pre-existing compile break in MergeAddressResults (Math.Min has no
TimeSpan overload).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@MichalPetryka

MichalPetryka commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

I've worked with DNS apis here before so here's what I wanted to note:

  • This needs to account for this pre Windows 11 bug and fallback to DnsQueryW for affected cases (it's only when DnsQueryEx would return synchronously).
  • IIRC DnsQueryEx is also broken under impersonization, so this needs identical handling like the GetAddrInfoW handling today has.

- Dns.DefaultResolver: use LazyInitializer.EnsureInitialized for
  thread-safe one-time initialization instead of non-atomic ??=.
- DnsResolver ctor: reject null entries in the server snapshot with an
  ArgumentException pointing at the public Servers property.
- BuildAddrArray: surface mixed-address-family ArgumentException with the
  public-facing 'Servers' parameter name instead of the internal 'servers'.
- LoopbackDnsServer: use ConcurrentDictionary for _responses to avoid
  races between test-thread mutations and listener-thread reads.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 18, 2026 11:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 21 out of 21 changed files in this pull request and generated 6 comments.

Comment on lines +678 to +687
private static void WriteSockAddr(Span<byte> dest, IPEndPoint endpoint)
{
// IPEndPoint.Serialize() yields the platform SOCKADDR layout (family, port,
// address, and for IPv6 the flow info and scope id) via SocketAddressPal, so
// there's no need to lay out the bytes by hand. DnsQueryEx always queries DNS
// servers on port 53 and requires the sockaddr port field to be left as 0;
// serializing a port-0 endpoint satisfies that.
SocketAddress socketAddress = endpoint.Serialize();
socketAddress.Buffer.Span.Slice(0, socketAddress.Size).CopyTo(dest);
}
Comment on lines +556 to +560
// DnsQueryEx only supports DNS servers reachable on the standard port 53.
// The sockaddr port field passed to the API must be 0 (the API always
// queries port 53); supplying any non-zero port - even 53 itself - results
// in ERROR_INVALID_PARAMETER. We therefore reject any server endpoint that
// requests a non-default port, since it cannot be honored on Windows.
Comment on lines +101 to +105
// ---- DNS_RECORD (variable layout: header + Data union) ----
// We declare the fixed header layout and read the data area as a byte blob,
// re-interpreting per record type. The Data field begins at offset 24 on 32-bit
// and at offset 24 on 64-bit pointer layouts as documented; we use a
// Sequential struct with explicit Next/pName pointers.
Comment on lines +14 to +33
public class WindowsLoopbackServer : IAsyncDisposable
{
private LoopbackDnsServer _server;

public WindowsLoopbackServer()
{
_server = LoopbackDnsServer.Start();
}

internal LoopbackDnsServer Server => _server;

public async ValueTask DisposeAsync()
{
if (_server != null)
{
await _server.DisposeAsync();
_server = null;
}
}
}
Comment on lines +48 to +54
[OuterLoop("Binds the loopback DNS port 53 and issues real DnsQueryEx calls.")]
[Collection(nameof(DisableParallelization))]
[PlatformSpecific(TestPlatforms.Windows)]
public class DnsResolverLoopbackTest : IClassFixture<WindowsLoopbackServer>
{
private static DnsResolver CreateResolver(LoopbackDnsServer server)
=> new DnsResolver(new DnsResolverOptions { Servers = { server.EndPoint } });
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants