fix(backend): report ingest correctness: 500 on a refused report, 413 on oversize, global body cap (closes #23)
check / check (push) Successful in 10s
check / check (push) Successful in 10s
A report the buffer refuses now returns 500 instead of a false `ok`. Reports reach disk later, so a failed disk write is still answered 200 and shows in the log, and at shutdown as a failed stop with a non-zero exit. An over-limit body returns 413; malformed JSON stays 400. A MaxBodyBytes middleware caps every route at 1 MiB; a route group can only lower that limit. The raw geo blob is no longer logged; client_id, timestamp and decode error text are cut to 128 bytes before logging. Panic recovery logs the panic value and stack through slog. Model: opus-5-5
This commit was merged in pull request #58.
This commit is contained in:
@@ -23,6 +23,16 @@ latest run passes.
|
||||
|
||||
# Completed Steps
|
||||
|
||||
- 2026-09-28: report ingest correctness (issue #23): a storage failure now
|
||||
returns 500 instead of a false `ok`; oversize bodies return 413 (distinguished
|
||||
from malformed JSON, which stays 400); a `MaxBodyBytes` middleware caps every
|
||||
route, not just the report route; the raw attacker-controlled `geo` blob is no
|
||||
longer logged (only its length), and `client_id`, `timestamp` and decode error
|
||||
text are length-bounded before logging; a `decodeJSON` handler helper was
|
||||
added; panic recovery now routes the stack through slog instead of chi's
|
||||
plain-text stderr; and writing a report file now returns its error, so a
|
||||
failed final flush on shutdown makes the process exit non-zero instead of
|
||||
losing the buffered reports silently
|
||||
- 2026-09-21: shutdown lifecycle correctness. The process now shuts down through
|
||||
fx instead of `os.Exit`, so every component's `OnStop` runs and buffered
|
||||
reports are flushed to disk on `SIGTERM` — previously a full flush window of
|
||||
|
||||
@@ -0,0 +1,13 @@
|
||||
package handlers
|
||||
|
||||
import "log/slog"
|
||||
|
||||
// MaxLoggedFieldBytes exposes the log bound to the external tests.
|
||||
const MaxLoggedFieldBytes = maxLoggedFieldBytes
|
||||
|
||||
// NewForTest builds a Handlers around a report sink and logger,
|
||||
// bypassing the fx graph so handler behaviour (including the
|
||||
// storage failure path) is exercisable in unit tests.
|
||||
func NewForTest(buf reportAppender, log *slog.Logger) *Handlers {
|
||||
return &Handlers{buf: buf, log: log}
|
||||
}
|
||||
@@ -18,6 +18,13 @@ import (
|
||||
|
||||
const jsonContentType = "application/json; charset=utf-8"
|
||||
|
||||
// reportAppender is the subset of the report buffer the handlers
|
||||
// depend on. Defining it here keeps the storage failure path
|
||||
// exercisable with a stub in tests.
|
||||
type reportAppender interface {
|
||||
Append(v any) error
|
||||
}
|
||||
|
||||
// Params defines the dependencies for Handlers.
|
||||
type Params struct {
|
||||
fx.In
|
||||
@@ -30,7 +37,7 @@ type Params struct {
|
||||
|
||||
// Handlers provides HTTP handler factories for all endpoints.
|
||||
type Handlers struct {
|
||||
buf *reportbuf.Buffer
|
||||
buf reportAppender
|
||||
hc *healthcheck.Healthcheck
|
||||
log *slog.Logger
|
||||
params *Params
|
||||
@@ -72,3 +79,15 @@ func (s *Handlers) respondJSON(
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// decodeJSON decodes the request body into v. The body is
|
||||
// expected to already be bounded by the body-size middleware, so
|
||||
// a caller can distinguish an over-limit body from malformed
|
||||
// JSON by testing the returned error for *http.MaxBytesError.
|
||||
func (s *Handlers) decodeJSON(
|
||||
_ http.ResponseWriter,
|
||||
r *http.Request,
|
||||
v any,
|
||||
) error {
|
||||
return json.NewDecoder(r.Body).Decode(v)
|
||||
}
|
||||
|
||||
@@ -2,10 +2,14 @@ package handlers
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"net/http"
|
||||
)
|
||||
|
||||
const maxReportBodyBytes = 1 << 20 // 1 MiB
|
||||
// maxLoggedFieldBytes bounds untrusted text (string fields,
|
||||
// decode error text) before it is logged, so a caller cannot
|
||||
// inflate log volume with an oversized value.
|
||||
const maxLoggedFieldBytes = 128
|
||||
|
||||
type reportSample struct {
|
||||
T int64 `json:"t"`
|
||||
@@ -35,48 +39,80 @@ func (s *Handlers) HandleReport() http.HandlerFunc {
|
||||
}
|
||||
|
||||
return func(w http.ResponseWriter, r *http.Request) {
|
||||
r.Body = http.MaxBytesReader(
|
||||
w, r.Body, maxReportBodyBytes,
|
||||
)
|
||||
|
||||
var rpt report
|
||||
|
||||
err := json.NewDecoder(r.Body).Decode(&rpt)
|
||||
err := s.decodeJSON(w, r, &rpt)
|
||||
if err != nil {
|
||||
s.log.Error("failed to decode report",
|
||||
"error", err,
|
||||
)
|
||||
s.respondJSON(w, r,
|
||||
&response{Status: "error"},
|
||||
http.StatusBadRequest,
|
||||
s.decodeErrorStatus(err),
|
||||
)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
totalSamples := 0
|
||||
for _, h := range rpt.Hosts {
|
||||
totalSamples += len(h.History)
|
||||
}
|
||||
s.logReportReceived(rpt)
|
||||
|
||||
s.log.Info("report received",
|
||||
"client_id", rpt.ClientID,
|
||||
"timestamp", rpt.Timestamp,
|
||||
"host_count", len(rpt.Hosts),
|
||||
"total_samples", totalSamples,
|
||||
"geo", string(rpt.Geo),
|
||||
)
|
||||
|
||||
bufErr := s.buf.Append(rpt)
|
||||
if bufErr != nil {
|
||||
s.log.Error("failed to buffer report",
|
||||
"error", bufErr,
|
||||
err = s.buf.Append(rpt)
|
||||
if err != nil {
|
||||
s.log.Error("failed to buffer report", "error", err)
|
||||
s.respondJSON(w, r,
|
||||
&response{Status: "error"},
|
||||
http.StatusInternalServerError,
|
||||
)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
s.respondJSON(w, r,
|
||||
&response{Status: "ok"},
|
||||
http.StatusOK,
|
||||
)
|
||||
s.respondJSON(w, r, &response{Status: "ok"}, http.StatusOK)
|
||||
}
|
||||
}
|
||||
|
||||
// decodeErrorStatus logs a report decode failure and returns the
|
||||
// status to send: 413 when the body exceeded the size limit,
|
||||
// otherwise 400 for malformed JSON.
|
||||
func (s *Handlers) decodeErrorStatus(err error) int {
|
||||
var tooLarge *http.MaxBytesError
|
||||
if errors.As(err, &tooLarge) {
|
||||
s.log.Warn("report body too large", "limit_bytes", tooLarge.Limit)
|
||||
|
||||
return http.StatusRequestEntityTooLarge
|
||||
}
|
||||
|
||||
// The decoder's error text can quote request bytes (a whole
|
||||
// oversized number, for example), so it is bounded too.
|
||||
s.log.Error("failed to decode report",
|
||||
"error", boundedForLog(err.Error()),
|
||||
)
|
||||
|
||||
return http.StatusBadRequest
|
||||
}
|
||||
|
||||
// logReportReceived logs an accepted report. Untrusted fields are
|
||||
// bounded (client_id, timestamp) or reduced to a length
|
||||
// (geo_bytes) so the raw attacker-controlled body never reaches
|
||||
// the log.
|
||||
func (s *Handlers) logReportReceived(rpt report) {
|
||||
totalSamples := 0
|
||||
for _, h := range rpt.Hosts {
|
||||
totalSamples += len(h.History)
|
||||
}
|
||||
|
||||
s.log.Info("report received",
|
||||
"client_id", boundedForLog(rpt.ClientID),
|
||||
"timestamp", boundedForLog(rpt.Timestamp),
|
||||
"host_count", len(rpt.Hosts),
|
||||
"total_samples", totalSamples,
|
||||
"geo_bytes", len(rpt.Geo),
|
||||
)
|
||||
}
|
||||
|
||||
// boundedForLog truncates an untrusted string to a fixed byte
|
||||
// bound so an attacker-controlled field cannot dominate the log.
|
||||
func boundedForLog(s string) string {
|
||||
if len(s) > maxLoggedFieldBytes {
|
||||
return s[:maxLoggedFieldBytes]
|
||||
}
|
||||
|
||||
return s
|
||||
}
|
||||
|
||||
@@ -0,0 +1,212 @@
|
||||
package handlers_test
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"io"
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"sneak.berlin/go/netwatch/internal/handlers"
|
||||
"sneak.berlin/go/netwatch/internal/middleware"
|
||||
)
|
||||
|
||||
var errStorageFailed = errors.New("storage failed")
|
||||
|
||||
// stubAppender drives the storage success/failure path without a
|
||||
// real buffer or disk.
|
||||
type stubAppender struct {
|
||||
err error
|
||||
}
|
||||
|
||||
func (s stubAppender) Append(any) error { return s.err }
|
||||
|
||||
func newTestHandlers(buf stubAppender, out io.Writer) *handlers.Handlers {
|
||||
return handlers.NewForTest(buf, slog.New(slog.NewJSONHandler(out, nil)))
|
||||
}
|
||||
|
||||
func decodeStatus(t *testing.T, body []byte) string {
|
||||
t.Helper()
|
||||
|
||||
var resp struct {
|
||||
Status string `json:"status"`
|
||||
}
|
||||
|
||||
err := json.Unmarshal(body, &resp)
|
||||
if err != nil {
|
||||
t.Fatalf("response body not JSON: %v (%q)", err, body)
|
||||
}
|
||||
|
||||
return resp.Status
|
||||
}
|
||||
|
||||
func TestHandleReportStorageFailureIsNon2xx(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
h := newTestHandlers(stubAppender{err: errStorageFailed}, io.Discard)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequestWithContext(t.Context(),
|
||||
http.MethodPost, "/api/v1/reports",
|
||||
strings.NewReader(`{"clientId":"c1","hosts":[]}`),
|
||||
)
|
||||
|
||||
h.HandleReport().ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code < 500 {
|
||||
t.Fatalf("storage failure status = %d, want a 5xx", rec.Code)
|
||||
}
|
||||
|
||||
if got := decodeStatus(t, rec.Body.Bytes()); got != "error" {
|
||||
t.Fatalf("status field = %q, want %q", got, "error")
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleReportMalformedJSONIs400(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
h := newTestHandlers(stubAppender{}, io.Discard)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequestWithContext(t.Context(),
|
||||
http.MethodPost, "/api/v1/reports",
|
||||
strings.NewReader(`{not json`),
|
||||
)
|
||||
|
||||
h.HandleReport().ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusBadRequest {
|
||||
t.Fatalf("malformed status = %d, want %d",
|
||||
rec.Code, http.StatusBadRequest)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleReportOversizeIs413(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
const limit = 32
|
||||
|
||||
h := newTestHandlers(stubAppender{}, io.Discard)
|
||||
handler := (&middleware.Middleware{}).MaxBodyBytes(limit)(
|
||||
h.HandleReport(),
|
||||
)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequestWithContext(t.Context(),
|
||||
http.MethodPost, "/api/v1/reports",
|
||||
strings.NewReader(`{"clientId":"`+strings.Repeat("x", 200)+`"}`),
|
||||
)
|
||||
// No declared length, so only the middleware's read cap can
|
||||
// stop this body.
|
||||
req.ContentLength = -1
|
||||
|
||||
handler.ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusRequestEntityTooLarge {
|
||||
t.Fatalf("oversize status = %d, want %d",
|
||||
rec.Code, http.StatusRequestEntityTooLarge)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleReportDoesNotLogRawGeo(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
const sentinel = "SENSITIVE-GEO-BLOB"
|
||||
|
||||
var logbuf bytes.Buffer
|
||||
|
||||
h := newTestHandlers(stubAppender{}, &logbuf)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequestWithContext(t.Context(),
|
||||
http.MethodPost, "/api/v1/reports",
|
||||
strings.NewReader(
|
||||
`{"clientId":"c1","geo":{"raw":"`+sentinel+`"},"hosts":[]}`,
|
||||
),
|
||||
)
|
||||
|
||||
h.HandleReport().ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want %d", rec.Code, http.StatusOK)
|
||||
}
|
||||
|
||||
if strings.Contains(logbuf.String(), sentinel) {
|
||||
t.Fatal("raw geo bytes were written to the log")
|
||||
}
|
||||
|
||||
if !strings.Contains(logbuf.String(), "geo_bytes") {
|
||||
t.Fatal("expected a bounded geo_bytes field in the log")
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleReportLogsClientIDCutToBound(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
long := strings.Repeat("c", 2*handlers.MaxLoggedFieldBytes)
|
||||
|
||||
var logbuf bytes.Buffer
|
||||
|
||||
h := newTestHandlers(stubAppender{}, &logbuf)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequestWithContext(t.Context(),
|
||||
http.MethodPost, "/api/v1/reports",
|
||||
strings.NewReader(
|
||||
`{"clientId":"`+long+`","timestamp":"`+long+`","hosts":[]}`,
|
||||
),
|
||||
)
|
||||
|
||||
h.HandleReport().ServeHTTP(rec, req)
|
||||
|
||||
var logged map[string]any
|
||||
|
||||
err := json.Unmarshal(logbuf.Bytes(), &logged)
|
||||
if err != nil {
|
||||
t.Fatalf("log line not JSON: %v (%q)", err, logbuf.String())
|
||||
}
|
||||
|
||||
want := long[:handlers.MaxLoggedFieldBytes]
|
||||
|
||||
if logged["client_id"] != want {
|
||||
t.Fatalf("logged client_id not cut to %d bytes: %q",
|
||||
handlers.MaxLoggedFieldBytes, logged["client_id"])
|
||||
}
|
||||
|
||||
if logged["timestamp"] != want {
|
||||
t.Fatalf("logged timestamp not cut to %d bytes: %q",
|
||||
handlers.MaxLoggedFieldBytes, logged["timestamp"])
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleReportDecodeErrorLogIsBounded(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// A number too large for its int64 field makes the decoder's
|
||||
// error text quote the whole number.
|
||||
huge := strings.Repeat("9", 2*handlers.MaxLoggedFieldBytes)
|
||||
|
||||
var logbuf bytes.Buffer
|
||||
|
||||
h := newTestHandlers(stubAppender{}, &logbuf)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequestWithContext(t.Context(),
|
||||
http.MethodPost, "/api/v1/reports",
|
||||
strings.NewReader(`{"hosts":[{"history":[{"t":`+huge+`}]}]}`),
|
||||
)
|
||||
|
||||
h.HandleReport().ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusBadRequest {
|
||||
t.Fatalf("status = %d, want %d", rec.Code, http.StatusBadRequest)
|
||||
}
|
||||
|
||||
if strings.Contains(logbuf.String(), huge) {
|
||||
t.Fatal("the whole oversized number was written to the log")
|
||||
}
|
||||
}
|
||||
@@ -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())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -80,12 +80,16 @@ func New(
|
||||
// stopOnce makes OnStop idempotent: a second
|
||||
// invocation must not close an already-closed channel
|
||||
// (which would panic) or flush again.
|
||||
var err error
|
||||
|
||||
b.stopOnce.Do(func() {
|
||||
close(b.done)
|
||||
b.flushLocked()
|
||||
err = b.flushLocked()
|
||||
})
|
||||
|
||||
return nil
|
||||
// A failed final flush fails the stop, so the process
|
||||
// exits non-zero.
|
||||
return err
|
||||
},
|
||||
})
|
||||
|
||||
@@ -109,7 +113,12 @@ func (b *Buffer) Append(v any) error {
|
||||
data := b.drainBuf()
|
||||
b.mu.Unlock()
|
||||
|
||||
go b.writeFile(data)
|
||||
go func() {
|
||||
writeErr := b.writeFile(data)
|
||||
if writeErr != nil {
|
||||
b.log.Error("flush reports failed", "error", writeErr)
|
||||
}
|
||||
}()
|
||||
|
||||
return nil
|
||||
}
|
||||
@@ -128,7 +137,10 @@ func (b *Buffer) flushLoop() {
|
||||
for {
|
||||
select {
|
||||
case <-ticker.C:
|
||||
b.flushLocked()
|
||||
err := b.flushLocked()
|
||||
if err != nil {
|
||||
b.log.Error("flush reports failed", "error", err)
|
||||
}
|
||||
case <-b.done:
|
||||
return
|
||||
}
|
||||
@@ -137,19 +149,19 @@ func (b *Buffer) flushLoop() {
|
||||
|
||||
// flushLocked acquires the lock, drains the buffer, and
|
||||
// writes the data to a compressed file.
|
||||
func (b *Buffer) flushLocked() {
|
||||
func (b *Buffer) flushLocked() error {
|
||||
b.mu.Lock()
|
||||
|
||||
if b.buf.Len() == 0 {
|
||||
b.mu.Unlock()
|
||||
|
||||
return
|
||||
return nil
|
||||
}
|
||||
|
||||
data := b.drainBuf()
|
||||
b.mu.Unlock()
|
||||
|
||||
b.writeFile(data)
|
||||
return b.writeFile(data)
|
||||
}
|
||||
|
||||
// drainBuf copies the buffer contents and resets it.
|
||||
@@ -164,7 +176,7 @@ func (b *Buffer) drainBuf() []byte {
|
||||
|
||||
// writeFile creates a timestamped zstd-compressed JSONL file
|
||||
// in the data directory.
|
||||
func (b *Buffer) writeFile(data []byte) {
|
||||
func (b *Buffer) writeFile(data []byte) error {
|
||||
ts := time.Now().UTC().Format("2006-01-02T15-04-05.000Z")
|
||||
name := fmt.Sprintf("reports-%s.jsonl.zst", ts)
|
||||
path := filepath.Join(b.dataDir, name)
|
||||
@@ -177,31 +189,35 @@ func (b *Buffer) writeFile(data []byte) {
|
||||
filePerms,
|
||||
)
|
||||
if err != nil {
|
||||
b.log.Error("create report file", "error", err)
|
||||
|
||||
return
|
||||
return fmt.Errorf("create report file: %w", err)
|
||||
}
|
||||
|
||||
// Closes the file on the early returns below. The success
|
||||
// path closes it explicitly to check the error; closing it
|
||||
// a second time here is harmless.
|
||||
defer func() { _ = f.Close() }()
|
||||
|
||||
enc, err := zstd.NewWriter(f)
|
||||
if err != nil {
|
||||
b.log.Error("create zstd encoder", "error", err)
|
||||
|
||||
return
|
||||
return fmt.Errorf("create zstd encoder: %w", err)
|
||||
}
|
||||
|
||||
_, writeErr := enc.Write(data)
|
||||
if writeErr != nil {
|
||||
b.log.Error("write compressed data", "error", writeErr)
|
||||
|
||||
_, err = enc.Write(data)
|
||||
if err != nil {
|
||||
_ = enc.Close()
|
||||
|
||||
return
|
||||
return fmt.Errorf("write compressed data: %w", err)
|
||||
}
|
||||
|
||||
closeErr := enc.Close()
|
||||
if closeErr != nil {
|
||||
b.log.Error("close zstd encoder", "error", closeErr)
|
||||
err = enc.Close()
|
||||
if err != nil {
|
||||
return fmt.Errorf("close zstd encoder: %w", err)
|
||||
}
|
||||
|
||||
err = f.Close()
|
||||
if err != nil {
|
||||
return fmt.Errorf("close report file: %w", err)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
package reportbuf_test
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"io/fs"
|
||||
"os"
|
||||
"strings"
|
||||
"testing"
|
||||
@@ -52,6 +54,46 @@ func TestFlushOnShutdown(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestFailedFinalFlushFailsStop proves a final flush that cannot
|
||||
// write its file makes the stop fail, which makes the process
|
||||
// exit non-zero instead of dropping the buffered reports silently.
|
||||
func TestFailedFinalFlushFailsStop(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
t.Setenv("DATA_DIR", dir)
|
||||
|
||||
var buf *reportbuf.Buffer
|
||||
|
||||
app := fxtest.New(t,
|
||||
fx.Provide(
|
||||
globals.New,
|
||||
logger.New,
|
||||
config.New,
|
||||
reportbuf.New,
|
||||
),
|
||||
fx.Populate(&buf),
|
||||
)
|
||||
|
||||
app.RequireStart()
|
||||
|
||||
err := buf.Append(map[string]string{"probe": "shutdown"})
|
||||
if err != nil {
|
||||
t.Fatalf("append report: %v", err)
|
||||
}
|
||||
|
||||
// Removing the data directory leaves the final flush nowhere to
|
||||
// write. A read-only directory would not do: tests run as root
|
||||
// in the backend image, and root ignores the read-only bit.
|
||||
err = os.RemoveAll(dir)
|
||||
if err != nil {
|
||||
t.Fatalf("remove data dir: %v", err)
|
||||
}
|
||||
|
||||
err = app.Stop(t.Context())
|
||||
if !errors.Is(err, fs.ErrNotExist) {
|
||||
t.Fatalf("stop error = %v, want the final flush's error", err)
|
||||
}
|
||||
}
|
||||
|
||||
func hasReportFile(t *testing.T, dir string) bool {
|
||||
t.Helper()
|
||||
|
||||
|
||||
@@ -0,0 +1,5 @@
|
||||
package server
|
||||
|
||||
// MaxRequestBodyBytes exposes the router-wide body limit to the
|
||||
// external tests.
|
||||
const MaxRequestBodyBytes = maxRequestBodyBytes
|
||||
@@ -7,18 +7,26 @@ import (
|
||||
"github.com/go-chi/chi/v5/middleware"
|
||||
)
|
||||
|
||||
const requestTimeout = 60 * time.Second
|
||||
const (
|
||||
requestTimeout = 60 * time.Second
|
||||
|
||||
// maxRequestBodyBytes caps every request body. A route group
|
||||
// can mount s.mw.MaxBodyBytes with a smaller value to lower
|
||||
// its bound, but cannot raise it: this cap runs first.
|
||||
maxRequestBodyBytes int64 = 1 << 20 // 1 MiB
|
||||
)
|
||||
|
||||
// SetupRoutes configures the chi router with middleware and
|
||||
// all application routes.
|
||||
func (s *Server) SetupRoutes() {
|
||||
s.router = chi.NewRouter()
|
||||
|
||||
s.router.Use(middleware.Recoverer)
|
||||
s.router.Use(s.mw.Recoverer())
|
||||
s.router.Use(middleware.RequestID)
|
||||
s.router.Use(s.mw.Logging())
|
||||
s.router.Use(s.mw.SecurityHeaders())
|
||||
s.router.Use(s.mw.CORS())
|
||||
s.router.Use(s.mw.MaxBodyBytes(maxRequestBodyBytes))
|
||||
s.router.Use(middleware.Timeout(requestTimeout))
|
||||
|
||||
s.router.Get(
|
||||
|
||||
@@ -0,0 +1,76 @@
|
||||
package server_test
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"sneak.berlin/go/netwatch/internal/config"
|
||||
"sneak.berlin/go/netwatch/internal/globals"
|
||||
"sneak.berlin/go/netwatch/internal/handlers"
|
||||
"sneak.berlin/go/netwatch/internal/healthcheck"
|
||||
"sneak.berlin/go/netwatch/internal/logger"
|
||||
"sneak.berlin/go/netwatch/internal/middleware"
|
||||
"sneak.berlin/go/netwatch/internal/reportbuf"
|
||||
"sneak.berlin/go/netwatch/internal/server"
|
||||
|
||||
"go.uber.org/fx"
|
||||
"go.uber.org/fx/fxtest"
|
||||
)
|
||||
|
||||
// TestHealthCheckRejectsOversizeBody sends the health check, which
|
||||
// never reads its body, a body one byte over the limit. Only the
|
||||
// router-wide body limit can reject it.
|
||||
func TestHealthCheckRejectsOversizeBody(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var srv *server.Server
|
||||
|
||||
// The same constructors as main, never started: SetupRoutes is
|
||||
// called directly, so nothing listens.
|
||||
app := fxtest.New(t,
|
||||
fx.Provide(
|
||||
config.New,
|
||||
globals.New,
|
||||
handlers.New,
|
||||
healthcheck.New,
|
||||
logger.New,
|
||||
middleware.New,
|
||||
reportbuf.New,
|
||||
server.New,
|
||||
),
|
||||
fx.Populate(&srv),
|
||||
)
|
||||
|
||||
err := app.Err()
|
||||
if err != nil {
|
||||
t.Fatalf("build server: %v", err)
|
||||
}
|
||||
|
||||
srv.SetupRoutes()
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
req := httptest.NewRequestWithContext(t.Context(),
|
||||
http.MethodGet, "/.well-known/healthcheck",
|
||||
strings.NewReader(
|
||||
strings.Repeat("x", int(server.MaxRequestBodyBytes)+1),
|
||||
),
|
||||
)
|
||||
|
||||
srv.ServeHTTP(rec, req)
|
||||
|
||||
if rec.Code != http.StatusRequestEntityTooLarge {
|
||||
t.Fatalf("status = %d, want %d",
|
||||
rec.Code, http.StatusRequestEntityTooLarge)
|
||||
}
|
||||
|
||||
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)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user