Add DNS over HTTPS (DoH) client and resolvers#36
Conversation
Implements DoH client and resolvers for secure DNS queries over HTTPS: - DoHClient: Core client supporting both GET and POST methods per RFC 8484 - DoH resolver base class for custom DoH endpoints - CloudflareDoH: Pre-configured Cloudflare DoH resolver - GoogleDoH: Pre-configured Google DoH resolver Includes comprehensive unit and E2E tests for all new functionality. https://claude.ai/code/session_01W9EN976RGtJV3B3Lu2DYrB
WalkthroughAdds DNS-over-HTTPS support. Introduces DoHClient (GET/POST, endpoint/method validation, request encoding, cURL calls, response decoding and transaction ID checking) and a DoH resolver that wraps it. Adds CloudflareDoH and GoogleDoH concrete resolvers with primary/backup endpoints. Adds unit tests for DoHClient constructor/validation and multiple end-to-end tests for DoH, CloudflareDoH, and GoogleDoH covering A/AAAA/MX/TXT/NS queries and GET/POST methods. Updates docker-compose DNS servers to 8.8.8.8 and 1.1.1.1. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Update docker-compose.yml to use public DNS servers (8.8.8.8, 1.1.1.1) instead of 127.0.0.1 to allow DoH E2E tests to resolve external hostnames like cloudflare-dns.com and dns.google. https://claude.ai/code/session_01W9EN976RGtJV3B3Lu2DYrB
| * @param string $method HTTP method to use (GET or POST) | ||
| */ | ||
| public function __construct( | ||
| bool $useBackup = false, |
There was a problem hiding this comment.
why do we expose useBackup?
There was a problem hiding this comment.
Kept it for symmetry with the existing UDP Resolver\Cloudflare (which also exposes bool $useBackup). Common usage is new Cloudflare() / new Cloudflare(useBackup: true), with the endpoint constants left as escape hatches for anyone who wants to bypass and pass a custom URL through Resolver\Http.
| CURLOPT_TIMEOUT => $this->timeout, | ||
| CURLOPT_CONNECTTIMEOUT => $this->timeout, |
There was a problem hiding this comment.
I think these should be different fields, timeouts are really important to avoid worker exhaustion at our scale
There was a problem hiding this comment.
Done. Split into $timeout (total request, default 5) and $connectTimeout (default 2), wired through Http\Client, Resolver\Http, and the Cloudflare/Google subclasses. Connect timeout maps to CURLOPT_CONNECTTIMEOUT, total to CURLOPT_TIMEOUT, so a slow upstream can no longer eat 5s of worker time before we even know it's unreachable.
| * A resolver that forwards DNS queries to a DoH server over HTTPS. | ||
| * Implements RFC 8484 for DNS queries over HTTP/HTTPS. | ||
| */ | ||
| class DoH implements Resolver |
There was a problem hiding this comment.
Can we call it Http instead of DoH?
There was a problem hiding this comment.
Done. Class is now Utopia\DNS\Resolver\Http (file src/DNS/Resolver/Http.php).
| * Implements DNS queries over HTTPS as specified in RFC 8484. | ||
| * Supports both GET and POST methods for sending DNS queries. | ||
| */ | ||
| class DoHClient |
There was a problem hiding this comment.
could we put this in Utopia\DNS\Http\Client
There was a problem hiding this comment.
Done. Moved to Utopia\DNS\Http\Client at src/DNS/Http/Client.php.
| @@ -0,0 +1,46 @@ | |||
| <?php | |||
|
|
|||
| namespace Utopia\DNS\Resolver; | |||
There was a problem hiding this comment.
Could we but this in Utopia\DNS\Resolver\Http\Cloudflare?
There was a problem hiding this comment.
Done. Now at Utopia\DNS\Resolver\Http\Cloudflare (src/DNS/Resolver/Http/Cloudflare.php). Same restructure applied to GoogleDoH → Utopia\DNS\Resolver\Http\Google.
…imeouts
- Move DoHClient to Utopia\DNS\Http\Client
- Rename Resolver\DoH to Resolver\Http
- Move CloudflareDoH/GoogleDoH under Resolver\Http\{Cloudflare,Google}
- Split single $timeout into $timeout (total) and $connectTimeout (connect)
for better worker exhaustion control under load
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Greptile SummaryThis PR adds a complete DNS-over-HTTPS implementation (RFC 8484) including a core
Confidence Score: 5/5Safe to merge; the core DoH implementation is solid and all issues from prior review rounds have been fixed All previously identified issues — curl handle failure guards, HTTPS-only redirects, Google backup endpoint deduplication, and HTTP scheme rejection — are correctly resolved in the current code. The one remaining gap (POST redirects silently becoming parameterless GET requests without CURLOPT_POSTREDIR) is a hardening concern for edge-case redirect scenarios that don't occur with Cloudflare or Google's current endpoints. src/DNS/Http/Client.php — the queryPost redirect handling is the only area that could behave unexpectedly with a redirecting DoH endpoint Important Files Changed
Reviews (4): Last reviewed commit: "fix: reject non-HTTPS DoH endpoints" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
- Throw Exception when curl_init() returns false in queryPost/queryGet (prevents fatal TypeError on rare cURL init failures) - Change Google ENDPOINT_BACKUP to https://8.8.8.8/dns-query so useBackup actually fails over to a distinct IP-addressed endpoint that survives a DNS resolution failure for dns.google Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add CURLOPT_REDIR_PROTOCOLS => CURLPROTO_HTTPS in both queryPost and queryGet so a malicious or misbehaving DoH server cannot redirect a query to plaintext HTTP, which would defeat the security guarantee of DNS-over-HTTPS. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
FILTER_VALIDATE_URL accepts any scheme (http, ftp, ...). For DoH the endpoint must be HTTPS, otherwise the encryption guarantee is silently lost. Add an explicit https scheme check and a unit test for the http rejection path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 participants
Summary
This PR introduces DNS over HTTPS (DoH) support to the DNS library, implementing RFC 8484 for secure DNS queries over HTTPS. It includes a core DoH client, generic DoH resolver, and pre-configured resolvers for Cloudflare and Google's public DNS services.
Key Changes
DoHClient (
src/DNS/DoHClient.php): Core implementation supporting both GET and POST methods for DNS queries over HTTPSDoH Resolver (
src/DNS/Resolver/DoH.php): Generic resolver implementing theResolverinterfaceCloudflareDoH Resolver (
src/DNS/Resolver/CloudflareDoH.php): Pre-configured resolver for Cloudflare's public DNShttps://cloudflare-dns.com/dns-queryhttps://one.one.one.one/dns-queryGoogleDoH Resolver (
src/DNS/Resolver/GoogleDoH.php): Pre-configured resolver for Google's public DNShttps://dns.google/dns-queryComprehensive Test Suite:
Implementation Details
https://claude.ai/code/session_01W9EN976RGtJV3B3Lu2DYrB
Summary by CodeRabbit
New Features
Tests
Chores