fix(backend): report ingest correctness — propagate storage failure, 413 on oversize, global body cap (closes #23)
check / check (push) Successful in 45s
check / check (push) Successful in 45s
A buffer failure on POST /api/v1/reports now returns 500 instead of a false `ok`: the failure is server-side and a client can retry. An over-limit body returns 413 (errors.As on `*http.MaxBytesError`); malformed JSON stays 400. A MaxBodyBytes middleware (1 MiB) caps every route; a route group can only lower that limit. The raw geo blob is no longer logged, only its length; client_id, timestamp and decode error text are length-bounded before logging. A decodeJSON handler helper is added. Panic recovery routes the stack through slog as structured JSON. Writing a report file now returns its error, so a failed final flush fails the stop and the process exits non-zero. Model: opus-5-5
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
package middleware
|
||||
|
||||
import (
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"net/netip"
|
||||
)
|
||||
@@ -8,6 +9,12 @@ import (
|
||||
// Test-only wrappers exposing unexported helpers to the
|
||||
// external middleware_test package.
|
||||
|
||||
// NewWithLogger builds a Middleware around a logger for tests
|
||||
// that exercise the logging paths without the fx graph.
|
||||
func NewWithLogger(log *slog.Logger) *Middleware {
|
||||
return &Middleware{log: log}
|
||||
}
|
||||
|
||||
func ClientIP(
|
||||
remoteAddr string,
|
||||
header http.Header,
|
||||
|
||||
@@ -3,11 +3,14 @@
|
||||
package middleware
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"log/slog"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/netip"
|
||||
"runtime/debug"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
@@ -22,6 +25,14 @@ import (
|
||||
|
||||
const corsMaxAgeSec = 300
|
||||
|
||||
// jsonErrorBody is the body written for errors raised inside
|
||||
// middleware, matching the {"status":"error"} shape the handlers
|
||||
// return so clients see one error contract across the API.
|
||||
const (
|
||||
jsonContentType = "application/json; charset=utf-8"
|
||||
jsonErrorBody = "{\"status\":\"error\"}\n"
|
||||
)
|
||||
|
||||
// Security header values. The backend is a JSON API with no
|
||||
// HTML surface, so the CSP forbids every resource type and
|
||||
// framing outright.
|
||||
@@ -236,6 +247,79 @@ func (s *Middleware) SecurityHeaders() func(http.Handler) http.Handler {
|
||||
}
|
||||
}
|
||||
|
||||
// writeJSONError writes the shared JSON error body with the
|
||||
// given status. Used where middleware must reject a request
|
||||
// before it reaches a handler.
|
||||
func writeJSONError(w http.ResponseWriter, status int) {
|
||||
w.Header().Set("Content-Type", jsonContentType)
|
||||
w.WriteHeader(status)
|
||||
_, _ = io.WriteString(w, jsonErrorBody)
|
||||
}
|
||||
|
||||
// MaxBodyBytes returns middleware that caps the request body at
|
||||
// limit bytes. A declared Content-Length over the limit is
|
||||
// rejected immediately with 413. Bodies without a declared
|
||||
// length (or that understate it) are capped as they are read, so
|
||||
// a handler that reads the body sees a *http.MaxBytesError it can
|
||||
// map to 413. Mounted again on a route group, it can only lower
|
||||
// the limit: a cap applied earlier in the chain still holds.
|
||||
func (s *Middleware) MaxBodyBytes(
|
||||
limit int64,
|
||||
) func(http.Handler) http.Handler {
|
||||
return func(next http.Handler) http.Handler {
|
||||
return http.HandlerFunc(
|
||||
func(w http.ResponseWriter, r *http.Request) {
|
||||
if r.ContentLength > limit {
|
||||
writeJSONError(
|
||||
w,
|
||||
http.StatusRequestEntityTooLarge,
|
||||
)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
r.Body = http.MaxBytesReader(w, r.Body, limit)
|
||||
|
||||
next.ServeHTTP(w, r)
|
||||
},
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// Recoverer returns middleware that recovers from a panic in a
|
||||
// downstream handler, logs the panic and stack trace through
|
||||
// slog, and responds 500 with no body. http.ErrAbortHandler is
|
||||
// re-panicked so the server can abort the response as intended.
|
||||
func (s *Middleware) Recoverer() func(http.Handler) http.Handler {
|
||||
return func(next http.Handler) http.Handler {
|
||||
return http.HandlerFunc(
|
||||
func(w http.ResponseWriter, r *http.Request) {
|
||||
defer func() {
|
||||
rec := recover()
|
||||
if rec == nil {
|
||||
return
|
||||
}
|
||||
|
||||
err, ok := rec.(error)
|
||||
if ok && errors.Is(err, http.ErrAbortHandler) {
|
||||
panic(rec)
|
||||
}
|
||||
|
||||
s.log.ErrorContext(r.Context(),
|
||||
"panic recovered",
|
||||
"panic", fmt.Sprintf("%v", rec),
|
||||
"stack", string(debug.Stack()),
|
||||
)
|
||||
|
||||
w.WriteHeader(http.StatusInternalServerError)
|
||||
}()
|
||||
|
||||
next.ServeHTTP(w, r)
|
||||
},
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// CORS returns middleware that adds permissive CORS headers.
|
||||
func (s *Middleware) CORS() func(http.Handler) http.Handler {
|
||||
return cors.Handler(cors.Options{
|
||||
|
||||
@@ -1,9 +1,14 @@
|
||||
package middleware_test
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/netip"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"sneak.berlin/go/netwatch/internal/middleware"
|
||||
@@ -146,3 +151,152 @@ func TestSecurityHeaders(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestMaxBodyBytesRejectsOversizeOnNonReadingRoute confirms the
|
||||
// limit is enforced even for a handler that never reads the body
|
||||
// (for example the health check), via the Content-Length check.
|
||||
func TestMaxBodyBytesRejectsOversizeOnNonReadingRoute(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
const limit = 16
|
||||
|
||||
called := false
|
||||
handler := (&middleware.Middleware{}).MaxBodyBytes(limit)(
|
||||
http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
|
||||
called = true
|
||||
}),
|
||||
)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequestWithContext(t.Context(),
|
||||
http.MethodPost, "/.well-known/healthcheck",
|
||||
strings.NewReader(strings.Repeat("x", limit+1)),
|
||||
)
|
||||
handler.ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusRequestEntityTooLarge {
|
||||
t.Fatalf("status = %d, want %d",
|
||||
rec.Code, http.StatusRequestEntityTooLarge)
|
||||
}
|
||||
|
||||
if called {
|
||||
t.Fatal("handler ran despite oversize body")
|
||||
}
|
||||
|
||||
if got := rec.Body.String(); got != "{\"status\":\"error\"}\n" {
|
||||
t.Errorf("body = %q, want %q", got, "{\"status\":\"error\"}\n")
|
||||
}
|
||||
|
||||
got := rec.Header().Get("Content-Type")
|
||||
if got != "application/json; charset=utf-8" {
|
||||
t.Errorf("Content-Type = %q, want a JSON content type", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestMaxBodyBytesAllowsWithinLimit(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
const limit = 64
|
||||
|
||||
handler := (&middleware.Middleware{}).MaxBodyBytes(limit)(
|
||||
http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}),
|
||||
)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequestWithContext(t.Context(),
|
||||
http.MethodPost, "/api/v1/reports",
|
||||
strings.NewReader(`{"clientId":"c1"}`),
|
||||
)
|
||||
handler.ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want %d", rec.Code, http.StatusOK)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRecovererReturns500AndLogsThroughSlog(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var logbuf bytes.Buffer
|
||||
|
||||
mw := middleware.NewWithLogger(
|
||||
slog.New(slog.NewJSONHandler(&logbuf, nil)),
|
||||
)
|
||||
|
||||
handler := mw.Recoverer()(
|
||||
http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
|
||||
panic("boom")
|
||||
}),
|
||||
)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", http.NoBody)
|
||||
handler.ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusInternalServerError {
|
||||
t.Fatalf("status = %d, want %d",
|
||||
rec.Code, http.StatusInternalServerError)
|
||||
}
|
||||
|
||||
var record map[string]any
|
||||
|
||||
err := json.Unmarshal(logbuf.Bytes(), &record)
|
||||
if err != nil {
|
||||
t.Fatalf("panic log is not one JSON record: %v (%q)", err, logbuf.String())
|
||||
}
|
||||
|
||||
if record["msg"] != "panic recovered" || record["level"] != "ERROR" {
|
||||
t.Errorf("log record = %v, want msg %q at level ERROR",
|
||||
record, "panic recovered")
|
||||
}
|
||||
|
||||
if record["panic"] != "boom" {
|
||||
t.Errorf("panic field = %v, want %q", record["panic"], "boom")
|
||||
}
|
||||
|
||||
stack, _ := record["stack"].(string)
|
||||
if !strings.HasPrefix(stack, "goroutine ") {
|
||||
t.Errorf("stack field = %q, want a stack trace", stack)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRecovererRepanicsOnAbortHandler checks that a handler aborting
|
||||
// with http.ErrAbortHandler is not treated as a crash: Recoverer
|
||||
// panics again so the server aborts the response, and logs nothing.
|
||||
func TestRecovererRepanicsOnAbortHandler(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var logbuf bytes.Buffer
|
||||
|
||||
mw := middleware.NewWithLogger(
|
||||
slog.New(slog.NewJSONHandler(&logbuf, nil)),
|
||||
)
|
||||
|
||||
handler := mw.Recoverer()(
|
||||
http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
|
||||
panic(http.ErrAbortHandler)
|
||||
}),
|
||||
)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", http.NoBody)
|
||||
|
||||
var recovered any
|
||||
|
||||
func() {
|
||||
defer func() { recovered = recover() }()
|
||||
|
||||
handler.ServeHTTP(rec, req)
|
||||
}()
|
||||
|
||||
err, _ := recovered.(error)
|
||||
if !errors.Is(err, http.ErrAbortHandler) {
|
||||
t.Errorf("Recoverer panicked with %v, want http.ErrAbortHandler", recovered)
|
||||
}
|
||||
|
||||
if logbuf.Len() != 0 {
|
||||
t.Errorf("abort was logged: %q", logbuf.String())
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user