Compare commits
1 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 02b63a4e65 |
21
README.md
21
README.md
@@ -182,6 +182,27 @@ dnswatcher exposes a lightweight HTTP API for operational visibility:
|
||||
| `GET /api/v1/status` | Current monitoring state |
|
||||
| `GET /metrics` | Prometheus metrics (optional) |
|
||||
|
||||
#### Server timeouts
|
||||
|
||||
The HTTP server sets all four socket-level timeouts. These are compile-time
|
||||
constants in `internal/server/server.go`, not configurable via environment
|
||||
variables.
|
||||
|
||||
| Timeout | Value | Purpose |
|
||||
|---------------------|-------|-----------------------------------------------|
|
||||
| `ReadHeaderTimeout` | 10s | Bounds the request header read (slowloris) |
|
||||
| `ReadTimeout` | 15s | Bounds the whole request read, headers + body |
|
||||
| `WriteTimeout` | 75s | Bounds handler execution plus response flush |
|
||||
| `IdleTimeout` | 120s | Reaps idle keep-alive connections |
|
||||
|
||||
These are distinct from the 60s per-request handler budget applied by
|
||||
`chimw.Timeout` in `internal/server/routes.go`, which cancels the request
|
||||
context but does not touch the socket. `WriteTimeout` is deliberately
|
||||
larger than that budget: the write deadline is armed once request headers
|
||||
are read, so a smaller value would sever the connection before a handler
|
||||
using its full budget could respond. `IdleTimeout` exceeds common
|
||||
Prometheus scrape intervals so the scraper reuses its connection.
|
||||
|
||||
---
|
||||
|
||||
## Architecture
|
||||
|
||||
8
TODO.md
8
TODO.md
@@ -25,6 +25,14 @@ confirm make check still passes.
|
||||
|
||||
# Completed Steps
|
||||
|
||||
- 2026-08-09: `http.Server` now sets all four socket-level timeouts
|
||||
(`ReadTimeout` 15s, `ReadHeaderTimeout` 10s, `WriteTimeout` 75s,
|
||||
`IdleTimeout` 120s) as named constants in `internal/server/server.go`,
|
||||
closing the slowloris / unreaped-keep-alive exposure required by
|
||||
`REPO_POLICIES.md` before 1.0; `WriteTimeout` is deliberately greater
|
||||
than the 60s `chimw.Timeout` handler budget so that budget stays
|
||||
reachable, and tests in `internal/server` pin both the non-zero
|
||||
values and that relationship (#99)
|
||||
- 2026-08-07: golangci-lint bumped to v2.12.2 (commit-pinned installs
|
||||
in `Dockerfile` and `script/bootstrap`); `.golangci.yml` set to the
|
||||
org-standard v2-schema config used across the org's repos
|
||||
|
||||
19
internal/server/export_test.go
Normal file
19
internal/server/export_test.go
Normal file
@@ -0,0 +1,19 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"time"
|
||||
)
|
||||
|
||||
// NewHTTPServer exports newHTTPServer for testing.
|
||||
func NewHTTPServer(
|
||||
listenAddr string,
|
||||
handler http.Handler,
|
||||
) *http.Server {
|
||||
return newHTTPServer(listenAddr, handler)
|
||||
}
|
||||
|
||||
// RequestTimeout exports the handler execution budget applied by
|
||||
// chimw.Timeout in SetupRoutes, so tests can assert the relationship
|
||||
// between it and the server's WriteTimeout.
|
||||
const RequestTimeout time.Duration = requestTimeout
|
||||
@@ -33,8 +33,52 @@ type Params struct {
|
||||
// shutdownTimeout is how long to wait for graceful shutdown.
|
||||
const shutdownTimeout = 30 * time.Second
|
||||
|
||||
// readHeaderTimeout is the max duration for reading request headers.
|
||||
const readHeaderTimeout = 10 * time.Second
|
||||
// Socket-level timeouts for the HTTP server.
|
||||
//
|
||||
// These bound time spent on the connection itself and are a distinct
|
||||
// control from the per-request handler budget enforced by
|
||||
// chimw.Timeout(requestTimeout) in routes.go: that one cancels the
|
||||
// request context after requestTimeout but never touches the socket,
|
||||
// so without the values below a peer can hold a connection open
|
||||
// forever (slowloris, unreaped keep-alives).
|
||||
//
|
||||
// The one hard constraint between the two controls is
|
||||
// writeTimeout > requestTimeout. net/http arms the write deadline
|
||||
// once the request headers have been read, so on a plaintext
|
||||
// connection it covers handler execution AND the response flush. If
|
||||
// writeTimeout were <= requestTimeout the server would sever the
|
||||
// connection before a handler that legitimately consumed its full
|
||||
// budget could emit anything, making the 60s budget unreachable in
|
||||
// practice. The margin between them is the response-flush allowance.
|
||||
//
|
||||
// The only clients of this service are browsers loading the dashboard
|
||||
// and a Prometheus scraper; the values are sized for those.
|
||||
const (
|
||||
// readHeaderTimeout is the max duration for reading request
|
||||
// headers.
|
||||
readHeaderTimeout = 10 * time.Second
|
||||
|
||||
// readTimeout bounds reading the entire request, headers plus
|
||||
// body. Every route here is a GET with no body, so this only
|
||||
// ever needs to cover headers; the extra 5s over
|
||||
// readHeaderTimeout is slack, not a real allowance, and keeps a
|
||||
// body dribbled one byte at a time from holding the read side
|
||||
// open indefinitely.
|
||||
readTimeout = 15 * time.Second
|
||||
|
||||
// writeTimeout must exceed the requestTimeout handler budget
|
||||
// (60s) per the note above. The 15s difference is the allowance
|
||||
// for flushing a completed response to a slow client.
|
||||
writeTimeout = 75 * time.Second
|
||||
|
||||
// idleTimeout reaps keep-alive connections between requests. It
|
||||
// is deliberately longer than the common Prometheus scrape
|
||||
// intervals (15s/30s/60s) so the scraper reuses its connection
|
||||
// rather than reconnecting every cycle, while a browser tab
|
||||
// left open on the dashboard stops occupying a connection
|
||||
// within two minutes of going quiet.
|
||||
idleTimeout = 120 * time.Second
|
||||
)
|
||||
|
||||
// Server is the HTTP server.
|
||||
type Server struct {
|
||||
@@ -76,16 +120,29 @@ func New(
|
||||
return srv, nil
|
||||
}
|
||||
|
||||
// newHTTPServer builds the listening http.Server with every
|
||||
// socket-level timeout set. All four are set deliberately: a zero
|
||||
// value in net/http means "no limit", not "some default".
|
||||
func newHTTPServer(
|
||||
listenAddr string,
|
||||
handler http.Handler,
|
||||
) *http.Server {
|
||||
return &http.Server{
|
||||
Addr: listenAddr,
|
||||
Handler: handler,
|
||||
ReadTimeout: readTimeout,
|
||||
ReadHeaderTimeout: readHeaderTimeout,
|
||||
WriteTimeout: writeTimeout,
|
||||
IdleTimeout: idleTimeout,
|
||||
}
|
||||
}
|
||||
|
||||
// Run starts the HTTP server.
|
||||
func (s *Server) Run() {
|
||||
s.SetupRoutes()
|
||||
|
||||
listenAddr := fmt.Sprintf(":%d", s.port)
|
||||
s.httpServer = &http.Server{
|
||||
Addr: listenAddr,
|
||||
Handler: s,
|
||||
ReadHeaderTimeout: readHeaderTimeout,
|
||||
}
|
||||
s.httpServer = newHTTPServer(listenAddr, s)
|
||||
|
||||
s.log.Info("http server starting", "addr", listenAddr)
|
||||
|
||||
|
||||
112
internal/server/server_test.go
Normal file
112
internal/server/server_test.go
Normal file
@@ -0,0 +1,112 @@
|
||||
package server_test
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"testing"
|
||||
|
||||
"sneak.berlin/go/dnswatcher/internal/server"
|
||||
)
|
||||
|
||||
// noopHandler stands in for the router; newHTTPServer only stores it.
|
||||
func noopHandler() http.Handler {
|
||||
return http.HandlerFunc(
|
||||
func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
// TestHTTPServerTimeoutsAreSet asserts that every socket-level
|
||||
// timeout is configured. A zero value in net/http means "no limit",
|
||||
// so a refactor that silently drops one of these reintroduces the
|
||||
// slowloris / unreaped-keep-alive exposure this guards against.
|
||||
//
|
||||
// The assertions are on the configured field values only; nothing
|
||||
// here measures elapsed time, so the test cannot flake on timing.
|
||||
func TestHTTPServerTimeoutsAreSet(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
srv := server.NewHTTPServer(":8080", noopHandler())
|
||||
|
||||
if srv.ReadTimeout <= 0 {
|
||||
t.Errorf(
|
||||
"ReadTimeout must be non-zero, got %v",
|
||||
srv.ReadTimeout,
|
||||
)
|
||||
}
|
||||
|
||||
if srv.ReadHeaderTimeout <= 0 {
|
||||
t.Errorf(
|
||||
"ReadHeaderTimeout must be non-zero, got %v",
|
||||
srv.ReadHeaderTimeout,
|
||||
)
|
||||
}
|
||||
|
||||
if srv.WriteTimeout <= 0 {
|
||||
t.Errorf(
|
||||
"WriteTimeout must be non-zero, got %v",
|
||||
srv.WriteTimeout,
|
||||
)
|
||||
}
|
||||
|
||||
if srv.IdleTimeout <= 0 {
|
||||
t.Errorf(
|
||||
"IdleTimeout must be non-zero, got %v",
|
||||
srv.IdleTimeout,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// TestWriteTimeoutExceedsHandlerBudget pins the one relationship the
|
||||
// values must satisfy. net/http arms the write deadline once request
|
||||
// headers are read, so it covers handler execution plus the response
|
||||
// flush. If WriteTimeout were not greater than the chimw.Timeout
|
||||
// handler budget, the connection would be severed before a handler
|
||||
// that used its full budget could respond, making that budget
|
||||
// unreachable.
|
||||
func TestWriteTimeoutExceedsHandlerBudget(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
srv := server.NewHTTPServer(":8080", noopHandler())
|
||||
|
||||
if srv.WriteTimeout <= server.RequestTimeout {
|
||||
t.Errorf(
|
||||
"WriteTimeout (%v) must exceed handler budget (%v)",
|
||||
srv.WriteTimeout,
|
||||
server.RequestTimeout,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// TestReadTimeoutCoversHeaderTimeout asserts the read deadline for
|
||||
// the whole request is at least as long as the header-only deadline;
|
||||
// a smaller ReadTimeout would make ReadHeaderTimeout unreachable.
|
||||
func TestReadTimeoutCoversHeaderTimeout(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
srv := server.NewHTTPServer(":8080", noopHandler())
|
||||
|
||||
if srv.ReadTimeout < srv.ReadHeaderTimeout {
|
||||
t.Errorf(
|
||||
"ReadTimeout (%v) must be >= ReadHeaderTimeout (%v)",
|
||||
srv.ReadTimeout,
|
||||
srv.ReadHeaderTimeout,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// TestHTTPServerAddrAndHandler covers the rest of the constructor so
|
||||
// a future edit cannot drop the listen address or the handler.
|
||||
func TestHTTPServerAddrAndHandler(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
srv := server.NewHTTPServer(":9999", noopHandler())
|
||||
|
||||
if srv.Addr != ":9999" {
|
||||
t.Errorf("Addr = %q, want %q", srv.Addr, ":9999")
|
||||
}
|
||||
|
||||
if srv.Handler == nil {
|
||||
t.Error("Handler must not be nil")
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user