From ac95c33cdbc5d536d47062c372d16de24a46dcc6 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 29 Sep 2026 00:44:54 +0000 Subject: [PATCH 1/3] Test that max-age never outlives an expiring image URL (closes #63) Route tests for both image routes. An image served through a signed URL expiring in 60 seconds, or an encrypted URL with a 60 second TTL, must get a max-age of at most 60. A URL with no expiry keeps one year. An allowlisted URL whose exp has already passed, which is served without checking exp, must get 0. The expiring cases fail until the fix that follows. Model: opus-5-5 --- .../image_cache_control_internal_test.go | 201 ++++++++++++++++++ .../handlers/image_signature_internal_test.go | 6 +- 2 files changed, 204 insertions(+), 3 deletions(-) create mode 100644 internal/handlers/image_cache_control_internal_test.go diff --git a/internal/handlers/image_cache_control_internal_test.go b/internal/handlers/image_cache_control_internal_test.go new file mode 100644 index 0000000..a7522da --- /dev/null +++ b/internal/handlers/image_cache_control_internal_test.go @@ -0,0 +1,201 @@ +package handlers + +import ( + "image/color" + "log/slog" + "net/http" + "net/http/httptest" + "strconv" + "strings" + "testing" + "testing/fstest" + "time" + + "github.com/go-chi/chi/v5" + + "sneak.berlin/go/pixa/internal/encurl" + "sneak.berlin/go/pixa/internal/imgcache" +) + +// photoPath is the path of the JPEG that newSignedHostServer serves. +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) { + t.Helper() + + cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{ + StateDir: t.TempDir(), + CacheTTL: time.Hour, + NegativeTTL: 5 * time.Minute, + }) + if err != nil { + t.Fatalf("imgcache.NewCache() error = %v", err) + } + + jpegData := generateTestJPEG(t, 100, 100, color.RGBA{255, 0, 0, 255}) + + svc, err := imgcache.NewService(&imgcache.ServiceConfig{ + Cache: cache, + Fetcher: newMockFetcher(fstest.MapFS{ + signedHost + photoPath: &fstest.MapFile{Data: jpegData}, + }), + SigningKey: testSigningKey, + }) + if err != nil { + t.Fatalf("imgcache.NewService() error = %v", err) + } + + encGen, err := encurl.NewGenerator(testSigningKey) + if err != nil { + t.Fatalf("encurl.NewGenerator() error = %v", err) + } + + h := &Handlers{ + log: slog.New(slog.DiscardHandler), + imgSvc: svc, + encGen: encGen, + } + + r := chi.NewRouter() + r.Get("/v1/image/*", h.HandleImage()) + r.Get("/v1/e/{token}/*", h.HandleImageEnc()) + + return h, r +} + +// getMaxAge sends a GET for target to srv, requires a 200, and returns the +// max-age of the response's Cache-Control header, which must read +// "public, max-age=, immutable". +func getMaxAge(t *testing.T, srv http.Handler, target string) int { + t.Helper() + + req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, target, nil) + rec := httptest.NewRecorder() + + srv.ServeHTTP(rec, req) + + header := rec.Header().Get("Cache-Control") + t.Logf("GET %s: %d, Cache-Control: %s", target, rec.Code, header) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d", rec.Code, http.StatusOK) + } + + value, hasPrefix := strings.CutPrefix(header, "public, max-age=") + value, hasSuffix := strings.CutSuffix(value, ", immutable") + + maxAge, err := strconv.Atoi(value) + if !hasPrefix || !hasSuffix || err != nil { + t.Fatalf("Cache-Control = %q, want public, max-age=, immutable", + header) + } + + return maxAge +} + +// TestHandleImage_SignedURL_MaxAgeEndsAtExp verifies that an image served +// through a signed URL expiring in 60 seconds may be cached for at most those +// 60 seconds. The lower bound of 50 shows the max-age is the time left, not 0. +func TestHandleImage_SignedURL_MaxAgeEndsAtExp(t *testing.T) { + t.Parallel() + + h, srv := newSignedHostServer(t) + + 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) + } + + maxAge := getMaxAge(t, srv, signedURL) + if maxAge < 50 || maxAge > 60 { + t.Errorf("max-age = %d, want 50 to 60", maxAge) + } +} + +// TestHandleImage_AllowlistedHost_MaxAge verifies the max-age of an image from +// an allowlisted host, which is served without checking sig or exp. A URL with +// no exp may be cached for a year. A URL whose exp has passed is the one request +// that reaches the header after its expiry, and must get 0, never less. +func TestHandleImage_AllowlistedHost_MaxAge(t *testing.T) { + t.Parallel() + + pastExp := strconv.FormatInt(time.Now().Add(-time.Hour).Unix(), 10) + + tests := []struct { + name string + query string + wantMaxAge int + }{ + {"no exp", "", 31536000}, + {"exp already past", "?exp=" + pastExp, 0}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + fix := setupTestHandler(t) + + r := chi.NewRouter() + r.Get("/v1/image/*", fix.handler.HandleImage()) + + maxAge := getMaxAge(t, r, + "/v1/image/"+fix.goodHost+"/images/photo.jpg/50x50.jpeg"+tt.query) + if maxAge != tt.wantMaxAge { + t.Errorf("max-age = %d, want %d", maxAge, tt.wantMaxAge) + } + }) + } +} + +// TestHandleImageEnc_MaxAge verifies that an image served through an encrypted +// URL with a 60 second TTL may be cached for at most those 60 seconds, and that +// one made without a TTL, which never expires, may be cached for a year. +func TestHandleImageEnc_MaxAge(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + expiresAt int64 + wantAtLeast int + wantAtMost int + }{ + {"60 second TTL", time.Now().Add(time.Minute).Unix(), 50, 60}, + {"no TTL", 0, 31536000, 31536000}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + h, srv := newSignedHostServer(t) + + token, err := h.encGen.Generate(&encurl.Payload{ + SourceHost: signedHost, + SourcePath: photoPath, + Width: 50, + Height: 50, + Format: imgcache.FormatJPEG, + ExpiresAt: tt.expiresAt, + }) + if err != nil { + t.Fatalf("Generate() error = %v", err) + } + + maxAge := getMaxAge(t, srv, "/v1/e/"+token+"/img.jpg") + if maxAge < tt.wantAtLeast || maxAge > tt.wantAtMost { + t.Errorf("max-age = %d, want %d to %d", + maxAge, tt.wantAtLeast, tt.wantAtMost) + } + }) + } +} diff --git a/internal/handlers/image_signature_internal_test.go b/internal/handlers/image_signature_internal_test.go index c435668..c057514 100644 --- a/internal/handlers/image_signature_internal_test.go +++ b/internal/handlers/image_signature_internal_test.go @@ -14,9 +14,9 @@ import ( ) // signedHost is not on the allowlist setupTestHandler builds, so a request -// for it needs a valid signature. No image is served for it: a request that -// passes the signature check gets 502 from the failed fetch, and one that -// fails the check gets 401. +// for it needs a valid signature. setupTestHandler serves no image for it: a +// request that passes the signature check gets 502 from the failed fetch, and +// one that fails the check gets 401. const signedHost = "signed.example.com" // getImage sends a GET for target to the image route of fix and returns the -- 2.54.0 From 0a0142d1afea759ec4fc91065347d199d6481e9d Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 29 Sep 2026 00:51:50 +0000 Subject: [PATCH 2/3] Keep max-age within an expiring image URL's lifetime (closes #63) Both image routes sent Cache-Control: public, max-age=31536000, immutable, so a browser or proxy could keep serving an image for a year after its signed or encrypted URL had expired. The header is now built from the request's Expires: max-age is the whole seconds left until the URL expires, never negative, or one year for a URL with no expiry. ToImageRequest now carries an encrypted URL's expiry onto the request, as the image route already does with exp. immutable stays: it only stops revalidation while a copy is fresh, and freshness now ends at the expiry. README.md documents the header. Model: opus-5-5 --- README.md | 6 ++++++ TODO.md | 7 +++++++ internal/encurl/encurl.go | 9 ++++++++- internal/handlers/image.go | 20 +++++++++++++++++++- internal/handlers/imageenc.go | 4 ++-- internal/imgcache/imgcache.go | 3 ++- 6 files changed, 44 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 4765703..50f39ef 100644 --- a/README.md +++ b/README.md @@ -100,6 +100,12 @@ than once, is refused with 400. - ``: one of `orig`, `png`, `jpeg`, `webp` - ``: `orig` or `x` (e.g. `800x600`) +An image is served with `Cache-Control: public, max-age=, immutable`. +When the URL has an expiry (an `exp`, or the TTL of an encrypted URL), +`max-age` is the whole seconds left until then, so no browser or proxy cache +keeps the image after pixa would refuse the URL. A URL with no expiry gets one +year. `immutable` only stops a client revalidating while its copy is fresh. + 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 refused with 429 and a `Retry-After` header. Behind a reverse proxy the client diff --git a/TODO.md b/TODO.md index 56d0745..c10609b 100644 --- a/TODO.md +++ b/TODO.md @@ -30,6 +30,13 @@ exhaustion # Completed Steps +- 2026-09-29 `max-age` never outlives an expiring URL (closes #63): both image + routes build `Cache-Control` from the request's `Expires`, which an encrypted + URL's expiry now fills too; `max-age` is one year, or the whole seconds left + until the `exp` of a `/v1/image/` URL or the expiry of an encrypted URL when + that is sooner, never negative; an allowlisted host's URL that has an `exp` + follows it too; `immutable` stays, as freshness now ends at the expiry; + documented in `README.md`. - 2026-09-28 strip metadata from processed images (closes #82): every output is exported with govips' `StripMetadata`, so it carries no EXIF, XMP, IPTC or ICC profile; the image is first turned upright with `AutoRotate` (before sizes are diff --git a/internal/encurl/encurl.go b/internal/encurl/encurl.go index 5c8819c..8815bf0 100644 --- a/internal/encurl/encurl.go +++ b/internal/encurl/encurl.go @@ -103,7 +103,8 @@ func (g *Generator) Parse(token string) (*Payload, error) { } // ToImageRequest converts the payload to an ImageRequest. -// Applies default values for omitted optional fields. +// Applies default values for omitted optional fields. An ExpiresAt of 0, a URL +// that never expires, gives the zero Expires. func (p *Payload) ToImageRequest() *imgcache.ImageRequest { format := p.Format if format == "" { @@ -120,6 +121,11 @@ func (p *Payload) ToImageRequest() *imgcache.ImageRequest { fitMode = DefaultFitMode } + var expires time.Time + if p.ExpiresAt != 0 { + expires = time.Unix(p.ExpiresAt, 0) + } + return &imgcache.ImageRequest{ SourceHost: p.SourceHost, SourcePath: p.SourcePath, @@ -131,6 +137,7 @@ func (p *Payload) ToImageRequest() *imgcache.ImageRequest { Format: format, Quality: quality, FitMode: fitMode, + Expires: expires, } } diff --git a/internal/handlers/image.go b/internal/handlers/image.go index 00b271e..561ca1d 100644 --- a/internal/handlers/image.go +++ b/internal/handlers/image.go @@ -220,6 +220,24 @@ func (s *Handlers) respondImageError( s.respondError(w, "internal error", http.StatusInternalServerError) } +// cacheControl returns the Cache-Control header for an image served through a +// URL that expires at expires, or never when expires is the zero time. A cache +// may keep the image for a year, but not past the URL's expiry, after which +// pixa refuses the URL. The seconds left are rounded down and never negative. +// immutable only stops revalidation while the image is fresh, so it also ends +// at the expiry. +func cacheControl(expires time.Time) string { + const oneYear = 365 * 24 * time.Hour + + maxAge := oneYear + + if !expires.IsZero() { + maxAge = min(maxAge, max(time.Until(expires), 0)) + } + + return fmt.Sprintf("public, max-age=%d, immutable", int64(maxAge/time.Second)) +} + // writeImageResponse writes headers and streams the image content, // handling conditional and HEAD requests. func (s *Handlers) writeImageResponse( @@ -235,7 +253,7 @@ func (s *Handlers) writeImageResponse( } // Cache control headers - w.Header().Set("Cache-Control", "public, max-age=31536000, immutable") + w.Header().Set("Cache-Control", cacheControl(req.Expires)) w.Header().Set("X-Pixa-Cache", string(resp.CacheStatus)) if resp.ETag != "" { diff --git a/internal/handlers/imageenc.go b/internal/handlers/imageenc.go index 8089f64..5effd8b 100644 --- a/internal/handlers/imageenc.go +++ b/internal/handlers/imageenc.go @@ -89,8 +89,8 @@ func (s *Handlers) HandleImageEnc() http.HandlerFunc { w.Header().Set("Content-Length", strconv.FormatInt(resp.ContentLength, 10)) } - // Cache headers - encrypted URLs can be cached since they're immutable - w.Header().Set("Cache-Control", "public, max-age=31536000, immutable") + // Cache headers: max-age ends at the URL's expiry + w.Header().Set("Cache-Control", cacheControl(req.Expires)) w.Header().Set("X-Pixa-Cache", string(resp.CacheStatus)) // Stream the response diff --git a/internal/imgcache/imgcache.go b/internal/imgcache/imgcache.go index 7fb069b..a6a591e 100644 --- a/internal/imgcache/imgcache.go +++ b/internal/imgcache/imgcache.go @@ -95,7 +95,8 @@ type ImageRequest struct { FitMode FitMode // Signature is the HMAC signature for non-allowlisted hosts Signature string - // Expires is the signature expiration timestamp + // Expires is when the URL expires: the exp of a signed URL, or the expiry + // of an encrypted URL; the zero time if it has none Expires time.Time // AllowHTTP indicates whether HTTP (non-TLS) is allowed for this request AllowHTTP bool -- 2.54.0 From 9742842975168978fb9baccae05a6a6cca86c9f5 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 29 Sep 2026 01:26:21 +0000 Subject: [PATCH 3/3] Document and test the one-year max-age cap (closes #63) The README said max-age is the seconds left until the URL's expiry, but a URL expiring more than a year away gets one year; it now says at most one year. A new case for an encrypted URL with a two-year TTL expects max-age=31536000. Model: opus-5-5 --- README.md | 7 ++++--- internal/handlers/image_cache_control_internal_test.go | 6 ++++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 50f39ef..23fe41b 100644 --- a/README.md +++ b/README.md @@ -102,9 +102,10 @@ than once, is refused with 400. An image is served with `Cache-Control: public, max-age=, immutable`. When the URL has an expiry (an `exp`, or the TTL of an encrypted URL), -`max-age` is the whole seconds left until then, so no browser or proxy cache -keeps the image after pixa would refuse the URL. A URL with no expiry gets one -year. `immutable` only stops a client revalidating while its copy is fresh. +`max-age` is the whole seconds left until then, at most one year, so no browser +or proxy cache keeps the image after pixa would refuse the URL. A URL with no +expiry gets one year. `immutable` only stops a client revalidating while its +copy is fresh. 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/internal/handlers/image_cache_control_internal_test.go b/internal/handlers/image_cache_control_internal_test.go index a7522da..bb6a6b9 100644 --- a/internal/handlers/image_cache_control_internal_test.go +++ b/internal/handlers/image_cache_control_internal_test.go @@ -158,8 +158,9 @@ func TestHandleImage_AllowlistedHost_MaxAge(t *testing.T) { } // TestHandleImageEnc_MaxAge verifies that an image served through an encrypted -// URL with a 60 second TTL may be cached for at most those 60 seconds, and that -// one made without a TTL, which never expires, may be cached for a year. +// URL with a 60 second TTL may be cached for at most those 60 seconds, that one +// with a two-year TTL may be cached for a year, and that one made without a +// TTL, which never expires, may be cached for a year. func TestHandleImageEnc_MaxAge(t *testing.T) { t.Parallel() @@ -170,6 +171,7 @@ func TestHandleImageEnc_MaxAge(t *testing.T) { wantAtMost int }{ {"60 second TTL", time.Now().Add(time.Minute).Unix(), 50, 60}, + {"two-year TTL", time.Now().Add(2 * 365 * 24 * time.Hour).Unix(), 31536000, 31536000}, {"no TTL", 0, 31536000, 31536000}, } -- 2.54.0