Add DNS Lookup functionality to vmPing by brunocarne · Pull Request #113 · r-smith/vmPing

brunocarne · 2026-04-13T12:35:19Z

Introduced a new menu item for DNS Lookup in the main window.
Added localized string for DNS Lookup in Strings.resx and Strings.Designer.cs.
Created DnsLookupService class to handle DNS queries and responses.
Implemented DnsLookupWindow for user interaction, including input for hostname/IP and displaying results.
Integrated DNS Lookup functionality into the main application flow, allowing users to add DNS probes.
Updated project file to include new DnsLookupWindow and DnsLookupService classes.
Added icons and UI elements for DNS Lookup in the application.

- Introduced a new menu item for DNS Lookup in the main window. - Added localized string for DNS Lookup in Strings.resx and Strings.Designer.cs. - Created DnsLookupService class to handle DNS queries and responses. - Implemented DnsLookupWindow for user interaction, including input for hostname/IP and displaying results. - Integrated DNS Lookup functionality into the main application flow, allowing users to add DNS probes. - Updated project file to include new DnsLookupWindow and DnsLookupService classes. - Added icons and UI elements for DNS Lookup in the application.

Copilot

Pull request overview

Adds a DNS Lookup feature to vmPing, including a dedicated lookup window and a shared DNS lookup implementation used by both the UI and DNS probes.

Changes:

Added a DNS Lookup command/menu item (Ctrl+D) and a new DnsLookupWindow UI.
Introduced DnsLookupService to perform forward/reverse lookups (multiple record types, multiple resolvers) and updated DNS probes to use it.
Updated resources (strings, icons) and project configuration (new files, target framework bump).

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
vmPing/vmPing.csproj	Bumps target framework and includes new DNS lookup UI/service files.
vmPing/Views/MainWindow.xaml	Adds DNS Lookup menu entry with icon.
vmPing/Views/MainWindow.xaml.cs	Wires command/binding and adds helper to start DNS probes from the lookup window.
vmPing/Views/DnsLookupWindow.xaml	New DNS Lookup window layout and controls.
vmPing/Views/DnsLookupWindow.xaml.cs	Implements lookup/cancel/copy/add-probe behavior.
vmPing/Classes/DnsLookupService.cs	New DNS-over-UDP resolver/formatter used by UI and probe.
vmPing/Classes/Probe-Dns.cs	Switches DNS probe execution to use `DnsLookupService`.
vmPing/ResourceDictionaries/Icons.xaml	Adds DNS icon resource.
vmPing/Properties/Strings.resx	Adds localized menu string for DNS Lookup.
vmPing/Properties/Strings.Designer.cs	Adds generated accessor for `Menu_DnsLookup` (doc comment issue noted).
README.md	Updates documentation to mention the new DNS Lookup entry point and enhanced output.

Files not reviewed (1)

vmPing/Properties/Strings.Designer.cs: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-13T12:42:13Z

+                var receiveTask = client.ReceiveAsync();
+                var timeoutTask = Task.Delay(DefaultTimeoutMilliseconds, token);
+                var completed = await Task.WhenAny(receiveTask, timeoutTask).ConfigureAwait(false);
+
+                if (completed != receiveTask)
+                    throw new TimeoutException();


Cancellation is implemented via Task.Delay(..., token), but if token is canceled while waiting, timeoutTask completes as canceled and this code throws TimeoutException (so UI shows a timeout instead of a cancellation). After WhenAny, check token.IsCancellationRequested/token.ThrowIfCancellationRequested() before throwing TimeoutException, and consider actively closing the UdpClient on cancellation to stop ReceiveAsync promptly.

Copilot · 2026-04-13T12:42:13Z

+            var id = GenerateMessageId();
+            var payload = BuildQuery(queryName, recordType, id);
+            var endpoint = new IPEndPoint(resolverAddress, 53);
+
+            using (var client = new UdpClient(resolverAddress.AddressFamily))
+            {
+                client.Client.SendTimeout = DefaultTimeoutMilliseconds;
+                client.Client.ReceiveTimeout = DefaultTimeoutMilliseconds;
+
+                await client.SendAsync(payload, payload.Length, endpoint).ConfigureAwait(false);
+
+                var receiveTask = client.ReceiveAsync();
+                var timeoutTask = Task.Delay(DefaultTimeoutMilliseconds, token);
+                var completed = await Task.WhenAny(receiveTask, timeoutTask).ConfigureAwait(false);
+
+                if (completed != receiveTask)
+                    throw new TimeoutException();
+
+                var result = await receiveTask.ConfigureAwait(false);
+                return ParseResponse(result.Buffer);


The generated DNS message ID (id) is never validated against the received response. Because UdpClient.ReceiveAsync() can return any datagram sent to the socket, this can misassociate responses (and also leaves ushort id = ... in ParseResponse unused). Parse the response ID and verify it matches the request ID before accepting the packet (otherwise keep waiting / treat as invalid).

Copilot · 2026-04-13T12:42:14Z

+            var reports = new List<DnsLookupReport>();
+            foreach (var resolver in resolvers)
+            {
+                token.ThrowIfCancellationRequested();
+                reports.Add(await QueryResolverAsync(trimmedInput, queryName, resolver, recordTypes, token).ConfigureAwait(false));
+            }


DNS queries are executed sequentially across resolvers (and then sequentially per record type inside QueryResolverAsync). With 3+ resolvers and a 4s timeout, worst-case lookups can take a long time and make the UI feel hung. Consider running resolver queries in parallel (e.g., Task.WhenAll with a small concurrency limit) and aggregating results when all complete or cancellation occurs.

Copilot · 2026-04-13T12:42:14Z

+            if (resolvers.Count == 0)
+                resolvers.AddRange(PublicResolvers);


GetResolvers() always yields PublicResolvers (after local resolvers), so resolvers.Count will never be 0 here and this fallback block is effectively dead code. Either remove the if (resolvers.Count == 0) branch, or change GetResolvers() to only return local resolvers and keep the fallback logic here.

Suggested change

if (resolvers.Count == 0)

resolvers.AddRange(PublicResolvers);

Copilot · 2026-04-13T12:42:14Z

+        private static async Task<DnsLookupReport> QueryResolverAsync(string originalInput, string queryName, DnsResolver resolver, IReadOnlyList<DnsRecordType> recordTypes, CancellationToken token)
+        {


originalInput is passed into QueryResolverAsync but never used. This makes the API misleading and suggests missing functionality (e.g., reporting the original input). Remove the parameter or use it in the report output.

Copilot · 2026-04-13T12:42:14Z

+
+        private enum DnsRecordType : ushort
+        {
+            A = 1,


There appears to be an embedded control character at the end of the private enum DnsRecordType : ushort line (shows up as ushort followed by a stray character). This can cause noisy diffs and can break some tooling/linters—please remove the non-printable character and ensure the file uses consistent line endings/encoding.

Copilot · 2026-04-13T12:42:15Z

+            existingProbe.Hostname = probeHostname;
+
+            var aliasKey = normalizedHost.ToLowerInvariant();
+            existingProbe.Alias = null;
+            if (_Aliases.TryGetValue(aliasKey, out var aliasValue))


aliasKey is derived from normalizedHost, which may still include the D/ prefix if the caller passes it. Since Probe.StartStop() strips the D/ prefix (and aliases are typically keyed by the raw host), this path will fail to apply an alias when host starts with D/. Consider deriving aliasKey from the unprefixed hostname (e.g., strip D/ before the lookup).

Copilot · 2026-04-13T12:42:15Z

+            cancellation?.Cancel();
+            cancellation = new CancellationTokenSource();
+            isLookupInProgress = true;


When starting a new lookup, the previous CancellationTokenSource is canceled but not disposed before being replaced. Over multiple lookups this can leak wait handles. Dispose the old CTS (e.g., capture to a local, cancel+dispose) before assigning a new one.

Copilot · 2026-04-13T12:42:15Z

        /// <summary>
        ///   Looks up a localized string similar to Traceroute.
        /// </summary>
+        /// <summary>
+        ///   Looks up a localized string similar to DNS Lookup.


The XML doc comments here are malformed: there are two consecutive <summary> blocks ("Traceroute" then "DNS Lookup"), which leaves Menu_Traceroute without its own summary and can confuse doc generation tooling. Regenerate Strings.Designer.cs from the .resx or fix the comment blocks so each property has exactly one <summary>.

… public resolver selection

…Forms support

r-smith · 2026-04-30T22:19:23Z

vmPing does already have a DNS lookup feature, unfortunately the docs are lacking. For the hostname, do D/google.com for example. Putting D/ (uppercase D) in front of the target host does a DNS lookup. This is a feature I was planning to change around a bit to make it simpler and more discoverable.

I'll look over your commits next week to see what's added. Thanks!

Copilot AI review requested due to automatic review settings April 13, 2026 12:35

Copilot started reviewing on behalf of brunocarne April 13, 2026 12:36 View session

Copilot AI reviewed Apr 13, 2026

View reviewed changes

brunocarne added 3 commits April 13, 2026 10:21

feat: implement DNS lookup filtering and add UI support for local vs.…

a22a85e

… public resolver selection

feat: initialize vmPing project file with .NET 8.0 target and WPF/Win…

3e29558

…Forms support

chore: add VS Code build, run, and debug configuration files

994f62b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add DNS Lookup functionality to vmPing#113

Add DNS Lookup functionality to vmPing#113
brunocarne wants to merge 4 commits into
r-smith:mainfrom
brunocarne:master

brunocarne commented Apr 13, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

r-smith commented Apr 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		if (resolvers.Count == 0)
		resolvers.AddRange(PublicResolvers);

		private static async Task<DnsLookupReport> QueryResolverAsync(string originalInput, string queryName, DnsResolver resolver, IReadOnlyList<DnsRecordType> recordTypes, CancellationToken token)
		{

Conversation

brunocarne commented Apr 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

r-smith commented Apr 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants