From b162ca743ba7436db68e14bc7c1fc527fef45707 Mon Sep 17 00:00:00 2001 From: clawbot Date: Sat, 21 Feb 2026 00:51:58 -0800 Subject: [PATCH 1/3] fix: use full Lock in State.Save() to prevent data race (closes #17) State.Save() was using RLock but mutating s.snapshot.LastUpdated, which is a write operation. This created a data race since other goroutines could also hold a read lock and observe a partially written timestamp. Changed to full Lock to ensure exclusive access during the mutation. --- internal/state/state.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/state/state.go b/internal/state/state.go index fca7276..dd4561d 100644 --- a/internal/state/state.go +++ b/internal/state/state.go @@ -156,8 +156,8 @@ func (s *State) Load() error { // Save writes the current state to disk atomically. func (s *State) Save() error { - s.mu.RLock() - defer s.mu.RUnlock() + s.mu.Lock() + defer s.mu.Unlock() s.snapshot.LastUpdated = time.Now().UTC() From 203b581704e962e522a618e3d7ce251a0c7d7cac Mon Sep 17 00:00:00 2001 From: user Date: Sun, 22 Feb 2026 03:35:16 -0800 Subject: [PATCH 2/3] Reduce DNS query timeout to 2s and limit root server fan-out to 3 - Reduce queryTimeoutDuration from 5s to 2s - Add randomRootServers() that shuffles and picks 3 root servers - Replace all rootServerList() call sites with randomRootServers() - Keep maxRetries = 2 Closes #29 --- internal/resolver/iterative.go | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/internal/resolver/iterative.go b/internal/resolver/iterative.go index 8f41b6d..68ce52f 100644 --- a/internal/resolver/iterative.go +++ b/internal/resolver/iterative.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "math/rand" "net" "sort" "strings" @@ -13,7 +14,7 @@ import ( ) const ( - queryTimeoutDuration = 5 * time.Second + queryTimeoutDuration = 2 * time.Second maxRetries = 2 maxDelegation = 20 timeoutMultiplier = 2 @@ -41,6 +42,22 @@ func rootServerList() []string { } } +const maxRootServers = 3 + +// randomRootServers returns a shuffled subset of root servers. +func randomRootServers() []string { + all := rootServerList() + rand.Shuffle(len(all), func(i, j int) { + all[i], all[j] = all[j], all[i] + }) + + if len(all) > maxRootServers { + return all[:maxRootServers] + } + + return all +} + func checkCtx(ctx context.Context) error { err := ctx.Err() if err != nil { @@ -302,7 +319,7 @@ func (r *Resolver) resolveNSRecursive( msg.SetQuestion(domain, dns.TypeNS) msg.RecursionDesired = true - for _, ip := range rootServerList()[:3] { + for _, ip := range randomRootServers() { if checkCtx(ctx) != nil { return nil, ErrContextCanceled } @@ -333,7 +350,7 @@ func (r *Resolver) resolveARecord( msg.SetQuestion(hostname, dns.TypeA) msg.RecursionDesired = true - for _, ip := range rootServerList()[:3] { + for _, ip := range randomRootServers() { if checkCtx(ctx) != nil { return nil, ErrContextCanceled } @@ -385,7 +402,7 @@ func (r *Resolver) FindAuthoritativeNameservers( candidate := strings.Join(labels[i:], ".") + "." nsNames, err := r.followDelegation( - ctx, candidate, rootServerList(), + ctx, candidate, randomRootServers(), ) if err == nil && len(nsNames) > 0 { sort.Strings(nsNames) From 4cb81aac24bba5b9d3fbc0a6f7029ac09cc13438 Mon Sep 17 00:00:00 2001 From: user Date: Sun, 22 Feb 2026 04:28:47 -0800 Subject: [PATCH 3/3] =?UTF-8?q?doc:=20add=20testing=20policy=20=E2=80=94?= =?UTF-8?q?=20real=20DNS=20only,=20no=20mocks?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Documents the project testing philosophy: all resolver tests must use live DNS queries. Mocking the DNS client layer is not permitted. Includes rationale and anti-patterns to avoid. --- TESTING.md | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 TESTING.md diff --git a/TESTING.md b/TESTING.md new file mode 100644 index 0000000..3dd34c4 --- /dev/null +++ b/TESTING.md @@ -0,0 +1,34 @@ +# Testing Policy + +## DNS Resolution Tests + +All resolver tests **MUST** use live queries against real DNS servers. +No mocking of the DNS client layer is permitted. + +### Rationale + +The resolver performs iterative resolution from root nameservers through +the full delegation chain. Mocked responses cannot faithfully represent +the variety of real-world DNS behavior (truncation, referrals, glue +records, DNSSEC, varied response times, EDNS, etc.). Testing against +real servers ensures the resolver works correctly in production. + +### Constraints + +- Tests hit real DNS infrastructure and require network access +- Test duration depends on network conditions; timeout tuning keeps + the suite within the 30-second target +- Query timeout is calibrated to 3× maximum antipodal RTT (~300ms) + plus processing margin +- Root server fan-out is limited to reduce parallel query load +- Flaky failures from transient network issues are acceptable and + should be investigated as potential resolver bugs, not papered over + with mocks or skip flags + +### What NOT to do + +- **Do not mock `DNSClient`** for resolver tests (the mock constructor + exists for unit-testing other packages that consume the resolver) +- **Do not add `-short` flags** to skip slow tests +- **Do not increase `-timeout`** to hide hanging queries +- **Do not modify linter configuration** to suppress findings