server: assert timeouts on the served http.Server, not just the constructor (closes #120)

The timeout tests called newHTTPServer directly, so a Run that built
its http.Server inline would drop every timeout with the suite still
green. TestRunWiresSocketTimeouts wires a Server as cmd/dnswatcher
does, minus the watcher and resolver so no live DNS is touched, drives
Run with an unbindable port so it stores its http.Server and returns
without listening, and checks that server carries all four timeouts
and both required relationships. The addr/handler test, which could
not fail, is dropped.

The ReadTimeout note now says what net/http does: a request whose
headers arrive after ReadTimeout but within ReadHeaderTimeout gets a
read deadline that has already passed, so reading its body fails at
once.

Model: opus-4-8 (implementation); opus-5-5 (rework)

TODO.md

@@ -23,10 +23,9 @@ Rationale, Design, TODO, License, Author) if any are still missing.
 
 # Completed Steps
 
-- 2026-09-21: server timeout test now drives `Run` and asserts the
+- 2026-09-28: the server timeout test now drives `Run` and checks the
-  served `http.Server` carries the timeouts; corrected the inverted
+  `http.Server` it serves carries the timeouts; corrected the `ReadTimeout`
-  `ReadTimeout` rationale note (#120).
+  note in that test (closes #120).
-
 - 2026-09-28: upaas deploy readiness — runtime image runs as unprivileged
   `dnswatcher`, Docker `HEALTHCHECK`, startup fails when the data directory is
   not writable, README "Running under upaas" (closes #147).
@@ -34,7 +33,6 @@ Rationale, Design, TODO, License, Author) if any are still missing.
   `internal/healthcheck`, and `internal/logger` (closes #110).
 - 2026-09-21: `go mod tidy` dropped the redundant `golang.org/x/sync`
   `// indirect` line so `script/bootstrap` leaves a clean tree (#132)
-
 - 2026-08-10: comment-only corrections to `script/bootstrap`,
   `script/cibuild`, and `Dockerfile.lint`. The `goimports` pin in
   `script/bootstrap` was justified by a claim that `script/fmt-check`

internal/server/server_test.go

@@ -55,10 +55,7 @@ func buildServer(t *testing.T) *server.Server {
 // TestRunWiresSocketTimeouts pins that the http.Server the running
 // server actually serves — the one Run builds and hands to
 // ListenAndServe — carries every socket-level timeout, plus the two
-// relationships the values must satisfy. Earlier tests asserted these
+// relationships the values must satisfy.
-// on newHTTPServer directly, which left the call site unguarded: a Run
-// that built its http.Server inline would drop every timeout with the
-// suite still green (https://git.eeqj.de/sneak/dnswatcher/issues/120).
 //
 // Run is driven to completion with an unbindable port: it builds and
 // stores s.httpServer, then ListenAndServe fails at once and Run
@@ -66,16 +63,13 @@ func buildServer(t *testing.T) *server.Server {
 // goroutine after Run returns, so reading s.httpServer is free of any
 // data race. Nothing here measures elapsed time.
 //
-// On the ReadTimeout >= ReadHeaderTimeout relationship: a smaller
+// ReadTimeout must be at least ReadHeaderTimeout. net/http reads the
-// ReadTimeout does NOT make the header phase unreachable. net/http's
+// headers under ReadHeaderTimeout, then sets the read deadline for the
-// (*Server).readHeaderTimeout applies ReadHeaderTimeout directly, so
+// rest of the request to ReadTimeout, counted from when it started
-// the header read keeps its full budget. What breaks is the
+// reading the request. If ReadTimeout were smaller, a request whose
-// whole-request deadline: once the headers are read, readRequest
+// headers arrived after ReadTimeout but within ReadHeaderTimeout would
-// installs a read deadline of t0+ReadTimeout, which is already in the
+// get a read deadline that had already passed, so reading its body
-// past when ReadTimeout is the smaller value, severing the request.
+// would fail at once.
-// Verified against the pinned go1.25 net/http (Dockerfile golang
-// 1.25-alpine; go.mod go 1.25.5): src/net/http/server.go readRequest
-// and (*Server).readHeaderTimeout.
 func TestRunWiresSocketTimeouts(t *testing.T) {
 	// Sets an env var and touches viper global state, so like the
 	// config tests it cannot use t.Parallel.