Compare commits

..
1 Commits
Author SHA1 Message Date
sneak 540c4590a9 handlers: a directory under /s/ answers 404, not a listing (closes #286)
check / check (push) Waiting to run
http.FileServer answered /s/ and /s/css/ with an HTML listing of the
embedded files, and /s/css with a redirect to that listing. HandleStatic
in internal/handlers now looks the name up in the embedded files first
and answers 404 when it names a directory; files are served as before.
The routes mount it under /s.

Model: opus-5-5
2026-10-15 14:34:06 +00:00
5 changed files with 30 additions and 128 deletions
+7 -9
View File
@@ -19,18 +19,16 @@ works correctly in production.
### Constraints
- Tests hit real DNS infrastructure and require network access
- Test duration depends on network conditions. `make test` must finish in under
60 seconds, the hard cap in `REPO_POLICIES.md`; under 20 seconds is the target
- Each query to a nameserver has a 2-second timeout, and a query over UDP that
gets no reply is sent once more (`queryTimeoutDuration` and `maxRetries` in
`internal/resolver/iterative.go`)
- In the walk to a name's nameservers, the servers of each zone are asked one at
a time in a random order, as the README's DNS Resolution Strategy describes
- Test duration depends on network conditions; timeout tuning keeps the suite
within the 60-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
- Live lookups that expect an answer go through `internal/livednstest`, which
limits how many run at once in a test binary and retries a lookup that got
none
- A flaky live test is a robustness problem, fixed with retries with backoff,
several independent nameservers or longer timeouts, never with mocks or skip
- 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
-11
View File
@@ -19,17 +19,6 @@ milestone: https://git.eeqj.de/sneak/dnswatcher/milestone/7
# Completed Steps
- 2026-10-15: a query its caller's context ends before it is sent records no
reason about the nameserver, and a try the context's deadline cuts short is a
timeout, so a nameserver whose only failures were timeouts is `timeout`, not a
network error or `ok` (closes #298).
- 2026-10-15: the resolver no longer says its query timeout grows on each try;
every try waits the same 2 seconds, and the unused `timeoutMultiplier` is gone
(closes #296).
- 2026-10-15: `TESTING.md` Constraints give the resolver's 2-second query
timeout and second try, say a zone's servers are asked one at a time in a
random order, give the 60-second cap and 20-second target for `make test`, and
say a flaky live test is fixed with robustness, not with mocks (closes #289).
- 2026-10-15: a request under `/s/` that names a directory, such as `/s/` or
`/s/css`, answers 404 instead of a listing of its files (closes #286).
- 2026-10-15: a notification endpoint that answers with a redirect, or any
+19 -16
View File
@@ -19,6 +19,7 @@ const (
queryTimeoutDuration = 2 * time.Second
maxRetries = 2
maxDelegation = 20
timeoutMultiplier = 2
// maxLookupDepth is how many lookups of nameserver addresses may be
// under way one inside another. Looking up a nameserver's address
@@ -61,11 +62,19 @@ func checkCtx(ctx context.Context) error {
return nil
}
// tryExchange sends msg to addr over UDP, up to maxRetries times, until
// a reply comes. Every try waits the same queryTimeoutDuration, or less
// when ctx's deadline comes first. When ctx has ended before a further
// try, it returns the error of the try already made, such as a timeout,
// or ErrContextCanceled when none was made.
func (r *Resolver) exchangeWithTimeout(
ctx context.Context,
msg *dns.Msg,
addr string,
attempt int,
) (*dns.Msg, error) {
_ = attempt // timeout escalation handled by client config
resp, _, err := r.client.ExchangeContext(ctx, msg, addr)
return resp, err
}
func (r *Resolver) tryExchange(
ctx context.Context,
msg *dns.Msg,
@@ -75,16 +84,14 @@ func (r *Resolver) tryExchange(
var err error
for range maxRetries {
for attempt := range maxRetries {
if checkCtx(ctx) != nil {
if err == nil {
return nil, ErrContextCanceled
}
return nil, err
return nil, ErrContextCanceled
}
resp, _, err = r.client.ExchangeContext(ctx, msg, addr)
resp, err = r.exchangeWithTimeout(
ctx, msg, addr, attempt,
)
if err == nil {
break
}
@@ -870,10 +877,6 @@ func (r *Resolver) querySingleType(
if err != nil {
err = fmt.Errorf("query %s @%s: %w", hostname, nsIP, err)
}
case errors.Is(err, ErrContextCanceled):
// The caller's context ended before the query was sent.
// That says nothing about the nameserver, so no reason is
// recorded.
case isTimeout(err):
state.gotTimeout = true
case errors.Is(err, ErrRefused):
@@ -1,8 +1,6 @@
package resolver
import (
"context"
"log/slog"
"strconv"
"syscall"
"testing"
@@ -134,33 +132,3 @@ func TestReadReply(t *testing.T) {
})
}
}
// TestQuerySingleType_ContextEnded asks a nameserver about one record
// type with a context that has already ended, so no query is sent. A
// query the context ended before it was sent says nothing about the
// nameserver and records no reason: a nameserver whose earlier query
// timed out is still reported as timed out, not as a network error.
func TestQuerySingleType_ContextEnded(t *testing.T) {
t.Parallel()
r := NewFromLogger(slog.New(slog.DiscardHandler))
ctx, cancel := context.WithCancel(t.Context())
cancel()
resp := &NameserverResponse{Records: map[string][]string{}, Status: StatusOK}
// The query for an earlier record type timed out.
state := queryState{gotTimeout: true}
_, err := r.querySingleType(
ctx, []string{"192.0.2.1"}, "example.com.", dns.TypeA, resp, &state,
)
require.ErrorIs(t, err, ErrContextCanceled)
assert.Equal(t, queryState{gotTimeout: true}, state)
classifyResponse(resp, state)
assert.Equal(t, StatusTimeout, resp.Status)
assert.Equal(t, "all queries timed out", resp.Error)
}
+4 -60
View File
@@ -1380,10 +1380,10 @@ func TestQueryNameserverIP_Timeout(t *testing.T) {
r := newTestResolver(t)
// Nothing answers at 192.0.2.1, a documentation address. The
// resolver tries each query twice, and each try gives up after two
// seconds. This deadline ends during the second try of the first
// record type; TestQueryNameserverIP_DeadlineDuringFirstTry covers
// one that ends during the first.
// resolver tries each query twice, and the first try gives up
// after two seconds. A deadline that ends during the first try
// makes the status vary from run to run between error and
// timeout, so the deadline must outlast the first try.
ctx, cancel := context.WithTimeout(
context.Background(), 3*time.Second,
)
@@ -1399,62 +1399,6 @@ func TestQueryNameserverIP_Timeout(t *testing.T) {
assert.NotEmpty(t, resp.Error)
}
// TestQueryNameserverIP_DeadlineDuringFirstTry gives a query to
// 192.0.2.1, where nothing answers, a deadline that ends during its
// first try. That try ends at the deadline as a timeout, so the
// nameserver is reported as timed out, not as answering.
func TestQueryNameserverIP_DeadlineDuringFirstTry(t *testing.T) {
t.Parallel()
r := newTestResolver(t)
// Well under the two seconds one try waits.
ctx, cancel := context.WithTimeout(
context.Background(), 500*time.Millisecond,
)
t.Cleanup(cancel)
resp, err := r.QueryNameserverIP(
ctx, "unreachable.test.", "192.0.2.1",
"example.com",
)
require.NoError(t, err)
assert.Equal(t, resolver.StatusTimeout, resp.Status)
assert.NotEmpty(t, resp.Error)
}
// TestQueryNameserverIP_CancelDuringFirstTry cancels the context while
// the first try of a query to 192.0.2.1, where nothing answers, waits
// for a reply. That try's timeout is all that is learned about the
// nameserver, so it is reported as timed out, not as answering.
func TestQueryNameserverIP_CancelDuringFirstTry(t *testing.T) {
t.Parallel()
r := newTestResolver(t)
// The first try starts at once and waits two seconds for a reply;
// the cancel comes a second in. The DNS client cuts a try short at
// the context's deadline but not at a cancel, so the try still
// waits its full two seconds and ends as a timeout a second after
// the cancel. By then the context has certainly ended, so no second
// try is made. With a deadline instead, the try ends at the moment
// the context does, and whether a second try is made depends on
// which the resolver sees first.
ctx, cancel := context.WithCancel(context.Background())
t.Cleanup(cancel)
time.AfterFunc(time.Second, cancel)
resp, err := r.QueryNameserverIP(
ctx, "unreachable.test.", "192.0.2.1",
"example.com",
)
require.NoError(t, err)
assert.Equal(t, resolver.StatusTimeout, resp.Status)
assert.NotEmpty(t, resp.Error)
}
// TestQueryNameserverIP_CancelledLogsNothing cancels the context while
// a query to 192.0.2.1, where nothing answers, is waiting for a reply,
// as shutdown does. The query was cut short, not failed, so nothing is