Run the reconciliation pass at startup and once a day (closes #250)

The pass makes a database query for every cached file and ran on every
eviction tick, so on a large cache one pass could outlast the tick and
the next started at once. It now runs on a timer of its own that starts
when a pass ends, a day long. Eviction and the removal of expired
negative cache rows keep the eviction interval.

Model: opus-5-5

README.md

@@ -131,34 +131,6 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for pixa needs:
       the cache can grow to three quarters or more of the space they all have
       left, and to all of it when that is 500 MiB or less.
     - the rest are in the table under Configuration below
-- **Memory:** pixa can use up to `max_concurrent_processing` times 4.2 GiB for
-  the images it processes at once, plus `upstream_connections` times
-  `upstream_max_response_size` for the source images it has fetched that wait
-  for processing. 4.2 GiB is the most resident memory was measured to grow while
-  pixa, with pixad's libvips settings, processed one 8192x8192 source, the
-  largest it accepts, saving it at full size to each output format in turn, one
-  at a time. The sources were a JPEG, a GIF, a WebP, an 8-bit and a 16-bit PNG,
-  an 8-bit and a 12-bit AVIF, and an 8-bit, a 16-bit and a 32-bit floating-point
-  JPEG XL; memory grew most for the 16-bit JPEG XL, of random pixels, saved as
-  WebP. At the defaults that is half the CPUs pixa sees times 4.2 GiB, plus 64
-  times 50 MiB: on a host where pixa sees 48 CPUs, 24 times 4.2 GiB plus 3.1
-  GiB, about 104 GiB. Set `PIXA_MAX_CONCURRENT_PROCESSING` and
-  `PIXA_UPSTREAM_CONNECTIONS` so that this fits the memory the container may
-  use.
-- **Temporary files:** while pixa processes a large source, libvips writes its
-  decoded pixels to a file in `/tmp`. The file disappears from `/tmp` once it is
-  written, so a listing of `/tmp` does not show it, but it keeps its space, on
-  disk or, where `/tmp` is a tmpfs, in memory, until the image is done. For a
-  GIF, WebP, AVIF or JPEG XL source the decoded pixels are held in memory as
-  well as written to that file, and the 4.2 GiB above includes that memory. For
-  an 8192x8192 source the file was measured on pixad at 192 MiB for an 8-bit RGB
-  JPEG, 512 MiB for a 16-bit RGBA PNG or JPEG XL, and 1 GiB for a JPEG XL of
-  32-bit floating-point RGBA, the largest measured: up to 1 GiB per image
-  processed at once, 24 GiB at the defaults on a host where pixa sees 48 CPUs.
-  This `/tmp` is in the container's own filesystem, not on the volume at
-  `/var/lib/pixa`, and `cache_max_bytes` does not count it. The 4.2 GiB above
-  does not include these files; where `/tmp` is a tmpfs, they are memory on top
-  of it.
 - **Health check:** the image's `HEALTHCHECK` requests
   `/.well-known/healthcheck.json`. upaas reads the container's health 60 seconds
   after a deploy and marks the deploy failed unless it is `healthy`. The probe
@@ -196,7 +168,7 @@ references rather than using filesystem refcounting.
 Source and transformed image files, and the `.meta` file beside each transformed
 image, are written under a temporary name, synced to disk, and only then renamed
 into place, so a crash leaves no file under its own name with part of its data.
-When pixa starts, and every five minutes after, it removes each cached source or
+When pixa starts, and once a day after, it removes each cached source or
 transformed image whose file is empty, or not the size the database records for
 it, with a warning naming the file; the next request for it fetches the source
 or converts the image again.
@@ -316,20 +288,6 @@ once, is refused with 400.
   both optional (values under Signature Specification). Both are part of what is
   cached, so each value of either is a separate cached image.
 
-A source URL's query follows its path after `%3F`, the encoded `?`:
-`/v1/image/<host>/<path>%3F<query>/<size>.<format>`. pixa decodes the
-percent-escapes in the image URL's path once, then reads the host up to the
-first `/`, the size and format after the last `/`, and between them the source
-path up to the first `?` and the source query after it. So the `?` before the
-query, any `?` in the query, and every `%` in the source path or query must be
-percent-encoded, as `%3F` and `%25`. Any other character may be written as
-itself or percent-encoded: `/`, `;`, `,`, `=` and `&` work either way. For
-example,
-`https://cdn.example.com/img?url=https://origin.example.com/my%20cat.jpg` at
-800x600 WebP is
-`/v1/image/cdn.example.com/img%3Furl=https://origin.example.com/my%2520cat.jpg/800x600.webp`,
-signed as shown under Signature Specification.
-
 A size with one side `0` has that side scaled to keep the source's proportions,
 and is refused with 400 when that side would come out above 8192, the most
 either side may be; so is a size with `fit=outside`, which scales the image to
@@ -549,17 +507,6 @@ For the same image at quality 40 with fit `contain`, the input ends in
 and the URL is
 `/v1/image/cdn.example.com/photos/cat.jpg/800x600.webp?sig=5IwXUx6vf7yefhaUvFzgXZvG2o0Df4RJxPTK3pKq5VU=&exp=1704067200&q=40&fit=contain`.
 
-For a source URL with a query,
-`https://cdn.example.com/img?url=https://origin.example.com/my%20cat.jpg`, at
-800x600 WebP with the same key and expiration and the default quality and fit:
-
-1. Build input, with the query's `:` written as `%3A` and its `%` as `%25`:
-   `cdn.example.com:/img:url=https%3A//origin.example.com/my%2520cat.jpg:800:600:webp:1704067200:85:cover`
-2. The signature is `37aFa9GSVCZM1kRY9SzMOk2fOg_M7Wt5gKNR4xM-FYA=`
-3. URL, with the `?` before the query written as `%3F` and the query's `%` as
-   `%25` (see Routes):
-   `/v1/image/cdn.example.com/img%3Furl=https://origin.example.com/my%2520cat.jpg/800x600.webp?sig=37aFa9GSVCZM1kRY9SzMOk2fOg_M7Wt5gKNR4xM-FYA=&exp=1704067200`
-
 **Allowlist patterns:**
 
 - **Exact match**: `cdn.example.com` — matches only that host
@@ -619,7 +566,7 @@ tell. With no file, pixa uses the environment and the defaults.
 | `PIXA_ALLOW_HTTP`                    | `allow_http`                    | Allow plain-HTTP upstreams, for testing only; default `false`                |
 | `PIXA_UPSTREAM_CONNECTIONS_PER_HOST` | `upstream_connections_per_host` | Concurrent connections per upstream host; default `20`                       |
 | `PIXA_UPSTREAM_CONNECTIONS`          | `upstream_connections`          | Concurrent connections to all upstream hosts together; default `64`          |
-| `PIXA_MAX_CONCURRENT_PROCESSING`     | `max_concurrent_processing`     | Images processed at once; default half the CPUs, at least 1                  |
+| `PIXA_MAX_CONCURRENT_PROCESSING`     | `max_concurrent_processing`     | Images processed at once; default the number of CPUs                         |
 | `PIXA_IMAGE_REQUESTS_PER_MINUTE`     | `image_requests_per_minute`     | Image requests per client per minute; `0` turns it off; default `600`        |
 | `PIXA_UPSTREAM_FETCH_TIMEOUT`        | `upstream_fetch_timeout`        | Time allowed for one fetch from an upstream host; default `30s`              |
 | `PIXA_UPSTREAM_MAX_RESPONSE_SIZE`    | `upstream_max_response_size`    | Largest upstream response accepted, in bytes; default 50 MiB                 |
@@ -705,12 +652,10 @@ Key settings in more detail:
   does, and `downstream_timeout` has not ended first, the request is answered
   503 with the error `server busy, try again later`
 - `max_concurrent_processing` — the most images decoded and encoded at once;
-  default half the CPUs pixa can use (`GOMAXPROCS`, which follows a container's
-  CPU limit), rounded down, and at least 1. A value that is not a whole number
-  of at least 1 aborts startup. A request that finds all of them in use waits up
-  to 10 seconds for one to free up; if none does, and `downstream_timeout` has
-  not ended first, it is answered 503 the same way. Each one can take a lot of
-  memory: see "Running under upaas"
+  default the number of CPUs pixa can use (`GOMAXPROCS`), which follows a
+  container's CPU limit. A request that finds all of them in use waits up to 10
+  seconds for one to free up; if none does, and `downstream_timeout` has not
+  ended first, it is answered 503 the same way
 - `image_requests_per_minute` — how many requests one client may send to the
   image routes per minute (see the limit on them under Routes); default `600`.
   `0` turns the limit off, and a negative or non-integer value aborts startup.

TODO.md

@@ -30,44 +30,16 @@ P2: security: per-origin rate limiting
 
 # Completed Steps
 
-- 2026-10-15 a thin source requested with `fit=cover` no longer makes libvips
-  build a huge image (closes #282): libvips' thumbnail scaled the whole source
-  to cover the requested size and copied that into memory before it cut out the
-  centre, so an 8192x64 source at 8192x8192 made it allocate about 24 GiB, and
-  an 8192x1 source answered 500. The image processor now first cuts the centre
-  of the source, widened to whole source pixels, and thumbnail scales and cuts
-  only that, by the same amount on both sides as before. What it copies into
-  memory is then larger than the output by less than one scaled source pixel on
-  each side. With the runtime image's libvips and JPEG output at 8192x8192,
-  resident memory peaked at about 242 MiB for the 8192x64 source and 431 MiB for
-  the 8192x1 one. The output can sit a fraction of an output pixel from where it
-  was.
-- 2026-10-15 the README says how a source URL with a query is written into an
-  image URL, and the image route accepts it (closes #255): the query follows the
-  source path after `%3F`. pixa now reads the image URL's path decoded once, so
-  the `?` before the query, any `?` in it and every `%` in the source path or
-  query must be percent-encoded, and any other character may be. The route used
-  to parse chi's wildcard, which holds the path as the client wrote it whenever
-  that is not Go's own encoding of it; a `/`, `;` or `,` in the query written as
-  `%2F`, `%3B` or `%2C`, as `GenerateSignedURL` wrote them, kept the `%3F` from
-  being read as `?`, and the request failed with 401. `GenerateSignedURL` now
-  writes Go's own encoding of the path. The README has an example with a query
-  and its signature. The path traversal check no longer reads the source query,
-  and keeps a `%` that does not start a percent-escape as it is instead of
-  refusing the URL, so a source path or query holding `%25` is accepted, while a
-  double-encoded `..` next to such a `%` is still refused.
-- 2026-10-15 `max_concurrent_processing` defaults to half the CPUs pixa sees,
-  rounded down, and at least 1 (closes #242): it defaulted to one image per CPU,
-  and a container with no CPU limit sees every CPU of the host, so on a 48-CPU
-  host pixa processed 48 images at once, each of which can take about 4.2 GiB.
-  `config.DefaultMaxConcurrentProcessing` gives the default to the config and to
-  `imageprocessor.New`; `PIXA_MAX_CONCURRENT_PROCESSING` still overrides it.
-  "Running under upaas" in `README.md` gives the worst-case memory, measured,
-  and tells the operator to set `PIXA_MAX_CONCURRENT_PROCESSING` and
-  `PIXA_UPSTREAM_CONNECTIONS` to fit the container's memory. It also gives the
-  file libvips writes to `/tmp` for each large source, up to 1 GiB, measured,
-  which is outside `/var/lib/pixa` and `cache_max_bytes` and is memory where
-  `/tmp` is a tmpfs.
+- 2026-10-15 the reconciliation pass runs at startup and then once a day (closes
+  #250): it ran on every eviction tick, every 5 minutes in pixad, and makes a
+  database query for every cached file, on the one connection requests also use.
+  On a large cache a pass could take longer than 5 minutes, and the next one
+  then started at once, so the passes never stopped. The next pass now starts a
+  day after the previous one ends, so passes never run back to back. Eviction
+  and the removal of expired negative cache rows keep their 5-minute tick, and
+  eviction also runs after each reconciliation pass. A variant file whose
+  database row failed to be written is now counted in the cache size up to about
+  a day later, rather than 5 minutes.
 - 2026-10-15 the canonical files are re-vendored from `sneak/prompts` commit
   `e3f79c4` (closes #221): `.dockerignore`, `.editorconfig`,
   `.gitea/workflows/check.yml`, `.gitignore`, `.golangci.yml`,
@@ -119,13 +91,13 @@ P2: security: per-origin rate limiting
   each transformed image's `.meta` file, are synced to disk before they are
   renamed into place, and their directory after; the `.meta` file is written
   before the image, and a failed directory sync removes the renamed file. The
-  reconciliation pass, at startup and every five minutes, removes a cached
-  source or transformed image whose file is empty, or not the size the database
-  records for it, with its rows and a warning naming the file, so the next
-  request fetches the source or converts the image again; requests compare no
-  sizes. Storing a source over a file of another size replaces that file. A
-  source that fails to decode is not removed for that alone: a damaged upstream
-  image would then be fetched on every request.
+  reconciliation pass, at startup and once a day, removes a cached source or
+  transformed image whose file is empty, or not the size the database records
+  for it, with its rows and a warning naming the file, so the next request
+  fetches the source or converts the image again; requests compare no sizes.
+  Storing a source over a file of another size replaces that file. A source that
+  fails to decode is not removed for that alone: a damaged upstream image would
+  then be fetched on every request.
 - 2026-10-15 a transcode stops once its request's `downstream_timeout` has
   ended, unless it has begun encoding (closes #243): the image processor looked
   at its context only while it waited for a processing slot, so an image was

configs/config.example.yml

@@ -108,11 +108,10 @@ upstream_connections_per_host: 20
 # downstream_timeout has ended first.
 upstream_connections: 64
 
-# Maximum number of images decoded and encoded at once (default: half the
-# number of CPUs pixa can use, at least 1; the number of CPUs follows a
-# container's CPU limit). A request that finds none free waits up to 10
-# seconds for one, and if none frees up it is answered 503, unless
-# downstream_timeout has ended first.
+# Maximum number of images decoded and encoded at once (default: the
+# number of CPUs pixa can use, which follows a container's CPU limit). A
+# request that finds none free waits up to 10 seconds for one, and if none
+# frees up it is answered 503, unless downstream_timeout has ended first.
 # max_concurrent_processing: 4
 
 # How many requests to the image routes (/v1/image/ and /v1/e/) one client

internal/config/concurrency_limits_internal_test.go

@@ -1,7 +1,6 @@
 package config
 
 import (
-	"fmt"
 	"runtime"
 	"testing"
 )
@@ -13,8 +12,8 @@ const (
 )
 
 // TestOmittedConcurrencyLimitsUseDefaults checks that an omitted
-// max_concurrent_processing is half the CPUs Go uses, and at least 1, and
-// an omitted upstream_connections is 64.
+// max_concurrent_processing is the number of CPUs Go uses and an omitted
+// upstream_connections is 64.
 func TestOmittedConcurrencyLimitsUseDefaults(t *testing.T) {
 	t.Parallel()
 
@@ -23,9 +22,9 @@ func TestOmittedConcurrencyLimitsUseDefaults(t *testing.T) {
 		t.Fatalf("minimal config should be valid, got error: %v", err)
 	}
 
-	if c.MaxConcurrentProcessing != max(runtime.GOMAXPROCS(0)/2, 1) {
-		t.Errorf("MaxConcurrentProcessing = %d, want %d, half the CPUs",
-			c.MaxConcurrentProcessing, max(runtime.GOMAXPROCS(0)/2, 1))
+	if c.MaxConcurrentProcessing != runtime.GOMAXPROCS(0) {
+		t.Errorf("MaxConcurrentProcessing = %d, want %d, one per CPU",
+			c.MaxConcurrentProcessing, runtime.GOMAXPROCS(0))
 	}
 
 	if c.UpstreamConnections != 64 {
@@ -33,36 +32,6 @@ func TestOmittedConcurrencyLimitsUseDefaults(t *testing.T) {
 	}
 }
 
-// TestOmittedMaxConcurrentProcessingIsHalfTheCPUsAndAtLeastOne checks
-// that an omitted max_concurrent_processing is half the CPUs Go uses,
-// rounded down, and 1 when Go uses one CPU.
-//
-//nolint:paralleltest // it sets GOMAXPROCS, which other tests read
-func TestOmittedMaxConcurrentProcessingIsHalfTheCPUsAndAtLeastOne(t *testing.T) {
-	cpusBefore := runtime.GOMAXPROCS(0)
-
-	t.Cleanup(func() { runtime.GOMAXPROCS(cpusBefore) })
-
-	for _, tc := range []struct{ cpus, want int }{
-		{1, 1}, {2, 1}, {3, 1}, {8, 4},
-	} {
-		runtime.GOMAXPROCS(tc.cpus)
-
-		c, err := configFromYAML(t, signingKeyLine)
-		if err != nil {
-			t.Fatalf("minimal config should be valid, got error: %v", err)
-		}
-
-		t.Logf("GOMAXPROCS %d: MaxConcurrentProcessing %d",
-			tc.cpus, c.MaxConcurrentProcessing)
-
-		if c.MaxConcurrentProcessing != tc.want {
-			t.Errorf("GOMAXPROCS %d: MaxConcurrentProcessing = %d, want %d",
-				tc.cpus, c.MaxConcurrentProcessing, tc.want)
-		}
-	}
-}
-
 // TestExplicitConcurrencyLimitsAreUsed checks that valid values for the
 // two limits are used as given.
 func TestExplicitConcurrencyLimitsAreUsed(t *testing.T) {
@@ -191,19 +160,3 @@ func TestConcurrencyLimitsFromEnvironment(t *testing.T) {
 		})
 	}
 }
-
-// TestInvalidMaxConcurrentProcessingFromEnvironmentAbortsStartup checks
-// that a PIXA_MAX_CONCURRENT_PROCESSING that is not a whole number of at
-// least 1, an empty one included, aborts startup naming the variable and
-// the key instead of taking the default.
-func TestInvalidMaxConcurrentProcessingFromEnvironmentAbortsStartup(t *testing.T) {
-	for _, value := range []string{"", " ", "0.5", "half", "-1", "4 images"} {
-		t.Run(fmt.Sprintf("value %q", value), func(t *testing.T) {
-			t.Setenv(testMaxConcurrentProcessingVar, value)
-
-			_, err := configFromYAML(t, signingKeyLine)
-			wantStartupError(t, err,
-				testMaxConcurrentProcessingVar, keyMaxConcurrentProcessing)
-		})
-	}
-}

internal/config/config.go

@@ -37,14 +37,6 @@ const (
 	DefaultImageRequestsPerMinute     = 600
 )
 
-// DefaultMaxConcurrentProcessing returns the most images processed at once
-// when max_concurrent_processing is omitted: half the CPUs Go uses
-// (runtime.GOMAXPROCS(0), which follows a container's CPU limit), rounded
-// down, and at least 1.
-func DefaultMaxConcurrentProcessing() int {
-	return max(runtime.GOMAXPROCS(0)/2, 1) //nolint:mnd // half the CPUs
-}
-
 // Configuration key names.
 const (
 	keyDebug                      = "debug"
@@ -305,8 +297,10 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
 			keyUpstreamConnectionsPerHost, DefaultUpstreamConnectionsPerHost),
 		UpstreamConnections: loader.intVal(
 			keyUpstreamConnections, DefaultUpstreamConnections),
+		// Decoding and encoding are CPU-bound, so the default is one image
+		// per CPU Go uses, which follows a container's CPU limit.
 		MaxConcurrentProcessing: loader.intVal(
-			keyMaxConcurrentProcessing, DefaultMaxConcurrentProcessing()),
+			keyMaxConcurrentProcessing, runtime.GOMAXPROCS(0)),
 		UpstreamFetchTimeout: loader.durationVal(
 			keyUpstreamFetchTimeout, DefaultUpstreamFetchTimeout),
 		UpstreamMaxResponseSize: loader.int64Val(

internal/handlers/image.go

@@ -9,6 +9,7 @@ import (
 	"strconv"
 	"time"
 
+	"github.com/go-chi/chi/v5"
 	"github.com/go-chi/chi/v5/middleware"
 	"sneak.berlin/go/pixa/internal/encurl"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
@@ -79,21 +80,20 @@ func (s *Handlers) HandleRobotsTxt() http.HandlerFunc {
 	}
 }
 
-// parseImageRequest parses the image URL's path and query parameters into
+// parseImageRequest parses the wildcard path and query parameters into
 // an ImageRequest. On invalid input it writes an error response and
 // returns false.
 func (s *Handlers) parseImageRequest(
 	w http.ResponseWriter, r *http.Request,
 ) (*imgcache.ImageRequest, 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
-	// the %3F before a source query when, say, a "/" in the query is written
-	// as %2F
-	parsed, err := imgcache.ParseImageURL(r.URL.Path)
+	// Get the wildcard path from chi
+	pathParam := chi.URLParam(r, "*")
+
+	// Parse the URL path
+	parsed, err := imgcache.ParseImagePath(pathParam)
 	if err != nil {
 		s.log.Warn("failed to parse image URL",
-			"path", r.URL.Path,
+			"path", pathParam,
 			"error", err,
 		)
 		s.respondError(w, "invalid image URL: "+err.Error(), http.StatusBadRequest)

internal/handlers/image_signature_internal_test.go

@@ -169,166 +169,3 @@ func TestHandleImage_InvalidExp_Returns400(t *testing.T) {
 		})
 	}
 }
-
-// TestHandleImage_GeneratedSignedURLWithSourceQueryVerifies sends URLs built
-// by the service's signed-URL generator for source URLs with a query to the
-// route, through a chi router: chi routes on the path as the client wrote it
-// whenever that is not Go's own encoding of it. A 502 means the signature
-// passed, so the route read the source path and query that were signed.
-func TestHandleImage_GeneratedSignedURLWithSourceQueryVerifies(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		name  string
-		query string
-	}{
-		{"slash and colon", "url=https://origin.example.com/cat.jpg"},
-		{"semicolon", "a=1;b=2"},
-		{"comma", "crop=0,0,50,50"},
-		{"equals and ampersand", "a=1&b=2"},
-		{"percent-escape", "name=my%20cat"},
-		{"question mark", "next=/a?b=1"},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			fix := setupTestHandler(t)
-
-			signedURL, err := fix.service.GenerateSignedURL("", &imgcache.ImageRequest{
-				SourceHost:  signedHost,
-				SourcePath:  photoPath,
-				SourceQuery: tt.query,
-				Size:        imgcache.Size{Width: 50, Height: 50},
-				Format:      imgcache.FormatJPEG,
-			}, time.Hour)
-			if err != nil {
-				t.Fatalf("GenerateSignedURL() error = %v", err)
-			}
-
-			status := getImage(t, fix, signedURL)
-			if status != http.StatusBadGateway {
-				t.Errorf("status = %d, want %d", status, http.StatusBadGateway)
-			}
-		})
-	}
-}
-
-// TestHandleImage_SourceQueryEncodedAnyWay signs one source URL with a query
-// and writes it into the image URL in ways the README allows: the "?" before
-// the query and each "%" in it percent-encoded, and every other character as
-// itself or percent-encoded. A 502 means the signature passed.
-func TestHandleImage_SourceQueryEncodedAnyWay(t *testing.T) {
-	t.Parallel()
-
-	expires := time.Now().Add(time.Hour)
-	signer := signature.New("test-signing-key-must-be-32-chars")
-	sig := signer.Sign(&signature.Request{
-		SourceHost:  signedHost,
-		SourcePath:  photoPath,
-		SourceQuery: "url=https://origin.example.com/my%20cat.jpg",
-		Width:       50,
-		Height:      50,
-		Format:      string(imgcache.FormatJPEG),
-		Quality:     85,
-		FitMode:     string(imgcache.FitCover),
-		Expires:     expires,
-	})
-
-	tests := []struct {
-		name         string
-		writtenQuery string
-	}{
-		{"only ? and % encoded",
-			"%3Furl=https://origin.example.com/my%2520cat.jpg"},
-		{"every reserved character encoded",
-			"%3Furl%3Dhttps%3A%2F%2Forigin.example.com%2Fmy%2520cat.jpg"},
-		{"lower-case hex digits",
-			"%3furl=https://origin.example.com/my%2520cat.jpg"},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			target := fmt.Sprintf("/v1/image/%s%s%s/50x50.jpeg?sig=%s&exp=%d",
-				signedHost, photoPath, tt.writtenQuery, sig, expires.Unix())
-
-			status := getImage(t, setupTestHandler(t), target)
-			if status != http.StatusBadGateway {
-				t.Errorf("status = %d, want %d", status, http.StatusBadGateway)
-			}
-		})
-	}
-}
-
-// TestHandleImage_SourceURLHoldingPercent25Verifies signs a source path and a
-// source query that each hold %25, the percent-escape of a literal "%", and
-// sends each to the route written as the README says, with that "%" written as
-// %25, and as GenerateSignedURL writes it. A 502 means the signature passed.
-func TestHandleImage_SourceURLHoldingPercent25Verifies(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		name        string
-		sourcePath  string
-		sourceQuery string
-		// writtenPathAndQuery is the source path and query as the README says
-		// to write them into the image URL
-		writtenPathAndQuery string
-	}{
-		{"path", "/sale/50%25off.jpg", "", "/sale/50%2525off.jpg"},
-		{"query", photoPath, "discount=50%25", photoPath + "%3Fdiscount=50%2525"},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name+" written by the README rule", func(t *testing.T) {
-			t.Parallel()
-
-			expires := time.Now().Add(time.Hour)
-			signer := signature.New("test-signing-key-must-be-32-chars")
-			sig := signer.Sign(&signature.Request{
-				SourceHost:  signedHost,
-				SourcePath:  tt.sourcePath,
-				SourceQuery: tt.sourceQuery,
-				Width:       50,
-				Height:      50,
-				Format:      string(imgcache.FormatJPEG),
-				Quality:     85,
-				FitMode:     string(imgcache.FitCover),
-				Expires:     expires,
-			})
-
-			target := fmt.Sprintf("/v1/image/%s%s/50x50.jpeg?sig=%s&exp=%d",
-				signedHost, tt.writtenPathAndQuery, sig, expires.Unix())
-
-			status := getImage(t, setupTestHandler(t), target)
-			if status != http.StatusBadGateway {
-				t.Errorf("status = %d, want %d", status, http.StatusBadGateway)
-			}
-		})
-
-		t.Run(tt.name+" written by GenerateSignedURL", func(t *testing.T) {
-			t.Parallel()
-
-			fix := setupTestHandler(t)
-
-			signedURL, err := fix.service.GenerateSignedURL("", &imgcache.ImageRequest{
-				SourceHost:  signedHost,
-				SourcePath:  tt.sourcePath,
-				SourceQuery: tt.sourceQuery,
-				Size:        imgcache.Size{Width: 50, Height: 50},
-				Format:      imgcache.FormatJPEG,
-			}, time.Hour)
-			if err != nil {
-				t.Fatalf("GenerateSignedURL() error = %v", err)
-			}
-
-			status := getImage(t, fix, signedURL)
-			if status != http.StatusBadGateway {
-				t.Errorf("status = %d, want %d", status, http.StatusBadGateway)
-			}
-		})
-	}
-}

internal/imageprocessor/fit_cover_internal_test.go

@@ -1,268 +0,0 @@
-package imageprocessor
-
-import (
-	"bytes"
-	"image"
-	"image/color"
-	"image/png"
-	"math"
-	"testing"
-)
-
-// TestImageProcessor_FitCoverServesThinSource verifies that FitCover serves
-// a thin source at the requested size. An 8192x1 source scaled to cover
-// 8192x8192 is 8192 times as long as the output on its long side. The centre
-// of an 8192x1 source with the proportions of 1x8192 is 1/8192 of a pixel
-// wide. Each case is also turned on its side.
-func TestImageProcessor_FitCoverServesThinSource(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		name                    string
-		inputWidth, inputHeight int
-		size                    Size
-	}{
-		{"8192x1 at 8192x8192", 8192, 1, Size{Width: 8192, Height: 8192}},
-		{"8192x1 at 1x8192", 8192, 1, Size{Width: 1, Height: 8192}},
-		{"1x8192 at 8192x8192", 1, 8192, Size{Width: 8192, Height: 8192}},
-		{"1x8192 at 8192x1", 1, 8192, Size{Width: 8192, Height: 1}},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			input := createTestJPEG(t, tt.inputWidth, tt.inputHeight)
-
-			result, err := New(Params{}).Process(t.Context(),
-				bytes.NewReader(input),
-				&Request{Size: tt.size, Format: FormatJPEG, FitMode: FitCover})
-			if err != nil {
-				t.Fatalf("Process() error = %v", err)
-			}
-
-			defer func() { _ = result.Content.Close() }()
-
-			t.Logf("Process() made %dx%d", result.Width, result.Height)
-
-			if result.Width != tt.size.Width || result.Height != tt.size.Height {
-				t.Errorf("Process() made %dx%d, want %dx%d",
-					result.Width, result.Height, tt.size.Width, tt.size.Height)
-			}
-		})
-	}
-}
-
-// createBandsPNG creates a PNG of three equal bands, red, green and blue,
-// side by side when it is wider than tall and top to bottom otherwise.
-func createBandsPNG(t *testing.T, width, height int) []byte {
-	t.Helper()
-
-	bands := []color.RGBA{
-		{R: 255, A: 255},
-		{G: 255, A: 255},
-		{B: 255, A: 255},
-	}
-
-	img := image.NewRGBA(image.Rect(0, 0, width, height))
-	for y := range height {
-		for x := range width {
-			band := 3 * y / height
-			if width > height {
-				band = 3 * x / width
-			}
-
-			img.Set(x, y, bands[band])
-		}
-	}
-
-	return encodePNG(t, img)
-}
-
-// TestImageProcessor_FitCoverCropsTheCentre verifies that FitCover keeps the
-// centre of the source: a 300x100 source of red, green and blue bands side
-// by side, or a 100x300 one with them top to bottom, comes out at 50x50 as
-// its green band.
-func TestImageProcessor_FitCoverCropsTheCentre(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		name                    string
-		inputWidth, inputHeight int
-	}{
-		{"bands side by side", 300, 100},
-		{"bands top to bottom", 100, 300},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			input := createBandsPNG(t, tt.inputWidth, tt.inputHeight)
-
-			result, err := New(Params{}).Process(t.Context(),
-				bytes.NewReader(input),
-				&Request{
-					Size:    Size{Width: 50, Height: 50},
-					Format:  FormatPNG,
-					FitMode: FitCover,
-				})
-			if err != nil {
-				t.Fatalf("Process() error = %v", err)
-			}
-
-			defer func() { _ = result.Content.Close() }()
-
-			output, err := png.Decode(result.Content)
-			if err != nil {
-				t.Fatalf("failed to decode output: %v", err)
-			}
-
-			bounds := output.Bounds()
-			t.Logf("Process() made %dx%d", bounds.Dx(), bounds.Dy())
-
-			if bounds.Dx() != 50 || bounds.Dy() != 50 {
-				t.Fatalf("Process() made %dx%d, want 50x50",
-					bounds.Dx(), bounds.Dy())
-			}
-
-			// Resampling may blend a little of the bands beside the
-			// green one into the output's edges.
-			for y := bounds.Min.Y; y < bounds.Max.Y; y++ {
-				for x := bounds.Min.X; x < bounds.Max.X; x++ {
-					r, g, b, _ := output.At(x, y).RGBA()
-					if r>>8 > 64 || g>>8 < 192 || b>>8 > 64 {
-						t.Fatalf("output pixel %d,%d is %d,%d,%d, want green",
-							x, y, r>>8, g>>8, b>>8)
-					}
-				}
-			}
-		})
-	}
-}
-
-// The greys of createSquarePNG, and the grey halfway between them.
-const (
-	backgroundGrey = 192
-	squareGrey     = 64
-	halfwayGrey    = (backgroundGrey + squareGrey) / 2
-)
-
-// createSquarePNG creates a grey PNG side pixels square with a darker grey
-// square squareSide pixels wide in its centre.
-func createSquarePNG(t *testing.T, side, squareSide int) []byte {
-	t.Helper()
-
-	first := (side - squareSide) / 2
-	last := first + squareSide - 1
-
-	img := image.NewGray(image.Rect(0, 0, side, side))
-	for y := range side {
-		for x := range side {
-			grey := uint8(backgroundGrey)
-			if x >= first && x <= last && y >= first && y <= last {
-				grey = squareGrey
-			}
-
-			img.SetGray(x, y, color.Gray{Y: grey})
-		}
-	}
-
-	return encodePNG(t, img)
-}
-
-// squareLength returns how long the square is along greys, the greys of a
-// row or column of the output that crosses it: the distance between the
-// first and the last point where greys passes halfwayGrey, or 0 when there
-// is no such point. Resampling blurs the square's edges over several
-// pixels, and each point is placed between the two pixels on either side
-// of it in proportion to their greys, so the length is measured to a
-// fraction of a pixel.
-func squareLength(greys []float64) float64 {
-	first, last := -1.0, -1.0
-
-	for i := 1; i < len(greys); i++ {
-		before, after := greys[i-1], greys[i]
-		if (before > halfwayGrey) == (after > halfwayGrey) {
-			continue
-		}
-
-		point := float64(i-1) + (before-halfwayGrey)/(before-after)
-		if first < 0 {
-			first = point
-		}
-
-		last = point
-	}
-
-	return last - first
-}
-
-// TestImageProcessor_FitCoverKeepsProportions verifies that FitCover scales
-// both sides of the source by the same amount: a square in the centre of
-// the source comes out square. Its width and height are measured along the
-// output's middle row and column.
-func TestImageProcessor_FitCoverKeepsProportions(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		name             string
-		side, squareSide int
-		size             Size
-	}{
-		{"16x16 at 100x60", 16, 8, Size{Width: 100, Height: 60}},
-		{"16x16 at 60x100", 16, 8, Size{Width: 60, Height: 100}},
-		{"10x10 at 100x35", 10, 2, Size{Width: 100, Height: 35}},
-		{"10x10 at 35x100", 10, 2, Size{Width: 35, Height: 100}},
-		{"64x64 at 300x100", 64, 16, Size{Width: 300, Height: 100}},
-		{"64x64 at 100x300", 64, 16, Size{Width: 100, Height: 300}},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			input := createSquarePNG(t, tt.side, tt.squareSide)
-
-			result, err := New(Params{}).Process(t.Context(),
-				bytes.NewReader(input),
-				&Request{Size: tt.size, Format: FormatPNG, FitMode: FitCover})
-			if err != nil {
-				t.Fatalf("Process() error = %v", err)
-			}
-
-			defer func() { _ = result.Content.Close() }()
-
-			output, err := png.Decode(result.Content)
-			if err != nil {
-				t.Fatalf("failed to decode output: %v", err)
-			}
-
-			bounds := output.Bounds()
-			if bounds.Dx() != tt.size.Width || bounds.Dy() != tt.size.Height {
-				t.Fatalf("Process() made %dx%d, want %dx%d",
-					bounds.Dx(), bounds.Dy(), tt.size.Width, tt.size.Height)
-			}
-
-			row := make([]float64, bounds.Dx())
-			for x := range row {
-				r, _, _, _ := output.At(x, bounds.Dy()/2).RGBA()
-				row[x] = float64(r >> 8)
-			}
-
-			column := make([]float64, bounds.Dy())
-			for y := range column {
-				r, _, _, _ := output.At(bounds.Dx()/2, y).RGBA()
-				column[y] = float64(r >> 8)
-			}
-
-			width, height := squareLength(row), squareLength(column)
-			t.Logf("square is %.2f wide and %.2f high", width, height)
-
-			if width == 0 || math.Abs(width-height) > 0.5 {
-				t.Errorf("square is %.2f wide and %.2f high, want it square",
-					width, height)
-			}
-		})
-	}
-}

internal/imageprocessor/imageprocessor.go

@@ -7,11 +7,11 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"runtime"
 	"sync"
 	"time"
 
 	"github.com/davidbyttow/govips/v2/vips"
-	"sneak.berlin/go/pixa/internal/config"
 )
 
 // vipsOnce ensures vips is initialized exactly once.
@@ -20,10 +20,11 @@ import (
 var vipsOnce sync.Once
 
 // initVips initializes libvips with quiet logging, one worker thread per
-// image and no operation cache. Each request decodes different source
-// bytes, so the operation cache would rarely be hit and would hold memory
-// outside MaxConcurrentProcessing; repeated requests are served from
-// pixa's disk cache instead.
+// image and no operation cache. Process already works on one image per CPU
+// by default, so more threads per image would only compete for the CPUs.
+// Each request decodes different source bytes, so the operation cache
+// would rarely be hit and would hold memory outside MaxConcurrentProcessing;
+// repeated requests are served from pixa's disk cache instead.
 func initVips() {
 	vipsOnce.Do(func() {
 		vips.LoggingSettings(nil, vips.LogLevelError)
@@ -178,7 +179,7 @@ type Params struct {
 	// If <= 0, DefaultMaxInputBytes is used.
 	MaxInputBytes int64
 	// MaxConcurrentProcessing is the most images processed at once.
-	// If <= 0, config.DefaultMaxConcurrentProcessing() is used.
+	// If <= 0, the number of CPUs Go uses (runtime.GOMAXPROCS(0)) is used.
 	MaxConcurrentProcessing int
 }
 
@@ -194,7 +195,7 @@ func New(params Params) *ImageProcessor {
 
 	maxConcurrentProcessing := params.MaxConcurrentProcessing
 	if maxConcurrentProcessing <= 0 {
-		maxConcurrentProcessing = config.DefaultMaxConcurrentProcessing()
+		maxConcurrentProcessing = runtime.GOMAXPROCS(0)
 	}
 
 	return &ImageProcessor{
@@ -475,8 +476,8 @@ func (p *ImageProcessor) resize(
 ) error {
 	switch fit {
 	case FitCover, "":
-		// Crop the centre and resize to fill exact dimensions (default)
-		return cropCentreAndResize(img, width, height)
+		// Resize and crop to fill exact dimensions (default)
+		return img.Thumbnail(width, height, vips.InterestingCentre)
 
 	case FitContain:
 		// Resize to fit within dimensions, maintaining aspect ratio
@@ -530,38 +531,6 @@ func (p *ImageProcessor) resize(
 	}
 }
 
-// cropCentreAndResize scales img by the same amount on both sides, the
-// least that makes it cover width x height, and cuts the centre width x
-// height out of it, as libvips' thumbnail does with InterestingCentre.
-// thumbnail alone scales the whole image and copies the result into memory
-// before it cuts, and for a thin image that is far larger than the image or
-// the output: 8192x64 at 8192x8192 is 1048576x8192, about 24 GiB. So the
-// centre is cut from img first, widened to whole pixels of img, which adds
-// less than one pixel of img on each side, and thumbnail scales and cuts
-// only that.
-func cropCentreAndResize(img *vips.ImageRef, width, height int) error {
-	imgW, imgH := img.Width(), img.Height()
-
-	// An image wider than the requested proportions keeps its height and
-	// loses width down to imgH*width/height, any other keeps its width and
-	// loses height down to imgW*height/width. Half of what is lost comes
-	// off each side, rounded down to whole pixels, so the part kept holds
-	// all of the centre.
-	left, top := 0, 0
-	if imgW*height > imgH*width {
-		left = (imgW*height - imgH*width) / (2 * height) //nolint:mnd // each side
-	} else {
-		top = (imgH*width - imgW*height) / (2 * width) //nolint:mnd // each side
-	}
-
-	err := img.ExtractArea(left, top, imgW-2*left, imgH-2*top)
-	if err != nil {
-		return err
-	}
-
-	return img.Thumbnail(width, height, vips.InterestingCentre)
-}
-
 const defaultQuality = 85
 
 // encode encodes an image to the specified format. govips gives pixa no way

internal/imageprocessor/max_concurrent_processing_internal_test.go

@@ -115,14 +115,14 @@ func waitForEntries(t *testing.T, entered <-chan struct{}, count int) {
 	}
 }
 
-func TestNewDefaultsMaxConcurrentProcessingToHalfTheCPUs(t *testing.T) {
+func TestNewDefaultsMaxConcurrentProcessingToCPUs(t *testing.T) {
 	t.Parallel()
 
 	for _, limit := range []int{0, -1} {
 		proc := New(Params{MaxConcurrentProcessing: limit})
-		if got := cap(proc.processingSemaphore); got != max(runtime.GOMAXPROCS(0)/2, 1) {
-			t.Errorf("MaxConcurrentProcessing %d: %d slots, want %d, half the CPUs",
-				limit, got, max(runtime.GOMAXPROCS(0)/2, 1))
+		if got := cap(proc.processingSemaphore); got != runtime.GOMAXPROCS(0) {
+			t.Errorf("MaxConcurrentProcessing %d: %d slots, want %d, one per CPU",
+				limit, got, runtime.GOMAXPROCS(0))
 		}
 	}
 
@@ -132,30 +132,6 @@ func TestNewDefaultsMaxConcurrentProcessingToHalfTheCPUs(t *testing.T) {
 	}
 }
 
-// TestNewDefaultIsHalfTheCPUsAndAtLeastOne checks that New, given no
-// MaxConcurrentProcessing, makes one slot for every two CPUs Go uses,
-// rounded down, and one slot when Go uses one CPU.
-//
-//nolint:paralleltest // it sets GOMAXPROCS, which other tests read
-func TestNewDefaultIsHalfTheCPUsAndAtLeastOne(t *testing.T) {
-	cpusBefore := runtime.GOMAXPROCS(0)
-
-	t.Cleanup(func() { runtime.GOMAXPROCS(cpusBefore) })
-
-	for _, tc := range []struct{ cpus, want int }{
-		{1, 1}, {2, 1}, {3, 1}, {8, 4},
-	} {
-		runtime.GOMAXPROCS(tc.cpus)
-
-		got := cap(New(Params{}).processingSemaphore)
-		t.Logf("GOMAXPROCS %d: %d slots", tc.cpus, got)
-
-		if got != tc.want {
-			t.Errorf("GOMAXPROCS %d: %d slots, want %d", tc.cpus, got, tc.want)
-		}
-	}
-}
-
 // TestProcessNeverExceedsMaxConcurrentProcessing starts more Process calls
 // than MaxConcurrentProcessing allows and holds the first ones inside
 // Process until the test lets them go. No more than the limit may be

internal/imgcache/eviction.go

@@ -18,6 +18,11 @@ import (
 // write-pressure wakeups triggered by stores.
 const DefaultEvictionInterval = 5 * time.Minute
 
+// reconciliationInterval is how long after one reconciliation pass ends
+// the next one starts. A pass reads a row for every cached file, so its
+// cost grows with the cache, and it runs this rarely for that reason.
+const reconciliationInterval = 24 * time.Hour
+
 // evictionBatchSize is how many LRU candidates of each class (variants
 // and source blobs) one eviction pass fetches from the database.
 const evictionBatchSize = 100
@@ -437,15 +442,17 @@ func (c *Cache) notifyWritePressure() {
 	}
 }
 
-// StartEviction launches the background eviction goroutine, which
-// 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.
+// StartEviction launches the background eviction goroutine. At startup
+// it reconciles the database accounting with the cache directories,
+// removes the expired negative cache rows and evicts to the configured
+// limit. After that it reconciles again a day after each reconciliation
+// pass ends, removes the expired negative cache rows on every tick of
+// the given interval, and evicts to the limit on every tick, after each
+// reconciliation pass 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.evictionCancel != nil {
 		return
@@ -486,6 +493,16 @@ func (c *Cache) evictionLoop(ctx context.Context, interval time.Duration) {
 	c.runCleanExpired(ctx)
 	c.runEvictionPass(ctx)
 
+	// Reconciliation walks the cache directories and reads a row for
+	// every file, so it runs on a timer of its own rather than on the
+	// periodic tick or on write-pressure wakeups. The timer starts only
+	// once a pass has ended, so passes never run back to back, however
+	// long one takes. Running it daily, not just at startup, bounds how
+	// long a store's best-effort accounting insert can stay silently
+	// unaccounted for to about one day, plus the time the passes take.
+	reconciliationTimer := time.NewTimer(reconciliationInterval)
+	defer reconciliationTimer.Stop()
+
 	ticker := time.NewTicker(interval)
 	defer ticker.Stop()
 
@@ -493,17 +510,10 @@ func (c *Cache) evictionLoop(ctx context.Context, interval time.Duration) {
 		select {
 		case <-ctx.Done():
 			return
-		case <-ticker.C:
-			// Reconciliation walks the cache directories, so it only
-			// runs on the periodic ticker rather than on every
-			// write-pressure wakeup, keeping it off the per-store hot
-			// path. Reusing the eviction interval itself (rather than a
-			// separate, longer one) is a deliberate choice: it is the
-			// simplest option that still bounds how long a store's
-			// best-effort accounting insert can stay silently
-			// unaccounted for to one interval, on a process that is
-			// already running this loop regardless.
+		case <-reconciliationTimer.C:
 			c.runReconciliationPass(ctx)
+			reconciliationTimer.Reset(reconciliationInterval)
+		case <-ticker.C:
 			c.runCleanExpired(ctx)
 		case <-c.evictionPressure:
 		}
@@ -556,19 +566,18 @@ func (c *Cache) runCleanExpired(ctx context.Context) {
 
 // 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
-// hot path: it adopts variant files that predate the accounting table
-// (or whose accounting insert failed, e.g. StoreVariant's best-effort
-// insert under transient DB contention), drops accounting rows whose
-// files are missing, removes source blob files the database does not
-// know (and rows whose files are gone), removes source and variant files
-// that are empty or not the size their rows record, sweeps stale temp
-// files left behind by crashed writes, and last checks the total cache
-// usage against the tables. It reads the tables a page at a time.
-// Running it periodically, not just once, bounds how long such drift can
-// accumulate unaccounted for on a long-running process to one eviction
-// interval. Once ctx is cancelled, it stops at the next file or row and
-// returns ctx's error.
+// then once a day, off the request hot path: it adopts variant files
+// that predate the accounting table (or whose accounting insert failed,
+// e.g. StoreVariant's best-effort insert under transient DB contention),
+// drops accounting rows whose files are missing, removes source blob
+// files the database does not know (and rows whose files are gone),
+// removes source and variant files that are empty or not the size their
+// rows record, sweeps stale temp files left behind by crashed writes,
+// and last checks the total cache usage against the tables. It reads the
+// tables a page at a time. Running it daily, not just once, bounds how
+// long such drift can accumulate unaccounted for on a long-running
+// process to about one day. Once ctx is cancelled, it stops at the next
+// file or row and returns ctx's error.
 func (c *Cache) reconcileAccounting(ctx context.Context) error {
 	if c.disabled {
 		return nil

internal/imgcache/eviction_internal_test.go

@@ -9,7 +9,9 @@ import (
 	"log/slog"
 	"os"
 	"path/filepath"
+	"strconv"
 	"strings"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -918,6 +920,84 @@ func TestPeriodicReconciliationAdoptsFileThatAppearsAfterStartup(t *testing.T) {
 	}
 }
 
+// TestEvictionTicksDoNotReconcile starts the eviction loop with an
+// interval of 10 ms. The startup pass reconciles before it removes the
+// expired negative cache rows, so once the expired row stored before the
+// start is gone, the startup reconciliation has made all its reads. The
+// test then waits for three periodic ticks, each seen when it removes an
+// expired row stored after the previous one went. The reconciliation
+// pass runs at startup and then once a day, so none of those ticks may
+// read a content table for reconciliation.
+func TestEvictionTicksDoNotReconcile(t *testing.T) {
+	t.Parallel()
+
+	cache, _ := newEvictionTestCache(t, 1<<30)
+
+	var reconciliationReads atomic.Int64
+
+	cache.reconciliationReadTestHook = func(int) { reconciliationReads.Add(1) }
+
+	insertExpiredNegativeCacheRow(t, cache, "/startup.jpg")
+
+	cache.StartEviction(10 * time.Millisecond)
+	defer func() { _ = cache.StopEviction(t.Context()) }()
+
+	waitForNegativeCacheRowRemoved(t, cache, "/startup.jpg")
+
+	startupReads := reconciliationReads.Load()
+	t.Logf("the startup pass made %d reconciliation reads", startupReads)
+
+	if startupReads == 0 {
+		t.Fatal("the startup pass made no reconciliation read")
+	}
+
+	for tick := range 3 {
+		path := "/tick-" + strconv.Itoa(tick) + ".jpg"
+
+		insertExpiredNegativeCacheRow(t, cache, path)
+		waitForNegativeCacheRowRemoved(t, cache, path)
+	}
+
+	tickReads := reconciliationReads.Load() - startupReads
+	t.Logf("three eviction ticks made %d reconciliation reads", tickReads)
+
+	if tickReads != 0 {
+		t.Errorf("three eviction ticks made %d reconciliation reads, want 0: "+
+			"the reconciliation pass ran on an eviction tick", tickReads)
+	}
+}
+
+// insertExpiredNegativeCacheRow adds a negative_cache row for sourcePath
+// that expired an hour ago.
+func insertExpiredNegativeCacheRow(t *testing.T, cache *Cache, sourcePath string) {
+	t.Helper()
+
+	_, 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, sourcePath)
+	if err != nil {
+		t.Fatalf("failed to insert an expired negative cache row: %v", err)
+	}
+}
+
+// waitForNegativeCacheRowRemoved waits up to five seconds for the
+// eviction loop to remove the negative_cache row for sourcePath.
+func waitForNegativeCacheRowRemoved(t *testing.T, cache *Cache, sourcePath string) {
+	t.Helper()
+
+	deadline := time.Now().Add(5 * time.Second)
+
+	for negativeCacheRowCount(t, cache, sourcePath) > 0 {
+		if time.Now().After(deadline) {
+			t.Fatalf("the expired negative cache row for %s was not removed", sourcePath)
+		}
+
+		time.Sleep(10 * time.Millisecond)
+	}
+}
+
 // TestStopEvictionInterruptsPassInProgress holds the test database's
 // only connection, so the startup reconciliation pass waits for it, and
 // checks that StopEviction stops that pass instead of waiting for the

internal/imgcache/service.go

@@ -53,7 +53,7 @@ type ServiceConfig struct {
 	// Allowlist is the list of hosts that don't require signatures
 	Allowlist []string
 	// MaxConcurrentProcessing is the most images processed at once; zero
-	// uses the image processor's default, half the CPUs
+	// uses the image processor's default, one per CPU
 	MaxConcurrentProcessing int
 	// Logger for logging
 	Logger *slog.Logger

internal/imgcache/urlparser.go

@@ -47,9 +47,6 @@ func ValidateDimension(name string, value int) error {
 var sizeFormatRegex = regexp.MustCompile(
 	`^(\d+)x(\d+)(?:\.(\w+))?$|^(orig)(?:\.(\w+))?$`)
 
-// percentEscapeRegex matches one percent-escape, such as "%2e"
-var percentEscapeRegex = regexp.MustCompile(`%[0-9A-Fa-f]{2}`)
-
 // ParsedURL contains the parsed components of an image proxy URL.
 type ParsedURL struct {
 	// Host is the source origin host (e.g., "cdn.example.com")
@@ -64,8 +61,9 @@ type ParsedURL struct {
 	Format ImageFormat
 }
 
-// ParseImagePath parses an image URL's path after /v1/image/, decoded once:
-// <host>/<path>/<size>.<format>, or <host>/<path>/<size> for JPEG XL.
+// ParseImagePath parses the path captured by chi's wildcard:
+// <host>/<path>/<size>.<format>, or <host>/<path>/<size> for JPEG XL
+// This is the primary entry point when using chi routing.
 // Examples:
 //   - cdn.example.com/photos/cat.jpg/800x600.webp
 //   - cdn.example.com/photos/cat.jpg/0x0.jpeg
@@ -81,10 +79,9 @@ func ParseImagePath(path string) (*ParsedURL, error) {
 	return parseImageComponents(path)
 }
 
-// ParseImageURL parses an image URL's path, decoded once as r.URL.Path holds
-// it: /v1/image/<host>/<path>/<size>.<format>, or /v1/image/<host>/<path>/<size>
-// for JPEG XL. A source query follows the source path after a "?", which the
-// URL writes as %3F.
+// ParseImageURL parses a full URL path like /v1/image/<host>/<path>/<size>.<format>,
+// or /v1/image/<host>/<path>/<size> for JPEG XL
+// Use ParseImagePath instead when working with chi's wildcard capture.
 func ParseImageURL(urlPath string) (*ParsedURL, error) {
 	// Remove the /v1/image/ prefix
 	const prefix = "/v1/image/"
@@ -103,6 +100,12 @@ func ParseImageURL(urlPath string) (*ParsedURL, error) {
 // parseImageComponents parses <host>/<path>/<size>.<format>, or
 // <host>/<path>/<size> for JPEG XL.
 func parseImageComponents(remainder string) (*ParsedURL, error) {
+	// Check for path traversal before any other processing
+	err := checkPathTraversal(remainder)
+	if err != nil {
+		return nil, err
+	}
+
 	// Find the last path segment, which holds "size" or "size.format"
 	lastSlash := strings.LastIndex(remainder, "/")
 	if lastSlash == -1 {
@@ -158,13 +161,6 @@ func parseImageComponents(remainder string) (*ParsedURL, error) {
 		path = path[:qIndex]
 	}
 
-	// The source query names no file on the upstream, so only the host and
-	// the source path are checked for path traversal
-	err = checkPathTraversal(host + path)
-	if err != nil {
-		return nil, err
-	}
-
 	// Ensure path starts with /
 	if !strings.HasPrefix(path, "/") {
 		path = "/" + path
@@ -190,7 +186,12 @@ func checkPathTraversal(path string) error {
 	// Decode multiple times to catch double-encoding
 	decoded := path
 	for range 3 {
-		newDecoded := decodePercentEscapes(decoded)
+		newDecoded, err := url.PathUnescape(decoded)
+		if err != nil {
+			// Malformed encoding is suspicious
+			return ErrPathTraversal
+		}
+
 		if newDecoded == decoded {
 			break
 		}
@@ -231,18 +232,6 @@ func checkPathTraversal(path string) error {
 	return nil
 }
 
-// decodePercentEscapes decodes every percent-escape in s, such as %2e, and
-// keeps a "%" that does not start one as it is, where url.PathUnescape would
-// refuse the whole string. A source path holding %25, a literal "%", has such
-// a "%" after one more decoding.
-func decodePercentEscapes(s string) string {
-	return percentEscapeRegex.ReplaceAllStringFunc(s, func(escape string) string {
-		decoded, _ := url.PathUnescape(escape)
-
-		return decoded
-	})
-}
-
 // parseSizeFormat parses strings like "800x600.webp", "orig.png" or "800x600"
 func parseSizeFormat(s string) (Size, ImageFormat, error) {
 	matches := sizeFormatRegex.FindStringSubmatch(s)

internal/imgcache/urlparser_internal_test.go

@@ -407,71 +407,6 @@ func TestParseImagePath_PathTraversal(t *testing.T) {
 	}
 }
 
-// TestParseImageURL_SourceURLHoldingPercent25 verifies that a source path and
-// a source query holding %25, the percent-escape of a literal "%", are
-// accepted as they are, although decoding them once more leaves a "%" that
-// does not start a percent-escape.
-func TestParseImageURL_SourceURLHoldingPercent25(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		name  string
-		input string
-		want  *ParsedURL
-	}{
-		{
-			name:  "path",
-			input: "/v1/image/cdn.example.com/sale/50%25off.jpg/800x600.webp",
-			want: &ParsedURL{
-				Host: testHostCDN, Path: "/sale/50%25off.jpg",
-				Size: Size{Width: 800, Height: 600}, Format: FormatWebP,
-			},
-		},
-		{
-			name:  "query",
-			input: "/v1/image/cdn.example.com/photos/cat.jpg?discount=50%25/800x600.webp",
-			want: &ParsedURL{
-				Host: testHostCDN, Path: testPathCat, Query: "discount=50%25",
-				Size: Size{Width: 800, Height: 600}, Format: FormatWebP,
-			},
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			got, err := ParseImageURL(tt.input)
-			if err != nil {
-				t.Fatalf("ParseImageURL() unexpected error = %v", err)
-			}
-
-			assertParsedURL(t, got, tt.want)
-		})
-	}
-}
-
-// TestParseImageURL_DoubleEncodedTraversalNextToPercent verifies that a
-// double-encoded ".." is still refused when the path also holds a "%" that
-// does not start a percent-escape, either as written or after one more
-// decoding.
-func TestParseImageURL_DoubleEncodedTraversalNextToPercent(t *testing.T) {
-	t.Parallel()
-
-	for _, input := range []string{
-		"/v1/image/cdn.example.com/photos/%252e%252e/secret/50%25off.jpg/800x600.jpeg",
-		"/v1/image/cdn.example.com/photos/%252e%252e/secret/100%/800x600.jpeg",
-	} {
-		_, err := ParseImageURL(input)
-		t.Logf("ParseImageURL(%q) error = %v", input, err)
-
-		if !errors.Is(err, ErrPathTraversal) {
-			t.Errorf("ParseImageURL(%q) error = %v, want ErrPathTraversal",
-				input, err)
-		}
-	}
-}
-
 // errorIs checks if err matches target (handles wrapped errors).
 func errorIs(err, target error) bool {
 	if errors.Is(err, target) {

internal/signature/signature.go

@@ -126,20 +126,31 @@ func (s *Signer) GenerateSignedURL(
 		sizeStr = fmt.Sprintf("%dx%d", req.Width, req.Height)
 	}
 
-	// Build the path. 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.SourcePath
+	// Build the path.
+	// When a source query is present, it is embedded as a path segment
+	// (e.g. /host/path?query/size.fmt) so that the URL parser can extract
+	// it from the last-slash split. The "?" inside a path segment is
+	// percent-encoded by clients but chi delivers it decoded, which is
+	// exactly what the URL parser expects.
+	var path string
 	if req.SourceQuery != "" {
-		sourcePathAndQuery += "?" + req.SourceQuery
+		path = fmt.Sprintf("/v1/image/%s%s%%3F%s/%s.%s",
+			req.SourceHost,
+			req.SourcePath,
+			url.PathEscape(req.SourceQuery),
+			sizeStr,
+			req.Format,
+		)
+	} else {
+		path = fmt.Sprintf("/v1/image/%s%s/%s.%s",
+			req.SourceHost,
+			req.SourcePath,
+			sizeStr,
+			req.Format,
+		)
 	}
 
-	unescapedPath := fmt.Sprintf("/v1/image/%s%s/%s.%s",
-		req.SourceHost, sourcePathAndQuery, sizeStr, req.Format)
-
-	return (&url.URL{Path: unescapedPath}).EscapedPath(), sig, exp
+	return path, sig, exp
 }
 
 // buildSignatureData creates the string to be signed.

internal/signature/signature_test.go

@@ -475,44 +475,6 @@ func TestGenerateSignedURL_WithoutQueryString(t *testing.T) {
 	}
 }
 
-// TestGenerateSignedURL_ReadmeQueryExample checks the README's example of a
-// source URL with a query, the image at
-// https://cdn.example.com/img?url=https://origin.example.com/my%20cat.jpg: its
-// signature with the README's key, and the path GenerateSignedURL writes for
-// it, with the "?" written as %3F and the "%" as %25.
-func TestGenerateSignedURL_ReadmeQueryExample(t *testing.T) {
-	t.Parallel()
-
-	signer := signature.New("example-signing-key-for-documentation")
-
-	req := signature.Request{
-		SourceHost:  testHost,
-		SourcePath:  "/img",
-		SourceQuery: "url=https://origin.example.com/my%20cat.jpg",
-		Width:       800,
-		Height:      600,
-		Format:      testFormatWebP,
-		Quality:     85,
-		FitMode:     testFitCover,
-		Expires:     time.Unix(1704067200, 0),
-	}
-
-	const wantSig = "37aFa9GSVCZM1kRY9SzMOk2fOg_M7Wt5gKNR4xM-FYA="
-
-	gotSig := signer.Sign(&req)
-	if gotSig != wantSig {
-		t.Errorf("Sign() = %q, want %q", gotSig, wantSig)
-	}
-
-	const wantPath = "/v1/image/cdn.example.com" +
-		"/img%3Furl=https://origin.example.com/my%2520cat.jpg/800x600.webp"
-
-	gotPath, _, _ := signer.GenerateSignedURL(&req, time.Hour)
-	if gotPath != wantPath {
-		t.Errorf("GenerateSignedURL() path = %q, want %q", gotPath, wantPath)
-	}
-}
-
 func TestParseParams(t *testing.T) {
 	t.Parallel()