diff --git a/README.md b/README.md index e847aaf..2ab577b 100644 --- a/README.md +++ b/README.md @@ -139,12 +139,13 @@ path under `/v1/` answers 200, in maintenance mode too. 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` -when it sent one, as a reverse proxy in front of pixa may, otherwise one pixa -makes up from its host name, a random string chosen at startup and a counter. -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`. +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 diff --git a/TODO.md b/TODO.md index 2188291..2138144 100644 --- a/TODO.md +++ b/TODO.md @@ -30,9 +30,10 @@ P2: security: referer blacklist # Completed Steps - 2026-10-04 request IDs returned and passed on, and `/v1/e/` revalidates - (closes #84): a middleware right after chi's `RequestID` sets `X-Request-ID` - on every response from the ID `RequestID` stores in the request context, which - is the request's own `X-Request-ID` when it sent one; the upstream fetch sends + (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` diff --git a/internal/httpfetcher/request_id_internal_test.go b/internal/httpfetcher/request_id_internal_test.go index 374bdbb..a3c998e 100644 --- a/internal/httpfetcher/request_id_internal_test.go +++ b/internal/httpfetcher/request_id_internal_test.go @@ -11,7 +11,7 @@ import ( ) // TestFetchSendsRequestID verifies that a fetch sends the ID of the request -// it serves, which chi's RequestID middleware stores in the request context, +// 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) { diff --git a/internal/middleware/middleware.go b/internal/middleware/middleware.go index 758f774..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,16 +118,28 @@ func (s *Middleware) RateLimit( }) } -// RequestIDResponseHeader returns a middleware that sends the request's ID as +// 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 one chi's RequestID middleware stored in the request -// context, so RequestID must run first. -func (s *Middleware) RequestIDResponseHeader() func(http.Handler) http.Handler { +// 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) { - w.Header().Set(middleware.RequestIDHeader, - middleware.GetReqID(r.Context())) - next.ServeHTTP(w, r) + 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)) }) } } diff --git a/internal/server/routes.go b/internal/server/routes.go index d7d515b..7c81795 100644 --- a/internal/server/routes.go +++ b/internal/server/routes.go @@ -28,8 +28,7 @@ func (s *Server) SetupRoutes() { s.router = chi.NewRouter() s.router.Use(middleware.Recoverer) - s.router.Use(middleware.RequestID) - s.router.Use(s.mw.RequestIDResponseHeader()) + s.router.Use(s.mw.RequestID()) s.router.Use(s.mw.ClientIP()) s.router.Use(s.mw.SecurityHeaders()) s.router.Use(s.mw.Logging())