diff --git a/README.md b/README.md index 588e2db..2ab577b 100644 --- a/README.md +++ b/README.md @@ -120,8 +120,9 @@ path under `/v1/` answers 200, in maintenance mode too. `blocked_networks`); 502 when the upstream answered with an error status, and for 5 minutes after that for the same source URL; 503 when pixa is busy or in maintenance mode; 500 for any other failure. -- `GET /v1/e//` — an image through an encrypted URL (see Encrypted - URLs). Needs: nothing but the URL. Answers: 200; 400 for a token that does not +- `GET` or `HEAD` `/v1/e//` — an image through an encrypted URL + (see Encrypted URLs). Needs: nothing but the URL. Answers: 200; 304 when + `If-None-Match` matches the image's `ETag`; 400 for a token that does not decrypt, or that asks for a size or fit that is not valid; 410 once it has expired; 504 when the upstream has not sent its response headers within `upstream_fetch_timeout`, but 500 when that time runs out while the image @@ -137,6 +138,15 @@ path under `/v1/` answers 200, in maintenance mode too. authentication with `metrics.username` and `metrics.password`. Answers: 200; 401 without them; 404 when they are not set, as the route then does not exist. +Every response carries an `X-Request-ID` header holding the request's ID, which +a client can quote when reporting a problem: the request's own `X-Request-ID`, +as a reverse proxy in front of pixa may send, when it is at most 64 letters, +digits, `-`, `_` or `.`; otherwise a random one pixa makes for the request, +which tells nothing about the machine or the other requests. pixa's log line for +the request carries the same ID as `request_id`, and so do the lines it logs +when it fetches, converts and serves an image; the fetch sends it to the +upstream host as `X-Request-ID`. + Both `POST` routes accept only a form that pixa's own page served: the page puts a token in the form and sets a cookie to match, and a request without both is refused with 403, so another site cannot submit the form from a visitor's @@ -186,7 +196,9 @@ source) and one transcode: the first request does the work, and the others wait for its image or its error, holding no upstream connection or processing slot of their own. A waiting request stops waiting when its own client goes away. The work goes on for the others even if the first request's client goes away, -until that request's `downstream_timeout` ends. +until that request's `downstream_timeout` ends. The shared fetch sends the first +request's ID upstream, and the lines logged for the fetch and the transcode +carry that ID. The login form (`POST /`) is limited to 5 attempts per minute per client address, counting an IPv6 client by its /64; an attempt over the limit is diff --git a/TODO.md b/TODO.md index 0c4ac74..2138144 100644 --- a/TODO.md +++ b/TODO.md @@ -29,6 +29,20 @@ P2: security: referer blacklist # Completed Steps +- 2026-10-04 request IDs returned and passed on, and `/v1/e/` revalidates + (closes #84): pixa's own `RequestID` middleware, in place of chi's, gives each + request an ID, its own `X-Request-ID` when that is at most 64 letters, digits, + `-`, `_` or `.` and a random one otherwise, stores it where chi's did and + sends it back as `X-Request-ID` on every response; the upstream fetch sends + that ID, and the "upstream fetched", "image converted" and "image served" log + lines carry it as `request_id`, a fetch shared by several requests carrying + the first request's; `/v1/e/` sets `ETag`, answers a matching `If-None-Match` + with 304 and is routed for `HEAD`, the `ETag` and 304 code being + `notModified`, which `/v1/image/` calls too; its token checks moved unchanged + into `parseImageEncRequest` to keep `HandleImageEnc` within the line limit; no + `Vary` is added, as no response depends on a request header except the image + routes' CORS headers, for which `go-chi/cors` already sends `Vary: Origin`; + `Vary: Accept` is left to #88. - 2026-10-04 routes, encrypted URLs and config file documented (closes #75): "Routes" in `README.md` lists every route with its method, purpose, what it needs and the status codes it answers with, and says `q` and `fit` are part @@ -438,7 +452,6 @@ P2: security: referer blacklist - P2: HTTP response handling - Last-Modified headers - Vary header for content negotiation - - X-Request-ID propagation - P2: auto format selection (format=auto based on Accept header) - P2: configuration - YAML config file support diff --git a/internal/handlers/image.go b/internal/handlers/image.go index 3c94bc8..1a46a58 100644 --- a/internal/handlers/image.go +++ b/internal/handlers/image.go @@ -10,6 +10,7 @@ import ( "time" "github.com/go-chi/chi/v5" + "github.com/go-chi/chi/v5/middleware" "sneak.berlin/go/pixa/internal/encurl" "sneak.berlin/go/pixa/internal/httpfetcher" "sneak.berlin/go/pixa/internal/imageprocessor" @@ -247,6 +248,25 @@ func cacheControl(expires time.Time) string { return fmt.Sprintf("public, max-age=%d, immutable", int64(maxAge/time.Second)) } +// notModified sets the ETag header to etag and, when the request's +// If-None-Match is that ETag, answers 304 Not Modified. It reports whether it +// answered. An empty etag sets no header and never answers. +func notModified(w http.ResponseWriter, r *http.Request, etag string) bool { + if etag == "" { + return false + } + + w.Header().Set("ETag", etag) + + if r.Header.Get("If-None-Match") != etag { + return false + } + + w.WriteHeader(http.StatusNotModified) + + return true +} + // writeImageResponse writes headers and streams the image content, // handling conditional and HEAD requests. func (s *Handlers) writeImageResponse( @@ -265,17 +285,8 @@ func (s *Handlers) writeImageResponse( w.Header().Set("Cache-Control", cacheControl(req.Expires)) w.Header().Set("X-Pixa-Cache", string(resp.CacheStatus)) - if resp.ETag != "" { - w.Header().Set("ETag", resp.ETag) - - // Check for conditional request (If-None-Match) - if ifNoneMatch := r.Header.Get("If-None-Match"); ifNoneMatch != "" { - if ifNoneMatch == resp.ETag { - w.WriteHeader(http.StatusNotModified) - - return - } - } + if notModified(w, r, resp.ETag) { + return } // Handle HEAD request - return headers only @@ -298,6 +309,7 @@ func (s *Handlers) writeImageResponse( // Log cache status and timing after serving duration := time.Since(startTime) s.log.Info("image served", + "request_id", middleware.GetReqID(r.Context()), "cache_key", cacheKey, "cache_status", resp.CacheStatus, "duration_ms", duration.Milliseconds(), diff --git a/internal/handlers/image_cache_control_internal_test.go b/internal/handlers/image_cache_control_internal_test.go index bb6a6b9..7845df5 100644 --- a/internal/handlers/image_cache_control_internal_test.go +++ b/internal/handlers/image_cache_control_internal_test.go @@ -23,8 +23,10 @@ const photoPath = "/images/photo.jpg" // newSignedHostServer returns a router for both image routes, and the Handlers // behind it, whose fetcher serves a JPEG at photoPath on signedHost. signedHost // is not on the allowlist, so a /v1/image/ URL for it is served only with a -// valid signature. -func newSignedHostServer(t *testing.T) (*Handlers, http.Handler) { +// valid signature. The handlers and the image service log to log. +func newSignedHostServer( + t *testing.T, log *slog.Logger, +) (*Handlers, http.Handler) { t.Helper() cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{ @@ -44,6 +46,7 @@ func newSignedHostServer(t *testing.T) (*Handlers, http.Handler) { signedHost + photoPath: &fstest.MapFile{Data: jpegData}, }), SigningKey: testSigningKey, + Logger: log, }) if err != nil { t.Fatalf("imgcache.NewService() error = %v", err) @@ -55,7 +58,7 @@ func newSignedHostServer(t *testing.T) (*Handlers, http.Handler) { } h := &Handlers{ - log: slog.New(slog.DiscardHandler), + log: log, imgSvc: svc, encGen: encGen, } @@ -103,7 +106,7 @@ func getMaxAge(t *testing.T, srv http.Handler, target string) int { func TestHandleImage_SignedURL_MaxAgeEndsAtExp(t *testing.T) { t.Parallel() - h, srv := newSignedHostServer(t) + h, srv := newSignedHostServer(t, slog.New(slog.DiscardHandler)) signedURL, err := h.imgSvc.GenerateSignedURL("", &imgcache.ImageRequest{ SourceHost: signedHost, @@ -179,7 +182,7 @@ func TestHandleImageEnc_MaxAge(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - h, srv := newSignedHostServer(t) + h, srv := newSignedHostServer(t, slog.New(slog.DiscardHandler)) token, err := h.encGen.Generate(&encurl.Payload{ SourceHost: signedHost, diff --git a/internal/handlers/imageenc.go b/internal/handlers/imageenc.go index 56af31b..dadb6bd 100644 --- a/internal/handlers/imageenc.go +++ b/internal/handlers/imageenc.go @@ -9,6 +9,7 @@ import ( "time" "github.com/go-chi/chi/v5" + "github.com/go-chi/chi/v5/middleware" "sneak.berlin/go/pixa/internal/encurl" "sneak.berlin/go/pixa/internal/httpfetcher" @@ -24,43 +25,8 @@ func (s *Handlers) HandleImageEnc() http.HandlerFunc { ctx := r.Context() start := time.Now() - // Extract token from URL - token := chi.URLParam(r, "token") - if token == "" { - s.respondError(w, "missing token", http.StatusBadRequest) - - return - } - - // Decrypt and validate the payload - payload, err := s.encGen.Parse(token) - if err != nil { - if errors.Is(err, encurl.ErrExpired) { - s.log.Debug("encrypted URL expired", "error", err) - s.respondError(w, "URL has expired", http.StatusGone) - - return - } - - s.log.Debug("failed to decrypt URL", "error", err) - s.respondError(w, "invalid encrypted URL", http.StatusBadRequest) - - return - } - - // Convert payload to ImageRequest - req := payload.ToImageRequest() - - // Apply the same dimension and fit-mode bounds as the plain image - // route: a sealed payload is trusted for its origin, not for staying - // within limits, so an over-limit size or unknown fit mode is a 400 - // here rather than an out-of-memory or a 500 from the processor. - err = imgcache.ValidateImageRequest(req) - if err != nil { - s.log.Debug("encrypted URL failed validation", "error", err) - s.respondError(w, "invalid encrypted URL: "+err.Error(), - http.StatusBadRequest) - + req, ok := s.parseImageEncRequest(w, r) + if !ok { return } @@ -94,6 +60,17 @@ func (s *Handlers) HandleImageEnc() http.HandlerFunc { w.Header().Set("Cache-Control", cacheControl(req.Expires)) w.Header().Set("X-Pixa-Cache", string(resp.CacheStatus)) + if notModified(w, r, resp.ETag) { + return + } + + // A HEAD request gets the headers only + if r.Method == http.MethodHead { + w.WriteHeader(http.StatusOK) + + return + } + // Stream the response written, err := io.Copy(w, resp.Content) if err != nil { @@ -105,6 +82,7 @@ func (s *Handlers) HandleImageEnc() http.HandlerFunc { // Log completion duration := time.Since(start) s.log.Info("image served", + "request_id", middleware.GetReqID(ctx), "cache_key", imgcache.CacheKey(req), "host", req.SourceHost, "path", req.SourcePath, @@ -116,6 +94,56 @@ func (s *Handlers) HandleImageEnc() http.HandlerFunc { } } +// parseImageEncRequest decrypts the token of an encrypted image URL into an +// ImageRequest and checks it. On a token that is missing, does not decrypt, +// has expired or asks for something not valid, it writes an error response +// and returns false. +func (s *Handlers) parseImageEncRequest( + w http.ResponseWriter, r *http.Request, +) (*imgcache.ImageRequest, bool) { + // Extract token from URL + token := chi.URLParam(r, "token") + if token == "" { + s.respondError(w, "missing token", http.StatusBadRequest) + + return nil, false + } + + // Decrypt and validate the payload + payload, err := s.encGen.Parse(token) + if err != nil { + if errors.Is(err, encurl.ErrExpired) { + s.log.Debug("encrypted URL expired", "error", err) + s.respondError(w, "URL has expired", http.StatusGone) + + return nil, false + } + + s.log.Debug("failed to decrypt URL", "error", err) + s.respondError(w, "invalid encrypted URL", http.StatusBadRequest) + + return nil, false + } + + // Convert payload to ImageRequest + req := payload.ToImageRequest() + + // Apply the same dimension and fit-mode bounds as the plain image + // route: a sealed payload is trusted for its origin, not for staying + // within limits, so an over-limit size or unknown fit mode is a 400 + // here rather than an out-of-memory or a 500 from the processor. + err = imgcache.ValidateImageRequest(req) + if err != nil { + s.log.Debug("encrypted URL failed validation", "error", err) + s.respondError(w, "invalid encrypted URL: "+err.Error(), + http.StatusBadRequest) + + return nil, false + } + + return req, true +} + // handleImageError converts image service errors to HTTP responses. func (s *Handlers) handleImageError(w http.ResponseWriter, err error) { switch { diff --git a/internal/handlers/imageenc_internal_test.go b/internal/handlers/imageenc_internal_test.go index be2db22..d4f8c6b 100644 --- a/internal/handlers/imageenc_internal_test.go +++ b/internal/handlers/imageenc_internal_test.go @@ -96,3 +96,70 @@ func TestHandleImageEnc_InvalidFitMode_Returns400(t *testing.T) { t.Fatalf("status = %d, want %d", rec.Code, http.StatusBadRequest) } } + +// TestHandleImageEnc_IfNoneMatch_Returns304 verifies that an image served +// through an encrypted URL carries an ETag, and that a request whose +// If-None-Match is that ETag is answered 304 Not Modified with no body. +func TestHandleImageEnc_IfNoneMatch_Returns304(t *testing.T) { + t.Parallel() + + h, srv := newSignedHostServer(t, slog.New(slog.DiscardHandler)) + target := encPhotoURL(t, h) + + rec := httptest.NewRecorder() + srv.ServeHTTP(rec, httptest.NewRequestWithContext( + t.Context(), http.MethodGet, target, nil)) + + etag := rec.Header().Get("ETag") + t.Logf("GET: %d, ETag %q", rec.Code, etag) + + if rec.Code != http.StatusOK || etag == "" { + t.Fatalf("GET: status = %d, ETag = %q, want %d and an ETag", + rec.Code, etag, http.StatusOK) + } + + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, target, nil) + req.Header.Set("If-None-Match", etag) + + rec = httptest.NewRecorder() + srv.ServeHTTP(rec, req) + t.Logf("GET with If-None-Match: %d, %d body bytes", rec.Code, rec.Body.Len()) + + if rec.Code != http.StatusNotModified || rec.Body.Len() != 0 { + t.Errorf("status = %d with %d body bytes, want %d with none", + rec.Code, rec.Body.Len(), http.StatusNotModified) + } +} + +// TestHandleImageEnc_HEAD_ReturnsHeadersOnly verifies that HEAD on an +// encrypted URL is answered 200 with the headers GET sends and no body. +func TestHandleImageEnc_HEAD_ReturnsHeadersOnly(t *testing.T) { + t.Parallel() + + h, _ := newSignedHostServer(t, slog.New(slog.DiscardHandler)) + + r := chi.NewRouter() + r.Head("/v1/e/{token}/*", h.HandleImageEnc()) + + rec := httptest.NewRecorder() + r.ServeHTTP(rec, httptest.NewRequestWithContext( + t.Context(), http.MethodHead, encPhotoURL(t, h), nil)) + t.Logf("HEAD: %d, headers %v, %d body bytes", + rec.Code, rec.Header(), rec.Body.Len()) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d", rec.Code, http.StatusOK) + } + + for _, name := range []string{ + "Content-Type", "Content-Length", "Cache-Control", "ETag", + } { + if rec.Header().Get(name) == "" { + t.Errorf("HEAD response has no %s", name) + } + } + + if rec.Body.Len() != 0 { + t.Errorf("HEAD response body has %d bytes, want none", rec.Body.Len()) + } +} diff --git a/internal/handlers/request_id_internal_test.go b/internal/handlers/request_id_internal_test.go new file mode 100644 index 0000000..d26a2f6 --- /dev/null +++ b/internal/handlers/request_id_internal_test.go @@ -0,0 +1,131 @@ +package handlers + +import ( + "bytes" + "context" + "encoding/json" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/go-chi/chi/v5/middleware" + + "sneak.berlin/go/pixa/internal/encurl" + "sneak.berlin/go/pixa/internal/imgcache" +) + +// signedPhotoURL returns a signed /v1/image/ URL, valid for a minute, for the +// JPEG at photoPath on signedHost at 50x50, made with h's image service. +func signedPhotoURL(t *testing.T, h *Handlers) string { + t.Helper() + + signedURL, err := h.imgSvc.GenerateSignedURL("", &imgcache.ImageRequest{ + SourceHost: signedHost, + SourcePath: photoPath, + Size: imgcache.Size{Width: 50, Height: 50}, + Format: imgcache.FormatJPEG, + }, time.Minute) + if err != nil { + t.Fatalf("GenerateSignedURL() error = %v", err) + } + + return signedURL +} + +// encPhotoURL returns an encrypted /v1/e/ URL, which never expires, for the +// JPEG at photoPath on signedHost at 50x50, made with h's generator. +func encPhotoURL(t *testing.T, h *Handlers) string { + t.Helper() + + token, err := h.encGen.Generate(&encurl.Payload{ + SourceHost: signedHost, + SourcePath: photoPath, + Width: 50, + Height: 50, + Format: imgcache.FormatJPEG, + }) + if err != nil { + t.Fatalf("Generate() error = %v", err) + } + + return "/v1/e/" + token + "/img.jpg" +} + +// requestIDByMessage reads the JSON log lines in logs and returns the +// request_id of each line, by its message. +func requestIDByMessage(t *testing.T, logs io.Reader) map[string]string { + t.Helper() + + logged := make(map[string]string) + + dec := json.NewDecoder(logs) + for dec.More() { + var line map[string]any + + err := dec.Decode(&line) + if err != nil { + t.Fatalf("decoding log line: %v", err) + } + + msg, _ := line["msg"].(string) + requestID, _ := line["request_id"].(string) + logged[msg] = requestID + } + + return logged +} + +// TestImageLogLinesCarryRequestID verifies that the lines logged when an image +// is fetched, converted and served through either image route carry the +// request's ID as request_id, as the request log line does, so they can be +// found from it. +func TestImageLogLinesCarryRequestID(t *testing.T) { + t.Parallel() + + const requestID = "test-request-id" + + imageURLs := map[string]func(*testing.T, *Handlers) string{ + "/v1/image/": signedPhotoURL, + "/v1/e/": encPhotoURL, + } + + for route, imageURL := range imageURLs { + t.Run(route, func(t *testing.T) { + t.Parallel() + + var logs bytes.Buffer + + h, srv := newSignedHostServer(t, + slog.New(slog.NewJSONHandler(&logs, nil))) + + ctx := context.WithValue(t.Context(), + middleware.RequestIDKey, requestID) + rec := httptest.NewRecorder() + srv.ServeHTTP(rec, httptest.NewRequestWithContext( + ctx, http.MethodGet, imageURL(t, h), nil)) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d", rec.Code, http.StatusOK) + } + + t.Logf("logged:\n%s", logs.String()) + + logged := requestIDByMessage(t, &logs) + + for _, msg := range []string{ + "upstream fetched", "image converted", "image served", + } { + got, ok := logged[msg] + if !ok { + t.Errorf("no %q line logged", msg) + } else if got != requestID { + t.Errorf("%q line has request_id %q, want %q", + msg, got, requestID) + } + } + }) + } +} diff --git a/internal/httpfetcher/httpfetcher.go b/internal/httpfetcher/httpfetcher.go index 3cfee1c..f76e8b1 100644 --- a/internal/httpfetcher/httpfetcher.go +++ b/internal/httpfetcher/httpfetcher.go @@ -18,6 +18,8 @@ import ( "strings" "sync" "time" + + "github.com/go-chi/chi/v5/middleware" ) // Fetcher configuration constants. @@ -267,6 +269,13 @@ func (f *HTTPFetcher) Fetch(ctx context.Context, url string) (*FetchResult, erro req.Header.Set("User-Agent", f.config.UserAgent) req.Header.Set("Accept", strings.Join(f.config.AllowedContentTypes, ", ")) + // The ID of the request this fetch serves, so the fetch can be found in + // the upstream host's logs + requestID := middleware.GetReqID(ctx) + if requestID != "" { + req.Header.Set(middleware.RequestIDHeader, requestID) + } + // Use httptrace to capture connection details var remoteAddr string diff --git a/internal/httpfetcher/request_id_internal_test.go b/internal/httpfetcher/request_id_internal_test.go new file mode 100644 index 0000000..a3c998e --- /dev/null +++ b/internal/httpfetcher/request_id_internal_test.go @@ -0,0 +1,50 @@ +package httpfetcher + +import ( + "context" + "io" + "net/http" + "net/http/httptest" + "testing" + + "github.com/go-chi/chi/v5/middleware" +) + +// TestFetchSendsRequestID verifies that a fetch sends the ID of the request +// it serves, which the RequestID middleware stores in the request context, +// to the upstream host as X-Request-Id, so the fetch can be found in that +// host's logs. +func TestFetchSendsRequestID(t *testing.T) { + t.Parallel() + + const requestID = "test-request-id" + + received := make(chan string, 1) + + srv := httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + received <- r.Header.Get("X-Request-Id") + + w.Header().Set("Content-Type", contentTypeJPEG) + _, _ = io.WriteString(w, imagePayload) + })) + t.Cleanup(srv.Close) + + f, _ := newServerFetcher(t, srv, nil) + + ctx := context.WithValue(testContext(t), middleware.RequestIDKey, requestID) + + res, err := f.Fetch(ctx, upstreamURL("/image")) + if err != nil { + t.Fatalf("Fetch() error = %v", err) + } + + _ = res.Content.Close() + + got := <-received + t.Logf("upstream received X-Request-Id %q", got) + + if got != requestID { + t.Errorf("upstream X-Request-Id = %q, want %q", got, requestID) + } +} diff --git a/internal/imgcache/service.go b/internal/imgcache/service.go index 968dd95..562595c 100644 --- a/internal/imgcache/service.go +++ b/internal/imgcache/service.go @@ -13,6 +13,7 @@ import ( "github.com/dustin/go-humanize" "github.com/getsentry/sentry-go" + "github.com/go-chi/chi/v5/middleware" "golang.org/x/sync/singleflight" "sneak.berlin/go/pixa/internal/allowlist" "sneak.berlin/go/pixa/internal/httpfetcher" @@ -461,6 +462,7 @@ func (s *Service) fetchAndProcess( // Log upstream fetch details s.log.Info("upstream fetched", + "request_id", middleware.GetReqID(ctx), "host", req.SourceHost, "path", req.SourcePath, "bytes", fetchBytes, @@ -545,6 +547,7 @@ func (s *Service) processAndStore( } s.log.Info("image converted", + "request_id", middleware.GetReqID(ctx), "host", req.SourceHost, "path", req.SourcePath, "src_format", processResult.InputFormat, diff --git a/internal/middleware/middleware.go b/internal/middleware/middleware.go index 492ba87..358cbdc 100644 --- a/internal/middleware/middleware.go +++ b/internal/middleware/middleware.go @@ -2,9 +2,12 @@ package middleware import ( + "context" + "crypto/rand" "log/slog" "net/http" "net/netip" + "regexp" "time" basicauth "github.com/99designs/basicauth-go" @@ -115,6 +118,32 @@ func (s *Middleware) RateLimit( }) } +// requestIDPattern is what a request's own X-Request-Id must look like to be +// kept as its ID: 1 to 64 letters, digits, '-', '_' or '.'. +var requestIDPattern = regexp.MustCompile(`^[A-Za-z0-9._-]{1,64}$`) + +// RequestID returns a middleware that gives each request an ID and sends it as +// the X-Request-Id response header, so a client can quote it when reporting a +// problem. The ID is the request's own X-Request-Id when that matches +// requestIDPattern, and otherwise a random one, which tells nothing about the +// machine or the traffic. It is stored in the request context under chi's +// RequestIDKey, where the logging middleware, the handlers and the upstream +// fetch read it. +func (s *Middleware) RequestID() func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + id := r.Header.Get(middleware.RequestIDHeader) + if !requestIDPattern.MatchString(id) { + id = rand.Text() + } + + w.Header().Set(middleware.RequestIDHeader, id) + ctx := context.WithValue(r.Context(), middleware.RequestIDKey, id) + next.ServeHTTP(w, r.WithContext(ctx)) + }) + } +} + type loggingResponseWriter struct { http.ResponseWriter diff --git a/internal/middleware/request_id_internal_test.go b/internal/middleware/request_id_internal_test.go new file mode 100644 index 0000000..5b381d9 --- /dev/null +++ b/internal/middleware/request_id_internal_test.go @@ -0,0 +1,118 @@ +package middleware + +import ( + "log/slog" + "net/http" + "net/http/httptest" + "os" + "strings" + "testing" + + "github.com/go-chi/chi/v5/middleware" + + "sneak.berlin/go/pixa/internal/config" +) + +// sendRequestID sends a request through the RequestID middleware, carrying +// incoming as its X-Request-Id unless that is empty. It returns the ID the +// next handler found in the request context, which the upstream fetch sends +// and the log lines carry, and the X-Request-Id of the response. +func sendRequestID(t *testing.T, incoming string) (string, string) { + t.Helper() + + mw := &Middleware{log: slog.Default(), config: &config.Config{}} + + var inContext string + + handler := mw.RequestID()(http.HandlerFunc( + func(_ http.ResponseWriter, r *http.Request) { + inContext = middleware.GetReqID(r.Context()) + })) + + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil) + if incoming != "" { + req.Header.Set("X-Request-Id", incoming) + } + + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + return inContext, rec.Header().Get("X-Request-Id") +} + +// TestRequestIDKeepsShortPlainID verifies that a request's own X-Request-Id of +// at most 64 letters, digits, '-', '_' or '.' is kept as its ID. +func TestRequestIDKeepsShortPlainID(t *testing.T) { + t.Parallel() + + for _, incoming := range []string{ + "client-request-id", + "A1_b2.c3-d4", + strings.Repeat("a", 64), + } { + inContext, inResponse := sendRequestID(t, incoming) + + if inContext != incoming || inResponse != incoming { + t.Errorf("incoming %q: context has %q, response %q, want both %q", + incoming, inContext, inResponse, incoming) + } + } +} + +// TestRequestIDReplacesLongOrUnusualID verifies that a request's own +// X-Request-Id that is over 64 characters or holds anything but letters, +// digits, '-', '_' or '.' is neither sent back nor sent upstream: the request +// gets a fresh ID instead. +func TestRequestIDReplacesLongOrUnusualID(t *testing.T) { + t.Parallel() + + for _, incoming := range []string{ + strings.Repeat("a", 65), + strings.Repeat("a", 9000), + "has space", + "a/b", + "a,b", + "