Fix: double DNS resolution#157
Merged
NGTmeaty merged 2 commits intoNov 13, 2025
Merged
Conversation
Problem: - DNS was resolved twice for every connection - First by gowarc (archiveDNS) for archiving/caching - Then by Go stdlib/proxy for actual connection - Result: wasted resources, duplicate DNS queries Solution: - Track if proxy requires hostname (socks5h, http) vs IP (socks5) - Use gowarc's resolved IP for direct connections and socks5 proxies - Pass hostname only to socks5h/http proxies (remote DNS) Changes: - Add proxyNeedsHostname bool to customDialer struct - Detect proxy scheme in newCustomDialer() to set flag - Extract port and construct IP:port in CustomDialContext() - Extract port and construct IP:port in CustomDialTLSContext() - Use resolved IP when proxy doesn't need hostname OR no proxy Benefits: - Single DNS query per connection (50% reduction) - Respects proxy DNS semantics (socks5h = remote DNS) - Preserves WARC DNS archiving functionality - Improves performance and reduces DNS server load
When using remote DNS proxies (socks5h, socks4a, http, https), DNS archiving is now skipped to avoid privacy leaks and ensure accuracy. Changes: - Modified CustomDialContext() and CustomDialTLSContext() to conditionally skip archiveDNS() call when using remote DNS proxies - For remote DNS proxies: hostname is sent directly to proxy (no local DNS) - For direct/local DNS proxies: DNS is resolved and archived as before - Added comprehensive documentation in README.md explaining DNS behavior for different proxy types Benefits: 1. Privacy: No local DNS queries when using socks5h (prevents DNS leak) 2. Accuracy: WARC reflects actual connection (no local DNS mismatch) 3. Performance: Eliminates unnecessary DNS query for remote DNS proxies Trade-off: No DNS WARC records are created for remote DNS proxy connections, which is acceptable since the proxy handles DNS resolution remotely and local DNS results may differ from the proxy's resolution. This change is part of the DNS optimization work to eliminate duplicate DNS queries and improve privacy when using remote DNS proxies.
Contributor
There was a problem hiding this comment.
Pull Request Overview
This pull request fixes double DNS resolution by introducing logic to distinguish between proxy types that require hostname-based (remote) DNS resolution versus those that can use IP addresses (local DNS). The key improvement is the addition of a proxyNeedsHostname field that determines whether to pass the hostname or resolved IP address to the proxy dialer.
Key changes:
- Added
proxyNeedsHostnamefield to track proxy DNS resolution requirements - Modified dial logic to conditionally use resolved IP or hostname based on proxy type
- Applied changes consistently to both non-TLS and TLS dial contexts
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
NGTmeaty
approved these changes
Nov 13, 2025
NGTmeaty
left a comment
Collaborator
There was a problem hiding this comment.
Great catch! I have one question around why we are setting dialAddr, but otherwise LGTM!
NGTmeaty
added a commit
that referenced
this pull request
Feb 2, 2026
* feat: add happy eyeballs support This resolves issue #172. This issue was a result of #157 dialing to the IP address directly, rather than the DNS name (resulting in a secondary lookup anyways...). In fixing it, we removed our Happy Eyeballs support. This adds it back. * Update dialer.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update dialer.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update dialer.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update dialer.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix: ensure both IPv4 and IPv6 cache entries match expected results. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.
This pull request updates the proxy dialing logic in
dialer.goto correctly handle DNS resolution for different proxy types. The main improvement is that it now distinguishes between proxies that require hostnames (remote DNS resolution) and those that can use resolved IP addresses (local DNS resolution), ensuring proper connection behavior for each proxy type.Right now, DNS resolutions are done twice, with the proxy and without..
This PR disable local DNS queries when socks5h is used. This behavior is documented in the README.