Compare commits
1
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
b425692bfd |
@@ -203,6 +203,46 @@ 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.
|
||||
|
||||
### Security Headers
|
||||
|
||||
Every response — the dashboard, the static assets under `/s/...`, the
|
||||
healthchecks, the JSON API, and `/metrics` — carries the following
|
||||
headers, set by a global middleware:
|
||||
|
||||
| Header | Value |
|
||||
|-----------------------------|---------------------------------------|
|
||||
| `Strict-Transport-Security` | `max-age=31536000; includeSubDomains` |
|
||||
| `Content-Security-Policy` | see below |
|
||||
| `X-Frame-Options` | `DENY` |
|
||||
| `X-Content-Type-Options` | `nosniff` |
|
||||
| `Referrer-Policy` | `no-referrer` |
|
||||
| `Permissions-Policy` | all unused browser features denied |
|
||||
|
||||
The content security policy is:
|
||||
|
||||
```
|
||||
default-src 'self'; script-src 'none'; style-src 'self'; img-src 'self';
|
||||
font-src 'none'; connect-src 'none'; object-src 'none'; base-uri 'none';
|
||||
form-action 'none'; frame-ancestors 'none'
|
||||
```
|
||||
|
||||
The dashboard ships no JavaScript (the 30-second refresh is a
|
||||
`<meta http-equiv="refresh">`), no inline styles, no inline event
|
||||
handlers, and no images; its only subresource is the embedded stylesheet
|
||||
at `/s/css/tailwind.min.css`, which `style-src 'self'` permits. The
|
||||
policy therefore needs neither `unsafe-inline` nor `unsafe-eval`.
|
||||
`frame-ancestors 'none'` is the primary anti-framing control, with
|
||||
`X-Frame-Options: DENY` retained as the legacy fallback.
|
||||
|
||||
HSTS is emitted unconditionally, including over plain HTTP. dnswatcher is
|
||||
expected to run behind a TLS-terminating reverse proxy, and the browser
|
||||
must still be told to enforce HTTPS end to end, so the header is never
|
||||
gated on whether the request itself arrived over TLS.
|
||||
|
||||
`Referrer-Policy: no-referrer` is stricter than the
|
||||
`strict-origin-when-cross-origin` baseline: the dashboard has no
|
||||
cross-origin navigation needs, and its URL may name internal hosts.
|
||||
|
||||
---
|
||||
|
||||
## Architecture
|
||||
@@ -215,7 +255,8 @@ internal/
|
||||
globals/globals.go Build-time variables (version)
|
||||
logger/logger.go slog structured logging (TTY detection)
|
||||
healthcheck/healthcheck.go Health check service
|
||||
middleware/middleware.go HTTP middleware (logging, CORS, metrics auth)
|
||||
middleware/middleware.go HTTP middleware (logging, CORS, security
|
||||
headers, metrics auth)
|
||||
handlers/handlers.go HTTP request handlers
|
||||
server/
|
||||
server.go HTTP server lifecycle
|
||||
|
||||
@@ -23,13 +23,8 @@ Rationale, Design, TODO, License, Author) if any are still missing.
|
||||
|
||||
# Completed Steps
|
||||
|
||||
- 2026-09-21: server timeout test now drives `Run` and asserts the
|
||||
served `http.Server` carries the timeouts; corrected the inverted
|
||||
`ReadTimeout` rationale note (#120).
|
||||
|
||||
- 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`
|
||||
@@ -116,6 +111,16 @@ Rationale, Design, TODO, License, Author) if any are still missing.
|
||||
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-09: security response headers middleware
|
||||
(`SecurityHeaders()` in `internal/middleware/middleware.go`)
|
||||
registered globally in `internal/server/routes.go`, so HSTS, CSP,
|
||||
`X-Frame-Options`, `X-Content-Type-Options`, `Referrer-Policy`, and
|
||||
`Permissions-Policy` are set on every response including `/s/...` and
|
||||
`/metrics`; the CSP needs no `unsafe-inline`/`unsafe-eval` because the
|
||||
dashboard ships no JavaScript and no inline styles; HSTS is emitted
|
||||
unconditionally per policy (TLS-terminating proxy in front). Remaining
|
||||
1.0 hardening items — `http.Server` timeouts, request body limits,
|
||||
rate limiting, CORS scoping — are tracked separately
|
||||
- 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
|
||||
|
||||
@@ -21,6 +21,60 @@ import (
|
||||
// corsMaxAge is the maximum age for CORS preflight responses.
|
||||
const corsMaxAge = 300
|
||||
|
||||
// Security response header values applied to every response.
|
||||
//
|
||||
// The CSP is as strict as the dashboard allows: the template ships no
|
||||
// JavaScript, no inline styles, no inline event handlers and no images,
|
||||
// and its only subresource is the embedded stylesheet at
|
||||
// /s/css/tailwind.min.css, which style-src 'self' permits. Neither
|
||||
// unsafe-inline nor unsafe-eval is used. frame-ancestors 'none' is the
|
||||
// primary anti-framing control; X-Frame-Options is the legacy fallback.
|
||||
const (
|
||||
// hstsValue is emitted unconditionally, including over plain HTTP,
|
||||
// because the service runs behind a TLS-terminating proxy and the
|
||||
// browser must still enforce HTTPS end to end.
|
||||
hstsValue = "max-age=31536000; includeSubDomains"
|
||||
|
||||
cspValue = "default-src 'self'; " +
|
||||
"script-src 'none'; " +
|
||||
"style-src 'self'; " +
|
||||
"img-src 'self'; " +
|
||||
"font-src 'none'; " +
|
||||
"connect-src 'none'; " +
|
||||
"object-src 'none'; " +
|
||||
"base-uri 'none'; " +
|
||||
"form-action 'none'; " +
|
||||
"frame-ancestors 'none'"
|
||||
|
||||
frameOptionsValue = "DENY"
|
||||
|
||||
contentTypeOptionsValue = "nosniff"
|
||||
|
||||
// referrerPolicyValue is stricter than the policy minimum of
|
||||
// strict-origin-when-cross-origin: the dashboard has no
|
||||
// cross-origin navigation needs and its URL may name internal
|
||||
// hosts.
|
||||
referrerPolicyValue = "no-referrer"
|
||||
|
||||
permissionsPolicyValue = "accelerometer=(), " +
|
||||
"autoplay=(), " +
|
||||
"camera=(), " +
|
||||
"display-capture=(), " +
|
||||
"encrypted-media=(), " +
|
||||
"fullscreen=(), " +
|
||||
"geolocation=(), " +
|
||||
"gyroscope=(), " +
|
||||
"magnetometer=(), " +
|
||||
"microphone=(), " +
|
||||
"midi=(), " +
|
||||
"payment=(), " +
|
||||
"picture-in-picture=(), " +
|
||||
"publickey-credentials-get=(), " +
|
||||
"screen-wake-lock=(), " +
|
||||
"usb=(), " +
|
||||
"xr-spatial-tracking=()"
|
||||
)
|
||||
|
||||
// Params contains dependencies for Middleware.
|
||||
type Params struct {
|
||||
fx.In
|
||||
@@ -186,6 +240,37 @@ func (m *Middleware) CORS() func(http.Handler) http.Handler {
|
||||
})
|
||||
}
|
||||
|
||||
// SecurityHeaders returns middleware that sets the security response
|
||||
// headers required for production internet exposure on every response.
|
||||
//
|
||||
// The headers are set before the request reaches the next handler so
|
||||
// that they are present on every response, including panics recovered
|
||||
// by chi's Recoverer and timeouts produced by chi's Timeout.
|
||||
func (m *Middleware) SecurityHeaders() func(http.Handler) http.Handler {
|
||||
return func(next http.Handler) http.Handler {
|
||||
return http.HandlerFunc(func(
|
||||
writer http.ResponseWriter,
|
||||
request *http.Request,
|
||||
) {
|
||||
header := writer.Header()
|
||||
header.Set("Strict-Transport-Security", hstsValue)
|
||||
header.Set("Content-Security-Policy", cspValue)
|
||||
header.Set("X-Frame-Options", frameOptionsValue)
|
||||
header.Set(
|
||||
"X-Content-Type-Options",
|
||||
contentTypeOptionsValue,
|
||||
)
|
||||
header.Set("Referrer-Policy", referrerPolicyValue)
|
||||
header.Set(
|
||||
"Permissions-Policy",
|
||||
permissionsPolicyValue,
|
||||
)
|
||||
|
||||
next.ServeHTTP(writer, request)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// MetricsAuth returns basic auth middleware for /metrics.
|
||||
func (m *Middleware) MetricsAuth() func(http.Handler) http.Handler {
|
||||
if m.params.Config.MetricsUsername == "" {
|
||||
|
||||
@@ -0,0 +1,334 @@
|
||||
package middleware_test
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/go-chi/chi/v5"
|
||||
"go.uber.org/fx/fxtest"
|
||||
|
||||
"sneak.berlin/go/dnswatcher/internal/config"
|
||||
"sneak.berlin/go/dnswatcher/internal/globals"
|
||||
"sneak.berlin/go/dnswatcher/internal/handlers"
|
||||
"sneak.berlin/go/dnswatcher/internal/logger"
|
||||
"sneak.berlin/go/dnswatcher/internal/middleware"
|
||||
"sneak.berlin/go/dnswatcher/internal/notify"
|
||||
"sneak.berlin/go/dnswatcher/internal/state"
|
||||
)
|
||||
|
||||
// Expected security header values, spelled out literally so that any
|
||||
// change to the middleware has to be made deliberately here as well.
|
||||
const (
|
||||
wantHSTS = "max-age=31536000; includeSubDomains"
|
||||
|
||||
wantCSP = "default-src 'self'; " +
|
||||
"script-src 'none'; " +
|
||||
"style-src 'self'; " +
|
||||
"img-src 'self'; " +
|
||||
"font-src 'none'; " +
|
||||
"connect-src 'none'; " +
|
||||
"object-src 'none'; " +
|
||||
"base-uri 'none'; " +
|
||||
"form-action 'none'; " +
|
||||
"frame-ancestors 'none'"
|
||||
|
||||
wantFrameOptions = "DENY"
|
||||
|
||||
wantContentTypeOptions = "nosniff"
|
||||
|
||||
wantReferrerPolicy = "no-referrer"
|
||||
|
||||
wantPermissionsPolicy = "accelerometer=(), " +
|
||||
"autoplay=(), " +
|
||||
"camera=(), " +
|
||||
"display-capture=(), " +
|
||||
"encrypted-media=(), " +
|
||||
"fullscreen=(), " +
|
||||
"geolocation=(), " +
|
||||
"gyroscope=(), " +
|
||||
"magnetometer=(), " +
|
||||
"microphone=(), " +
|
||||
"midi=(), " +
|
||||
"payment=(), " +
|
||||
"picture-in-picture=(), " +
|
||||
"publickey-credentials-get=(), " +
|
||||
"screen-wake-lock=(), " +
|
||||
"usb=(), " +
|
||||
"xr-spatial-tracking=()"
|
||||
)
|
||||
|
||||
// stylesheetPath is the only subresource the dashboard loads.
|
||||
const stylesheetPath = "/s/css/tailwind.min.css"
|
||||
|
||||
// newTestLogger builds a logger for direct component construction.
|
||||
func newTestLogger(t *testing.T) *logger.Logger {
|
||||
t.Helper()
|
||||
|
||||
glob, err := globals.New(nil)
|
||||
if err != nil {
|
||||
t.Fatalf("globals.New: %v", err)
|
||||
}
|
||||
|
||||
log, err := logger.New(nil, logger.Params{Globals: glob})
|
||||
if err != nil {
|
||||
t.Fatalf("logger.New: %v", err)
|
||||
}
|
||||
|
||||
return log
|
||||
}
|
||||
|
||||
// newTestMiddleware builds a Middleware without an fx application.
|
||||
func newTestMiddleware(t *testing.T) *middleware.Middleware {
|
||||
t.Helper()
|
||||
|
||||
glob, err := globals.New(nil)
|
||||
if err != nil {
|
||||
t.Fatalf("globals.New: %v", err)
|
||||
}
|
||||
|
||||
mw, err := middleware.New(nil, middleware.Params{
|
||||
Logger: newTestLogger(t),
|
||||
Globals: glob,
|
||||
Config: &config.Config{},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("middleware.New: %v", err)
|
||||
}
|
||||
|
||||
return mw
|
||||
}
|
||||
|
||||
// serveWithSecurityHeaders runs a GET through SecurityHeaders and
|
||||
// returns the recorded response.
|
||||
func serveWithSecurityHeaders(
|
||||
t *testing.T,
|
||||
target string,
|
||||
handler http.Handler,
|
||||
) *httptest.ResponseRecorder {
|
||||
t.Helper()
|
||||
|
||||
mw := newTestMiddleware(t)
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequestWithContext(
|
||||
t.Context(), http.MethodGet, target, nil,
|
||||
)
|
||||
|
||||
mw.SecurityHeaders()(handler).ServeHTTP(rec, req)
|
||||
|
||||
return rec
|
||||
}
|
||||
|
||||
// okHandler writes a trivial 200 response.
|
||||
func okHandler() http.Handler {
|
||||
return http.HandlerFunc(func(
|
||||
writer http.ResponseWriter,
|
||||
_ *http.Request,
|
||||
) {
|
||||
writer.WriteHeader(http.StatusOK)
|
||||
})
|
||||
}
|
||||
|
||||
func TestSecurityHeaders(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
header string
|
||||
want string
|
||||
}{
|
||||
{
|
||||
"hsts",
|
||||
"Strict-Transport-Security",
|
||||
wantHSTS,
|
||||
},
|
||||
{
|
||||
"csp",
|
||||
"Content-Security-Policy",
|
||||
wantCSP,
|
||||
},
|
||||
{
|
||||
"frame options",
|
||||
"X-Frame-Options",
|
||||
wantFrameOptions,
|
||||
},
|
||||
{
|
||||
"content type options",
|
||||
"X-Content-Type-Options",
|
||||
wantContentTypeOptions,
|
||||
},
|
||||
{
|
||||
"referrer policy",
|
||||
"Referrer-Policy",
|
||||
wantReferrerPolicy,
|
||||
},
|
||||
{
|
||||
"permissions policy",
|
||||
"Permissions-Policy",
|
||||
wantPermissionsPolicy,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
rec := serveWithSecurityHeaders(t, "/", okHandler())
|
||||
|
||||
got := rec.Header().Get(tt.header)
|
||||
if got != tt.want {
|
||||
t.Errorf(
|
||||
"%s = %q, want %q",
|
||||
tt.header, got, tt.want,
|
||||
)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestSecurityHeadersCSPDirectives guards the properties the repo
|
||||
// policy requires of the content security policy itself.
|
||||
func TestSecurityHeadersCSPDirectives(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
rec := serveWithSecurityHeaders(t, "/", okHandler())
|
||||
csp := rec.Header().Get("Content-Security-Policy")
|
||||
|
||||
forbidden := []string{"unsafe-inline", "unsafe-eval"}
|
||||
for _, directive := range forbidden {
|
||||
if strings.Contains(csp, directive) {
|
||||
t.Errorf("CSP must not contain %q: %q", directive, csp)
|
||||
}
|
||||
}
|
||||
|
||||
required := []string{
|
||||
"default-src 'self'",
|
||||
"script-src 'none'",
|
||||
"style-src 'self'",
|
||||
"frame-ancestors 'none'",
|
||||
}
|
||||
for _, directive := range required {
|
||||
if !strings.Contains(csp, directive) {
|
||||
t.Errorf("CSP must contain %q: %q", directive, csp)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestSecurityHeadersOnErrorResponse verifies the headers are emitted
|
||||
// even when the wrapped handler fails, since they are set before the
|
||||
// handler runs.
|
||||
func TestSecurityHeadersOnErrorResponse(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
failing := http.HandlerFunc(func(
|
||||
writer http.ResponseWriter,
|
||||
_ *http.Request,
|
||||
) {
|
||||
http.Error(
|
||||
writer,
|
||||
"boom",
|
||||
http.StatusInternalServerError,
|
||||
)
|
||||
})
|
||||
|
||||
rec := serveWithSecurityHeaders(t, "/api/v1/status", failing)
|
||||
|
||||
if rec.Code != http.StatusInternalServerError {
|
||||
t.Fatalf("status = %d, want 500", rec.Code)
|
||||
}
|
||||
|
||||
if got := rec.Header().Get(
|
||||
"X-Content-Type-Options",
|
||||
); got != wantContentTypeOptions {
|
||||
t.Errorf(
|
||||
"X-Content-Type-Options = %q, want %q",
|
||||
got, wantContentTypeOptions,
|
||||
)
|
||||
}
|
||||
|
||||
if got := rec.Header().Get(
|
||||
"Strict-Transport-Security",
|
||||
); got != wantHSTS {
|
||||
t.Errorf(
|
||||
"Strict-Transport-Security = %q, want %q",
|
||||
got, wantHSTS,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// newTestHandlers builds real Handlers with empty monitoring state.
|
||||
func newTestHandlers(t *testing.T) *handlers.Handlers {
|
||||
t.Helper()
|
||||
|
||||
glob, err := globals.New(nil)
|
||||
if err != nil {
|
||||
t.Fatalf("globals.New: %v", err)
|
||||
}
|
||||
|
||||
log := newTestLogger(t)
|
||||
|
||||
notifier, err := notify.New(fxtest.NewLifecycle(t), notify.Params{
|
||||
Logger: log,
|
||||
Config: &config.Config{},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("notify.New: %v", err)
|
||||
}
|
||||
|
||||
hnd, err := handlers.New(nil, handlers.Params{
|
||||
Logger: log,
|
||||
Globals: glob,
|
||||
State: state.NewForTest(),
|
||||
Notify: notifier,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("handlers.New: %v", err)
|
||||
}
|
||||
|
||||
return hnd
|
||||
}
|
||||
|
||||
// TestDashboardRendersWithSecurityHeaders renders the real dashboard
|
||||
// through the middleware and checks that the policy still permits the
|
||||
// one stylesheet the page loads.
|
||||
func TestDashboardRendersWithSecurityHeaders(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
mw := newTestMiddleware(t)
|
||||
hnd := newTestHandlers(t)
|
||||
|
||||
router := chi.NewRouter()
|
||||
router.Use(mw.SecurityHeaders())
|
||||
router.Get("/", hnd.HandleDashboard())
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequestWithContext(
|
||||
t.Context(), http.MethodGet, "/", nil,
|
||||
)
|
||||
|
||||
router.ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200", rec.Code)
|
||||
}
|
||||
|
||||
body := rec.Body.String()
|
||||
if !strings.Contains(body, stylesheetPath) {
|
||||
t.Errorf("dashboard does not reference %q", stylesheetPath)
|
||||
}
|
||||
|
||||
if !strings.Contains(body, "dnswatcher") {
|
||||
t.Errorf("dashboard body looks empty: %d bytes", len(body))
|
||||
}
|
||||
|
||||
csp := rec.Header().Get("Content-Security-Policy")
|
||||
if csp != wantCSP {
|
||||
t.Errorf("CSP = %q, want %q", csp, wantCSP)
|
||||
}
|
||||
|
||||
// The stylesheet is same-origin, so style-src 'self' allows it.
|
||||
if !strings.Contains(csp, "style-src 'self'") {
|
||||
t.Errorf("CSP would block %q: %q", stylesheetPath, csp)
|
||||
}
|
||||
}
|
||||
@@ -5,20 +5,15 @@ import (
|
||||
"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
|
||||
|
||||
// SetListenPort overrides the port Run binds. A test uses it to hand
|
||||
// Run an unbindable port so ListenAndServe fails immediately and Run
|
||||
// returns after storing its http.Server.
|
||||
func SetListenPort(s *Server, port int) {
|
||||
s.port = port
|
||||
}
|
||||
|
||||
// HTTPServerOf returns the http.Server that Run built and stored, so a
|
||||
// test can inspect the timeouts the running server actually carries.
|
||||
func HTTPServerOf(s *Server) *http.Server {
|
||||
return s.httpServer
|
||||
}
|
||||
|
||||
@@ -21,6 +21,7 @@ func (s *Server) SetupRoutes() {
|
||||
// Global middleware
|
||||
s.router.Use(chimw.Recoverer)
|
||||
s.router.Use(chimw.RequestID)
|
||||
s.router.Use(s.mw.SecurityHeaders())
|
||||
s.router.Use(s.mw.Logging())
|
||||
s.router.Use(s.mw.CORS())
|
||||
s.router.Use(chimw.Timeout(requestTimeout))
|
||||
|
||||
@@ -1,136 +1,112 @@
|
||||
package server_test
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"testing"
|
||||
|
||||
"github.com/spf13/viper"
|
||||
"go.uber.org/fx"
|
||||
|
||||
"sneak.berlin/go/dnswatcher/internal/config"
|
||||
"sneak.berlin/go/dnswatcher/internal/globals"
|
||||
"sneak.berlin/go/dnswatcher/internal/handlers"
|
||||
"sneak.berlin/go/dnswatcher/internal/healthcheck"
|
||||
"sneak.berlin/go/dnswatcher/internal/logger"
|
||||
"sneak.berlin/go/dnswatcher/internal/middleware"
|
||||
"sneak.berlin/go/dnswatcher/internal/notify"
|
||||
"sneak.berlin/go/dnswatcher/internal/server"
|
||||
"sneak.berlin/go/dnswatcher/internal/state"
|
||||
)
|
||||
|
||||
// buildServer wires a *server.Server exactly as cmd/dnswatcher does,
|
||||
// minus the watcher/resolver subtree that would touch live DNS. fx
|
||||
// builds the object graph but the lifecycle is never started, so no
|
||||
// OnStart hook runs and nothing listens or resolves. The caller must
|
||||
// first configure viper (config.New reads it), which is also why the
|
||||
// caller cannot run in parallel.
|
||||
func buildServer(t *testing.T) *server.Server {
|
||||
t.Helper()
|
||||
|
||||
var srv *server.Server
|
||||
|
||||
app := fx.New(
|
||||
fx.NopLogger,
|
||||
fx.Provide(
|
||||
globals.New,
|
||||
logger.New,
|
||||
config.New,
|
||||
state.New,
|
||||
healthcheck.New,
|
||||
notify.New,
|
||||
middleware.New,
|
||||
handlers.New,
|
||||
server.New,
|
||||
),
|
||||
fx.Populate(&srv),
|
||||
// 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)
|
||||
},
|
||||
)
|
||||
|
||||
err := app.Err()
|
||||
if err != nil {
|
||||
t.Fatalf("building server graph: %v", err)
|
||||
}
|
||||
|
||||
return srv
|
||||
}
|
||||
|
||||
// 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
|
||||
// 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).
|
||||
// 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.
|
||||
//
|
||||
// Run is driven to completion with an unbindable port: it builds and
|
||||
// stores s.httpServer, then ListenAndServe fails at once and Run
|
||||
// returns without ever listening. The assertions run in the same
|
||||
// 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 does NOT make the header phase unreachable. net/http's
|
||||
// (*Server).readHeaderTimeout applies ReadHeaderTimeout directly, so
|
||||
// the header read keeps its full budget. What breaks is the
|
||||
// whole-request deadline: once the headers are read, readRequest
|
||||
// installs a read deadline of t0+ReadTimeout, which is already in the
|
||||
// past when ReadTimeout is the smaller value, severing the request.
|
||||
// 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.
|
||||
viper.Reset()
|
||||
t.Setenv("DNSWATCHER_TARGETS", "example.com")
|
||||
// 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 := buildServer(t)
|
||||
server.SetListenPort(srv, -1)
|
||||
srv := server.NewHTTPServer(":8080", noopHandler())
|
||||
|
||||
srv.Run()
|
||||
|
||||
hs := server.HTTPServerOf(srv)
|
||||
if hs == nil {
|
||||
t.Fatal("Run did not build an http.Server")
|
||||
if srv.ReadTimeout <= 0 {
|
||||
t.Errorf(
|
||||
"ReadTimeout must be non-zero, got %v",
|
||||
srv.ReadTimeout,
|
||||
)
|
||||
}
|
||||
|
||||
if hs.ReadTimeout <= 0 {
|
||||
t.Errorf("ReadTimeout must be non-zero, got %v", hs.ReadTimeout)
|
||||
}
|
||||
|
||||
if hs.ReadHeaderTimeout <= 0 {
|
||||
if srv.ReadHeaderTimeout <= 0 {
|
||||
t.Errorf(
|
||||
"ReadHeaderTimeout must be non-zero, got %v",
|
||||
hs.ReadHeaderTimeout,
|
||||
srv.ReadHeaderTimeout,
|
||||
)
|
||||
}
|
||||
|
||||
if hs.WriteTimeout <= 0 {
|
||||
t.Errorf("WriteTimeout must be non-zero, got %v", hs.WriteTimeout)
|
||||
if srv.WriteTimeout <= 0 {
|
||||
t.Errorf(
|
||||
"WriteTimeout must be non-zero, got %v",
|
||||
srv.WriteTimeout,
|
||||
)
|
||||
}
|
||||
|
||||
if hs.IdleTimeout <= 0 {
|
||||
t.Errorf("IdleTimeout must be non-zero, got %v", hs.IdleTimeout)
|
||||
if srv.IdleTimeout <= 0 {
|
||||
t.Errorf(
|
||||
"IdleTimeout must be non-zero, got %v",
|
||||
srv.IdleTimeout,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
if hs.WriteTimeout <= server.RequestTimeout {
|
||||
// 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)",
|
||||
hs.WriteTimeout,
|
||||
srv.WriteTimeout,
|
||||
server.RequestTimeout,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
if hs.ReadTimeout < hs.ReadHeaderTimeout {
|
||||
// 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)",
|
||||
hs.ReadTimeout,
|
||||
hs.ReadHeaderTimeout,
|
||||
srv.ReadTimeout,
|
||||
srv.ReadHeaderTimeout,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
if hs.Handler != srv {
|
||||
t.Errorf(
|
||||
"Run wired handler %T, want the *server.Server",
|
||||
hs.Handler,
|
||||
)
|
||||
// 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