Test that logging out needs the form's CSRF token (part of #256)

A GET /logout, or a POST /logout without the CSRF token, must end no
login session; the generator page must carry a logout form with the
token; and after a logout from that form, a login opens the generator
page while a cookie copied before it is refused, also after a restart.

Model: opus-5-5

README.md

@@ -204,9 +204,10 @@ path under `/v1/` answers 200, in maintenance mode too.
   `image_requests_per_minute` (below); 415 when the upstream answers with a
   `Content-Type` that is not one of the source formats (below), or with an image
   whose first bytes libvips reads as another format; 502 when the upstream
-  answered with an error status, and for 5 minutes after that for the same
-  source URL; 503 when pixa is busy or in maintenance mode; 500 for any other
-  failure.
+  answered with an error status, and for 5 minutes after that for an image of
+  the same source URL that pixa can neither serve from its cache nor make from
+  the source image still in its cache; 503 when pixa is busy or in maintenance
+  mode; 500 for any other failure.
 - `GET` or `HEAD` `/v1/e/<token>/<name>` — an image through an encrypted URL
   (see Encrypted URLs). Needs: nothing but the URL. Answers: 200; 304 when
   `If-None-Match` matches the image's `ETag`; 400 for a token that does not
@@ -426,9 +427,14 @@ HMAC-SHA256(secret, "host:path:query:width:height:format:expiration:quality:fit"
 
 Where:
 
-- `host` — source origin hostname (e.g. `cdn.example.com`)
-- `path` — source path (e.g. `/photos/cat.jpg`)
-- `query` — source query string, empty string if none
+- `host` — source host as the URL writes it up to the first `/`, in lower case,
+  keeping an IPv6 address's brackets and any `:` and port after it (e.g.
+  `cdn.example.com`, `cdn.example.com:8443`, `[2001:db8::1]:8443`)
+- `path` — source path (e.g. `/photos/cat.jpg`), with each `%` written as `%25`
+  and each `:` as `%3A`, so `/photos/a:b.jpg` is signed as `/photos/a%3Ab.jpg`
+- `query` — source query string, empty string if none, with `%` and `:` written
+  as in `path`; neither then holds a `:`, so path `/a:b` with no query and path
+  `/a` with query `b:` are signed differently
 - `width` — requested width in pixels, `0` for original
 - `height` — requested height in pixels, `0` for original
 - `format` — output format, one of those listed under Routes, with `original`
@@ -478,6 +484,12 @@ entry that is neither a host name (letters, digits, hyphens, underscores and
 dots, with at most one leading dot) nor an IP address, such as one with a port
 or a `*.` wildcard, aborts startup.
 
+pixa lower-cases the host of a `/v1/image/` URL before anything else uses it, so
+`CDN.Example.com` is matched, fetched, cached and counted against
+`upstream_connections_per_host` as `cdn.example.com`. Such a URL whose host has
+a port, as in `cdn.example.com:8443`, or a `:` with no port, needs a signature
+even when the host is allowlisted. A host holding `@` is refused with 400.
+
 ### Configuration
 
 Every setting can be given as an environment variable, in a YAML config file

TODO.md

@@ -30,6 +30,46 @@ P2: security: per-origin rate limiting
 
 # Completed Steps
 
+- 2026-10-15 expired negative cache rows are removed on a schedule (closes
+  #244): the row of a failed fetch, which makes pixa answer 502 for that source
+  URL for 5 minutes, was deleted only when the same URL was requested again
+  after that, so a client asking for a new missing path each time added rows
+  that stayed for good. The eviction goroutine now calls `CleanExpired` at
+  startup and on each periodic tick, every 5 minutes in pixad, so a row stays at
+  most about 10 minutes. `CleanExpired` deletes at most 1000 rows per statement,
+  so a request waits behind 1000 deletions at most, however many rows have
+  expired. The goroutine runs with the disk cache disabled
+  (`cache_max_bytes: 0`) too, where it does nothing else, as the negative cache
+  is used either way.
+- 2026-10-15 a cached variant is served while its source URL is in the negative
+  cache (closes #245): `Get` checked the negative cache before the variant
+  cache, so once a fetch of the source failed, as one does when another variant
+  misses after the source was evicted and the upstream is down, every variant of
+  that source answered 502 for 5 minutes, also one on disk or one that could be
+  made from the cached source. The negative cache is now checked only just
+  before a fetch from the upstream, so a cached variant is served and a variant
+  not in the cache is made from the cached source when there is one. `README.md`
+  says so under `/v1/image/`. A refusal from the negative cache now counts as a
+  miss, as any other request that misses the variant cache does.
+- 2026-10-15 the host of a `/v1/image/` URL is lower-cased, and one with a port
+  needs a signature (closes #246): with `cdn.example.com` allowlisted,
+  `CDN.Example.com` and `cdn.example.com:8443` passed the allowlist too, and
+  each spelling was fetched and cached on its own and had connections of its own
+  under `upstream_connections_per_host`. The image URL parser now lower-cases
+  the host and refuses one holding `@`, which a URL reads as a user name and
+  password, with 400. A host with a port, or with a `:` and no port, needs a
+  signature even when it is allowlisted. That is checked in the image service,
+  not in `allowlist.IsAllowed`, which also matches `referer_blocklist`, where a
+  `Referer` is blocked whatever its port. A URL is signed over its host as it
+  writes it, in lower case, with any IPv6 brackets, `:` and port kept.
+- 2026-10-15 a `:` in the source path or query no longer moves where the path
+  ends and the query begins (closes #247): the signed data and the cache key
+  joined host, path and query with `:` and no escaping, so a URL signed for
+  `https://h/a:b` verified for `https://h/a?b:`; the two also shared a cache
+  key, and whichever was fetched first was served for both. In both, the path
+  and the query now have each `%` written as `%25` and each `:` as `%3A`; a path
+  and query holding neither are signed and keyed as before. Signature
+  Specification in `README.md` says so.
 - 2026-10-15 logging out ends every login session (closes #256): a login session
   was a cookie with nothing kept on the server, so `/logout` removed it from one
   browser and a copy of it kept working for its 30 days. Every login is made

internal/handlers/auth_session_internal_test.go

@@ -185,7 +185,7 @@ func TestHandleLogout_ClearsSessionCookie(t *testing.T) {
 	}
 }
 
-// TestHandleLogout_EndsCopiedSession verifies that after GET /logout a copy
+// TestHandleLogout_EndsCopiedSession verifies that after POST /logout a copy
 // of the session cookie taken before it shows the login page, not the
 // generator page.
 func TestHandleLogout_EndsCopiedSession(t *testing.T) {
@@ -196,14 +196,14 @@ func TestHandleLogout_EndsCopiedSession(t *testing.T) {
 	copied := newSessionCookie(t, h)
 
 	req := httptest.NewRequestWithContext(
-		t.Context(), http.MethodGet, "/logout", nil)
+		t.Context(), http.MethodPost, "/logout", nil)
 	req.AddCookie(copied)
 
 	rec := httptest.NewRecorder()
 	h.HandleLogout().ServeHTTP(rec, req)
 
 	if rec.Code != http.StatusSeeOther {
-		t.Fatalf("GET /logout status = %d, want %d",
+		t.Fatalf("POST /logout status = %d, want %d",
 			rec.Code, http.StatusSeeOther)
 	}
 
@@ -223,7 +223,7 @@ func TestHandleLogout_EndsCopiedSession(t *testing.T) {
 }
 
 // TestHandleLogout_SaveFails_Answers500 verifies that when the logout cannot
-// be saved in the database, GET /logout answers 500, leaves the session
+// be saved in the database, POST /logout answers 500, leaves the session
 // cookie in place and ends no session.
 func TestHandleLogout_SaveFails_Answers500(t *testing.T) {
 	t.Parallel()
@@ -248,17 +248,17 @@ func TestHandleLogout_SaveFails_Answers500(t *testing.T) {
 	}
 
 	req := httptest.NewRequestWithContext(
-		t.Context(), http.MethodGet, "/logout", nil)
+		t.Context(), http.MethodPost, "/logout", nil)
 	req.AddCookie(cookie)
 
 	rec := httptest.NewRecorder()
 	h.HandleLogout().ServeHTTP(rec, req)
 
-	t.Logf("GET /logout: status %d, Set-Cookie %q",
+	t.Logf("POST /logout: status %d, Set-Cookie %q",
 		rec.Code, rec.Header().Get("Set-Cookie"))
 
 	if rec.Code != http.StatusInternalServerError {
-		t.Errorf("GET /logout status = %d, want %d",
+		t.Errorf("POST /logout status = %d, want %d",
 			rec.Code, http.StatusInternalServerError)
 	}
 

internal/handlers/handlers.go

@@ -124,7 +124,8 @@ func (s *Handlers) initImageService() error {
 	s.imgCache = cache
 
 	// Background eviction: startup reconciliation, then periodic and
-	// write-pressure passes. No-op when the disk cache is disabled.
+	// write-pressure passes, and the removal of expired negative cache
+	// rows, which is all it does when the disk cache is disabled.
 	cache.StartEviction(imgcache.DefaultEvictionInterval)
 
 	// Writes the counts requests could not write by their deadline

internal/imgcache/cache.go

@@ -32,6 +32,12 @@ const httpStatusOK = 200
 // variant not among them is served as before, reading its .meta file.
 const metaCacheSize = 10000
 
+// cleanExpiredBatchSize is the most expired negative cache rows one
+// statement of CleanExpired deletes. Each batch is a statement of its
+// own, so a request waits behind one batch at most, however many rows
+// have expired.
+const cleanExpiredBatchSize = 1000
+
 // CacheConfig holds cache configuration.
 type CacheConfig struct {
 	StateDir    string
@@ -50,8 +56,9 @@ type CacheConfig struct {
 
 	// DisableDiskCache turns the disk cache off entirely: no cache
 	// directories are created, lookups always miss, stores are
-	// no-ops, and no eviction machinery runs. The config layer sets
-	// this when the operator configures cache_max_bytes: 0.
+	// no-ops, and the eviction goroutine only removes the expired
+	// negative cache rows. The config layer sets this when the
+	// operator configures cache_max_bytes: 0.
 	DisableDiskCache bool
 
 	// Logger receives accounting and eviction log output. A nil
@@ -482,17 +489,27 @@ func (c *Cache) GetSourceContent(
 	return c.srcContent.LoadWithSize(contentHash)
 }
 
-// CleanExpired removes expired entries from the cache.
+// CleanExpired removes the negative cache rows past their expiry, one
+// batch per statement, until none is left. Once ctx ends it starts no
+// further batch and returns ctx's error. The eviction goroutine calls it
+// at startup and on every periodic tick, so a row stays about
+// NegativeTTL plus one eviction interval, whether or not its URL is
+// requested again.
 func (c *Cache) CleanExpired(ctx context.Context) error {
-	// Clean expired negative cache entries
-	_, err := c.db.ExecContext(ctx, `
-		DELETE FROM negative_cache WHERE expires_at < CURRENT_TIMESTAMP
-	`)
-	if err != nil {
-		return fmt.Errorf("failed to clean negative cache: %w", err)
-	}
+	for {
+		if ctx.Err() != nil {
+			return ctx.Err()
+		}
 
-	return nil
+		deleted, err := c.cleanExpiredBatch(ctx)
+		if err != nil {
+			return err
+		}
+
+		if deleted < cleanExpiredBatchSize {
+			return nil
+		}
+	}
 }
 
 // Stats returns cache statistics.
@@ -756,3 +773,27 @@ func (c *Cache) checkNegativeCache(
 
 	return true, nil
 }
+
+// cleanExpiredBatch deletes up to cleanExpiredBatchSize negative cache
+// rows past their expiry in one statement, and returns how many it
+// deleted. DELETE ... LIMIT needs a SQLite build option, so the subquery
+// picks the rows.
+func (c *Cache) cleanExpiredBatch(ctx context.Context) (int64, error) {
+	result, err := c.db.ExecContext(ctx, `
+		DELETE FROM negative_cache WHERE id IN (
+			SELECT id FROM negative_cache
+			WHERE expires_at < CURRENT_TIMESTAMP
+			LIMIT ?
+		)
+	`, cleanExpiredBatchSize)
+	if err != nil {
+		return 0, fmt.Errorf("failed to clean negative cache: %w", err)
+	}
+
+	deleted, err := result.RowsAffected()
+	if err != nil {
+		return 0, fmt.Errorf("failed to count cleaned negative cache rows: %w", err)
+	}
+
+	return deleted, nil
+}

internal/imgcache/eviction.go

@@ -437,14 +437,16 @@ func (c *Cache) notifyWritePressure() {
 }
 
 // StartEviction launches the background eviction goroutine, which
-// reconciles the database accounting with the cache directories at
-// startup and again on every periodic tick thereafter, and evicts to
-// the configured limit on the given periodic interval and on
-// write-pressure notifications. It is a no-op on a disabled cache or
-// when already started. The goroutine outlives the caller, so it runs
-// with its own context, which StopEviction cancels.
+// reconciles the database accounting with the cache directories and
+// removes the expired negative cache rows at startup and again on every
+// periodic tick thereafter, and evicts to the configured limit on the
+// given periodic interval and on write-pressure notifications. On a
+// disabled cache it only removes the expired negative cache rows, as
+// the negative cache is used either way. It is a no-op when already
+// started. The goroutine outlives the caller, so it runs with its own
+// context, which StopEviction cancels.
 func (c *Cache) StartEviction(interval time.Duration) {
-	if c.disabled || c.evictionCancel != nil {
+	if c.evictionCancel != nil {
 		return
 	}
 
@@ -480,6 +482,7 @@ func (c *Cache) evictionLoop(ctx context.Context, interval time.Duration) {
 	defer close(c.evictionDone)
 
 	c.runReconciliationPass(ctx)
+	c.runCleanExpired(ctx)
 	c.runEvictionPass(ctx)
 
 	ticker := time.NewTicker(interval)
@@ -500,6 +503,7 @@ func (c *Cache) evictionLoop(ctx context.Context, interval time.Duration) {
 			// unaccounted for to one interval, on a process that is
 			// already running this loop regardless.
 			c.runReconciliationPass(ctx)
+			c.runCleanExpired(ctx)
 		case <-c.evictionPressure:
 		}
 
@@ -535,6 +539,20 @@ func (c *Cache) runReconciliationPass(ctx context.Context) {
 	}
 }
 
+// runCleanExpired runs CleanExpired, logging a failure instead of
+// propagating it (the loop must keep running). It does nothing once ctx
+// is cancelled.
+func (c *Cache) runCleanExpired(ctx context.Context) {
+	if ctx.Err() != nil {
+		return
+	}
+
+	err := c.CleanExpired(ctx)
+	if err != nil {
+		c.log.Warn("negative cache cleanup failed", "error", err)
+	}
+}
+
 // reconcileAccounting synchronizes the database size accounting with
 // the actual contents of the cache directories. It runs at startup and
 // again on every periodic eviction tick thereafter, off the request

internal/imgcache/negative_cache_internal_test.go

@@ -3,6 +3,7 @@ package imgcache
 import (
 	"context"
 	"errors"
+	"strconv"
 	"testing"
 	"time"
 )
@@ -94,6 +95,307 @@ func TestNegativeCache_Expired(t *testing.T) {
 	}
 }
 
+// TestService_Get_ServesCachedVariantOfNegativeCachedURL caches a variant of
+// the test photo, then keeps a failed fetch of the photo's URL in the negative
+// cache, as a failed fetch for another variant does once the source has been
+// evicted. The cached variant is still served, and a variant not in the cache
+// is made from the cached source. Once the source is evicted, a variant not in
+// the cache needs a fetch and is refused.
+func TestService_Get_ServesCachedVariantOfNegativeCachedURL(t *testing.T) {
+	t.Parallel()
+
+	svc, fixtures := SetupTestService(t)
+
+	resp, err := svc.Get(t.Context(), photoVariant(fixtures, 85, FitCover))
+	if err != nil {
+		t.Fatalf("Get() first request error = %v", err)
+	}
+
+	_ = resp.Content.Close()
+
+	err = svc.cache.StoreNegative(t.Context(), photoVariant(fixtures, 85, FitCover),
+		httpStatusBadGateway, "upstream answered 503")
+	if err != nil {
+		t.Fatalf("StoreNegative() error = %v", err)
+	}
+
+	resp, err = svc.Get(t.Context(), photoVariant(fixtures, 85, FitCover))
+	if err != nil {
+		t.Fatalf("Get() of the cached variant error = %v", err)
+	}
+
+	_ = resp.Content.Close()
+
+	if resp.CacheStatus != CacheHit {
+		t.Errorf("cached variant CacheStatus = %q, want %q",
+			resp.CacheStatus, CacheHit)
+	}
+
+	resp, err = svc.Get(t.Context(), photoVariant(fixtures, 40, FitCover))
+	t.Logf("Get() of a variant made from the cached source: error = %v", err)
+
+	if err != nil {
+		t.Fatalf("Get() of a variant made from the cached source error = %v", err)
+	}
+
+	_ = resp.Content.Close()
+
+	contentHash, _, err := svc.cache.LookupSource(t.Context(),
+		photoVariant(fixtures, 85, FitCover))
+	if err != nil || contentHash == "" {
+		t.Fatalf("LookupSource() = %q, %v, want the cached source", contentHash, err)
+	}
+
+	err = svc.cache.evictSourceBlob(t.Context(), contentHash)
+	if err != nil {
+		t.Fatalf("evictSourceBlob() error = %v", err)
+	}
+
+	_, err = svc.Get(t.Context(), photoVariant(fixtures, 60, FitCover))
+	t.Logf("Get() of a variant whose source was evicted: error = %v", err)
+
+	if !errors.Is(err, ErrNegativeCached) {
+		t.Errorf("Get() of a variant whose source was evicted error = %v, want %v",
+			err, ErrNegativeCached)
+	}
+}
+
+// TestEvictionLoopRemovesExpiredNegativeCacheRows stores a failed fetch
+// that expires a second later and starts the eviction loop with a short
+// interval: a later tick must remove its row although its URL is never
+// requested again, and must keep a row that has not expired. It runs
+// with the disk cache enabled and disabled, as the negative cache is
+// used either way.
+func TestEvictionLoopRemovesExpiredNegativeCacheRows(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name             string
+		disableDiskCache bool
+	}{
+		{name: "disk cache enabled", disableDiskCache: false},
+		{name: "disk cache disabled", disableDiskCache: true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			cache, err := NewCache(evictionTestDB(t), CacheConfig{
+				StateDir:         t.TempDir(),
+				NegativeTTL:      time.Second,
+				DisableDiskCache: tt.disableDiskCache,
+			})
+			if err != nil {
+				t.Fatalf("failed to create cache: %v", err)
+			}
+
+			expired := &ImageRequest{
+				SourceHost: testHostExample, SourcePath: "/gone.jpg",
+			}
+
+			err = cache.StoreNegative(t.Context(), expired, 404, "not found")
+			if err != nil {
+				t.Fatalf("StoreNegative failed: %v", err)
+			}
+
+			const notExpiredPath = "/later.jpg"
+
+			_, err = cache.db.ExecContext(t.Context(), `
+				INSERT INTO negative_cache
+					(source_host, source_path, status_code, expires_at)
+				VALUES (?, ?, 404, datetime('now', '+1 hour'))
+			`, testHostExample, notExpiredPath)
+			if err != nil {
+				t.Fatalf("failed to insert a row that has not expired: %v", err)
+			}
+
+			started := time.Now()
+
+			cache.StartEviction(100 * time.Millisecond)
+			defer func() { _ = cache.StopEviction(t.Context()) }()
+
+			for negativeCacheRowCount(t, cache, expired.SourcePath) > 0 {
+				if time.Since(started) > 10*time.Second {
+					t.Fatal("the expired row is still in negative_cache")
+				}
+
+				time.Sleep(50 * time.Millisecond)
+			}
+
+			t.Logf("the expired row was removed %s after the loop started",
+				time.Since(started))
+
+			if negativeCacheRowCount(t, cache, notExpiredPath) != 1 {
+				t.Error("the row that has not expired was removed")
+			}
+		})
+	}
+}
+
+// negativeCacheRowCount returns how many negative_cache rows hold
+// sourcePath.
+func negativeCacheRowCount(tb testing.TB, cache *Cache, sourcePath string) int {
+	tb.Helper()
+
+	var count int
+
+	err := cache.db.QueryRowContext(tb.Context(),
+		`SELECT COUNT(*) FROM negative_cache WHERE source_path = ?`,
+		sourcePath,
+	).Scan(&count)
+	if err != nil {
+		tb.Fatalf("failed to count negative_cache rows: %v", err)
+	}
+
+	return count
+}
+
+// TestCleanExpiredDeletesInBatches stores one expired negative cache row
+// more than one batch holds, and one row that has not expired. One
+// statement must delete one batch and no more, so a request waits behind
+// one batch at most; CleanExpired must then remove the expired row left
+// over and keep the row that has not expired.
+func TestCleanExpiredDeletesInBatches(t *testing.T) {
+	t.Parallel()
+
+	cache, err := NewCache(evictionTestDB(t), CacheConfig{StateDir: t.TempDir()})
+	if err != nil {
+		t.Fatalf("failed to create cache: %v", err)
+	}
+
+	ctx := t.Context()
+
+	insertExpiredNegativeCacheRows(t, cache, cleanExpiredBatchSize+1)
+
+	const notExpiredPath = "/later.jpg"
+
+	_, err = cache.db.ExecContext(ctx, `
+		INSERT INTO negative_cache
+			(source_host, source_path, status_code, expires_at)
+		VALUES (?, ?, 404, datetime('now', '+1 hour'))
+	`, testHostExample, notExpiredPath)
+	if err != nil {
+		t.Fatalf("failed to insert a row that has not expired: %v", err)
+	}
+
+	deleted, err := cache.cleanExpiredBatch(ctx)
+	if err != nil {
+		t.Fatalf("cleanExpiredBatch failed: %v", err)
+	}
+
+	t.Logf("one statement deleted %d of %d expired rows",
+		deleted, cleanExpiredBatchSize+1)
+
+	if deleted != cleanExpiredBatchSize {
+		t.Errorf("one statement deleted %d rows, want %d",
+			deleted, cleanExpiredBatchSize)
+	}
+
+	err = cache.CleanExpired(ctx)
+	if err != nil {
+		t.Fatalf("CleanExpired failed: %v", err)
+	}
+
+	if count := negativeCacheTableRowCount(t, cache); count != 1 {
+		t.Errorf("negative_cache holds %d rows after CleanExpired, want 1", count)
+	}
+
+	if negativeCacheRowCount(t, cache, notExpiredPath) != 1 {
+		t.Error("the row that has not expired was removed")
+	}
+}
+
+// TestCleanExpiredRemovesMoreThanOneBatch stores one expired negative
+// cache row more than one batch holds and calls CleanExpired once: it
+// must remove them all, so it fails if CleanExpired stops after its
+// first batch.
+func TestCleanExpiredRemovesMoreThanOneBatch(t *testing.T) {
+	t.Parallel()
+
+	cache, err := NewCache(evictionTestDB(t), CacheConfig{StateDir: t.TempDir()})
+	if err != nil {
+		t.Fatalf("failed to create cache: %v", err)
+	}
+
+	insertExpiredNegativeCacheRows(t, cache, cleanExpiredBatchSize+1)
+
+	before := negativeCacheTableRowCount(t, cache)
+
+	err = cache.CleanExpired(t.Context())
+	if err != nil {
+		t.Fatalf("CleanExpired failed: %v", err)
+	}
+
+	after := negativeCacheTableRowCount(t, cache)
+
+	t.Logf("CleanExpired removed %d of %d expired rows, %d per batch",
+		before-after, before, cleanExpiredBatchSize)
+
+	if after != 0 {
+		t.Errorf("negative_cache holds %d rows after CleanExpired, want 0", after)
+	}
+}
+
+// TestCleanExpiredStopsOnceContextEnds runs CleanExpired with a context
+// that has already ended: it must return the context's error and delete
+// no row.
+func TestCleanExpiredStopsOnceContextEnds(t *testing.T) {
+	t.Parallel()
+
+	cache, err := NewCache(evictionTestDB(t), CacheConfig{StateDir: t.TempDir()})
+	if err != nil {
+		t.Fatalf("failed to create cache: %v", err)
+	}
+
+	insertExpiredNegativeCacheRows(t, cache, 1)
+
+	ctx, cancel := context.WithCancel(t.Context())
+	cancel()
+
+	err = cache.CleanExpired(ctx)
+	if !errors.Is(err, context.Canceled) {
+		t.Errorf("CleanExpired() error = %v, want context.Canceled", err)
+	}
+
+	if count := negativeCacheTableRowCount(t, cache); count != 1 {
+		t.Errorf("negative_cache holds %d rows, want the expired row kept", count)
+	}
+}
+
+// insertExpiredNegativeCacheRows inserts count negative_cache rows that
+// expired an hour ago, each for a different source path.
+func insertExpiredNegativeCacheRows(t *testing.T, cache *Cache, count int) {
+	t.Helper()
+
+	for i := range count {
+		_, err := cache.db.ExecContext(t.Context(), `
+			INSERT INTO negative_cache
+				(source_host, source_path, status_code, expires_at)
+			VALUES (?, ?, 404, datetime('now', '-1 hour'))
+		`, testHostExample, "/expired/"+strconv.Itoa(i)+".jpg")
+		if err != nil {
+			t.Fatalf("failed to insert an expired row: %v", err)
+		}
+	}
+}
+
+// negativeCacheTableRowCount returns how many rows negative_cache holds.
+func negativeCacheTableRowCount(tb testing.TB, cache *Cache) int {
+	tb.Helper()
+
+	var count int
+
+	err := cache.db.QueryRowContext(tb.Context(),
+		`SELECT COUNT(*) FROM negative_cache`,
+	).Scan(&count)
+	if err != nil {
+		tb.Fatalf("failed to count negative_cache rows: %v", err)
+	}
+
+	return count
+}
+
 func TestService_Get_ReturnsErrorForNegativeCachedURL(t *testing.T) {
 	t.Parallel()
 

internal/imgcache/service.go

@@ -9,6 +9,7 @@ import (
 	"log/slog"
 	"net/url"
 	"runtime/debug"
+	"strings"
 	"time"
 
 	"github.com/dustin/go-humanize"
@@ -129,25 +130,13 @@ func NewService(cfg *ServiceConfig) (*Service, error) {
 var ErrNegativeCached = errors.New("request is in negative cache (recently failed)")
 
 // Get retrieves a processed image, fetching and processing if necessary.
+// While its source URL is in the negative cache, a cached variant is still
+// served and a variant not in the cache is still made from a cached source;
+// only a request that needs the source fetched is refused.
 func (s *Service) Get(ctx context.Context, req *ImageRequest) (*ImageResponse, error) {
 	// Propagate AllowHTTP setting to the request
 	req.AllowHTTP = s.allowHTTP
 
-	// Check negative cache first - skip fetching for recently-failed URLs
-	negHit, err := s.cache.checkNegativeCache(ctx, req)
-	if err != nil {
-		s.log.Warn("negative cache check failed", "error", err)
-	}
-
-	if negHit {
-		s.log.Debug("negative cache hit",
-			"host", req.SourceHost,
-			"path", req.SourcePath,
-		)
-
-		return nil, fmt.Errorf("%w: %w", httpfetcher.ErrUpstreamError, ErrNegativeCached)
-	}
-
 	// Check variant cache first (disk only, no DB)
 	result, err := s.cache.Lookup(ctx, req)
 	if err != nil {
@@ -218,7 +207,16 @@ func (s *Service) ValidateRequest(req *ImageRequest) error {
 		return fmt.Errorf("invalid source URL: %w", err)
 	}
 
-	if s.allowlist.IsAllowed(parsedURL) {
+	// 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
 	}
 
@@ -423,12 +421,28 @@ func (s *Service) processFromSourceOrFetch(
 
 // fetchAndProcess fetches from upstream, processes, and caches the result.
 // It counts the fetch with the bytes read from upstream, including when
-// reading the response or a later step fails.
+// reading the response or a later step fails. It fetches nothing while the
+// source URL is in the negative cache.
 func (s *Service) fetchAndProcess(
 	ctx context.Context,
 	req *ImageRequest,
 	cacheKey VariantKey,
 ) (*processedVariant, error) {
+	// Check negative cache - skip fetching for recently-failed URLs
+	negHit, err := s.cache.checkNegativeCache(ctx, req)
+	if err != nil {
+		s.log.Warn("negative cache check failed", "error", err)
+	}
+
+	if negHit {
+		s.log.Debug("negative cache hit",
+			"host", req.SourceHost,
+			"path", req.SourcePath,
+		)
+
+		return nil, fmt.Errorf("%w: %w", httpfetcher.ErrUpstreamError, ErrNegativeCached)
+	}
+
 	// Fetch from upstream
 	sourceURL := req.SourceURL()
 

internal/imgcache/service_internal_test.go

@@ -250,6 +250,97 @@ func TestService_ValidateRequest_SignatureExactHostMatch(t *testing.T) {
 	}
 }
 
+// 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{
+				SourceHost: tt.host,
+				SourcePath: testPathCat,
+				Size:       Size{Width: 50, Height: 50},
+				Format:     FormatJPEG,
+				Quality:    85,
+				FitMode:    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{
+				SourceHost: host,
+				SourcePath: testPathCat,
+				Size:       Size{Width: 50, Height: 50},
+				Format:     FormatJPEG,
+				Quality:    85,
+				FitMode:    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()
 

internal/imgcache/storage.go

@@ -10,6 +10,7 @@ import (
 	"log/slog"
 	"os"
 	"path/filepath"
+	"strings"
 	"time"
 )
 
@@ -371,11 +372,13 @@ func HashPath(path string) PathHash {
 
 // CacheKey generates a unique key for a request variant.
 // Format: sha256(host:path:query:width:height:format:quality:fit_mode)
+// The path and query go through escapePercentAndColon, so that a ":" in
+// either cannot be taken for the one between them.
 func CacheKey(req *ImageRequest) VariantKey {
 	data := fmt.Sprintf("%s:%s:%s:%d:%d:%s:%d:%s",
 		req.SourceHost,
-		req.SourcePath,
-		req.SourceQuery,
+		escapePercentAndColon(req.SourcePath),
+		escapePercentAndColon(req.SourceQuery),
 		req.Size.Width,
 		req.Size.Height,
 		req.Format,
@@ -387,6 +390,14 @@ func CacheKey(req *ImageRequest) VariantKey {
 	return VariantKey(hex.EncodeToString(h[:]))
 }
 
+// escapePercentAndColon returns s with each "%" written as "%25" and each
+// ":" as "%3A", so that the result holds no ":".
+func escapePercentAndColon(s string) string {
+	s = strings.ReplaceAll(s, "%", "%25")
+
+	return strings.ReplaceAll(s, ":", "%3A")
+}
+
 // VariantStorage handles key-based file storage for processed image variants.
 // Files are stored at: <basedir>/<ab>/<cd>/<cache_key>
 // Metadata is stored at: <basedir>/<ab>/<cd>/<cache_key>.meta

internal/imgcache/storage_internal_test.go

@@ -407,6 +407,62 @@ func TestCacheKey(t *testing.T) {
 	}
 }
 
+// TestCacheKey_ColonOrPercentInPathOrQuery checks that two different source
+// URLs whose path or query holds ":" or "%" get different keys.
+func TestCacheKey_ColonOrPercentInPathOrQuery(t *testing.T) {
+	t.Parallel()
+
+	cases := []struct {
+		name          string
+		path1, query1 string
+		path2, query2 string
+	}{
+		{
+			name:  "colon moved from path to query",
+			path1: "/a:b", query1: "",
+			path2: "/a", query2: "b:",
+		},
+		{
+			name:  "colon in path and its percent-encoding",
+			path1: "/a:b", query1: "",
+			path2: "/a%3Ab", query2: "",
+		},
+	}
+
+	for _, tt := range cases {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			key1 := CacheKey(&ImageRequest{
+				SourceHost:  testHostCDN,
+				SourcePath:  tt.path1,
+				SourceQuery: tt.query1,
+				Size:        Size{Width: 800, Height: 600},
+				Format:      FormatWebP,
+				Quality:     85,
+				FitMode:     FitCover,
+			})
+			key2 := CacheKey(&ImageRequest{
+				SourceHost:  testHostCDN,
+				SourcePath:  tt.path2,
+				SourceQuery: tt.query2,
+				Size:        Size{Width: 800, Height: 600},
+				Format:      FormatWebP,
+				Quality:     85,
+				FitMode:     FitCover,
+			})
+
+			t.Logf("CacheKey() of path %q query %q = %s", tt.path1, tt.query1, key1)
+			t.Logf("CacheKey() of path %q query %q = %s", tt.path2, tt.query2, key2)
+
+			if key1 == key2 {
+				t.Errorf("CacheKey() gave path %q query %q and path %q query %q "+
+					"the same key", tt.path1, tt.query1, tt.path2, tt.query2)
+			}
+		})
+	}
+}
+
 // TestVariantStorage_StoreLogsFailedMetaWrite verifies that a .meta write
 // that fails is logged at warn and the store still succeeds.
 func TestVariantStorage_StoreLogsFailedMetaWrite(t *testing.T) {

internal/imgcache/urlparser.go

@@ -15,6 +15,7 @@ import (
 var (
 	ErrInvalidPath       = errors.New("invalid image path")
 	ErrMissingHost       = errors.New("missing source host")
+	ErrInvalidHost       = errors.New("invalid source host")
 	ErrMissingSize       = errors.New("missing size specification")
 	ErrInvalidSize       = errors.New("invalid size format")
 	ErrInvalidFormat     = errors.New("invalid or unsupported format")
@@ -143,6 +144,17 @@ func parseImageComponents(remainder string) (*ParsedURL, error) {
 		return nil, ErrMissingHost
 	}
 
+	// A URL reads what comes before "@" as a user name and password, and the
+	// rest as the host
+	if strings.Contains(host, "@") {
+		return nil, fmt.Errorf("%w: %q holds @", ErrInvalidHost, host)
+	}
+
+	// Host names do not depend on letter case, so the allowlist, the upstream
+	// connection limits, the cache and the signature see one spelling of each
+	// host
+	host = strings.ToLower(host)
+
 	// Extract query string if present in path
 	if qIndex := strings.Index(path, "?"); qIndex != -1 {
 		query = path[qIndex+1:]

internal/imgcache/urlparser_internal_test.go

@@ -234,6 +234,49 @@ func TestParseImagePath(t *testing.T) {
 	}
 }
 
+// TestParseImagePath_LowerCasesHost verifies that the host is lower-cased, so
+// that every spelling of its letter case is the same host to the allowlist,
+// the upstream connection limits and the cache, and that the path keeps its
+// case.
+func TestParseImagePath_LowerCasesHost(t *testing.T) {
+	t.Parallel()
+
+	got, err := ParseImagePath("CDN.Example.COM/Photos/Cat.jpg/800x600.webp")
+	if err != nil {
+		t.Fatalf("ParseImagePath() error = %v", err)
+	}
+
+	t.Logf("parsed host %q, path %q", got.Host, got.Path)
+
+	if got.Host != testHostCDN {
+		t.Errorf("Host = %q, want %q", got.Host, testHostCDN)
+	}
+
+	if got.Path != "/Photos/Cat.jpg" {
+		t.Errorf("Path = %q, want %q", got.Path, "/Photos/Cat.jpg")
+	}
+}
+
+// TestParseImagePath_RefusesUserinfo verifies that a host holding "@" is
+// refused: a URL reads what comes before the "@" as a user name and password
+// and the rest as the host.
+func TestParseImagePath_RefusesUserinfo(t *testing.T) {
+	t.Parallel()
+
+	for _, input := range []string{
+		"user@cdn.example.com/photos/cat.jpg/800x600.webp",
+		"user:password@cdn.example.com/photos/cat.jpg/800x600.webp",
+	} {
+		got, err := ParseImagePath(input)
+		t.Logf("ParseImagePath(%q) = %+v, error = %v", input, got, err)
+
+		if !errors.Is(err, ErrInvalidHost) {
+			t.Errorf("ParseImagePath(%q) error = %v, want %v",
+				input, err, ErrInvalidHost)
+		}
+	}
+}
+
 func TestParsedURL_ToImageRequest(t *testing.T) {
 	t.Parallel()
 

internal/server/logout_internal_test.go

@@ -3,6 +3,8 @@ package server
 import (
 	"net/http"
 	"net/http/httptest"
+	"net/url"
+	"regexp"
 	"strings"
 	"testing"
 
@@ -19,6 +21,11 @@ import (
 	"sneak.berlin/go/pixa/internal/session"
 )
 
+// logoutFormPattern extracts the CSRF token rendered into the generator
+// page's logout form.
+var logoutFormPattern = regexp.MustCompile(
+	`(?s)action="/logout".*?name="gorilla\.csrf\.Token" value="([^"]+)"`)
+
 // startPixa starts the components pixad's fx app builds, from cfg, and
 // returns the server with its routes, as newTestServer does.
 func startPixa(t *testing.T, cfg *config.Config) *Server {
@@ -50,6 +57,24 @@ func startPixa(t *testing.T, cfg *config.Config) *Server {
 	return s
 }
 
+// logIn logs in to s with the signing key, as postLogin does, and returns
+// the session cookie it sets.
+func logIn(t *testing.T, s *Server) *http.Cookie {
+	t.Helper()
+
+	rec := postLogin(t, s, firstClient, "", testSigningKey)
+
+	for _, c := range rec.Result().Cookies() {
+		if c.Name == session.CookieName {
+			return c
+		}
+	}
+
+	t.Fatal("logging in set no session cookie")
+
+	return nil
+}
+
 // opensGeneratorPage reports whether GET / with cookie shows s's generator
 // page, which only a valid login session opens.
 func opensGeneratorPage(t *testing.T, s *Server, cookie *http.Cookie) bool {
@@ -65,37 +90,132 @@ func opensGeneratorPage(t *testing.T, s *Server, cookie *http.Cookie) bool {
 		strings.Contains(rec.Body.String(), `action="/generate"`)
 }
 
+// postLogout loads the generator page with GET / and sessionCookie and
+// submits its logout form with POST /logout, as a browser does.
+func postLogout(
+	t *testing.T, s *Server, sessionCookie *http.Cookie,
+) *httptest.ResponseRecorder {
+	t.Helper()
+
+	req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
+	req.AddCookie(sessionCookie)
+
+	page := httptest.NewRecorder()
+	s.ServeHTTP(page, req)
+
+	match := logoutFormPattern.FindStringSubmatch(page.Body.String())
+	if match == nil {
+		t.Fatal("the generator page has no logout form with a CSRF token")
+	}
+
+	form := url.Values{"gorilla.csrf.Token": {match[1]}}
+	req = httptest.NewRequestWithContext(t.Context(), http.MethodPost,
+		"/logout", strings.NewReader(form.Encode()))
+	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	req.AddCookie(sessionCookie)
+
+	for _, c := range page.Result().Cookies() {
+		req.AddCookie(c)
+	}
+
+	rec := httptest.NewRecorder()
+	s.ServeHTTP(rec, req)
+
+	return rec
+}
+
+// TestLogoutWithoutCSRFTokenEndsNoSession verifies that a request to /logout
+// carrying the session cookie but not the logout form's CSRF token ends no
+// session: GET /logout, as an image on another page of the same site makes a
+// browser send, answers 405, and POST /logout without the token answers 403.
+func TestLogoutWithoutCSRFTokenEndsNoSession(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		method string
+		want   int
+	}{
+		{http.MethodGet, http.StatusMethodNotAllowed},
+		{http.MethodPost, http.StatusForbidden},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.method, func(t *testing.T) {
+			t.Parallel()
+
+			s := newTestServer(t)
+			cookie := logIn(t, s)
+
+			req := httptest.NewRequestWithContext(
+				t.Context(), tc.method, "/logout", nil)
+			req.AddCookie(cookie)
+
+			rec := httptest.NewRecorder()
+			s.ServeHTTP(rec, req)
+			t.Logf("%s /logout without the CSRF token: status %d",
+				tc.method, rec.Code)
+
+			if rec.Code != tc.want {
+				t.Errorf("%s /logout without the CSRF token status = %d, "+
+					"want %d", tc.method, rec.Code, tc.want)
+			}
+
+			if !opensGeneratorPage(t, s, cookie) {
+				t.Errorf("%s /logout without the CSRF token ended the "+
+					"login session", tc.method)
+			}
+		})
+	}
+}
+
+// TestLoginAfterLogoutOpensGeneratorPage verifies that after a logout from
+// the generator page a copy of the session cookie taken before it is
+// refused, and that a login after it opens the generator page.
+func TestLoginAfterLogoutOpensGeneratorPage(t *testing.T) {
+	t.Parallel()
+
+	s := newTestServer(t)
+
+	copied := logIn(t, s)
+	if !opensGeneratorPage(t, s, copied) {
+		t.Fatal("logging in did not open the generator page")
+	}
+
+	logout := postLogout(t, s, copied)
+	if logout.Code != http.StatusSeeOther {
+		t.Fatalf("POST /logout status = %d, want %d",
+			logout.Code, http.StatusSeeOther)
+	}
+
+	if opensGeneratorPage(t, s, copied) {
+		t.Error("a cookie copied before the logout still opens the " +
+			"generator page")
+	}
+
+	opens := opensGeneratorPage(t, s, logIn(t, s))
+	t.Logf("a login after the logout opens the generator page: %v", opens)
+
+	if !opens {
+		t.Error("a login after the logout does not open the generator page")
+	}
+}
+
 // TestLogoutOutlastsRestart verifies that a copy of the session cookie taken
-// before /logout is still refused by a pixa started again on the same state
+// before a logout is still refused by a pixa started again on the same state
 // directory, as pixad is after a restart.
 func TestLogoutOutlastsRestart(t *testing.T) {
 	t.Parallel()
 
 	s := newTestServer(t)
 
-	login := postLogin(t, s, firstClient, "", testSigningKey)
-
-	var copied *http.Cookie
-
-	for _, c := range login.Result().Cookies() {
-		if c.Name == session.CookieName {
-			copied = c
-		}
+	copied := logIn(t, s)
+	if !opensGeneratorPage(t, s, copied) {
+		t.Fatal("logging in did not open the generator page")
 	}
 
-	if copied == nil || !opensGeneratorPage(t, s, copied) {
-		t.Fatal("logging in gave no session cookie that opens the generator page")
-	}
-
-	req := httptest.NewRequestWithContext(
-		t.Context(), http.MethodGet, "/logout", nil)
-	req.AddCookie(copied)
-
-	logout := httptest.NewRecorder()
-	s.ServeHTTP(logout, req)
-
+	logout := postLogout(t, s, copied)
 	if logout.Code != http.StatusSeeOther {
-		t.Fatalf("GET /logout status = %d, want %d",
+		t.Fatalf("POST /logout status = %d, want %d",
 			logout.Code, http.StatusSeeOther)
 	}
 
@@ -106,7 +226,7 @@ func TestLogoutOutlastsRestart(t *testing.T) {
 		opens)
 
 	if opens {
-		t.Error("after a restart, a cookie copied before /logout " +
+		t.Error("after a restart, a cookie copied before the logout " +
 			"still opens the generator page")
 	}
 }

internal/session/session_logout_test.go

@@ -124,9 +124,8 @@ func TestLogout_EndsEverySessionMadeBeforeIt(t *testing.T) {
 }
 
 // TestLogout_WithoutSession_EndsNoSession verifies that a logout from a
-// request without a session cookie ends no session. Another site can make a
-// browser request /logout, but the browser leaves the SameSite=Strict
-// session cookie out of that request.
+// request without a session cookie ends no session, so someone who has not
+// logged in cannot end the sessions of those who have.
 func TestLogout_WithoutSession_EndsNoSession(t *testing.T) {
 	t.Parallel()
 

internal/signature/escaping_test.go

@@ -0,0 +1,113 @@
+package signature_test
+
+import (
+	"errors"
+	"testing"
+	"time"
+
+	"sneak.berlin/go/pixa/internal/signature"
+)
+
+// TestSigner_Verify_ColonCannotMovePathQueryBoundary checks that a URL
+// signed for path "/a:b" with no query does not verify for path "/a" with
+// query "b:", a different resource on the same host.
+func TestSigner_Verify_ColonCannotMovePathQueryBoundary(t *testing.T) {
+	t.Parallel()
+
+	signer := signature.New(goldenSigningKey)
+
+	signed := &signature.Request{
+		SourceHost: testHost,
+		SourcePath: "/a:b",
+		Width:      800,
+		Height:     600,
+		Format:     testFormatWebP,
+		Quality:    85,
+		FitMode:    testFitCover,
+		Expires:    time.Now().Add(1 * time.Hour),
+	}
+	signed.Signature = signer.Sign(signed)
+
+	replayed := *signed
+	replayed.SourcePath = "/a"
+	replayed.SourceQuery = "b:"
+
+	t.Logf("signature for path %q query %q: %s",
+		signed.SourcePath, signed.SourceQuery, signed.Signature)
+	t.Logf("signature for path %q query %q: %s",
+		replayed.SourcePath, replayed.SourceQuery, signer.Sign(&replayed))
+
+	err := signer.Verify(&replayed)
+	if !errors.Is(err, signature.ErrInvalid) {
+		t.Errorf("Verify() of path %q query %q with the signature for "+
+			"path %q = %v, want %v",
+			replayed.SourcePath, replayed.SourceQuery, signed.SourcePath,
+			err, signature.ErrInvalid)
+	}
+}
+
+// TestSigner_EscapedGoldenVectors pins the signature of requests whose path
+// or query holds ":" or "%", which are signed with "%" written as "%25" and
+// ":" as "%3A". The expected values were computed once, outside pixa, from
+// the signed data in each comment.
+func TestSigner_EscapedGoldenVectors(t *testing.T) {
+	t.Parallel()
+
+	signer := signature.New(goldenSigningKey)
+
+	vectors := []struct {
+		name          string
+		path          string
+		query         string
+		wantSignature string
+	}{
+		{
+			name: "colon in path",
+			path: "/photos/a:b.jpg",
+			// Signed data:
+			// "cdn.example.com:/photos/a%3Ab.jpg::800:600:webp:1704067200:85:cover"
+			wantSignature: "WnEpZ7bQp1Zvw8WDatlj2vu-4wYKWE3BFBizV8m__6k=",
+		},
+		{
+			name:  "colon in query",
+			path:  testPath,
+			query: "t=1:2",
+			// Signed data:
+			// "cdn.example.com:/photos/cat.jpg:t=1%3A2:800:600:webp:1704067200:85:cover"
+			wantSignature: "85kRoN8hhr4cCUkEUjNvUsvVYKjYYfWdeffx5TjJjqA=",
+		},
+		{
+			name:  "percent in path and query",
+			path:  "/photos/a%20b.jpg",
+			query: "q=50%",
+			// Signed data:
+			// "cdn.example.com:/photos/a%2520b.jpg:q=50%25:800:600:webp:1704067200:85:cover"
+			wantSignature: "MjDiiRdgXMrsjqPMsENtD4aG1hp1e-a6zy67vKTX4CQ=",
+		},
+	}
+
+	for _, tt := range vectors {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			req := &signature.Request{
+				SourceHost:  testHost,
+				SourcePath:  tt.path,
+				SourceQuery: tt.query,
+				Width:       800,
+				Height:      600,
+				Format:      testFormatWebP,
+				Quality:     85,
+				FitMode:     testFitCover,
+				Expires:     time.Unix(goldenExpiresUnix, 0),
+			}
+
+			gotSignature := signer.Sign(req)
+			t.Logf("Sign() of path %q query %q = %s", tt.path, tt.query, gotSignature)
+
+			if gotSignature != tt.wantSignature {
+				t.Errorf("Sign() = %q, want %q", gotSignature, tt.wantSignature)
+			}
+		})
+	}
+}

internal/signature/signature.go

@@ -10,6 +10,7 @@ import (
 	"fmt"
 	"net/url"
 	"strconv"
+	"strings"
 	"time"
 )
 
@@ -154,15 +155,17 @@ func (s *Signer) GenerateSignedURL(
 
 // buildSignatureData creates the string to be signed.
 // Format: "host:path:query:width:height:format:expiration:quality:fit"
-// All components are used verbatim (exact match). No normalization,
-// suffix matching, or wildcard 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.
+// The path and query go through escapePercentAndColon, so that a ":" in
+// either cannot be taken for the one between them; every other component
+// is used verbatim. No normalization, suffix matching, or wildcard
+// 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 *Request) string {
 	return fmt.Sprintf("%s:%s:%s:%d:%d:%s:%d:%d:%s",
 		req.SourceHost,
-		req.SourcePath,
-		req.SourceQuery,
+		escapePercentAndColon(req.SourcePath),
+		escapePercentAndColon(req.SourceQuery),
 		req.Width,
 		req.Height,
 		req.Format,
@@ -172,6 +175,14 @@ func (s *Signer) buildSignatureData(req *Request) string {
 	)
 }
 
+// escapePercentAndColon returns s with each "%" written as "%25" and each
+// ":" as "%3A", so that the result holds no ":".
+func escapePercentAndColon(s string) string {
+	s = strings.ReplaceAll(s, "%", "%25")
+
+	return strings.ReplaceAll(s, ":", "%3A")
+}
+
 // ParseParams extracts signature and expiration from query parameters.
 func ParseParams(sig, expStr string) (string, time.Time, error) {
 	if expStr == "" {