TODO.md

@@ -30,6 +30,27 @@ P2: security: per-origin rate limiting
 
 # Completed Steps
 
+- 2026-10-26 the image request holds only what names the cached object (closes
+  #314): its source and its transform, which the variant key covers. The signed
+  URL format, the signing algorithm and every route's answers are unchanged.
+    - The `/v1/image/` handler checks `allowlist_hosts` and verifies the
+      signature itself, with `Signer.Verify(req, sig, expires)`.
+    - Each image handler keeps its URL's expiry as a value of its own, for
+      `Cache-Control`. `encurl`'s `Generate` takes the expiry and its `Parse`
+      returns it.
+    - The fetcher takes the source and builds its URL with `Source.URL`, over
+      http only when the fetcher's own `AllowHTTP` is set. `validateURL` takes
+      that URL, so no URL string is built and parsed again.
+    - `chooseAutoFormat` returns the format it chooses, and the handler puts it
+      into the request it parsed.
+    - The image service serves images through `Get`, and has `WaitForProcessing`
+      for shutdown.
+    - Removed:
+        - `Signature`, `Expires`, `AllowHTTP` and `SourceURL` of the image
+          request
+        - `Service.ValidateRequest`, and the service's signer, allowlist and
+          `AllowHTTP`
+        - `extractHost` of the fetcher and `urlToFSPath` of the mock fetcher
 - 2026-10-26 pixad's fx graph builds the image pipeline (closes #313).
   `database.New` now opens the database, so the parts below can read it while
   they are built; its stop hook still runs after the cache's. The cache's

internal/encurl/encurl.go

@@ -59,12 +59,14 @@ func NewGenerator(signingKey string) (*Generator, error) {
 	return &Generator{key: key}, nil
 }
 
-// Generate creates an encrypted URL token from the request's source,
-// transform and expiry, each as the request holds it; an empty format, a
-// quality of 0 or an empty fit mode is left out of the token, and a zero
-// Expires makes a token that never expires. The token is CBOR-encoded,
+// Generate creates an encrypted URL token from the request's source and
+// transform, each as the request holds it, and from expires; an empty format,
+// a quality of 0 or an empty fit mode is left out of the token, and a zero
+// expires makes a token that never expires. The token is CBOR-encoded,
 // encrypted with NaCl secretbox, and base64url-encoded.
-func (g *Generator) Generate(req *imagerequest.Request) (string, error) {
+func (g *Generator) Generate(
+	req *imagerequest.Request, expires time.Time,
+) (string, error) {
 	p := &payload{
 		SourceHost:  req.Source.Host,
 		SourcePath:  req.Source.Path,
@@ -76,8 +78,8 @@ func (g *Generator) Generate(req *imagerequest.Request) (string, error) {
 		FitMode:     req.Transform.FitMode,
 	}
 
-	if !req.Expires.IsZero() {
-		p.ExpiresAt = req.Expires.Unix()
+	if !expires.IsZero() {
+		p.ExpiresAt = expires.Unix()
 	}
 
 	// CBOR encode the payload
@@ -91,19 +93,19 @@ func (g *Generator) Generate(req *imagerequest.Request) (string, error) {
 }
 
 // Parse decrypts and validates an encrypted URL token and returns the
-// request it holds, with default values for omitted optional fields. A token
-// that never expires gives the zero Expires.
+// request it holds, with default values for omitted optional fields, and its
+// expiry. A token that never expires gives the zero time.
 // Returns ErrExpired if the token has expired, or other errors for invalid tokens.
-func (g *Generator) Parse(token string) (*imagerequest.Request, error) {
+func (g *Generator) Parse(token string) (*imagerequest.Request, time.Time, error) {
 	// Decrypt
 	data, err := seal.Decrypt(g.key, token)
 	if err != nil {
 		if errors.Is(err, seal.ErrDecryptionFailed) ||
 			errors.Is(err, seal.ErrInvalidPayload) {
-			return nil, ErrDecryptFailed
+			return nil, time.Time{}, ErrDecryptFailed
 		}
 
-		return nil, err
+		return nil, time.Time{}, err
 	}
 
 	// CBOR decode
@@ -111,12 +113,12 @@ func (g *Generator) Parse(token string) (*imagerequest.Request, error) {
 
 	err = cbor.Unmarshal(data, &p)
 	if err != nil {
-		return nil, ErrInvalidFormat
+		return nil, time.Time{}, ErrInvalidFormat
 	}
 
 	// Check expiration (0 = never expires)
 	if p.ExpiresAt != 0 && time.Now().Unix() > p.ExpiresAt {
-		return nil, ErrExpired
+		return nil, time.Time{}, ErrExpired
 	}
 
 	format := p.Format
@@ -151,6 +153,5 @@ func (g *Generator) Parse(token string) (*imagerequest.Request, error) {
 			Quality: quality,
 			FitMode: fitMode,
 		},
-		Expires: expires,
-	}, nil
+	}, expires, nil
 }

internal/encurl/encurl_test.go

@@ -36,10 +36,10 @@ func TestGenerator_GenerateAndParse(t *testing.T) {
 			Quality: 90,
 			FitMode: imagerequest.FitContain,
 		},
-		Expires: time.Now().Add(time.Hour),
 	}
+	expires := time.Now().Add(time.Hour)
 
-	token, err := gen.Generate(req)
+	token, err := gen.Generate(req, expires)
 	if err != nil {
 		t.Fatalf("Generate() error = %v", err)
 	}
@@ -48,7 +48,7 @@ func TestGenerator_GenerateAndParse(t *testing.T) {
 		t.Fatal("Generate() returned empty token")
 	}
 
-	parsed, err := gen.Parse(token)
+	parsed, parsedExpires, err := gen.Parse(token)
 	if err != nil {
 		t.Fatalf("Parse() error = %v", err)
 	}
@@ -91,8 +91,8 @@ func TestGenerator_GenerateAndParse(t *testing.T) {
 			parsed.Transform.FitMode, req.Transform.FitMode)
 	}
 
-	if parsed.Expires.Unix() != req.Expires.Unix() {
-		t.Errorf("ExpiresAt = %d, want %d", parsed.Expires.Unix(), req.Expires.Unix())
+	if parsedExpires.Unix() != expires.Unix() {
+		t.Errorf("ExpiresAt = %d, want %d", parsedExpires.Unix(), expires.Unix())
 	}
 }
 
@@ -106,15 +106,14 @@ func TestGenerator_Parse_Expired(t *testing.T) {
 			Host: testSourceHost,
 			Path: testSourcePath,
 		},
-		Expires: time.Now().Add(-time.Hour), // Already expired
 	}
 
-	token, err := gen.Generate(req)
+	token, err := gen.Generate(req, time.Now().Add(-time.Hour)) // Already expired
 	if err != nil {
 		t.Fatalf("Generate() error = %v", err)
 	}
 
-	_, err = gen.Parse(token)
+	_, _, err = gen.Parse(token)
 	if err == nil {
 		t.Error("Parse() should fail for expired token")
 	}
@@ -129,7 +128,7 @@ func TestGenerator_Parse_InvalidToken(t *testing.T) {
 
 	gen, _ := encurl.NewGenerator("test-signing-key-12345")
 
-	_, err := gen.Parse("not-a-valid-token")
+	_, _, err := gen.Parse("not-a-valid-token")
 	if err == nil {
 		t.Error("Parse() should fail for invalid token")
 	}
@@ -145,10 +144,9 @@ func TestGenerator_Parse_TamperedToken(t *testing.T) {
 			Host: testSourceHost,
 			Path: testSourcePath,
 		},
-		Expires: time.Now().Add(time.Hour),
 	}
 
-	token, _ := gen.Generate(req)
+	token, _ := gen.Generate(req, time.Now().Add(time.Hour))
 
 	// Tamper with the token
 	tampered := []byte(token)
@@ -156,7 +154,7 @@ func TestGenerator_Parse_TamperedToken(t *testing.T) {
 		tampered[10] ^= 0x01
 	}
 
-	_, err := gen.Parse(string(tampered))
+	_, _, err := gen.Parse(string(tampered))
 	if err == nil {
 		t.Error("Parse() should fail for tampered token")
 	}
@@ -173,12 +171,11 @@ func TestGenerator_Parse_WrongKey(t *testing.T) {
 			Host: testSourceHost,
 			Path: testSourcePath,
 		},
-		Expires: time.Now().Add(time.Hour),
 	}
 
-	token, _ := gen1.Generate(req)
+	token, _ := gen1.Generate(req, time.Now().Add(time.Hour))
 
-	_, err := gen2.Parse(token)
+	_, _, err := gen2.Parse(token)
 	if err == nil {
 		t.Error("Parse() should fail with different signing key")
 	}
@@ -195,13 +192,12 @@ func TestGenerator_Parse_Defaults(t *testing.T) {
 			Host: testSourceHost,
 			Path: testSourcePath,
 		},
-		Expires: time.Now().Add(time.Hour),
-	})
+	}, time.Now().Add(time.Hour))
 	if err != nil {
 		t.Fatalf("Generate() error = %v", err)
 	}
 
-	req, err := gen.Parse(token)
+	req, _, err := gen.Parse(token)
 	if err != nil {
 		t.Fatalf("Parse() error = %v", err)
 	}
@@ -232,10 +228,9 @@ func TestGenerator_TokenIsURLSafe(t *testing.T) {
 			Host: testSourceHost,
 			Path: testSourcePath,
 		},
-		Expires: time.Now().Add(time.Hour),
 	}
 
-	token, _ := gen.Generate(req)
+	token, _ := gen.Generate(req, time.Now().Add(time.Hour))
 
 	// Check that token only contains URL-safe characters
 	for _, c := range token {

internal/handlers/auth.go

@@ -153,20 +153,20 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc {
 			return
 		}
 
-		// The image URL must fetch the source URL as it was written
-		req.AllowHTTP = s.config.AllowHTTP
-		fetchURL := req.SourceURL()
+		// The image URL must fetch the source URL as it was written. The
+		// fetcher fetches over http only when allow_http is set.
+		fetchURL := req.Source.URL(s.config.AllowHTTP)
 
 		if !fetchedAsWritten(parsed, fetchURL) {
 			s.renderGeneratorWithForm(w, r, "Source URL cannot be fetched as "+
-				"written: pixa would fetch "+fetchURL,
+				"written: pixa would fetch "+fetchURL.String(),
 				r.Form, http.StatusBadRequest)
 
 			return
 		}
 
 		// Generate encrypted token
-		token, err := s.encGen.Generate(req)
+		token, err := s.encGen.Generate(req, expiresAt)
 		if err != nil {
 			s.log.Error("failed to generate encrypted URL", "error", err)
 			s.renderGeneratorWithForm(w, r, "Failed to generate URL", r.Form,
@@ -226,25 +226,23 @@ func parseSourceURL(sourceURL string) (*url.URL, error) {
 // in RawPath when it is not the decoded path percent-encoded again.
 // EscapedPath ignores RawPath when it also holds a character net/url encodes,
 // such as a space, so RawPath is compared itself.
-func fetchedAsWritten(source *url.URL, fetchURL string) bool {
+func fetchedAsWritten(source, fetchURL *url.URL) bool {
 	writtenPath := source.RawPath
 	if writtenPath == "" {
 		writtenPath = source.EscapedPath()
 	}
 
-	fetched, err := url.Parse(fetchURL)
-
-	return err == nil && source.User == nil && !source.ForceQuery &&
-		fetched.EscapedPath() == writtenPath &&
-		fetched.RawQuery == source.RawQuery
+	return source.User == nil && !source.ForceQuery &&
+		fetchURL.EscapedPath() == writtenPath &&
+		fetchURL.RawQuery == source.RawQuery
 }
 
 // buildGenerateRequest parses the numeric form fields and assembles the
-// image request the encrypted URL's token is made from. ttl=0 means never
-// expires (Expires stays the zero time). A non-numeric or out-of-range
-// width, height, quality or ttl, or an unrecognized fit mode, is a client
-// error naming the offending field. The format field is passed through
-// unchecked.
+// image request the encrypted URL's token is made from, and its expiry.
+// ttl=0 means never expires (the expiry is the zero time). A non-numeric or
+// out-of-range width, height, quality or ttl, or an unrecognized fit mode, is
+// a client error naming the offending field. The format field is passed
+// through unchecked.
 func buildGenerateRequest(
 	parsed *url.URL, form url.Values,
 ) (*imagerequest.Request, time.Time, int, error) {
@@ -294,7 +292,6 @@ func buildGenerateRequest(
 			Quality: quality,
 			FitMode: fitMode,
 		},
-		Expires: expiresAt,
 	}
 
 	return req, expiresAt, ttl, nil

internal/handlers/cache_hit_internal_test.go

@@ -19,11 +19,13 @@ import (
 	"github.com/go-chi/chi/v5"
 	"github.com/prometheus/client_golang/prometheus"
 
+	"sneak.berlin/go/pixa/internal/allowlist"
 	"sneak.berlin/go/pixa/internal/database"
 	"sneak.berlin/go/pixa/internal/encurl"
 	"sneak.berlin/go/pixa/internal/imageprocessor"
 	"sneak.berlin/go/pixa/internal/imagerequest"
 	"sneak.berlin/go/pixa/internal/imgcache"
+	"sneak.berlin/go/pixa/internal/signature"
 )
 
 // countingHost is the allowlisted host whose photoPath the server of
@@ -137,8 +139,6 @@ func newCountingDatabaseServer(
 			MaxInputBytes:           50 << 20,
 			MaxConcurrentProcessing: 2,
 		}),
-		SigningKey:      testSigningKey,
-		Allowlist:       []string{countingHost},
 		MaxResponseSize: 50 << 20,
 		Logger:          slog.New(slog.DiscardHandler),
 	})
@@ -159,15 +159,17 @@ func newCountingDatabaseServer(
 			Quality: 85,
 			FitMode: imagerequest.FitCover,
 		},
-	})
+	}, time.Time{})
 	if err != nil {
 		tb.Fatalf("encGen.Generate() error = %v", err)
 	}
 
 	h := &Handlers{
-		log:    slog.New(slog.DiscardHandler),
-		imgSvc: svc,
-		encGen: encGen,
+		log:       slog.New(slog.DiscardHandler),
+		imgSvc:    svc,
+		encGen:    encGen,
+		signer:    signature.New(testSigningKey),
+		allowlist: allowlist.New([]string{countingHost}),
 	}
 
 	r := chi.NewRouter()

internal/handlers/first_bytes_internal_test.go

@@ -13,11 +13,13 @@ import (
 
 	"github.com/go-chi/chi/v5"
 	"github.com/prometheus/client_golang/prometheus"
+	"sneak.berlin/go/pixa/internal/allowlist"
 	"sneak.berlin/go/pixa/internal/encurl"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
 	"sneak.berlin/go/pixa/internal/imageprocessor"
 	"sneak.berlin/go/pixa/internal/imagerequest"
 	"sneak.berlin/go/pixa/internal/imgcache"
+	"sneak.berlin/go/pixa/internal/signature"
 )
 
 // mismatchedSourcePath is the path of the source that
@@ -27,8 +29,9 @@ import (
 const mismatchedSourcePath = "/images/source.png"
 
 // newRoutesFetchingThrough returns both image routes, and the Handlers behind
-// them, whose image service fetches through fetcher and has allowlistedHost on
-// its allowlist. The handlers and the image service log to log.
+// them, whose image service fetches through fetcher, and which have
+// allowlistedHost on their allowlist. The handlers and the image service log
+// to log.
 func newRoutesFetchingThrough(
 	t *testing.T, fetcher httpfetcher.Fetcher, log *slog.Logger,
 ) (*Handlers, http.Handler) {
@@ -50,8 +53,6 @@ func newRoutesFetchingThrough(
 			MaxInputBytes:           50 << 20,
 			MaxConcurrentProcessing: 2,
 		}),
-		SigningKey:      testSigningKey,
-		Allowlist:       []string{allowlistedHost},
 		MaxResponseSize: 50 << 20,
 		Logger:          log,
 	})
@@ -64,7 +65,13 @@ func newRoutesFetchingThrough(
 		t.Fatalf("encurl.NewGenerator() error = %v", err)
 	}
 
-	h := &Handlers{log: log, imgSvc: svc, encGen: encGen}
+	h := &Handlers{
+		log:       log,
+		imgSvc:    svc,
+		encGen:    encGen,
+		signer:    signature.New(testSigningKey),
+		allowlist: allowlist.New([]string{allowlistedHost}),
+	}
 
 	r := chi.NewRouter()
 	r.Get("/v1/image/*", h.HandleImage())
@@ -93,7 +100,7 @@ func mismatchedSourceEncURL(t *testing.T, h *Handlers) string {
 			Size:   imagerequest.Size{Width: 50, Height: 50},
 			Format: imagerequest.FormatJPEG,
 		},
-	})
+	}, time.Time{})
 	if err != nil {
 		t.Fatalf("Generate() error = %v", err)
 	}

internal/handlers/format_auto.go

@@ -18,16 +18,17 @@ var (
 		"image/jxl, image/avif, image/webp or image/jpeg")
 )
 
-// chooseAutoFormat replaces the format auto in req with the format
-// formatForAccept chooses from r's Accept header, and adds Vary: Accept to the
+// chooseAutoFormat returns the format to serve for the requested format: for
+// the format auto, the format formatForAccept chooses from r's Accept header,
+// and any other format as it is. For auto it adds Vary: Accept to the
 // response, which then depends on that header. It answers 400 for an Accept
 // header that is not valid and 406 for one that allows none of the formats,
-// and reports whether req can be served. Any other format is left as it is.
+// and reports whether the image can be served.
 func (s *Handlers) chooseAutoFormat(
-	w http.ResponseWriter, r *http.Request, req *imagerequest.Request,
-) bool {
-	if req.Transform.Format != imagerequest.FormatAuto {
-		return true
+	w http.ResponseWriter, r *http.Request, requested imagerequest.Format,
+) (imagerequest.Format, bool) {
+	if requested != imagerequest.FormatAuto {
+		return requested, true
 	}
 
 	w.Header().Add("Vary", "Accept")
@@ -36,18 +37,16 @@ func (s *Handlers) chooseAutoFormat(
 	if errors.Is(err, errNotAcceptable) {
 		s.respondError(w, err.Error(), http.StatusNotAcceptable)
 
-		return false
+		return "", false
 	}
 
 	if err != nil {
 		s.respondError(w, err.Error(), http.StatusBadRequest)
 
-		return false
+		return "", false
 	}
 
-	req.Transform.Format = format
-
-	return true
+	return format, true
 }
 
 // formatForAccept returns the format an auto URL is served in for the Accept

internal/handlers/format_auto_internal_test.go

@@ -75,9 +75,9 @@ func TestFormatForAccept(t *testing.T) {
 
 // autoPhotoURLs returns a signed /v1/image/ URL and an encrypted /v1/e/ URL,
 // both valid for a minute, for the JPEG at photoPath on signedHost at 50x50 in
-// the format auto. The first is signed with testSigningKey, the key of the
-// image service newSignedHostServer builds, and the second is made with h's
-// generator.
+// the format auto. The first is signed with testSigningKey, the key the
+// handlers that newSignedHostServer builds check signatures with. The second
+// is made with h's generator.
 func autoPhotoURLs(t *testing.T, h *Handlers) (string, string) {
 	t.Helper()
 
@@ -101,8 +101,7 @@ func autoPhotoURLs(t *testing.T, h *Handlers) (string, string) {
 			Size:   imagerequest.Size{Width: 50, Height: 50},
 			Format: imagerequest.FormatAuto,
 		},
-		Expires: time.Now().Add(time.Minute),
-	})
+	}, time.Now().Add(time.Minute))
 	if err != nil {
 		t.Fatalf("Generate() error = %v", err)
 	}

internal/handlers/handlers.go

@@ -16,6 +16,7 @@ import (
 	"sneak.berlin/go/pixa/internal/imgcache"
 	"sneak.berlin/go/pixa/internal/logger"
 	"sneak.berlin/go/pixa/internal/session"
+	"sneak.berlin/go/pixa/internal/signature"
 )
 
 // Params defines dependencies for Handlers.
@@ -41,6 +42,11 @@ type Handlers struct {
 	encGen      *encurl.Generator
 	csrfProtect func(http.Handler) http.Handler
 
+	// signer checks the signature of a /v1/image/ URL with signing_key.
+	signer *signature.Signer
+	// allowlist matches the hosts of allowlist_hosts, which a /v1/image/
+	// URL may name without a signature.
+	allowlist *allowlist.HostAllowList
 	// refererBlocklist matches the hosts of referer_blocklist; its IsAllowed
 	// reports whether a URL's host is on that list.
 	refererBlocklist *allowlist.HostAllowList
@@ -61,6 +67,8 @@ func New(_ fx.Lifecycle, params Params) (*Handlers, error) {
 		sessMgr:          params.SessionManager,
 		encGen:           params.EncryptedURLGenerator,
 		csrfProtect:      csrfProtect,
+		signer:           signature.New(params.Config.SigningKey),
+		allowlist:        allowlist.New(params.Config.AllowlistHosts),
 		refererBlocklist: allowlist.New(params.Config.RefererBlocklist),
 	}, nil
 }

internal/handlers/handlers_internal_test.go

@@ -19,10 +19,13 @@ import (
 
 	"github.com/go-chi/chi/v5"
 	"github.com/prometheus/client_golang/prometheus"
+	"sneak.berlin/go/pixa/internal/allowlist"
 	"sneak.berlin/go/pixa/internal/database"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
 	"sneak.berlin/go/pixa/internal/imageprocessor"
+	"sneak.berlin/go/pixa/internal/imagerequest"
 	"sneak.berlin/go/pixa/internal/imgcache"
+	"sneak.berlin/go/pixa/internal/signature"
 )
 
 // testFixtures contains test data.
@@ -62,8 +65,6 @@ func setupTestHandler(t *testing.T) *testFixtures {
 			MaxInputBytes:           50 << 20,
 			MaxConcurrentProcessing: 2,
 		}),
-		SigningKey:      "test-signing-key-must-be-32-chars",
-		Allowlist:       []string{goodHost},
 		MaxResponseSize: 50 << 20,
 	})
 	if err != nil {
@@ -71,8 +72,10 @@ func setupTestHandler(t *testing.T) *testFixtures {
 	}
 
 	h := &Handlers{
-		imgSvc: svc,
-		log:    slog.Default(),
+		imgSvc:    svc,
+		log:       slog.Default(),
+		signer:    signature.New("test-signing-key-must-be-32-chars"),
+		allowlist: allowlist.New([]string{goodHost}),
 	}
 
 	return &testFixtures{
@@ -127,10 +130,9 @@ func newMockFetcher(fs fs.FS) *mockFetcher {
 }
 
 func (f *mockFetcher) Fetch(
-	_ context.Context, url string,
+	_ context.Context, source imagerequest.Source,
 ) (*httpfetcher.FetchResult, error) {
-	// Remove https:// prefix
-	path := url[8:] // Remove "https://"
+	path := source.Host + source.Path
 
 	data, err := fs.ReadFile(f.fs, path)
 	if err != nil {

internal/handlers/image.go

@@ -7,6 +7,7 @@ import (
 	"net/http"
 	"net/url"
 	"strconv"
+	"strings"
 	"time"
 
 	"github.com/go-chi/chi/v5/middleware"
@@ -27,13 +28,13 @@ func (s *Handlers) HandleImage() http.HandlerFunc {
 			return
 		}
 
-		req, ok := s.parseImageRequest(w, r)
+		req, sig, expires, ok := s.parseImageRequest(w, r)
 		if !ok {
 			return
 		}
 
 		// Validate signature if required
-		err := s.imgSvc.ValidateRequest(req)
+		err := s.validateSignature(req, sig, expires)
 		if err != nil {
 			s.log.Warn("signature validation failed",
 				"host", req.Source.Host,
@@ -46,10 +47,13 @@ func (s *Handlers) HandleImage() http.HandlerFunc {
 		}
 
 		// The signature covers the format auto, not the format chosen
-		if !s.chooseAutoFormat(w, r, req) {
+		format, ok := s.chooseAutoFormat(w, r, req.Transform.Format)
+		if !ok {
 			return
 		}
 
+		req.Transform.Format = format
+
 		// Get cache key for logging
 		cacheKey := imagerequest.CacheKey(req)
 
@@ -65,7 +69,7 @@ func (s *Handlers) HandleImage() http.HandlerFunc {
 
 		defer func() { _ = resp.Content.Close() }()
 
-		s.writeImageResponse(w, r, req, resp, cacheKey, startTime)
+		s.writeImageResponse(w, r, req, expires, resp, cacheKey, startTime)
 	}
 }
 
@@ -82,11 +86,11 @@ func (s *Handlers) HandleRobotsTxt() http.HandlerFunc {
 }
 
 // parseImageRequest parses the image URL's path and query parameters into
-// an image request. On invalid input it writes an error response and
-// returns false.
+// an image request, and returns it with the URL's signature and expiry. On
+// invalid input it writes an error response and returns false.
 func (s *Handlers) parseImageRequest(
 	w http.ResponseWriter, r *http.Request,
-) (*imagerequest.Request, bool) {
+) (*imagerequest.Request, string, time.Time, bool) {
 	// The path is parsed decoded once, as r.URL.Path holds it, and not from
 	// chi's wildcard: chi routes on the path as the client wrote it whenever
 	// that is not Go's own encoding of it, so its wildcard would still hold
@@ -100,7 +104,7 @@ func (s *Handlers) parseImageRequest(
 		)
 		s.respondError(w, "invalid image URL: "+err.Error(), http.StatusBadRequest)
 
-		return nil, false
+		return nil, "", time.Time{}, false
 	}
 
 	// Parse signature params from query string. r.URL.Query() would silently
@@ -113,7 +117,7 @@ func (s *Handlers) parseImageRequest(
 		s.respondError(w, fmt.Sprintf("invalid query string %q: %v",
 			r.URL.RawQuery, err), http.StatusBadRequest)
 
-		return nil, false
+		return nil, "", time.Time{}, false
 	}
 
 	for name, values := range query {
@@ -121,17 +125,15 @@ func (s *Handlers) parseImageRequest(
 			s.respondError(w, fmt.Sprintf("invalid %s: given more than once",
 				name), http.StatusBadRequest)
 
-			return nil, false
+			return nil, "", time.Time{}, false
 		}
 	}
 
-	req.Signature = query.Get("sig")
-
-	req.Expires, err = parseExpires(query)
+	expires, err := parseExpires(query)
 	if err != nil {
 		s.respondError(w, err.Error(), http.StatusBadRequest)
 
-		return nil, false
+		return nil, "", time.Time{}, false
 	}
 
 	// Parse optional quality and fit params. Only a q missing from the URL is
@@ -143,7 +145,7 @@ func (s *Handlers) parseImageRequest(
 		s.respondError(w, `invalid q: not a number, got ""`,
 			http.StatusBadRequest)
 
-		return nil, false
+		return nil, "", time.Time{}, false
 	}
 
 	req.Transform.Quality, err = parseFormInt(query, "q",
@@ -152,7 +154,7 @@ func (s *Handlers) parseImageRequest(
 		s.respondError(w, fmt.Sprintf("%v, got %q", err, qStr),
 			http.StatusBadRequest)
 
-		return nil, false
+		return nil, "", time.Time{}, false
 	}
 
 	// Only a fit missing from the URL is cover. A fit in the URL that is not a
@@ -162,7 +164,7 @@ func (s *Handlers) parseImageRequest(
 	if query.Has("fit") && req.Transform.FitMode == "" {
 		s.respondError(w, `invalid fit: not a fit mode, got ""`, http.StatusBadRequest)
 
-		return nil, false
+		return nil, "", time.Time{}, false
 	}
 
 	// Default fit mode if not set
@@ -179,10 +181,35 @@ func (s *Handlers) parseImageRequest(
 		s.respondError(w, "invalid image request: "+err.Error(),
 			http.StatusBadRequest)
 
-		return nil, false
+		return nil, "", time.Time{}, false
 	}
 
-	return req, true
+	return req, query.Get("sig"), expires, true
+}
+
+// validateSignature returns nil when the source host of req is on
+// allowlist_hosts and names no port. Otherwise it verifies sig, the URL's
+// signature of req, and expires, the URL's expiry.
+func (s *Handlers) validateSignature(
+	req *imagerequest.Request, sig string, expires time.Time,
+) error {
+	// An allowlist entry names no port, so a host followed by ":", with or
+	// without a port, needs a signature. Go's URL type reads the port as the
+	// fetcher does: in an IPv6 address written without brackets, such as
+	// 2001:db8::1:8443, it is what follows the last ":". This is checked here
+	// and not in allowlist.IsAllowed, which also matches referer_blocklist,
+	// where a Referer must be blocked whatever its port. Port and IsAllowed
+	// read only the host of a URL.
+	sourceURL := &url.URL{Host: req.Source.Host}
+	hostHasPort := sourceURL.Port() != "" ||
+		strings.HasSuffix(sourceURL.Host, ":")
+
+	if !hostHasPort && s.allowlist.IsAllowed(sourceURL) {
+		return nil
+	}
+
+	// Signature required for non-allowed hosts
+	return s.signer.Verify(req, sig, expires)
 }
 
 // parseExpires reads the exp query parameter, a Unix time in seconds. An exp
@@ -341,10 +368,11 @@ func notModified(w http.ResponseWriter, r *http.Request, etag string) bool {
 }
 
 // writeImageResponse writes headers and streams the image content,
-// handling conditional and HEAD requests.
+// handling conditional and HEAD requests. expires is the URL's expiry, which
+// ends the Cache-Control max-age.
 func (s *Handlers) writeImageResponse(
 	w http.ResponseWriter, r *http.Request,
-	req *imagerequest.Request, resp *imgcache.ImageResponse,
+	req *imagerequest.Request, expires time.Time, resp *imgcache.ImageResponse,
 	cacheKey imagerequest.VariantKey, startTime time.Time,
 ) {
 	// Set response headers
@@ -355,7 +383,7 @@ func (s *Handlers) writeImageResponse(
 	}
 
 	// Cache control headers
-	w.Header().Set("Cache-Control", cacheControl(req.Expires))
+	w.Header().Set("Cache-Control", cacheControl(expires))
 	w.Header().Set("X-Pixa-Cache", string(resp.CacheStatus))
 
 	if notModified(w, r, resp.ETag) {

internal/handlers/image_cache_control_internal_test.go

@@ -14,10 +14,12 @@ import (
 	"github.com/go-chi/chi/v5"
 	"github.com/prometheus/client_golang/prometheus"
 
+	"sneak.berlin/go/pixa/internal/allowlist"
 	"sneak.berlin/go/pixa/internal/encurl"
 	"sneak.berlin/go/pixa/internal/imageprocessor"
 	"sneak.berlin/go/pixa/internal/imagerequest"
 	"sneak.berlin/go/pixa/internal/imgcache"
+	"sneak.berlin/go/pixa/internal/signature"
 )
 
 // photoPath is the path of the JPEG that newSignedHostServer serves.
@@ -52,7 +54,6 @@ func newSignedHostServer(
 			MaxInputBytes:           50 << 20,
 			MaxConcurrentProcessing: 2,
 		}),
-		SigningKey:      testSigningKey,
 		MaxResponseSize: 50 << 20,
 		Logger:          log,
 	})
@@ -66,9 +67,11 @@ func newSignedHostServer(
 	}
 
 	h := &Handlers{
-		log:    log,
-		imgSvc: svc,
-		encGen: encGen,
+		log:       log,
+		imgSvc:    svc,
+		encGen:    encGen,
+		signer:    signature.New(testSigningKey),
+		allowlist: allowlist.New(nil),
 	}
 
 	r := chi.NewRouter()
@@ -202,8 +205,7 @@ func TestHandleImageEnc_MaxAge(t *testing.T) {
 					Size:   imagerequest.Size{Width: 50, Height: 50},
 					Format: imagerequest.FormatJPEG,
 				},
-				Expires: tt.expires,
-			})
+			}, tt.expires)
 			if err != nil {
 				t.Fatalf("Generate() error = %v", err)
 			}

internal/handlers/image_errors_internal_test.go

@@ -14,6 +14,7 @@ import (
 
 	"github.com/go-chi/chi/v5"
 	"github.com/prometheus/client_golang/prometheus"
+	"sneak.berlin/go/pixa/internal/allowlist"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
 	"sneak.berlin/go/pixa/internal/imageprocessor"
 	"sneak.berlin/go/pixa/internal/imagerequest"
@@ -26,7 +27,7 @@ import (
 const allowlistedHost = "allowed.example.com"
 
 // newImageRoute returns the image route of a Handlers whose service fetches
-// with fetcher and checks signatures with testSigningKey.
+// with fetcher, and which checks signatures with testSigningKey.
 func newImageRoute(t *testing.T, fetcher httpfetcher.Fetcher) http.Handler {
 	t.Helper()
 
@@ -46,15 +47,18 @@ func newImageRoute(t *testing.T, fetcher httpfetcher.Fetcher) http.Handler {
 			MaxInputBytes:           50 << 20,
 			MaxConcurrentProcessing: 2,
 		}),
-		SigningKey:      testSigningKey,
-		Allowlist:       []string{allowlistedHost},
 		MaxResponseSize: 50 << 20,
 	})
 	if err != nil {
 		t.Fatalf("failed to create service: %v", err)
 	}
 
-	h := &Handlers{imgSvc: svc, log: slog.New(slog.DiscardHandler)}
+	h := &Handlers{
+		imgSvc:    svc,
+		log:       slog.New(slog.DiscardHandler),
+		signer:    signature.New(testSigningKey),
+		allowlist: allowlist.New([]string{allowlistedHost}),
+	}
 
 	r := chi.NewRouter()
 	r.Get("/v1/image/*", h.HandleImage())
@@ -63,7 +67,7 @@ func newImageRoute(t *testing.T, fetcher httpfetcher.Fetcher) http.Handler {
 }
 
 // newPhotoFetcher returns a mock fetcher that serves a JPEG at photoPath on
-// each of hosts, and answers any other URL with an upstream error.
+// each of hosts, and answers any other source with an upstream error.
 func newPhotoFetcher(t *testing.T, hosts ...string) *httpfetcher.MockFetcher {
 	t.Helper()
 
@@ -90,10 +94,10 @@ func photoURLWithSig(host, sig string, expires time.Time) string {
 	return fmt.Sprintf("%s?sig=%s&exp=%d", photoURL(host), sig, expires.Unix())
 }
 
-// photoSignature returns the signature of photoURL(host) at the default
-// quality and fit, made with key and expiring at expires.
-func photoSignature(key, host string, expires time.Time) string {
-	return signature.New(key).Sign(&imagerequest.Request{
+// photoRequest returns the image request of photoURL(host) at the default
+// quality and fit.
+func photoRequest(host string) *imagerequest.Request {
+	return &imagerequest.Request{
 		Source: imagerequest.Source{
 			Host: host,
 			Path: photoPath,
@@ -104,8 +108,13 @@ func photoSignature(key, host string, expires time.Time) string {
 			Quality: 85,
 			FitMode: imagerequest.FitCover,
 		},
-		Expires: expires,
-	})
+	}
+}
+
+// photoSignature returns the signature of photoURL(host) at the default
+// quality and fit, made with key and expiring at expires.
+func photoSignature(key, host string, expires time.Time) string {
+	return signature.New(key).Sign(photoRequest(host), expires)
 }
 
 // sendGet sends a GET for target to route and returns the response.
@@ -286,10 +295,9 @@ func TestHandleImage_AllowlistOrSignature(t *testing.T) {
 // host can be, and checks that each is refused with 400 naming the host as
 // decoded: allowlistedHost with a character after it that a URL host cannot
 // hold, percent-encoded, an IPv6 address without brackets, and brackets
-// around something that is not an IPv6 address. A "?" or "#" after
-// allowlistedHost would otherwise end the host when the source URL is read,
-// so the request would pass the allowlist and be fetched and cached under a
-// host that is not the one fetched.
+// around something that is not an IPv6 address. Later steps put the host
+// unparsed into the allowlist check, the cache key and the upstream URL, so
+// such a host is refused first.
 func TestHandleImage_InvalidHost_Returns400(t *testing.T) {
 	t.Parallel()
 
@@ -360,10 +368,9 @@ func TestHandleImage_HostForms_Served(t *testing.T) {
 
 // TestHandleImage_HashInSourcePathOrQuery_Returns400 sends image URLs whose
 // source path or source query holds a "#", written as %23, and checks that
-// each is refused with 400, the one signed for that source path included. The
-// source URL would otherwise end at the "#" when it is fetched, so each text
-// after it would fetch the image at photoPath and cache it under keys of its
-// own.
+// each is refused with 400, the one signed for that source path included.
+// A "#" starts a URL's fragment, so neither the source path nor the source
+// query may hold one.
 func TestHandleImage_HashInSourcePathOrQuery_Returns400(t *testing.T) {
 	t.Parallel()
 
@@ -381,8 +388,7 @@ func TestHandleImage_HashInSourcePathOrQuery_Returns400(t *testing.T) {
 			Quality: 85,
 			FitMode: imagerequest.FitCover,
 		},
-		Expires: exp,
-	})
+	}, exp)
 
 	tests := []struct {
 		name   string

internal/handlers/image_signature_internal_test.go

@@ -38,40 +38,44 @@ func getImage(t *testing.T, fix *testFixtures, target string) int {
 }
 
 // generateSignedURL returns the /v1/image/ URL of req, signed with key and
-// expiring after ttl, and sets req's expiry. An unset quality or fit of req is
-// first set to 85 or cover, the values the route uses when a URL has no q or
-// fit. The URL carries q and fit next to sig and exp, so the route verifies it
-// for the quality and fit it was signed with.
+// expiring after ttl. It signs a copy of req, so req itself is not changed.
+// An unset quality or fit in the copy is set to 85 or cover, the values the
+// route uses when a URL has no q or fit. The URL carries q and fit next to sig
+// and exp, so the route verifies it for the quality and fit it was signed
+// with.
 func generateSignedURL(
 	key string, req *imagerequest.Request, ttl time.Duration,
 ) string {
-	if req.Transform.Quality == 0 {
-		req.Transform.Quality = 85
+	signed := *req
+
+	if signed.Transform.Quality == 0 {
+		signed.Transform.Quality = 85
 	}
 
-	if req.Transform.FitMode == "" {
-		req.Transform.FitMode = imagerequest.FitCover
+	if signed.Transform.FitMode == "" {
+		signed.Transform.FitMode = imagerequest.FitCover
 	}
 
-	req.Expires = time.Now().Add(ttl)
-	sig := signature.New(key).Sign(req)
+	expires := time.Now().Add(ttl)
+	sig := signature.New(key).Sign(&signed, expires)
 
 	// A source query follows the source path after a "?". The image route
 	// reads the path decoded once, so the whole path is written in Go's own
 	// percent-encoding of a URL path, which writes "?" as %3F and "%" as %25,
 	// and leaves "/", ";", ",", "=" and "&" as they are.
-	sourcePathAndQuery := req.Source.Path
-	if req.Source.Query != "" {
-		sourcePathAndQuery += "?" + req.Source.Query
+	sourcePathAndQuery := signed.Source.Path
+	if signed.Source.Query != "" {
+		sourcePathAndQuery += "?" + signed.Source.Query
 	}
 
 	unescapedPath := fmt.Sprintf("/v1/image/%s%s/%dx%d.%s",
-		req.Source.Host, sourcePathAndQuery,
-		req.Transform.Size.Width, req.Transform.Size.Height, req.Transform.Format)
+		signed.Source.Host, sourcePathAndQuery,
+		signed.Transform.Size.Width, signed.Transform.Size.Height,
+		signed.Transform.Format)
 
 	return fmt.Sprintf("%s?sig=%s&exp=%d&q=%d&fit=%s",
-		(&url.URL{Path: unescapedPath}).EscapedPath(), sig, req.Expires.Unix(),
-		req.Transform.Quality, req.Transform.FitMode)
+		(&url.URL{Path: unescapedPath}).EscapedPath(), sig, expires.Unix(),
+		signed.Transform.Quality, signed.Transform.FitMode)
 }
 
 // TestHandleImage_SignatureCoversQualityAndFit signs a URL for quality 85
@@ -93,8 +97,7 @@ func TestHandleImage_SignatureCoversQualityAndFit(t *testing.T) {
 			Quality: 85,
 			FitMode: imagerequest.FitCover,
 		},
-		Expires: expires,
-	})
+	}, expires)
 	signedURL := fmt.Sprintf("/v1/image/%s/images/photo.jpg/50x50.jpeg?sig=%s&exp=%d",
 		signedHost, sig, expires.Unix())
 
@@ -277,8 +280,7 @@ func TestHandleImage_SourceQueryEncodedAnyWay(t *testing.T) {
 			Quality: 85,
 			FitMode: imagerequest.FitCover,
 		},
-		Expires: expires,
-	})
+	}, expires)
 
 	tests := []struct {
 		name         string
@@ -344,8 +346,7 @@ func TestHandleImage_SourceURLHoldingPercent25Verifies(t *testing.T) {
 					Quality: 85,
 					FitMode: imagerequest.FitCover,
 				},
-				Expires: expires,
-			})
+			}, expires)
 
 			target := fmt.Sprintf("/v1/image/%s%s/50x50.jpeg?sig=%s&exp=%d",
 				signedHost, tt.writtenPathAndQuery, sig, expires.Unix())

internal/handlers/imageenc.go

@@ -31,11 +31,18 @@ func (s *Handlers) HandleImageEnc() http.HandlerFunc {
 		ctx := r.Context()
 		start := time.Now()
 
-		req, ok := s.parseImageEncRequest(w, r)
-		if !ok || !s.chooseAutoFormat(w, r, req) {
+		req, expires, ok := s.parseImageEncRequest(w, r)
+		if !ok {
 			return
 		}
 
+		format, ok := s.chooseAutoFormat(w, r, req.Transform.Format)
+		if !ok {
+			return
+		}
+
+		req.Transform.Format = format
+
 		// Log the request
 		s.log.Debug("encrypted image request",
 			"host", req.Source.Host,
@@ -64,7 +71,7 @@ func (s *Handlers) HandleImageEnc() http.HandlerFunc {
 		}
 
 		// Cache headers: max-age ends at the URL's expiry
-		w.Header().Set("Cache-Control", cacheControl(req.Expires))
+		w.Header().Set("Cache-Control", cacheControl(expires))
 		w.Header().Set("X-Pixa-Cache", string(resp.CacheStatus))
 
 		if notModified(w, r, resp.ETag) {
@@ -102,34 +109,34 @@ func (s *Handlers) HandleImageEnc() http.HandlerFunc {
 }
 
 // parseImageEncRequest decrypts the token of an encrypted image URL into an
-// image request 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.
+// image request and checks it, and returns it with the token's expiry. 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,
-) (*imagerequest.Request, bool) {
+) (*imagerequest.Request, time.Time, bool) {
 	// Extract token from URL
 	token := chi.URLParam(r, "token")
 	if token == "" {
 		s.respondError(w, "missing token", http.StatusBadRequest)
 
-		return nil, false
+		return nil, time.Time{}, false
 	}
 
 	// Decrypt and validate the payload
-	req, err := s.encGen.Parse(token)
+	req, expires, 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
+			return nil, time.Time{}, false
 		}
 
 		s.log.Debug("failed to decrypt URL", "error", err)
 		s.respondError(w, "invalid encrypted URL", http.StatusBadRequest)
 
-		return nil, false
+		return nil, time.Time{}, false
 	}
 
 	// Apply the same dimension and fit-mode bounds as the plain image
@@ -142,10 +149,10 @@ func (s *Handlers) parseImageEncRequest(
 		s.respondError(w, "invalid encrypted URL: "+err.Error(),
 			http.StatusBadRequest)
 
-		return nil, false
+		return nil, time.Time{}, false
 	}
 
-	return req, true
+	return req, expires, true
 }
 
 // handleImageError converts image service errors to HTTP responses.

internal/handlers/imageenc_internal_test.go

@@ -6,6 +6,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"testing"
+	"time"
 
 	"github.com/go-chi/chi/v5"
 
@@ -62,7 +63,7 @@ func TestHandleImageEnc_OverLimitDimension_Returns400(t *testing.T) {
 		Transform: imagerequest.Transform{
 			Size: imagerequest.Size{Width: 100000, Height: 100000},
 		},
-	})
+	}, time.Time{})
 	if err != nil {
 		t.Fatalf("Generate() error = %v", err)
 	}
@@ -91,7 +92,7 @@ func TestHandleImageEnc_InvalidFitMode_Returns400(t *testing.T) {
 			Size:    imagerequest.Size{Width: 800, Height: 600},
 			FitMode: imagerequest.Fit("bogus"),
 		},
-	})
+	}, time.Time{})
 	if err != nil {
 		t.Fatalf("Generate() error = %v", err)
 	}

internal/handlers/imageenc_token_internal_test.go

@@ -70,15 +70,12 @@ func TestHandleImageEnc_RejectedToken(t *testing.T) {
 		},
 	}
 
-	valid, err := h.encGen.Generate(&photo)
+	valid, err := h.encGen.Generate(&photo, time.Time{})
 	if err != nil {
 		t.Fatalf("Generate() error = %v", err)
 	}
 
-	expiredPhoto := photo
-	expiredPhoto.Expires = time.Now().Add(-time.Minute)
-
-	expired, err := h.encGen.Generate(&expiredPhoto)
+	expired, err := h.encGen.Generate(&photo, time.Now().Add(-time.Minute))
 	if err != nil {
 		t.Fatalf("Generate() error = %v", err)
 	}
@@ -88,7 +85,7 @@ func TestHandleImageEnc_RejectedToken(t *testing.T) {
 		t.Fatalf("encurl.NewGenerator() error = %v", err)
 	}
 
-	otherKey, err := otherGen.Generate(&photo)
+	otherKey, err := otherGen.Generate(&photo, time.Time{})
 	if err != nil {
 		t.Fatalf("Generate() error = %v", err)
 	}

internal/handlers/jpegxl_default_internal_test.go

@@ -69,8 +69,7 @@ func TestImageWithoutFormat_SignedAsJXL(t *testing.T) {
 			Quality: encurl.DefaultQuality,
 			FitMode: imagerequest.FitCover,
 		},
-		Expires: expires,
-	})
+	}, expires)
 	query := fmt.Sprintf("?sig=%s&exp=%d", sig, expires.Unix())
 
 	for _, size := range []string{"50x50.jxl", "50x50"} {
@@ -94,7 +93,7 @@ func TestImageEncWithoutFormat_ServesJPEGXL(t *testing.T) {
 		Transform: imagerequest.Transform{
 			Size: imagerequest.Size{Width: 50, Height: 50},
 		},
-	})
+	}, time.Time{})
 	if err != nil {
 		t.Fatalf("Generate() error = %v", err)
 	}

internal/handlers/referer_blocklist_internal_test.go

@@ -15,7 +15,9 @@ import (
 	"sneak.berlin/go/pixa/internal/encurl"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
 	"sneak.berlin/go/pixa/internal/imageprocessor"
+	"sneak.berlin/go/pixa/internal/imagerequest"
 	"sneak.berlin/go/pixa/internal/imgcache"
+	"sneak.berlin/go/pixa/internal/signature"
 )
 
 // blockedReferer is a page on leech.example, which newRefererRoutes puts on
@@ -31,11 +33,11 @@ type countingFetcher struct {
 
 // Fetch counts the fetch and passes it on.
 func (f *countingFetcher) Fetch(
-	ctx context.Context, url string,
+	ctx context.Context, source imagerequest.Source,
 ) (*httpfetcher.FetchResult, error) {
 	f.fetches.Add(1)
 
-	return f.Fetcher.Fetch(ctx, url)
+	return f.Fetcher.Fetch(ctx, source)
 }
 
 // newRefererRoutes returns both image routes of a Handlers whose
@@ -65,8 +67,6 @@ func newRefererRoutes(t *testing.T) (http.Handler, *Handlers, *countingFetcher)
 			MaxInputBytes:           50 << 20,
 			MaxConcurrentProcessing: 2,
 		}),
-		SigningKey:      testSigningKey,
-		Allowlist:       []string{allowlistedHost},
 		MaxResponseSize: 50 << 20,
 	})
 	if err != nil {
@@ -79,9 +79,11 @@ func newRefererRoutes(t *testing.T) (http.Handler, *Handlers, *countingFetcher)
 	}
 
 	h := &Handlers{
-		log:    slog.New(slog.DiscardHandler),
-		imgSvc: svc,
-		encGen: encGen,
+		log:       slog.New(slog.DiscardHandler),
+		imgSvc:    svc,
+		encGen:    encGen,
+		signer:    signature.New(testSigningKey),
+		allowlist: allowlist.New([]string{allowlistedHost}),
 		refererBlocklist: allowlist.New(
 			[]string{"leech.example", ".hotlinker.example"}),
 	}

internal/handlers/request_id_internal_test.go

@@ -18,8 +18,8 @@ import (
 
 // signedPhotoURL returns a signed /v1/image/ URL, valid for a minute, for the
 // JPEG at photoPath on signedHost at 50x50. It is signed with testSigningKey,
-// the key of the image service newSignedHostServer builds. It takes a Handlers
-// it does not use, so that it has the type of encPhotoURL.
+// the key the handlers that newSignedHostServer builds check signatures with.
+// It takes a Handlers it does not use, so that it has the type of encPhotoURL.
 func signedPhotoURL(t *testing.T, _ *Handlers) string {
 	t.Helper()
 
@@ -49,7 +49,7 @@ func encPhotoURL(t *testing.T, h *Handlers) string {
 			Size:   imagerequest.Size{Width: 50, Height: 50},
 			Format: imagerequest.FormatJPEG,
 		},
-	})
+	}, time.Time{})
 	if err != nil {
 		t.Fatalf("Generate() error = %v", err)
 	}

internal/handlers/validate_signature_internal_test.go

@@ -0,0 +1,211 @@
+package handlers
+
+import (
+	"testing"
+	"time"
+
+	"sneak.berlin/go/pixa/internal/allowlist"
+	"sneak.berlin/go/pixa/internal/signature"
+)
+
+// Addresses the port tests below put on the allowlist.
+const (
+	allowlistedIPv4 = "203.0.113.5"
+	allowlistedIPv6 = "2001:db8::1"
+)
+
+// newSignatureChecker returns Handlers that check signatures with
+// testSigningKey and have allowlistHosts on their allowlist, as New builds
+// them from signing_key and allowlist_hosts.
+func newSignatureChecker(allowlistHosts ...string) *Handlers {
+	return &Handlers{
+		signer:    signature.New(testSigningKey),
+		allowlist: allowlist.New(allowlistHosts),
+	}
+}
+
+func TestValidateSignature_NonAllowlistedHost_NoSignature(t *testing.T) {
+	t.Parallel()
+
+	h := newSignatureChecker(allowlistedHost)
+
+	// Should fail validation - not allowlisted and no signature
+	err := h.validateSignature(photoRequest(signedHost), "", time.Time{})
+	if err == nil {
+		t.Error("validateSignature() expected error for non-allowlisted " +
+			"host without signature")
+	}
+}
+
+func TestValidateSignature_NonAllowlistedHost_ValidSignature(t *testing.T) {
+	t.Parallel()
+
+	h := newSignatureChecker(allowlistedHost)
+	req := photoRequest(signedHost)
+
+	// Generate a valid signature
+	expires := time.Now().Add(time.Hour)
+	sig := signature.New(testSigningKey).Sign(req, expires)
+
+	// Should pass validation
+	err := h.validateSignature(req, sig, expires)
+	if err != nil {
+		t.Errorf("validateSignature() error = %v", err)
+	}
+}
+
+func TestValidateSignature_NonAllowlistedHost_ExpiredSignature(t *testing.T) {
+	t.Parallel()
+
+	h := newSignatureChecker(allowlistedHost)
+	req := photoRequest(signedHost)
+
+	// Generate an expired signature
+	expires := time.Now().Add(-time.Hour)
+	sig := signature.New(testSigningKey).Sign(req, expires)
+
+	// Should fail validation
+	err := h.validateSignature(req, sig, expires)
+	if err == nil {
+		t.Error("validateSignature() expected error for expired signature")
+	}
+}
+
+func TestValidateSignature_NonAllowlistedHost_InvalidSignature(t *testing.T) {
+	t.Parallel()
+
+	h := newSignatureChecker(allowlistedHost)
+
+	// Should fail validation
+	err := h.validateSignature(photoRequest(signedHost), "invalid-signature",
+		time.Now().Add(time.Hour))
+	if err == nil {
+		t.Error("validateSignature() expected error for invalid signature")
+	}
+}
+
+// TestValidateSignature_NoAllowlist checks that with no host on the
+// allowlist, a request without a signature fails validation.
+func TestValidateSignature_NoAllowlist(t *testing.T) {
+	t.Parallel()
+
+	h := newSignatureChecker()
+
+	err := h.validateSignature(photoRequest(signedHost), "", time.Time{})
+	if err == nil {
+		t.Error("validateSignature() expected error when no host is allowlisted")
+	}
+}
+
+// TestValidateSignature_SignatureExactHostMatch verifies that
+// validateSignature enforces exact host matching for signatures. A
+// signature for one host must not verify for a different host, even
+// if they share a domain suffix.
+func TestValidateSignature_SignatureExactHostMatch(t *testing.T) {
+	t.Parallel()
+
+	h := newSignatureChecker()
+
+	// Sign a request for signedHost
+	signedReq := photoRequest(signedHost)
+	expires := time.Now().Add(time.Hour)
+	sig := signature.New(testSigningKey).Sign(signedReq, expires)
+
+	// The original request should pass validation
+	t.Run("exact host passes", func(t *testing.T) {
+		t.Parallel()
+
+		err := h.validateSignature(signedReq, sig, expires)
+		if err != nil {
+			t.Errorf("validateSignature() exact host failed: %v", err)
+		}
+	})
+
+	// Try to reuse the signature with different hosts
+	tests := []struct {
+		name string
+		host string
+	}{
+		{"parent domain", "example.com"},
+		{"sibling subdomain", "images.example.com"},
+		{"deeper subdomain", "a." + signedHost},
+		{"evil suffix domain", signedHost + ".evil.com"},
+		{"prefixed host", "evil" + signedHost},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name+" rejected", func(t *testing.T) {
+			t.Parallel()
+
+			err := h.validateSignature(photoRequest(tt.host), sig, expires)
+			if err == nil {
+				t.Errorf("validateSignature() should reject signature for "+
+					"host %q (signed for %q)", tt.host, signedHost)
+			}
+		})
+	}
+}
+
+// TestValidateSignature_HostWithPortNeedsSignature verifies that a host
+// followed by a port, or by a colon with no port, matches no allowlist entry,
+// so that a request for it without a signature is refused, while the same
+// host with no port is allowlisted.
+func TestValidateSignature_HostWithPortNeedsSignature(t *testing.T) {
+	t.Parallel()
+
+	h := newSignatureChecker(allowlistedHost, allowlistedIPv4, allowlistedIPv6)
+
+	tests := []struct {
+		host        string
+		allowlisted bool
+	}{
+		{allowlistedHost, true},
+		{allowlistedHost + ":8443", false},
+		{allowlistedHost + ":443", false},
+		{allowlistedHost + ":", false},
+		{allowlistedIPv4, true},
+		{allowlistedIPv4 + ":8080", false},
+		{"[" + allowlistedIPv6 + "]", true},
+		{"[" + allowlistedIPv6 + "]:8443", false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.host, func(t *testing.T) {
+			t.Parallel()
+
+			err := h.validateSignature(photoRequest(tt.host), "", time.Time{})
+			t.Logf("validateSignature() for host %q with no signature: %v",
+				tt.host, err)
+
+			if allowlisted := err == nil; allowlisted != tt.allowlisted {
+				t.Errorf("host %q served without a signature = %v, want %v",
+					tt.host, allowlisted, tt.allowlisted)
+			}
+		})
+	}
+}
+
+// TestValidateSignature_UnbracketedIPv6PortNeedsSignature verifies that an
+// allowlisted IPv6 address written without brackets and followed by a port,
+// or by a colon with no port, needs a signature. Go's URL type reads
+// 2001:db8::1:8443 as the host 2001:db8::1 with the port 8443.
+func TestValidateSignature_UnbracketedIPv6PortNeedsSignature(t *testing.T) {
+	t.Parallel()
+
+	h := newSignatureChecker(allowlistedIPv6)
+
+	for _, host := range []string{allowlistedIPv6 + ":8443", allowlistedIPv6 + ":"} {
+		t.Run(host, func(t *testing.T) {
+			t.Parallel()
+
+			err := h.validateSignature(photoRequest(host), "", time.Time{})
+			t.Logf("validateSignature() for host %q with no signature: %v",
+				host, err)
+
+			if err == nil {
+				t.Errorf("host %q served without a signature, want refused",
+					host)
+			}
+		})
+	}
+}

internal/httpfetcher/dial_context_internal_test.go

@@ -51,12 +51,12 @@ func TestDialContextReplacesOnlyTheDialer(t *testing.T) {
 		t.Errorf("body = %q, want %q", body, imagePayload)
 	}
 
-	_, err := f.Fetch(testContext(t), "http://127.0.0.1/image")
+	_, err := f.Fetch(testContext(t), imageSourceOn("127.0.0.1"))
 	if !errors.Is(err, ErrSSRFBlocked) {
 		t.Errorf("Fetch(loopback URL) error = %v, want ErrSSRFBlocked", err)
 	}
 
-	_, err = f.Fetch(testContext(t), upstreamURL("/redirect/private"))
+	_, err = f.Fetch(testContext(t), upstreamSource("/redirect/private"))
 	if !errors.Is(err, ErrSSRFBlocked) {
 		t.Errorf("Fetch(/redirect/private) error = %v, want ErrSSRFBlocked", err)
 	}

internal/httpfetcher/fetch_internal_test.go

@@ -14,6 +14,8 @@ import (
 	"sync"
 	"testing"
 	"time"
+
+	"sneak.berlin/go/pixa/internal/imagerequest"
 )
 
 // testPublicHost is a TEST-NET-1 (RFC 5737) literal. isPrivateIP treats it as
@@ -29,9 +31,9 @@ const imagePayload = "fake-jpeg-bytes"
 // would mean SSRF protection let a forbidden target reach the transport.
 var errUnexpectedDial = errors.New("unexpected dial target")
 
-// upstreamURL builds a fetch URL on the fake public host for the given path.
-func upstreamURL(path string) string {
-	return "http://" + testPublicHost + path
+// upstreamSource returns the source at path on the fake public host.
+func upstreamSource(path string) imagerequest.Source {
+	return imagerequest.Source{Host: testPublicHost, Path: path}
 }
 
 // recordingDialer records every address the transport asks it to dial and
@@ -158,7 +160,7 @@ func testContext(t *testing.T) context.Context {
 func fetchImage(t *testing.T, f *HTTPFetcher, path string) *FetchResult {
 	t.Helper()
 
-	res, err := f.Fetch(testContext(t), upstreamURL(path))
+	res, err := f.Fetch(testContext(t), upstreamSource(path))
 	if err != nil {
 		t.Fatalf("Fetch(%s) error = %v", path, err)
 	}
@@ -170,7 +172,7 @@ func fetchImage(t *testing.T, f *HTTPFetcher, path string) *FetchResult {
 func fetchExpectError(t *testing.T, f *HTTPFetcher, path string) error {
 	t.Helper()
 
-	res, err := f.Fetch(testContext(t), upstreamURL(path))
+	res, err := f.Fetch(testContext(t), upstreamSource(path))
 	if err == nil {
 		_ = res.Content.Close()
 
@@ -222,7 +224,7 @@ func TestFetchRedirectToPrivateIPBlocked(t *testing.T) {
 	srv := startUpstream(t)
 	f, dialer := newServerFetcher(t, srv, nil)
 
-	_, err := f.Fetch(testContext(t), upstreamURL("/redirect/private"))
+	_, err := f.Fetch(testContext(t), upstreamSource("/redirect/private"))
 	if !errors.Is(err, ErrSSRFBlocked) {
 		t.Fatalf("Fetch() error = %v, want ErrSSRFBlocked", err)
 	}
@@ -590,3 +592,53 @@ func TestSSRFSafeDialerAllowsPublicTarget(t *testing.T) {
 		t.Errorf("public target was SSRF-blocked: %v", err)
 	}
 }
+
+// TestFetchSchemeFollowsAllowHTTP checks that the fetcher fetches a source
+// over https, on port 443, unless AllowHTTP is set, and then over http, on
+// port 80. The test server speaks plain HTTP, so only the fetch over http
+// succeeds.
+func TestFetchSchemeFollowsAllowHTTP(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		allowHTTP bool
+		wantAddr  string
+	}{
+		{false, net.JoinHostPort(testPublicHost, "443")},
+		{true, net.JoinHostPort(testPublicHost, "80")},
+	}
+
+	for _, tt := range tests {
+		t.Run(fmt.Sprintf("AllowHTTP %v", tt.allowHTTP), func(t *testing.T) {
+			t.Parallel()
+
+			srv := startUpstream(t)
+			dialer := &recordingDialer{target: srv.Listener.Addr().String()}
+
+			f := New(&Config{
+				Timeout:               30 * time.Second,
+				MaxResponseSize:       50 << 20,
+				AllowHTTP:             tt.allowHTTP,
+				MaxConnectionsPerHost: 20,
+				MaxConnections:        64,
+				DialContext:           dialer.dialContext,
+			})
+
+			res, err := f.Fetch(testContext(t), upstreamSource("/image"))
+			if err == nil {
+				_ = res.Content.Close()
+			}
+
+			dialed := dialer.dialedAddrs()
+			t.Logf("Fetch(): error %v, dialed %v", err, dialed)
+
+			if served := err == nil; served != tt.allowHTTP {
+				t.Errorf("served = %v, want %v", served, tt.allowHTTP)
+			}
+
+			if len(dialed) != 1 || dialed[0] != tt.wantAddr {
+				t.Errorf("dialed %v, want only %s", dialed, tt.wantAddr)
+			}
+		})
+	}
+}

internal/httpfetcher/httpfetcher.go

@@ -20,6 +20,7 @@ import (
 	"time"
 
 	"github.com/go-chi/chi/v5/middleware"
+	"sneak.berlin/go/pixa/internal/imagerequest"
 )
 
 // Fetcher configuration constants.
@@ -91,8 +92,8 @@ var (
 
 // Fetcher retrieves content from upstream origins.
 type Fetcher interface {
-	// Fetch retrieves content from the given URL.
-	Fetch(ctx context.Context, url string) (*FetchResult, error)
+	// Fetch retrieves the content of the source image.
+	Fetch(ctx context.Context, source imagerequest.Source) (*FetchResult, error)
 }
 
 // FetchResult contains the result of fetching from upstream.
@@ -139,7 +140,8 @@ type Config struct {
 	Timeout time.Duration
 	// MaxResponseSize is the maximum allowed response body size.
 	MaxResponseSize int64
-	// AllowHTTP allows non-TLS connections (for testing only).
+	// AllowHTTP makes the fetcher fetch sources over http instead of https,
+	// and follow redirects to http (for testing only).
 	AllowHTTP bool
 	// MaxConnectionsPerHost limits concurrent connections to each upstream host.
 	MaxConnectionsPerHost int
@@ -211,7 +213,7 @@ func New(config *Config) *HTTPFetcher {
 			}
 
 			// Validate the redirect target
-			err := validateURL(req.Context(), req.URL.String(), config.AllowHTTP)
+			err := validateURL(req.Context(), req.URL, config.AllowHTTP)
 			if err != nil {
 				return fmt.Errorf("redirect blocked: %w", err)
 			}
@@ -229,18 +231,22 @@ func New(config *Config) *HTTPFetcher {
 	}
 }
 
-// Fetch retrieves content from the given URL with SSRF protection. When
-// MaxConnections fetches are already in progress, it waits up to
-// ConnectionWaitTimeout for one to finish, then fails with
-// ErrTooManyConnections.
-func (f *HTTPFetcher) Fetch(ctx context.Context, url string) (*FetchResult, error) {
+// Fetch retrieves the source image with SSRF protection, over https, or over
+// http when AllowHTTP is set. When MaxConnections fetches are already in
+// progress, it waits up to ConnectionWaitTimeout for one to finish, then
+// fails with ErrTooManyConnections.
+func (f *HTTPFetcher) Fetch(
+	ctx context.Context, source imagerequest.Source,
+) (*FetchResult, error) {
+	sourceURL := source.URL(f.config.AllowHTTP)
+
 	// Validate URL before making request
-	err := validateURL(ctx, url, f.config.AllowHTTP)
+	err := validateURL(ctx, sourceURL, f.config.AllowHTTP)
 	if err != nil {
 		return nil, err
 	}
 
-	release, err := f.acquireConnection(ctx, extractHost(url))
+	release, err := f.acquireConnection(ctx, source.Host)
 	if err != nil {
 		return nil, err
 	}
@@ -254,14 +260,9 @@ func (f *HTTPFetcher) Fetch(ctx context.Context, url string) (*FetchResult, erro
 		}
 	}()
 
-	parsedURL, err := neturl.Parse(url)
-	if err != nil {
-		return nil, fmt.Errorf("failed to parse URL: %w", err)
-	}
-
 	req := &http.Request{
 		Method: http.MethodGet,
-		URL:    parsedURL,
+		URL:    sourceURL,
 		Header: make(http.Header),
 	}
 
@@ -455,19 +456,14 @@ func (f *HTTPFetcher) isAllowedContentType(contentType string) bool {
 }
 
 // validateURL checks if a URL is safe to fetch (not internal/private).
-func validateURL(ctx context.Context, rawURL string, allowHTTP bool) error {
-	if !allowHTTP && !strings.HasPrefix(rawURL, "https://") {
+func validateURL(ctx context.Context, fetchURL *neturl.URL, allowHTTP bool) error {
+	if !allowHTTP && fetchURL.Scheme != "https" {
 		return ErrUnsupportedScheme
 	}
 
-	parsedURL, err := neturl.Parse(rawURL)
-	if err != nil {
-		return fmt.Errorf("%w: %w", ErrInvalidHost, err)
-	}
-
 	// Hostname drops any port, and the brackets of an IPv6 address whether
 	// or not a port follows them
-	host := parsedURL.Hostname()
+	host := fetchURL.Hostname()
 	if host == "" {
 		return ErrInvalidHost
 	}
@@ -493,25 +489,6 @@ func validateURL(ctx context.Context, rawURL string, allowHTTP bool) error {
 	return nil
 }
 
-// extractHost extracts the host from a URL string.
-func extractHost(rawURL string) string {
-	// Simple extraction without full URL parsing
-	url := rawURL
-	if idx := strings.Index(url, "://"); idx != -1 {
-		url = url[idx+3:]
-	}
-
-	if idx := strings.Index(url, "/"); idx != -1 {
-		url = url[:idx]
-	}
-
-	if idx := strings.Index(url, "?"); idx != -1 {
-		url = url[:idx]
-	}
-
-	return url
-}
-
 // isLocalhost checks if the host is localhost.
 func isLocalhost(host string) bool {
 	host = strings.ToLower(host)

internal/httpfetcher/httpfetcher_internal_test.go

@@ -5,9 +5,12 @@ import (
 	"errors"
 	"io"
 	"net"
+	"net/url"
 	"testing"
 	"testing/fstest"
 	"time"
+
+	"sneak.berlin/go/pixa/internal/imagerequest"
 )
 
 // testHost is the hostname used by mock fetch tests.
@@ -49,33 +52,6 @@ func TestIsAllowedContentType(t *testing.T) {
 	}
 }
 
-func TestExtractHost(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		url  string
-		want string
-	}{
-		{"https://example.com/path", testHost},
-		{"http://example.com:8080/path", "example.com:8080"},
-		{"https://example.com", testHost},
-		{"https://example.com?q=1", testHost},
-		{"example.com/path", testHost},
-		{"", ""},
-	}
-
-	for _, tc := range tests {
-		t.Run(tc.url, func(t *testing.T) {
-			t.Parallel()
-
-			got := extractHost(tc.url)
-			if got != tc.want {
-				t.Errorf("extractHost(%q) = %q, want %q", tc.url, got, tc.want)
-			}
-		})
-	}
-}
-
 func TestIsLocalhost(t *testing.T) {
 	t.Parallel()
 
@@ -188,7 +164,7 @@ func TestIsPrivateIPJudgesEmbeddedIPv4Address(t *testing.T) {
 func TestValidateURL_RejectsNonHTTPS(t *testing.T) {
 	t.Parallel()
 
-	err := validateURL(t.Context(), "http://example.com/path", false)
+	err := validateURL(t.Context(), &url.URL{Scheme: "http", Host: testHost}, false)
 	if !errors.Is(err, ErrUnsupportedScheme) {
 		t.Errorf("validateURL http = %v, want ErrUnsupportedScheme", err)
 	}
@@ -198,7 +174,8 @@ func TestValidateURL_AllowsHTTPWhenConfigured(t *testing.T) {
 	t.Parallel()
 
 	// Use a host that won't resolve (explicit .invalid TLD) so we don't hit DNS.
-	err := validateURL(t.Context(), "http://nonexistent.invalid/path", true)
+	err := validateURL(t.Context(),
+		&url.URL{Scheme: "http", Host: "nonexistent.invalid"}, true)
 	// We expect a host resolution error, not ErrUnsupportedScheme.
 	if errors.Is(err, ErrUnsupportedScheme) {
 		t.Error("validateURL with AllowHTTP should not return ErrUnsupportedScheme")
@@ -208,7 +185,7 @@ func TestValidateURL_AllowsHTTPWhenConfigured(t *testing.T) {
 func TestValidateURL_RejectsLocalhost(t *testing.T) {
 	t.Parallel()
 
-	err := validateURL(t.Context(), "https://localhost/path", false)
+	err := validateURL(t.Context(), &url.URL{Scheme: "https", Host: "localhost"}, false)
 	if !errors.Is(err, ErrSSRFBlocked) {
 		t.Errorf("validateURL localhost = %v, want ErrSSRFBlocked", err)
 	}
@@ -217,7 +194,7 @@ func TestValidateURL_RejectsLocalhost(t *testing.T) {
 func TestValidateURL_EmptyHost(t *testing.T) {
 	t.Parallel()
 
-	err := validateURL(t.Context(), "https:///path", false)
+	err := validateURL(t.Context(), &url.URL{Scheme: "https"}, false)
 	if !errors.Is(err, ErrInvalidHost) {
 		t.Errorf("validateURL empty host = %v, want ErrInvalidHost", err)
 	}
@@ -232,7 +209,8 @@ func TestMockFetcher_FetchesFile(t *testing.T) {
 
 	m := NewMock(mockFS)
 
-	result, err := m.Fetch(context.Background(), "https://example.com/images/photo.jpg")
+	result, err := m.Fetch(context.Background(),
+		imagerequest.Source{Host: testHost, Path: "/images/photo.jpg"})
 	if err != nil {
 		t.Fatalf("Fetch() error = %v", err)
 	}
@@ -263,7 +241,8 @@ func TestMockFetcher_MissingFileReturnsUpstreamError(t *testing.T) {
 	mockFS := fstest.MapFS{}
 	m := NewMock(mockFS)
 
-	_, err := m.Fetch(context.Background(), "https://example.com/missing.jpg")
+	_, err := m.Fetch(context.Background(),
+		imagerequest.Source{Host: testHost, Path: "/missing.jpg"})
 	if !errors.Is(err, ErrUpstreamError) {
 		t.Errorf("Fetch() error = %v, want ErrUpstreamError", err)
 	}
@@ -280,7 +259,7 @@ func TestMockFetcher_RespectsContextCancellation(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	cancel()
 
-	_, err := m.Fetch(ctx, "https://example.com/photo.jpg")
+	_, err := m.Fetch(ctx, imagerequest.Source{Host: testHost, Path: "/photo.jpg"})
 	if !errors.Is(err, context.Canceled) {
 		t.Errorf("Fetch() error = %v, want context.Canceled", err)
 	}

internal/httpfetcher/max_connections_internal_test.go

@@ -8,14 +8,20 @@ import (
 	"sync"
 	"testing"
 	"time"
+
+	"sneak.berlin/go/pixa/internal/imagerequest"
 )
 
-// imageURLOnPort is the fake upstream's image route on testPublicHost at
+// imageSourceOn is the fake upstream's image route on host.
+func imageSourceOn(host string) imagerequest.Source {
+	return imagerequest.Source{Host: host, Path: "/image"}
+}
+
+// imageSourceOnPort is the fake upstream's image route on testPublicHost at
 // port. Each port is a different host to the per-host limit, while the test
 // dialer sends every port to the one test server.
-func imageURLOnPort(port int) string {
-	return "http://" + net.JoinHostPort(testPublicHost, strconv.Itoa(port)) +
-		"/image"
+func imageSourceOnPort(port int) imagerequest.Source {
+	return imageSourceOn(net.JoinHostPort(testPublicHost, strconv.Itoa(port)))
 }
 
 // TestFetchLimitsConnectionsToAllHostsTogether checks that MaxConnections
@@ -39,12 +45,12 @@ func TestFetchLimitsConnectionsToAllHostsTogether(t *testing.T) {
 	f, _ := newServerFetcher(t, srv, cfg)
 	f.connectionWaitTimeout = 100 * time.Millisecond
 
-	first, err := f.Fetch(testContext(t), imageURLOnPort(81))
+	first, err := f.Fetch(testContext(t), imageSourceOnPort(81))
 	if err != nil {
 		t.Fatalf("first Fetch() error = %v", err)
 	}
 
-	second, err := f.Fetch(testContext(t), imageURLOnPort(82))
+	second, err := f.Fetch(testContext(t), imageSourceOnPort(82))
 	if err != nil {
 		t.Fatalf("second Fetch() error = %v", err)
 	}
@@ -53,7 +59,7 @@ func TestFetchLimitsConnectionsToAllHostsTogether(t *testing.T) {
 
 	start := time.Now()
 
-	_, err = f.Fetch(testContext(t), imageURLOnPort(83))
+	_, err = f.Fetch(testContext(t), imageSourceOnPort(83))
 	if !errors.Is(err, ErrTooManyConnections) {
 		t.Fatalf("third Fetch() error = %v, want ErrTooManyConnections", err)
 	}
@@ -72,7 +78,7 @@ func TestFetchLimitsConnectionsToAllHostsTogether(t *testing.T) {
 		t.Fatalf("close first body: %v", err)
 	}
 
-	third, err := f.Fetch(testContext(t), imageURLOnPort(83))
+	third, err := f.Fetch(testContext(t), imageSourceOnPort(83))
 	if err != nil {
 		t.Fatalf("Fetch() after a response was closed: error = %v", err)
 	}
@@ -99,7 +105,7 @@ func TestFetchFreesHostSlotWhenContextEndsWaitingForConnection(t *testing.T) {
 
 	f, _ := newServerFetcher(t, srv, cfg)
 
-	first, err := f.Fetch(testContext(t), imageURLOnPort(81))
+	first, err := f.Fetch(testContext(t), imageSourceOnPort(81))
 	if err != nil {
 		t.Fatalf("first Fetch() error = %v", err)
 	}
@@ -109,7 +115,7 @@ func TestFetchFreesHostSlotWhenContextEndsWaitingForConnection(t *testing.T) {
 	ctx, cancel := context.WithTimeout(t.Context(), 100*time.Millisecond)
 	defer cancel()
 
-	_, err = f.Fetch(ctx, imageURLOnPort(82))
+	_, err = f.Fetch(ctx, imageSourceOnPort(82))
 	if !errors.Is(err, context.DeadlineExceeded) {
 		t.Fatalf("second Fetch() error = %v, want context.DeadlineExceeded", err)
 	}
@@ -134,7 +140,7 @@ func TestFetchRemovesIdleHostSemaphores(t *testing.T) {
 
 	for i := range 100 {
 		wg.Go(func() {
-			res, err := f.Fetch(ctx, imageURLOnPort(1+i%50))
+			res, err := f.Fetch(ctx, imageSourceOnPort(1+i%50))
 			if err != nil {
 				t.Errorf("Fetch() error = %v", err)
 
@@ -173,12 +179,12 @@ func TestFetchRemovesHostSemaphoreWhenNoConnection(t *testing.T) {
 	f, _ := newServerFetcher(t, srv, cfg)
 	f.connectionWaitTimeout = 100 * time.Millisecond
 
-	open, err := f.Fetch(testContext(t), imageURLOnPort(81))
+	open, err := f.Fetch(testContext(t), imageSourceOnPort(81))
 	if err != nil {
 		t.Fatalf("first Fetch() error = %v", err)
 	}
 
-	_, err = f.Fetch(testContext(t), imageURLOnPort(82))
+	_, err = f.Fetch(testContext(t), imageSourceOnPort(82))
 	if !errors.Is(err, ErrTooManyConnections) {
 		t.Fatalf("Fetch() from another host: error = %v, "+
 			"want ErrTooManyConnections", err)
@@ -187,7 +193,7 @@ func TestFetchRemovesHostSemaphoreWhenNoConnection(t *testing.T) {
 	ctx, cancel := context.WithTimeout(t.Context(), 100*time.Millisecond)
 	defer cancel()
 
-	_, err = f.Fetch(ctx, imageURLOnPort(81))
+	_, err = f.Fetch(ctx, imageSourceOnPort(81))
 	if !errors.Is(err, context.DeadlineExceeded) {
 		t.Fatalf("Fetch() from the busy host: error = %v, "+
 			"want context.DeadlineExceeded", err)
@@ -199,7 +205,7 @@ func TestFetchRemovesHostSemaphoreWhenNoConnection(t *testing.T) {
 	ctx, cancel = context.WithTimeout(t.Context(), 100*time.Millisecond)
 	defer cancel()
 
-	_, err = f.Fetch(ctx, imageURLOnPort(83))
+	_, err = f.Fetch(ctx, imageSourceOnPort(83))
 	if !errors.Is(err, context.DeadlineExceeded) {
 		t.Fatalf("Fetch() from a host with nothing open: error = %v, "+
 			"want context.DeadlineExceeded", err)
@@ -222,15 +228,15 @@ func TestFetchReleasesConnectionOnError(t *testing.T) {
 	t.Parallel()
 
 	cases := []struct {
-		name string
-		url  string
-		want error
+		name   string
+		source imagerequest.Source
+		want   error
 	}{
-		{"upstream answers 500", upstreamURL("/status/500"), ErrUpstreamError},
-		{"upstream sends HTML", upstreamURL("/html"), ErrInvalidContentType},
+		{"upstream answers 500", upstreamSource("/status/500"), ErrUpstreamError},
+		{"upstream sends HTML", upstreamSource("/html"), ErrInvalidContentType},
 		// 198.51.100.7 (TEST-NET-2) passes the SSRF checks, and the test
 		// dialer refuses every host but testPublicHost.
-		{"connecting fails", "http://198.51.100.7/image", errUnexpectedDial},
+		{"connecting fails", imageSourceOn("198.51.100.7"), errUnexpectedDial},
 	}
 
 	for _, tc := range cases {
@@ -249,7 +255,7 @@ func TestFetchReleasesConnectionOnError(t *testing.T) {
 			f, _ := newServerFetcher(t, srv, cfg)
 			f.connectionWaitTimeout = 100 * time.Millisecond
 
-			_, err := f.Fetch(testContext(t), tc.url)
+			_, err := f.Fetch(testContext(t), tc.source)
 			if !errors.Is(err, tc.want) {
 				t.Fatalf("Fetch() error = %v, want %v", err, tc.want)
 			}

internal/httpfetcher/mock.go

@@ -7,14 +7,14 @@ import (
 	"io/fs"
 	"net/http"
 	"strings"
-)
 
-// errEmptyURLPath is returned when a mock URL has no usable path.
-var errEmptyURLPath = errors.New("empty URL path")
+	"sneak.berlin/go/pixa/internal/imagerequest"
+)
 
 // MockFetcher implements Fetcher using an embedded filesystem.
 // Files are organized as: hostname/path/to/file.ext
-// URLs like https://example.com/images/photo.jpg map to example.com/images/photo.jpg.
+// The source with host example.com and path /images/photo.jpg maps to
+// example.com/images/photo.jpg; the source query is ignored.
 type MockFetcher struct {
 	fs fs.FS
 }
@@ -24,8 +24,10 @@ func NewMock(fsys fs.FS) *MockFetcher {
 	return &MockFetcher{fs: fsys}
 }
 
-// Fetch retrieves content from the mock filesystem.
-func (m *MockFetcher) Fetch(ctx context.Context, url string) (*FetchResult, error) {
+// Fetch retrieves the source's content from the mock filesystem.
+func (m *MockFetcher) Fetch(
+	ctx context.Context, source imagerequest.Source,
+) (*FetchResult, error) {
 	// Check context cancellation
 	select {
 	case <-ctx.Done():
@@ -33,11 +35,7 @@ func (m *MockFetcher) Fetch(ctx context.Context, url string) (*FetchResult, erro
 	default:
 	}
 
-	// Parse URL to get filesystem path
-	path, err := urlToFSPath(url)
-	if err != nil {
-		return nil, err
-	}
+	path := source.Host + source.Path
 
 	// Open the file
 	f, err := m.fs.Open(path)
@@ -68,32 +66,6 @@ func (m *MockFetcher) Fetch(ctx context.Context, url string) (*FetchResult, erro
 	}, nil
 }
 
-// urlToFSPath converts a URL to a filesystem path.
-// https://example.com/images/photo.jpg -> example.com/images/photo.jpg
-func urlToFSPath(rawURL string) (string, error) {
-	// Strip scheme
-	url := rawURL
-	if idx := strings.Index(url, "://"); idx != -1 {
-		url = url[idx+3:]
-	}
-
-	// Remove query string
-	if idx := strings.Index(url, "?"); idx != -1 {
-		url = url[:idx]
-	}
-
-	// Remove fragment
-	if idx := strings.Index(url, "#"); idx != -1 {
-		url = url[:idx]
-	}
-
-	if url == "" {
-		return "", errEmptyURLPath
-	}
-
-	return url, nil
-}
-
 // detectContentTypeFromPath returns the MIME type based on file extension.
 func detectContentTypeFromPath(path string) string {
 	path = strings.ToLower(path)

internal/httpfetcher/request_id_internal_test.go

@@ -34,7 +34,7 @@ func TestFetchSendsRequestID(t *testing.T) {
 
 	ctx := context.WithValue(testContext(t), middleware.RequestIDKey, requestID)
 
-	res, err := f.Fetch(ctx, upstreamURL("/image"))
+	res, err := f.Fetch(ctx, upstreamSource("/image"))
 	if err != nil {
 		t.Fatalf("Fetch() error = %v", err)
 	}

internal/imagerequest/imagerequest.go

@@ -1,13 +1,12 @@
 // Package imagerequest holds the image request type, its parts, its checks,
-// the parsing of the /v1/image/ path, the variant key and the source key. It
-// imports no other package of this repo, so every package that handles an
-// image request can import it.
+// the parsing of the /v1/image/ path, the variant key, the source key and the
+// source's upstream URL. It imports no other package of this repo, so every
+// package that handles an image request can import it.
 package imagerequest
 
 import (
 	"errors"
 	"net/url"
-	"time"
 )
 
 // ErrInvalidFitMode is returned when an invalid fit mode is provided.
@@ -85,19 +84,13 @@ func ValidateImageRequest(req *Request) error {
 }
 
 // Request represents a request for a processed image: the source to fetch
-// and the transform to apply to it.
+// and the transform to apply to it. It holds exactly what the variant key
+// covers.
 type Request struct {
 	// Source is what to fetch
 	Source Source
 	// Transform is what to do to it
 	Transform Transform
-	// Signature is the HMAC signature for non-allowlisted hosts
-	Signature string
-	// 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
 }
 
 // Source is the image a request fetches from the origin.
@@ -125,23 +118,20 @@ type Transform struct {
 	FitMode Fit
 }
 
-// SourceURL returns the full upstream URL to fetch: the source path
-// percent-encoded again, as net/url writes a path, so each source path is one
-// upstream path, and the source query as it is, with the characters
-// escapeQuery encodes percent-encoded. Uses http:// scheme when AllowHTTP is
-// true, otherwise https://.
-func (r *Request) SourceURL() string {
+// URL returns the source's upstream URL, with the scheme http when allowHTTP
+// is true and https otherwise. net/url writes its path percent-encoded again,
+// so each source path is one upstream path. Its query is the source query as
+// it is, with the characters escapeQuery encodes percent-encoded.
+func (s Source) URL(allowHTTP bool) *url.URL {
 	scheme := "https"
-	if r.AllowHTTP {
+	if allowHTTP {
 		scheme = "http"
 	}
 
-	sourceURL := url.URL{
+	return &url.URL{
 		Scheme:   scheme,
-		Host:     r.Source.Host,
-		Path:     r.Source.Path,
-		RawQuery: escapeQuery(r.Source.Query),
+		Host:     s.Host,
+		Path:     s.Path,
+		RawQuery: escapeQuery(s.Query),
 	}
-
-	return sourceURL.String()
 }

internal/imagerequest/sourceurl_internal_test.go

@@ -2,57 +2,49 @@ package imagerequest
 
 import "testing"
 
-func TestRequest_SourceURL_DefaultHTTPS(t *testing.T) {
+func TestSource_URL_DefaultHTTPS(t *testing.T) {
 	t.Parallel()
 
-	req := &Request{
-		Source: Source{
-			Host:  testHostCDN,
-			Path:  testPathCat,
-			Query: "v=2",
-		},
+	source := Source{
+		Host:  testHostCDN,
+		Path:  testPathCat,
+		Query: "v=2",
 	}
 
-	got := req.SourceURL()
+	got := source.URL(false).String()
 
 	want := "https://cdn.example.com/photos/cat.jpg?v=2"
 	if got != want {
-		t.Errorf("SourceURL() = %q, want %q", got, want)
+		t.Errorf("URL() = %q, want %q", got, want)
 	}
 }
 
-func TestRequest_SourceURL_AllowHTTP(t *testing.T) {
+func TestSource_URL_AllowHTTP(t *testing.T) {
 	t.Parallel()
 
-	req := &Request{
-		Source: Source{
-			Host: "localhost:8080",
-			Path: testPathCat,
-		},
-		AllowHTTP: true,
+	source := Source{
+		Host: "localhost:8080",
+		Path: testPathCat,
 	}
 
-	got := req.SourceURL()
+	got := source.URL(true).String()
 
 	want := "http://localhost:8080/photos/cat.jpg"
 	if got != want {
-		t.Errorf("SourceURL() = %q, want %q", got, want)
+		t.Errorf("URL() = %q, want %q", got, want)
 	}
 }
 
-func TestRequest_SourceURL_AllowHTTPFalse(t *testing.T) {
+func TestSource_URL_AllowHTTPFalse(t *testing.T) {
 	t.Parallel()
 
-	req := &Request{
-		Source: Source{
-			Host: testHostCDN,
-			Path: "/img.jpg",
-		},
-		AllowHTTP: false,
+	source := Source{
+		Host: testHostCDN,
+		Path: "/img.jpg",
 	}
 
-	got := req.SourceURL()
+	got := source.URL(false).String()
 	if got != "https://cdn.example.com/img.jpg" {
-		t.Errorf("SourceURL() = %q, want https scheme", got)
+		t.Errorf("URL() = %q, want https scheme", got)
 	}
 }

internal/imagerequest/urlparser.go

@@ -126,15 +126,15 @@ func parseImageComponents(remainder string) (*Request, error) {
 		return nil, ErrMissingHost
 	}
 
-	// Only the forms hostRegex describes are accepted. A "?", "#" or "@" in
-	// the host would make Go's URL parser, which the allowlist check and the
-	// fetcher use, read a different host from the one allowlisted and cached
+	// Later steps put the host unparsed into the allowlist check, the cache key
+	// and the upstream URL, so only the forms hostRegex describes are accepted
 	hostMatch := hostRegex.FindStringSubmatch(host)
 	if hostMatch == nil {
 		return nil, fmt.Errorf("%w: %q", ErrInvalidHost, host)
 	}
 
-	// Go's URL parser refuses brackets around anything but an IPv6 address
+	// The allowlist check and the fetcher drop the brackets, so brackets
+	// around anything but an IPv6 address would give a host a second spelling
 	if inBrackets := hostMatch[1]; inBrackets != "" {
 		addr, err := netip.ParseAddr(inBrackets)
 		if err != nil || !addr.Is6() {
@@ -147,9 +147,8 @@ func parseImageComponents(remainder string) (*Request, error) {
 	// host
 	host = strings.ToLower(host)
 
-	// Go's URL parser, which the fetcher uses, reads a "#" as the start of a
-	// fragment and drops the rest, so each text after it would be another
-	// spelling, cached under its own keys, of one upstream request
+	// A "#" starts a URL's fragment, so neither the source path nor the source
+	// query may hold one
 	if strings.Contains(path, "#") {
 		return nil, fmt.Errorf(
 			`%w: the source path or query holds a "#"`, ErrInvalidPath)

internal/imgcache/concurrent_misses_internal_test.go

@@ -36,7 +36,7 @@ type heldFetcher struct {
 }
 
 func (f *heldFetcher) Fetch(
-	ctx context.Context, url string,
+	ctx context.Context, source imagerequest.Source,
 ) (*httpfetcher.FetchResult, error) {
 	f.fetches.Add(1)
 
@@ -48,7 +48,7 @@ func (f *heldFetcher) Fetch(
 		return nil, ctx.Err()
 	}
 
-	return f.upstream.Fetch(ctx, url)
+	return f.upstream.Fetch(ctx, source)
 }
 
 // releaseFetches lets every held fetch, and every later one, go on.
@@ -79,8 +79,8 @@ func setupHeldFetchService(t *testing.T) (*Service, *TestFixtures, *heldFetcher)
 }
 
 // photoVariant asks for the test photo, 100x100, at 50x25 as a JPEG of the
-// given quality and fit mode. Each call returns a new request, as Get writes
-// to the request it is given.
+// given quality and fit mode. Each call returns a new request, which a test
+// may change.
 func photoVariant(
 	fixtures *TestFixtures, quality int, fit imagerequest.Fit,
 ) *imagerequest.Request {
@@ -489,7 +489,7 @@ func TestService_Get_WaiterGetsProcessingTimedOut(t *testing.T) {
 type panickingFetcher struct{}
 
 func (panickingFetcher) Fetch(
-	context.Context, string,
+	context.Context, imagerequest.Source,
 ) (*httpfetcher.FetchResult, error) {
 	panic("upstream fetcher panicked")
 }

internal/imgcache/service.go

@@ -7,21 +7,17 @@ import (
 	"fmt"
 	"io"
 	"log/slog"
-	"net/url"
 	"runtime/debug"
-	"strings"
 	"time"
 
 	"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"
 	"sneak.berlin/go/pixa/internal/imageprocessor"
 	"sneak.berlin/go/pixa/internal/imagerequest"
 	"sneak.berlin/go/pixa/internal/magic"
-	"sneak.berlin/go/pixa/internal/signature"
 )
 
 // Service orchestrates the cache, fetcher, and processor.
@@ -29,10 +25,7 @@ type Service struct {
 	cache           *Cache
 	fetcher         httpfetcher.Fetcher
 	processor       *imageprocessor.ImageProcessor
-	signer          *signature.Signer
-	allowlist       *allowlist.HostAllowList
 	log             *slog.Logger
-	allowHTTP       bool
 	maxResponseSize int64
 	// variantsInProgress lets the requests that miss the same variant at the
 	// same time share one fetch and one transcode.
@@ -47,13 +40,6 @@ type ServiceConfig struct {
 	Fetcher httpfetcher.Fetcher
 	// Processor converts and resizes the images
 	Processor *imageprocessor.ImageProcessor
-	// SigningKey is the HMAC signing key (empty disables signing)
-	SigningKey string
-	// Allowlist is the list of hosts that don't require signatures
-	Allowlist []string
-	// AllowHTTP makes the service fetch sources over http instead of
-	// https (testing only), so the fetcher must allow http too
-	AllowHTTP bool
 	// MaxResponseSize is the largest source accepted, in bytes; a cached
 	// source larger than this is fetched again
 	MaxResponseSize int64
@@ -61,11 +47,8 @@ type ServiceConfig struct {
 	Logger *slog.Logger
 }
 
-// Static errors for service construction.
-var (
-	errCacheRequired      = errors.New("cache is required")
-	errSigningKeyRequired = errors.New("signing key is required")
-)
+// errCacheRequired is returned when the service is built without a cache.
+var errCacheRequired = errors.New("cache is required")
 
 // NewService creates a new image service.
 func NewService(cfg *ServiceConfig) (*Service, error) {
@@ -73,10 +56,6 @@ func NewService(cfg *ServiceConfig) (*Service, error) {
 		return nil, errCacheRequired
 	}
 
-	if cfg.SigningKey == "" {
-		return nil, errSigningKeyRequired
-	}
-
 	log := cfg.Logger
 	if log == nil {
 		log = slog.Default()
@@ -93,10 +72,7 @@ func NewService(cfg *ServiceConfig) (*Service, error) {
 		cache:           cfg.Cache,
 		fetcher:         cfg.Fetcher,
 		processor:       cfg.Processor,
-		signer:          signature.New(cfg.SigningKey),
-		allowlist:       allowlist.New(cfg.Allowlist),
 		log:             log,
-		allowHTTP:       cfg.AllowHTTP,
 		maxResponseSize: cfg.MaxResponseSize,
 	}, nil
 }
@@ -123,9 +99,6 @@ var ErrProcessingTimedOut = errors.New(
 func (s *Service) Get(
 	ctx context.Context, req *imagerequest.Request,
 ) (*ImageResponse, error) {
-	// Propagate AllowHTTP setting to the request
-	req.AllowHTTP = s.allowHTTP
-
 	cacheKey := imagerequest.CacheKey(req)
 
 	reader, size, contentType, err := s.cache.GetVariant(cacheKey)
@@ -169,33 +142,6 @@ func (s *Service) WaitForProcessing(ctx context.Context) int {
 	return s.processor.WaitForProcessing(ctx)
 }
 
-// ValidateRequest validates the request signature if required.
-func (s *Service) ValidateRequest(req *imagerequest.Request) error {
-	// Check if host is allowed (no signature required)
-	sourceURL := req.SourceURL()
-
-	parsedURL, err := url.Parse(sourceURL)
-	if err != nil {
-		return fmt.Errorf("invalid source URL: %w", err)
-	}
-
-	// An allowlist entry names no port, so a host followed by ":", with or
-	// without a port, needs a signature. The port is read from the parsed
-	// URL, as the fetcher reads it: in an IPv6 address written without
-	// brackets, such as 2001:db8::1:8443, it is what follows the last ":".
-	// This is checked here and not in allowlist.IsAllowed, which also matches
-	// referer_blocklist, where a Referer must be blocked whatever its port.
-	hostHasPort := parsedURL.Port() != "" ||
-		strings.HasSuffix(parsedURL.Host, ":")
-
-	if !hostHasPort && s.allowlist.IsAllowed(parsedURL) {
-		return nil
-	}
-
-	// Signature required for non-allowed hosts
-	return s.signer.Verify(req)
-}
-
 // errPanicked is returned when processing a variant panicked.
 var errPanicked = errors.New("panic while processing image")
 
@@ -388,11 +334,10 @@ func (s *Service) fetchAndProcess(
 	}
 
 	// Fetch from upstream
-	sourceURL := req.SourceURL()
+	s.log.Debug("fetching from upstream", "host", req.Source.Host,
+		"path", req.Source.Path, "query", req.Source.Query)
 
-	s.log.Debug("fetching from upstream", "url", sourceURL)
-
-	fetchResult, err := s.fetcher.Fetch(ctx, sourceURL)
+	fetchResult, err := s.fetcher.Fetch(ctx, req.Source)
 	if err != nil {
 		// Store negative cache for certain errors
 		if isNegativeCacheable(err) {

internal/imgcache/service_internal_test.go

@@ -9,17 +9,15 @@ import (
 
 	"sneak.berlin/go/pixa/internal/imagerequest"
 	"sneak.berlin/go/pixa/internal/magic"
-	"sneak.berlin/go/pixa/internal/signature"
 )
 
 // Test data literals used repeatedly in this file (goconst).
 const (
 	testPathPhoto  = "/images/photo.jpg"
 	testPathUpload = "/uploads/image.jpg"
-	testSigningKey = "test-signing-key-12345"
 )
 
-func TestService_Get_AllowlistedHost(t *testing.T) {
+func TestService_Get_GoodHost(t *testing.T) {
 	t.Parallel()
 
 	svc, fixtures := SetupTestService(t)
@@ -60,36 +58,12 @@ func TestService_Get_AllowlistedHost(t *testing.T) {
 	}
 }
 
-func TestService_Get_NonAllowlistedHost_NoSignature(t *testing.T) {
+// TestService_Get_OtherHost checks that the image service serves a second host
+// as it serves GoodHost.
+func TestService_Get_OtherHost(t *testing.T) {
 	t.Parallel()
 
-	svc, fixtures := SetupTestService(t, WithSigningKey("test-key"))
-
-	req := &imagerequest.Request{
-		Source: imagerequest.Source{
-			Host: fixtures.OtherHost,
-			Path: testPathUpload,
-		},
-		Transform: imagerequest.Transform{
-			Size:    imagerequest.Size{Width: 50, Height: 50},
-			Format:  imagerequest.FormatJPEG,
-			Quality: 85,
-			FitMode: imagerequest.FitCover,
-		},
-	}
-
-	// Should fail validation - not allowlisted and no signature
-	err := svc.ValidateRequest(req)
-	if err == nil {
-		t.Error("ValidateRequest() expected error for non-allowlisted host without signature")
-	}
-}
-
-func TestService_Get_NonAllowlistedHost_ValidSignature(t *testing.T) {
-	t.Parallel()
-
-	signingKey := testSigningKey
-	svc, fixtures := SetupTestService(t, WithSigningKey(signingKey))
+	svc, fixtures := SetupTestService(t)
 	ctx := context.Background()
 
 	req := &imagerequest.Request{
@@ -105,17 +79,6 @@ func TestService_Get_NonAllowlistedHost_ValidSignature(t *testing.T) {
 		},
 	}
 
-	// Generate a valid signature
-	signer := signature.New(signingKey)
-	req.Expires = time.Now().Add(time.Hour)
-	req.Signature = signer.Sign(req)
-
-	// Should pass validation
-	err := svc.ValidateRequest(req)
-	if err != nil {
-		t.Errorf("ValidateRequest() error = %v", err)
-	}
-
 	// Should fetch successfully
 	resp, err := svc.Get(ctx, req)
 	if err != nil {
@@ -134,244 +97,6 @@ func TestService_Get_NonAllowlistedHost_ValidSignature(t *testing.T) {
 	}
 }
 
-func TestService_Get_NonAllowlistedHost_ExpiredSignature(t *testing.T) {
-	t.Parallel()
-
-	signingKey := testSigningKey
-	svc, fixtures := SetupTestService(t, WithSigningKey(signingKey))
-
-	req := &imagerequest.Request{
-		Source: imagerequest.Source{
-			Host: fixtures.OtherHost,
-			Path: testPathUpload,
-		},
-		Transform: imagerequest.Transform{
-			Size:    imagerequest.Size{Width: 50, Height: 50},
-			Format:  imagerequest.FormatJPEG,
-			Quality: 85,
-			FitMode: imagerequest.FitCover,
-		},
-	}
-
-	// Generate an expired signature
-	signer := signature.New(signingKey)
-	req.Expires = time.Now().Add(-time.Hour) // Already expired
-	req.Signature = signer.Sign(req)
-
-	// Should fail validation
-	err := svc.ValidateRequest(req)
-	if err == nil {
-		t.Error("ValidateRequest() expected error for expired signature")
-	}
-}
-
-func TestService_Get_NonAllowlistedHost_InvalidSignature(t *testing.T) {
-	t.Parallel()
-
-	signingKey := testSigningKey
-	svc, fixtures := SetupTestService(t, WithSigningKey(signingKey))
-
-	req := &imagerequest.Request{
-		Source: imagerequest.Source{
-			Host: fixtures.OtherHost,
-			Path: testPathUpload,
-		},
-		Transform: imagerequest.Transform{
-			Size:    imagerequest.Size{Width: 50, Height: 50},
-			Format:  imagerequest.FormatJPEG,
-			Quality: 85,
-			FitMode: imagerequest.FitCover,
-		},
-	}
-
-	// Set an invalid signature
-	req.Signature = "invalid-signature"
-	req.Expires = time.Now().Add(time.Hour)
-
-	// Should fail validation
-	err := svc.ValidateRequest(req)
-	if err == nil {
-		t.Error("ValidateRequest() expected error for invalid signature")
-	}
-}
-
-// TestService_ValidateRequest_SignatureExactHostMatch verifies that
-// ValidateRequest enforces exact host matching for signatures. A
-// signature for one host must not verify for a different host, even
-// if they share a domain suffix.
-func TestService_ValidateRequest_SignatureExactHostMatch(t *testing.T) {
-	t.Parallel()
-
-	signingKey := "test-signing-key-must-be-32-chars"
-	svc, _ := SetupTestService(t,
-		WithSigningKey(signingKey),
-		WithNoAllowlist(),
-	)
-
-	signer := signature.New(signingKey)
-
-	// Sign a request for "cdn.example.com"
-	signedReq := &imagerequest.Request{
-		Source: imagerequest.Source{
-			Host: testHostCDN,
-			Path: testPathCat,
-		},
-		Transform: imagerequest.Transform{
-			Size:    imagerequest.Size{Width: 50, Height: 50},
-			Format:  imagerequest.FormatJPEG,
-			Quality: 85,
-			FitMode: imagerequest.FitCover,
-		},
-		Expires: time.Now().Add(time.Hour),
-	}
-	signedReq.Signature = signer.Sign(signedReq)
-
-	// The original request should pass validation
-	t.Run("exact host passes", func(t *testing.T) {
-		t.Parallel()
-
-		err := svc.ValidateRequest(signedReq)
-		if err != nil {
-			t.Errorf("ValidateRequest() exact host failed: %v", err)
-		}
-	})
-
-	// Try to reuse the signature with different hosts
-	tests := []struct {
-		name string
-		host string
-	}{
-		{"parent domain", testHostExample},
-		{"sibling subdomain", "images.example.com"},
-		{"deeper subdomain", "a.cdn.example.com"},
-		{"evil suffix domain", "cdn.example.com.evil.com"},
-		{"prefixed host", "evilcdn.example.com"},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name+" rejected", func(t *testing.T) {
-			t.Parallel()
-
-			req := &imagerequest.Request{
-				Source: imagerequest.Source{
-					Host:  tt.host,
-					Path:  signedReq.Source.Path,
-					Query: signedReq.Source.Query,
-				},
-				Transform: signedReq.Transform,
-				Expires:   signedReq.Expires,
-				Signature: signedReq.Signature,
-			}
-
-			err := svc.ValidateRequest(req)
-			if err == nil {
-				t.Errorf(
-					"ValidateRequest() should reject signature for host %q (signed for %q)",
-					tt.host, signedReq.Source.Host)
-			}
-		})
-	}
-}
-
-// TestService_ValidateRequest_HostWithPortNeedsSignature verifies that a host
-// followed by a port, or by a colon with no port, matches no allowlist entry,
-// so that a request for it without a signature is refused, while the same
-// host with no port is allowlisted.
-func TestService_ValidateRequest_HostWithPortNeedsSignature(t *testing.T) {
-	t.Parallel()
-
-	svc, _ := SetupTestService(t,
-		WithSigningKey(testSigningKey),
-		WithAllowlist(testHostCDN, "203.0.113.5", "2001:db8::1"),
-	)
-
-	tests := []struct {
-		host        string
-		allowlisted bool
-	}{
-		{testHostCDN, true},
-		{"cdn.example.com:8443", false},
-		{"cdn.example.com:443", false},
-		{"cdn.example.com:", false},
-		{"203.0.113.5", true},
-		{"203.0.113.5:8080", false},
-		{"[2001:db8::1]", true},
-		{"[2001:db8::1]:8443", false},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.host, func(t *testing.T) {
-			t.Parallel()
-
-			req := &imagerequest.Request{
-				Source: imagerequest.Source{
-					Host: tt.host,
-					Path: testPathCat,
-				},
-				Transform: imagerequest.Transform{
-					Size:    imagerequest.Size{Width: 50, Height: 50},
-					Format:  imagerequest.FormatJPEG,
-					Quality: 85,
-					FitMode: imagerequest.FitCover,
-				},
-			}
-
-			err := svc.ValidateRequest(req)
-			t.Logf("ValidateRequest() for host %q with no signature: %v",
-				tt.host, err)
-
-			if allowlisted := err == nil; allowlisted != tt.allowlisted {
-				t.Errorf("host %q served without a signature = %v, want %v",
-					tt.host, allowlisted, tt.allowlisted)
-			}
-		})
-	}
-}
-
-// TestService_ValidateRequest_UnbracketedIPv6PortNeedsSignature verifies that
-// an allowlisted IPv6 address written without brackets and followed by a
-// port, or by a colon with no port, needs a signature. Go's URL parser, which
-// the fetcher uses, reads 2001:db8::1:8443 as the host 2001:db8::1 with the
-// port 8443.
-func TestService_ValidateRequest_UnbracketedIPv6PortNeedsSignature(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	svc, _ := SetupTestService(t,
-		WithSigningKey(testSigningKey),
-		WithAllowlist("2001:db8::1"),
-	)
-
-	for _, host := range []string{"2001:db8::1:8443", "2001:db8::1:"} {
-		t.Run(host, func(t *testing.T) {
-			t.Parallel()
-
-			req := &imagerequest.Request{
-				Source: imagerequest.Source{
-					Host: host,
-					Path: testPathCat,
-				},
-				Transform: imagerequest.Transform{
-					Size:    imagerequest.Size{Width: 50, Height: 50},
-					Format:  imagerequest.FormatJPEG,
-					Quality: 85,
-					FitMode: imagerequest.FitCover,
-				},
-			}
-
-			err := svc.ValidateRequest(req)
-			t.Logf("ValidateRequest() for host %q with no signature: %v",
-				host, err)
-
-			if err == nil {
-				t.Errorf("host %q served without a signature, want refused",
-					host)
-			}
-		})
-	}
-}
-
 func TestService_Get_InvalidFile(t *testing.T) {
 	t.Parallel()
 
@@ -692,33 +417,6 @@ func TestService_Get_DifferentSizes(t *testing.T) {
 	}
 }
 
-func TestService_ValidateRequest_NoSigningKey(t *testing.T) {
-	t.Parallel()
-
-	// Service with no signing key - all non-allowlisted requests should fail
-	svc, fixtures := SetupTestService(t, WithNoAllowlist())
-
-	req := &imagerequest.Request{
-		Source: imagerequest.Source{
-			Host: fixtures.OtherHost,
-			Path: testPathUpload,
-		},
-		Transform: imagerequest.Transform{
-			Size:    imagerequest.Size{Width: 50, Height: 50},
-			Format:  imagerequest.FormatJPEG,
-			Quality: 85,
-			FitMode: imagerequest.FitCover,
-		},
-	}
-
-	err := svc.ValidateRequest(req)
-	if err == nil {
-		t.Error(
-			"ValidateRequest() expected error when no signing key and host not allowlisted",
-		)
-	}
-}
-
 func TestService_Get_ContextCancellation(t *testing.T) {
 	t.Parallel()
 

internal/imgcache/stats_internal_test.go

@@ -93,7 +93,7 @@ type fakeUpstream struct {
 }
 
 func (u *fakeUpstream) Fetch(
-	context.Context, string,
+	context.Context, imagerequest.Source,
 ) (*httpfetcher.FetchResult, error) {
 	return &httpfetcher.FetchResult{
 		Content:       io.NopCloser(u),

internal/imgcache/testutil_internal_test.go

@@ -31,11 +31,11 @@ const (
 // TestFixtures contains paths to test files in the mock filesystem.
 type TestFixtures struct {
 	// Valid image files
-	GoodHostJPEG  string // allowlisted host, valid JPEG
-	GoodHostPNG   string // allowlisted host, valid PNG
-	GoodHostGIF   string // allowlisted host, valid GIF
-	OtherHostJPEG string // non-allowlisted host, valid JPEG
-	OtherHostPNG  string // non-allowlisted host, valid PNG
+	GoodHostJPEG  string // valid JPEG on GoodHost
+	GoodHostPNG   string // valid PNG on GoodHost
+	GoodHostGIF   string // valid GIF on GoodHost
+	OtherHostJPEG string // valid JPEG on OtherHost
+	OtherHostPNG  string // valid PNG on OtherHost
 
 	// Invalid/edge case files
 	InvalidFile string // file with wrong magic bytes
@@ -43,8 +43,8 @@ type TestFixtures struct {
 	TextFile    string // text file masquerading as image
 
 	// Hostnames
-	GoodHost  string // allowlisted hostname
-	OtherHost string // non-allowlisted hostname
+	GoodHost  string // a host the mock fetcher serves
+	OtherHost string // a second host the mock fetcher serves
 }
 
 // DefaultFixtures returns the standard test fixture paths.
@@ -161,22 +161,11 @@ func generateTestGIF(t *testing.T, width, height int, c color.Color) []byte {
 }
 
 // SetupTestService creates a Service with mock fetcher for testing.
-func SetupTestService(
-	t *testing.T, opts ...TestServiceOption,
-) (*Service, *TestFixtures) {
+func SetupTestService(t *testing.T) (*Service, *TestFixtures) {
 	t.Helper()
 
 	mockFS, fixtures := NewTestFS(t)
 
-	cfg := &testServiceConfig{
-		allowlist:  []string{fixtures.GoodHost},
-		signingKey: "test-signing-key-must-be-32-chars",
-	}
-
-	for _, opt := range opts {
-		opt(cfg)
-	}
-
 	// Create temp directory for cache
 	tmpDir := t.TempDir()
 
@@ -199,8 +188,6 @@ func SetupTestService(
 			MaxInputBytes:           50 << 20,
 			MaxConcurrentProcessing: 2,
 		}),
-		SigningKey:      cfg.signingKey,
-		Allowlist:       cfg.allowlist,
 		MaxResponseSize: 50 << 20,
 	})
 	if err != nil {
@@ -228,32 +215,3 @@ func setupServiceTestDB(t *testing.T) *sql.DB {
 
 	return db
 }
-
-type testServiceConfig struct {
-	allowlist  []string
-	signingKey string
-}
-
-// TestServiceOption configures the test service.
-type TestServiceOption func(*testServiceConfig)
-
-// WithAllowlist sets the allowlist for the test service.
-func WithAllowlist(hosts ...string) TestServiceOption {
-	return func(c *testServiceConfig) {
-		c.allowlist = hosts
-	}
-}
-
-// WithSigningKey sets the signing key for the test service.
-func WithSigningKey(key string) TestServiceOption {
-	return func(c *testServiceConfig) {
-		c.signingKey = key
-	}
-}
-
-// WithNoAllowlist removes all allowlisted hosts.
-func WithNoAllowlist() TestServiceOption {
-	return func(c *testServiceConfig) {
-		c.allowlist = nil
-	}
-}

internal/providers/providers.go

@@ -212,21 +212,15 @@ type imageServiceParams struct {
 }
 
 // newImageService builds the image service from the cache, the fetcher
-// and the processor, with the signing key, the allowlist and the upstream
-// settings of the config.
+// and the processor, with the largest upstream response the config allows.
 func newImageService(
 	_ fx.Lifecycle, params imageServiceParams,
 ) (*imgcache.Service, error) {
-	cfg := params.Config
-
 	return imgcache.NewService(&imgcache.ServiceConfig{
 		Cache:           params.Cache,
 		Fetcher:         params.Fetcher,
 		Processor:       params.Processor,
-		SigningKey:      cfg.SigningKey,
-		Allowlist:       cfg.AllowlistHosts,
-		AllowHTTP:       cfg.AllowHTTP,
-		MaxResponseSize: cfg.UpstreamMaxResponseSize,
+		MaxResponseSize: params.Config.UpstreamMaxResponseSize,
 		Logger:          params.Logger.Get(),
 	})
 }

internal/server/computed_side_internal_test.go

@@ -7,6 +7,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"testing"
+	"time"
 
 	"sneak.berlin/go/pixa/internal/encurl"
 	"sneak.berlin/go/pixa/internal/imagerequest"
@@ -76,7 +77,7 @@ func TestImageProxyRefusesComputedSideAboveMaximum(t *testing.T) {
 				Size:   imagerequest.Size{Width: tt.width, Height: tt.height},
 				Format: imagerequest.FormatJPEG,
 			},
-		})
+		}, time.Time{})
 		if err != nil {
 			t.Fatalf("encurl Generate: %v", err)
 		}

internal/server/fit_outside_internal_test.go

@@ -5,6 +5,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"testing"
+	"time"
 
 	"sneak.berlin/go/pixa/internal/encurl"
 	"sneak.berlin/go/pixa/internal/imagerequest"
@@ -66,7 +67,7 @@ func TestImageProxyRefusesFitOutsideAboveMaximum(t *testing.T) {
 				Format:  imagerequest.FormatJPEG,
 				FitMode: imagerequest.FitOutside,
 			},
-		})
+		}, time.Time{})
 		if err != nil {
 			t.Fatalf("encurl Generate: %v", err)
 		}

internal/server/source_url_internal_test.go

@@ -524,8 +524,7 @@ func signedImageURL(t *testing.T, sourceHost string) string {
 			Quality: 85,
 			FitMode: imagerequest.FitCover,
 		},
-		Expires: expires,
-	})
+	}, expires)
 
 	// Go's percent-encoding of a URL path writes the brackets of the IPv6
 	// host as %5B and %5D.
@@ -549,7 +548,7 @@ func encryptedImageURL(t *testing.T, sourceHost string) string {
 	token, err := generator.Generate(&imagerequest.Request{
 		Source:    imagerequest.Source{Host: sourceHost, Path: ipv6SourcePath},
 		Transform: imagerequest.Transform{Format: imagerequest.FormatPNG},
-	})
+	}, time.Time{})
 	if err != nil {
 		t.Fatalf("encurl Generate: %v", err)
 	}

internal/server/svg_as_avif_internal_test.go

@@ -6,6 +6,7 @@ import (
 	"net/http/httptest"
 	"strings"
 	"testing"
+	"time"
 
 	"sneak.berlin/go/pixa/internal/encurl"
 	"sneak.berlin/go/pixa/internal/imagerequest"
@@ -45,7 +46,7 @@ func TestImageProxyRefusesSVGServedAsAVIF(t *testing.T) {
 				Size:   imagerequest.Size{Width: 32, Height: 24},
 				Format: format,
 			},
-		})
+		}, time.Time{})
 		if err != nil {
 			t.Fatalf("encurl Generate: %v", err)
 		}

internal/signature/escaping_test.go

@@ -28,20 +28,20 @@ func TestSigner_Verify_ColonCannotMovePathQueryBoundary(t *testing.T) {
 			Quality: 85,
 			FitMode: testFitCover,
 		},
-		Expires: time.Now().Add(1 * time.Hour),
 	}
-	signed.Signature = signer.Sign(signed)
+	expires := time.Now().Add(1 * time.Hour)
+	sig := signer.Sign(signed, expires)
 
 	replayed := *signed
 	replayed.Source.Path = "/a"
 	replayed.Source.Query = "b:"
 
 	t.Logf("signature for path %q query %q: %s",
-		signed.Source.Path, signed.Source.Query, signed.Signature)
+		signed.Source.Path, signed.Source.Query, sig)
 	t.Logf("signature for path %q query %q: %s",
-		replayed.Source.Path, replayed.Source.Query, signer.Sign(&replayed))
+		replayed.Source.Path, replayed.Source.Query, signer.Sign(&replayed, expires))
 
-	err := signer.Verify(&replayed)
+	err := signer.Verify(&replayed, sig, expires)
 	if !errors.Is(err, signature.ErrInvalid) {
 		t.Errorf("Verify() of path %q query %q with the signature for "+
 			"path %q = %v, want %v",
@@ -106,10 +106,9 @@ func TestSigner_EscapedGoldenVectors(t *testing.T) {
 					Quality: 85,
 					FitMode: testFitCover,
 				},
-				Expires: time.Unix(goldenExpiresUnix, 0),
 			}
 
-			gotSignature := signer.Sign(req)
+			gotSignature := signer.Sign(req, time.Unix(goldenExpiresUnix, 0))
 			t.Logf("Sign() of path %q query %q = %s", tt.path, tt.query, gotSignature)
 
 			if gotSignature != tt.wantSignature {

internal/signature/golden_test.go

@@ -19,8 +19,8 @@ type goldenVector struct {
 	name string
 	req  imagerequest.Request
 	// wantSignature is the exact base64url (RFC 4648 URL-safe,
-	// padded) HMAC-SHA256 signature for the request with Expires
-	// set to goldenExpiresUnix.
+	// padded) HMAC-SHA256 signature for the request expiring at
+	// goldenExpiresUnix.
 	wantSignature string
 }
 
@@ -124,9 +124,8 @@ func TestSigner_GoldenVectors(t *testing.T) {
 			t.Parallel()
 
 			signReq := tt.req
-			signReq.Expires = time.Unix(goldenExpiresUnix, 0)
 
-			gotSignature := signer.Sign(&signReq)
+			gotSignature := signer.Sign(&signReq, time.Unix(goldenExpiresUnix, 0))
 			if gotSignature != tt.wantSignature {
 				t.Errorf("Sign() = %q, want %q (signed byte format changed?)",
 					gotSignature, tt.wantSignature)

internal/signature/quality_fit_test.go

@@ -9,10 +9,12 @@ import (
 	"sneak.berlin/go/pixa/internal/signature"
 )
 
-// signedQualityFitRequest returns a request signed for quality 85 and fit
-// mode "cover", the effective defaults the handler applies before
-// verification.
-func signedQualityFitRequest(signer *signature.Signer) *imagerequest.Request {
+// signedQualityFitRequest returns a request, its signature for quality 85 and
+// fit mode "cover", the effective defaults the handler applies before
+// verification, and the expiry it was signed with.
+func signedQualityFitRequest(
+	signer *signature.Signer,
+) (*imagerequest.Request, string, time.Time) {
 	req := &imagerequest.Request{
 		Source: imagerequest.Source{
 			Host: testHost,
@@ -24,11 +26,10 @@ func signedQualityFitRequest(signer *signature.Signer) *imagerequest.Request {
 			Quality: 85,
 			FitMode: testFitCover,
 		},
-		Expires: time.Now().Add(1 * time.Hour),
 	}
-	req.Signature = signer.Sign(req)
+	expires := time.Now().Add(1 * time.Hour)
 
-	return req
+	return req, signer.Sign(req, expires), expires
 }
 
 // TestSigner_Verify_QualityAndFitAreSigned proves that quality and fit are
@@ -60,10 +61,10 @@ func TestSigner_Verify_QualityAndFitAreSigned(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 
-			req := signedQualityFitRequest(signer)
+			req, sig, expires := signedQualityFitRequest(signer)
 			tt.tamper(req)
 
-			err := signer.Verify(req)
+			err := signer.Verify(req, sig, expires)
 			if !errors.Is(err, signature.ErrInvalid) {
 				t.Errorf("Verify() = %v, want %v", err, signature.ErrInvalid)
 			}

internal/signature/signature.go

@@ -34,14 +34,14 @@ func New(secretKey string) *Signer {
 	}
 }
 
-// Sign generates an HMAC-SHA256 signature for the given request.
-// The signature covers: host + path + query + width + height + format +
-// expiration + quality + fit. The quality and fit mode are the effective
-// values the request resolves to: callers pass the defaults when the request
-// omits those parameters, so an omitted quality or fit mode signs
+// Sign generates an HMAC-SHA256 signature for the given request, expiring at
+// expires. The signature covers: host + path + query + width + height +
+// format + expiration + quality + fit. The quality and fit mode are the
+// effective values the request resolves to: callers pass the defaults when
+// the request omits those parameters, so an omitted quality or fit mode signs
 // identically to that same value stated explicitly.
-func (s *Signer) Sign(req *imagerequest.Request) string {
-	data := s.buildSignatureData(req)
+func (s *Signer) Sign(req *imagerequest.Request, expires time.Time) string {
+	data := s.buildSignatureData(req, expires)
 	mac := hmac.New(sha256.New, s.secretKey)
 	mac.Write([]byte(data))
 	sig := mac.Sum(nil)
@@ -49,28 +49,31 @@ func (s *Signer) Sign(req *imagerequest.Request) string {
 	return base64.URLEncoding.EncodeToString(sig)
 }
 
-// Verify checks if the signature on the request is valid and not expired.
+// Verify checks that sig is the signature of the request expiring at expires,
+// and that expires has not passed.
 // Signatures are exact-match only: every component of the signed data
 // (host, path, query, dimensions, format, expiration, quality, fit) must
 // match exactly.
 // No suffix matching, wildcard matching, or partial matching is supported.
 // A signature for "cdn.example.com" will NOT verify for "example.com" or
 // "other.cdn.example.com", and vice versa.
-func (s *Signer) Verify(req *imagerequest.Request) error {
+func (s *Signer) Verify(
+	req *imagerequest.Request, sig string, expires time.Time,
+) error {
 	// Check expiration first
-	if req.Expires.IsZero() {
+	if expires.IsZero() {
 		return ErrMissingExpiration
 	}
 
-	if time.Now().After(req.Expires) {
+	if time.Now().After(expires) {
 		return ErrExpired
 	}
 
 	// Compute expected signature
-	expected := s.Sign(req)
+	expected := s.Sign(req, expires)
 
 	// Constant-time comparison to prevent timing attacks
-	if !hmac.Equal([]byte(req.Signature), []byte(expected)) {
+	if !hmac.Equal([]byte(sig), []byte(expected)) {
 		return ErrInvalid
 	}
 
@@ -85,7 +88,9 @@ func (s *Signer) Verify(req *imagerequest.Request) error {
 // expansion is performed. Quality and fit are the effective transform
 // values, so replaying a signed URL with a different quality or fit mode
 // fails verification.
-func (s *Signer) buildSignatureData(req *imagerequest.Request) string {
+func (s *Signer) buildSignatureData(
+	req *imagerequest.Request, expires time.Time,
+) string {
 	return fmt.Sprintf("%s:%s:%s:%d:%d:%s:%d:%d:%s",
 		req.Source.Host,
 		escapePercentAndColon(req.Source.Path),
@@ -93,7 +98,7 @@ func (s *Signer) buildSignatureData(req *imagerequest.Request) string {
 		req.Transform.Size.Width,
 		req.Transform.Size.Height,
 		req.Transform.Format,
-		req.Expires.Unix(),
+		expires.Unix(),
 		req.Transform.Quality,
 		req.Transform.FitMode,
 	)

internal/signature/signature_test.go

@@ -34,11 +34,11 @@ func TestSigner_Sign(t *testing.T) {
 			Size:   imagerequest.Size{Width: 800, Height: 600},
 			Format: testFormatWebP,
 		},
-		Expires: time.Unix(1704067200, 0), // Fixed timestamp for reproducibility
 	}
+	expires := time.Unix(1704067200, 0) // Fixed timestamp for reproducibility
 
-	sig1 := signer.Sign(req)
-	sig2 := signer.Sign(req)
+	sig1 := signer.Sign(req, expires)
+	sig2 := signer.Sign(req, expires)
 
 	// Same input should produce same signature
 	if sig1 != sig2 {
@@ -62,18 +62,17 @@ func TestSigner_Sign(t *testing.T) {
 			Size:   imagerequest.Size{Width: 800, Height: 600},
 			Format: testFormatWebP,
 		},
-		Expires: time.Unix(1704067200, 0),
 	}
 
-	sig3 := signer.Sign(req2)
+	sig3 := signer.Sign(req2, expires)
 	if sig1 == sig3 {
 		t.Error("Sign() produced same signature for different input")
 	}
 }
 
-// validVerifyRequest returns a fully-populated request that verifies
-// successfully once signed.
-func validVerifyRequest() *imagerequest.Request {
+// validVerifyRequest returns a fully-populated request, and an expiry an hour
+// ahead, that verify successfully once signed.
+func validVerifyRequest() (*imagerequest.Request, time.Time) {
 	return &imagerequest.Request{
 		Source: imagerequest.Source{
 			Host: testHost,
@@ -83,13 +82,13 @@ func validVerifyRequest() *imagerequest.Request {
 			Size:   imagerequest.Size{Width: 800, Height: 600},
 			Format: testFormatWebP,
 		},
-		Expires: time.Now().Add(1 * time.Hour),
-	}
+	}, time.Now().Add(1 * time.Hour)
 }
 
 type verifyCase struct {
-	name    string
-	setup   func() *imagerequest.Request
+	name string
+	// setup returns the request, the signature and the expiry to verify
+	setup   func() (*imagerequest.Request, string, time.Time)
 	wantErr error
 }
 
@@ -97,54 +96,49 @@ func verifyCases(signer *signature.Signer) []verifyCase {
 	return []verifyCase{
 		{
 			name: "valid signature",
-			setup: func() *imagerequest.Request {
-				req := validVerifyRequest()
-				req.Signature = signer.Sign(req)
+			setup: func() (*imagerequest.Request, string, time.Time) {
+				req, expires := validVerifyRequest()
 
-				return req
+				return req, signer.Sign(req, expires), expires
 			},
 			wantErr: nil,
 		},
 		{
 			name: "expired signature",
-			setup: func() *imagerequest.Request {
-				req := validVerifyRequest()
-				req.Expires = time.Now().Add(-1 * time.Hour)
-				req.Signature = signer.Sign(req)
+			setup: func() (*imagerequest.Request, string, time.Time) {
+				req, _ := validVerifyRequest()
+				expires := time.Now().Add(-1 * time.Hour)
 
-				return req
+				return req, signer.Sign(req, expires), expires
 			},
 			wantErr: signature.ErrExpired,
 		},
 		{
 			name: "invalid signature",
-			setup: func() *imagerequest.Request {
-				req := validVerifyRequest()
-				req.Signature = "invalid-signature"
+			setup: func() (*imagerequest.Request, string, time.Time) {
+				req, expires := validVerifyRequest()
 
-				return req
+				return req, "invalid-signature", expires
 			},
 			wantErr: signature.ErrInvalid,
 		},
 		{
 			name: "missing expiration",
-			setup: func() *imagerequest.Request {
-				req := validVerifyRequest()
-				req.Expires = time.Time{}
-				req.Signature = "some-signature"
+			setup: func() (*imagerequest.Request, string, time.Time) {
+				req, _ := validVerifyRequest()
 
-				return req
+				return req, "some-signature", time.Time{}
 			},
 			wantErr: signature.ErrMissingExpiration,
 		},
 		{
 			name: "tampered request",
-			setup: func() *imagerequest.Request {
-				req := validVerifyRequest()
-				req.Signature = signer.Sign(req)
+			setup: func() (*imagerequest.Request, string, time.Time) {
+				req, expires := validVerifyRequest()
+				sig := signer.Sign(req, expires)
 				req.Source.Path = "/photos/secret.jpg"
 
-				return req
+				return req, sig, expires
 			},
 			wantErr: signature.ErrInvalid,
 		},
@@ -160,8 +154,8 @@ func TestSigner_Verify(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 
-			req := tt.setup()
-			err := signer.Verify(req)
+			req, sig, expires := tt.setup()
+			err := signer.Verify(req, sig, expires)
 
 			if tt.wantErr == nil {
 				if err != nil {
@@ -254,8 +248,9 @@ func TestSigner_Verify_ExactMatchOnly(t *testing.T) {
 
 	signer := signature.New("test-secret-key")
 
-	// Base request that we'll sign, then tamper with individual fields.
-	baseReq := func() *imagerequest.Request {
+	// Base request that we'll sign, then tamper with individual fields; it
+	// is returned with its signature and expiry.
+	baseReq := func() (*imagerequest.Request, string, time.Time) {
 		req := &imagerequest.Request{
 			Source: imagerequest.Source{
 				Host:  testHost,
@@ -266,21 +261,20 @@ func TestSigner_Verify_ExactMatchOnly(t *testing.T) {
 				Size:   imagerequest.Size{Width: 800, Height: 600},
 				Format: testFormatWebP,
 			},
-			Expires: time.Now().Add(1 * time.Hour),
 		}
-		req.Signature = signer.Sign(req)
+		expires := time.Now().Add(1 * time.Hour)
 
-		return req
+		return req, signer.Sign(req, expires), expires
 	}
 
 	for _, tt := range exactMatchTamperCases() {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 
-			req := baseReq()
+			req, sig, expires := baseReq()
 			tt.tamper(req)
 
-			err := signer.Verify(req)
+			err := signer.Verify(req, sig, expires)
 			if !errors.Is(err, signature.ErrInvalid) {
 				t.Errorf("Verify() = %v, want %v", err, signature.ErrInvalid)
 			}
@@ -291,9 +285,9 @@ func TestSigner_Verify_ExactMatchOnly(t *testing.T) {
 	t.Run("unmodified request passes", func(t *testing.T) {
 		t.Parallel()
 
-		req := baseReq()
+		req, sig, expires := baseReq()
 
-		err := signer.Verify(req)
+		err := signer.Verify(req, sig, expires)
 		if err != nil {
 			t.Errorf("Verify() unmodified request failed: %v", err)
 		}
@@ -329,10 +323,9 @@ func TestSigner_Sign_ExactHostInData(t *testing.T) {
 				Size:   imagerequest.Size{Width: 800, Height: 600},
 				Format: testFormatWebP,
 			},
-			Expires: time.Unix(1704067200, 0),
 		}
 
-		sig := signer.Sign(req)
+		sig := signer.Sign(req, time.Unix(1704067200, 0))
 		if existing, ok := sigs[sig]; ok {
 			t.Errorf("hosts %q and %q produced the same signature", existing, host)
 		}
@@ -356,20 +349,20 @@ func TestSigner_DifferentKeys(t *testing.T) {
 			Size:   imagerequest.Size{Width: 800, Height: 600},
 			Format: testFormatWebP,
 		},
-		Expires: time.Now().Add(1 * time.Hour),
 	}
+	expires := time.Now().Add(1 * time.Hour)
 
 	// Sign with key 1
-	req.Signature = signer1.Sign(req)
+	sig := signer1.Sign(req, expires)
 
 	// Verify with key 1 should succeed
-	err := signer1.Verify(req)
+	err := signer1.Verify(req, sig, expires)
 	if err != nil {
 		t.Errorf("Verify() with same key failed: %v", err)
 	}
 
 	// Verify with key 2 should fail
-	err = signer2.Verify(req)
+	err = signer2.Verify(req, sig, expires)
 	if !errors.Is(err, signature.ErrInvalid) {
 		t.Errorf("Verify() with different key should fail, got: %v", err)
 	}
@@ -395,12 +388,11 @@ func TestSign_ReadmeQueryExample(t *testing.T) {
 			Quality: 85,
 			FitMode: testFitCover,
 		},
-		Expires: time.Unix(1704067200, 0),
 	}
 
 	const wantSig = "37aFa9GSVCZM1kRY9SzMOk2fOg_M7Wt5gKNR4xM-FYA="
 
-	gotSig := signer.Sign(&req)
+	gotSig := signer.Sign(&req, time.Unix(1704067200, 0))
 	if gotSig != wantSig {
 		t.Errorf("Sign() = %q, want %q", gotSig, wantSig)
 	}