Keep this issue's TODO.md entry at the top of Completed Steps

The rebase onto next put the newer entry above it.

Model: opus-5-5

README.md

@@ -26,6 +26,11 @@ make docker
 docker run -p 8080:8080 -e PIXA_SIGNING_KEY="$(openssl rand -base64 32)" pixa:latest
 ```
 
+The example config allowlists no host (`allowlist_hosts`), so pixa serves an
+image only for a signed or encrypted URL. Every image on an allowlisted host is
+served to anyone without a signature, at any size and in any format, so
+allowlist only hosts you control, where nobody else can put images.
+
 A container takes its settings from environment variables (see Configuration
 below for the list). Only `PIXA_SIGNING_KEY` is required; if it is unset the
 container exits at startup naming the variable. Everything else has a built-in
@@ -46,9 +51,9 @@ everything in this list without further settings. The reverse proxy must:
   builds encrypted URLs from `Host`, and checks `Referer` against
   `referer_blocklist`;
 - set `X-Forwarded-For` to the client's address, with `trusted_proxies` set to
-  the address pixa sees the proxy's requests come from, so the login limit
-  counts each client by its own address (see `trusted_proxies` under
-  Configuration);
+  the address pixa sees the proxy's requests come from, so the login limit and
+  `image_requests_per_minute` count each client by its own address (see
+  `trusted_proxies` under Configuration);
 - wait for pixa's answer for at least `downstream_timeout` (default `60s`), the
   longest pixa takes to fetch, convert and send an image.
 
@@ -58,14 +63,16 @@ password there.
 
 pixa does the rest itself: it checks signatures and encrypted URLs, applies the
 allowlist, refuses upstream hosts with private or local addresses, limits login
-attempts, upstream response size and image dimensions, and sends the security
-headers, `Strict-Transport-Security` included, with every response.
+attempts, each client's image requests, upstream response size and image
+dimensions, and sends the security headers, `Strict-Transport-Security`
+included, with every response.
 
 The state directory (`state_dir`, `/var/lib/pixa` in the container) holds the
 database and the disk cache:
 
 - It needs a persistent volume: without one, every restart starts with an empty
-  cache. In the container, the startup script gives the directory to the user
+  cache. Losing the directory also ends every login session (see Encrypted
+  URLs). In the container, the startup script gives the directory to the user
   pixa runs as (uid 65532) and sets its mode to `750`; outside it, that user
   must be able to write the directory.
 - `cache_max_bytes` limits the source and transformed images together. The
@@ -114,10 +121,44 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for pixa needs:
     - `PIXA_SIGNING_KEY` (required): secret for signed and encrypted URLs and
       login, 32+ characters, for example from `openssl rand -base64 32`
     - `PIXA_ALLOWLIST_HOSTS`: upstream hosts served without a signature,
-      comma-separated
-    - `PIXA_CACHE_MAX_BYTES`: disk cache limit in bytes; `0` disables it;
-      default 75% of (free space + what the cache holds)
+      comma-separated; default none. Every image on a listed host is served to
+      anyone without a signature, so list only hosts you control, where nobody
+      else can put images.
+    - `PIXA_CACHE_MAX_BYTES`: disk cache limit in bytes; `0` disables it. Set
+      it: the volume is a directory on the host's filesystem, shared with the
+      host and every other app. The default is 75% of the sum of that
+      filesystem's free space and what the cache holds, and at least 500 MiB, so
+      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
@@ -152,6 +193,14 @@ name.
 Multiple source paths may reference the same content blob; the database tracks
 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
+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.
+
 pixa's target is 1-5k r/s, which has not been measured at that rate (see Load
 Test). Toward it, pixa keeps in memory the content types of the 10,000
 transformed images most recently cached or served, so a cache hit on one of them
@@ -176,7 +225,11 @@ path under `/v1/` answers 200, in maintenance mode too.
   session it answers 303 to `/`. Answers: 200 with the page showing the URL; 400
   with the page naming a field that is not valid; 500 when the URL cannot be
   made.
-- `GET /logout` — end the login session. Needs: nothing. Answers: 303 to `/`.
+- `POST /logout` — log out with the generator page's Logout button: end every
+  login session, in every browser (see Encrypted URLs), and remove the session
+  cookie. Needs: a login session and the generator page's form (below); without
+  a login session it only removes the cookie. Answers: 303 to `/`; 500 when the
+  logout cannot be saved in the database.
 - `GET` or `HEAD` `/v1/image/<host>/<path>/<size>.<format>`, or
   `/v1/image/<host>/<path>/<size>` with no format — an image, fetched, resized
   and converted (below). Needs: a signature, unless the host is allowlisted (see
@@ -188,10 +241,14 @@ path under `/v1/` answers 200, in maintenance mode too.
   `Referer` names a host in `referer_blocklist`, checked before the signature,
   the cache and the upstream fetch; 403 when the upstream host, or a host it
   redirects to, is `localhost`, ends in `.localhost` or `.local`, or has an
-  address in a blocked network (see `blocked_networks`); 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.
+  address in a blocked network (see `blocked_networks`); 429 over
+  `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 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
@@ -199,7 +256,7 @@ path under `/v1/` answers 200, in maintenance mode too.
   expired; 504 when the upstream has not sent its response headers within
   `upstream_fetch_timeout`, but 500 when that time runs out while the image
   itself is still arriving; 400 for an `Accept` header that is not valid, and
-  406, 403, 502, 503 and 500, as for `/v1/image/`.
+  406, 403, 415, 429, 502, 503 and 500, as for `/v1/image/`.
 - `GET /robots.txt` — asks every crawler to stay away (`Disallow: /`). Needs:
   nothing. Answers: 200.
 - `GET /.well-known/healthcheck.json` — JSON with `status` (`ok`), `now`,
@@ -220,7 +277,7 @@ the request carries the same ID as `request_id`, and so do the lines it logs
 when it fetches, converts and serves an image; the fetch sends it to the
 upstream host as `X-Request-ID`.
 
-Both `POST` routes accept only a form that pixa's own page served: the page puts
+The `POST` routes accept only a form that pixa's own page served: the page puts
 a token in the form and sets a cookie to match, and a request without both is
 refused with 403, so another site cannot submit the form from a visitor's
 browser. The login and generator pages are meant to be opened over HTTPS: while
@@ -259,9 +316,17 @@ 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.
 
-The source image may be JPEG, PNG, GIF, WebP, AVIF, JPEG XL or SVG. The
-upstream's `Content-Type` must name its format, and the image's first bytes must
-match it.
+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
+cover the size and so past it on one side, when that side would come out
+above 8192.
+
+The source image may be JPEG, PNG, GIF, WebP, AVIF or JPEG XL. The upstream's
+`Content-Type` must name its format, and the image's first bytes must match it.
+An upstream answering with any other `Content-Type`, SVG included, is refused
+with 415, and so is an image whose first bytes libvips reads as another format,
+such as an SVG whose first bytes pass the check for AVIF.
 
 With the format `auto`, pixa chooses the format for each request from its
 `Accept` header, in this order:
@@ -294,9 +359,26 @@ cache at once, they share one upstream fetch (or one read of the cached source)
 and one transcode: the first request does the work, and the others wait for its
 image or its error, holding no upstream connection or processing slot of their
 own. A waiting request stops waiting when its own client goes away. The work
-goes on for the others even if the first request's client goes away, until that
-request's `downstream_timeout` ends. The shared fetch sends the first request's
-ID upstream, and the lines logged for the fetch and the transcode carry that ID.
+goes on for the others even if the first request's client goes away. The shared
+fetch sends the first request's ID upstream, and the lines logged for the fetch
+and the transcode carry that ID.
+
+A transcode whose request's `downstream_timeout` has ended (for a shared one,
+the first request's) stops waiting for a processing slot, or, once it has one,
+gives it back without beginning to encode. A request waiting for a shared
+transcode that stops this way is answered 503 with the error
+`server busy, try again later`, though its own `downstream_timeout` may not have
+ended. govips gives pixa no way to stop libvips once an encode has begun, so an
+encode keeps its slot until it finishes, which can be after
+`downstream_timeout`. The limit of 8192 on each side of the source and of the
+output bounds how long that takes. With one libvips thread, as pixad runs it,
+encoding 8192x8192 random pixels takes up to about 1 second as JPEG, 15 as PNG,
+16 as GIF, 63 as WebP, 110 as AVIF and 96 as JPEG XL, the slowest of 8 or 16
+bits per sample, with or without an alpha channel. WebP, AVIF and JPEG XL are
+that slow only with an alpha channel: 8-bit RGB takes about 44, 53 and 28
+seconds. Decoding such a source adds up to about 11 seconds. A WebP, AVIF or
+JPEG XL encode can therefore hold its slot past the default `downstream_timeout`
+of 60 seconds.
 
 The login form (`POST /`) is limited to 5 attempts per minute per client
 address, counting an IPv6 client by its /64; an attempt over the limit is
@@ -315,6 +397,15 @@ its own `X-Forwarded-For`, whether it connects directly or through the proxy,
 because its own address is trusted too. Setting `trusted_proxies` to only the
 address pixa sees for requests that come through the proxy closes this.
 
+One client may send `image_requests_per_minute` requests per minute
+(default 600) to the image routes, `/v1/image/` and `/v1/e/` together, `GET` and
+`HEAD`, cache hits included; a request over the limit is refused with 429 and a
+`Retry-After` header. Clients are counted as for the login limit. A browser's
+CORS preflight is not counted. Behind a CDN or proxy whose addresses are not in
+`trusted_proxies`, all requests arriving from one proxy or CDN address count as
+one client, so such a deployment needs a higher limit, or `0`, which turns the
+limit off.
+
 ### Encrypted URLs
 
 An encrypted URL is an image URL made on pixa's own web page by someone who
@@ -324,13 +415,22 @@ nor change what it asks for.
 
 1. Open `/` in a browser over HTTPS (or over plain HTTP while `debug` is on, see
    Routes) and log in with the signing key (`signing_key`). The login session
-   lasts 30 days, or until `/logout`.
+   lasts 30 days, or until someone logs out with the generator page's Logout
+   button. Every login is made with the one signing key, so logging out ends
+   every login session, in every browser, a copy of the session cookie included.
+   pixa keeps a random value in its database, which it makes when the database
+   has none and replaces at each logout. A login session carries the value
+   current at its login and is refused once the value has changed, also after a
+   restart (see Deployment). The check for a logout compares no time, so a login
+   after a logout works whatever the host's clock says; the 30 days are still
+   measured by the host's clock.
 2. On the generator page, give the source image's URL, the width and height, the
    format, quality and fit, and how long the URL lasts, then submit the form
    (`POST /generate`). The format is JPEG XL unless another is chosen; a form
    sent with an empty format, or none, also makes a JPEG XL URL. Width and
    height both empty or `0` keep the original size; if only one of them is empty
-   or `0`, that side is scaled to keep the image's proportions.
+   or `0`, that side is scaled to keep the image's proportions. A URL whose
+   scaled side would come out above 8192 answers 400.
 3. The page shows the URL, `https://<host>/v1/e/<token>/img.<format>`, and when
    it expires. `<host>` is the host the page was opened on, and the URL starts
    with `http` instead while `debug` is on. The name after the token is ignored
@@ -389,9 +489,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`
@@ -441,6 +546,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
@@ -483,7 +594,8 @@ 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 the number of CPUs                         |
+| `PIXA_MAX_CONCURRENT_PROCESSING`     | `max_concurrent_processing`     | Images processed at once; default half the CPUs, at least 1                  |
+| `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                 |
 | `PIXA_DOWNSTREAM_TIMEOUT`            | `downstream_timeout`            | Time allowed for answering one client request; default `60s`                 |
@@ -533,13 +645,13 @@ Key settings in more detail:
   explicit list replaces the default. An invalid CIDR aborts startup. Set this
   to the address pixa sees for requests that come through your proxy, such as
   `172.17.0.1/32`, when the defaults do not cover it, or to trust nothing else
-  (see the login limit under Routes). For a proxy on the Docker host that
-  connects to pixa over `127.0.0.1`, that address is the gateway of the
-  container's Docker network (`172.17.0.1` on the default bridge), not the
-  proxy's own address; a proxy that connects through another of the host's
-  addresses is seen with that address. To be sure which address it is, set this
-  to `[]` (or `PIXA_TRUSTED_PROXIES` to empty), send a request through the
-  proxy, and read `remoteIP` in pixa's request log line for it
+  (see the login limit and `image_requests_per_minute` under Routes). For a
+  proxy on the Docker host that connects to pixa over `127.0.0.1`, that address
+  is the gateway of the container's Docker network (`172.17.0.1` on the default
+  bridge), not the proxy's own address; a proxy that connects through another of
+  the host's addresses is seen with that address. To be sure which address it
+  is, set this to `[]` (or `PIXA_TRUSTED_PROXIES` to empty), send a request
+  through the proxy, and read `remoteIP` in pixa's request log line for it
 - `upstream_fetch_timeout` — time allowed for one fetch from an upstream host,
   as a duration such as `30s` (the default) or `2m`
 - `upstream_max_response_size` — largest upstream response accepted, in bytes;
@@ -568,10 +680,18 @@ 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 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
+  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"
+- `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.
+  Behind a CDN or proxy whose addresses are not in `trusted_proxies`, all
+  requests arriving from one proxy or CDN address count as one client, so raise
+  the limit there, or set `0`
 - `maintenance_mode` — while `true`, the image routes (`/v1/image/` and
   `/v1/e/`) answer every request for an image with 503, a `Retry-After` header
   and a JSON error body. The health check (`/.well-known/healthcheck.json`)
@@ -592,7 +712,10 @@ See `configs/config.example.yml` for all options with defaults.
 - **Metrics**: Prometheus, at `/metrics`: generic HTTP request metrics
   (duration, response size, requests in flight) and the Go runtime and process
   metrics; requests are measured and `/metrics` is served only when
-  `metrics.username` and `metrics.password` are set
+  `metrics.username` and `metrics.password` are set. A request's `handler` label
+  is the pattern of the route it matches, such as `/v1/image/*`, never its path.
+  Requests that match no route share the label `unmatched`: those the router
+  answers 404 or 405, and CORS preflights
 - **Logging**: stdlib slog
 
 ## Entrypoints

TODO.md

@@ -26,10 +26,187 @@ resolved. The disk cache is now size-bounded with LRU eviction
 
 # Next Step
 
-P2: security: per-IP rate limiting on the image routes
+P2: security: per-origin rate limiting
 
 # Completed Steps
 
+- 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 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
+  with the one signing key, so logging out now ends them all: the session
+  manager keeps a random value, made with `crypto/rand` when the new `logout`
+  table has none and replaced by each logout, saves it there so that it outlasts
+  a restart, puts the value at login in each session, and refuses a session
+  carrying another value. The check for a logout compares no time, so a login
+  after a logout works whatever the host's clock does; a session's 30 days are
+  still checked against the clock. Losing the state directory ends every login
+  session, and no logout can make an ended one work again. Logging out is now
+  `POST /logout` from a form on the generator page, behind the CSRF check the
+  other forms have, so a link or image elsewhere on the same site cannot end the
+  sessions. A logout without a valid login session ends none and only removes
+  the cookie, as anyone can take a CSRF token from the login page. A logout that
+  cannot be saved answers 500. `session.NewManager`, which the tests use, keeps
+  the value in memory only; pixad uses `session.NewManagerWithDatabase`.
+- 2026-10-15 a cached file left empty or partial is no longer served, or fetched
+  again on every request (closes #252). Source and transformed image files, and
+  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.
+- 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
+  decoded and encoded, holding the slot, however long after its request's
+  deadline that took. `Process` now returns the context's error, and gives the
+  slot back, when the context has ended by the time it has the slot or before it
+  encodes. A request that waited for a shared transcode stopped at the first
+  request's deadline is answered 503 `server busy, try again later`, not 500,
+  though its own `downstream_timeout` may not have ended. govips gives pixa no
+  way to stop libvips once an encode has begun, so that encode keeps its slot
+  until it finishes; the limit of 8192 on each side of the source and of the
+  output bounds how long that takes. With one libvips thread, 8192x8192 random
+  pixels take up to about 1 second to encode as JPEG, 15 as PNG, 16 as GIF, 63
+  as WebP, 110 as AVIF and 96 as JPEG XL, the last three with an alpha channel,
+  plus up to about 11 seconds to decode the source; `README.md` gives these
+  times and no longer says the work stops when `downstream_timeout` ends. The
+  settings of the WebP, AVIF and JPEG XL encoders, which can hold a slot past
+  the default `downstream_timeout` of 60 seconds, are unchanged.
+- 2026-10-15 "Running under upaas" in `README.md` tells the operator to set
+  `PIXA_CACHE_MAX_BYTES` (closes #251): it listed the variable as optional, but
+  under upaas the volume is a directory on the host's filesystem. The default,
+  75% of the sum of that filesystem's free space and what the cache holds, and
+  at least 500 MiB, let one pixa take three quarters or more of the space the
+  host and every other app had left, and all of it when that was 500 MiB or
+  less. The section now says so. The default is unchanged.
+- 2026-10-15 the request metrics are labelled by route, not by path (closes
+  #254): their `handler` label was each request's path, so every new path, a 404
+  or an encrypted URL, added series to the Prometheus registry that were never
+  removed, and `/metrics` listed every encrypted URL's token. The label is now
+  the pattern of the chi route the request matches, such as `/v1/image/*`, or
+  `unmatched` for a request that matches none: one the router answers 404 or
+  405, or a CORS preflight. The metrics library takes the label before the
+  request is routed, so the metrics middleware asks the router which route
+  matches.
+- 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 the example config allowlists no host (closes #249): it allowlisted
+  `github.com` and `user-images.githubusercontent.com`, where anyone can put an
+  image, so a deployment set up as Getting Started says served those images to
+  anyone without a signature, at any size and in any format.
+  `configs/config.example.yml` now leaves `allowlist_hosts` out, with one
+  commented entry under `example.com`. Getting Started and "Running under upaas"
+  in `README.md` say that every image on an allowlisted host is served without a
+  signature, so an allowlisted host must be one the operator controls.
+- 2026-10-15 a chunked upstream body of exactly `upstream_max_response_size`
+  bytes is accepted (closes #248): once the limit was used up, the fetcher's
+  next read refused the body without looking for its end. Go's HTTP client
+  returns the end of a body sent with a `Content-Length` together with its last
+  bytes, but the end of a chunked body can come in a later read, so such a body
+  was refused after it had been downloaded whole, and the image routes
+  answered 500. Once the limit is used up, the fetcher now reads one more byte:
+  if the body has ended it is accepted, and if there is more it is refused with
+  `ErrResponseTooLarge`, as before.
+- 2026-10-13 `fit=outside` no longer makes an output above 8192 on a side
+  (closes #257): it scales the image to cover the requested size, and the side
+  scaled past that size was held to no maximum, so `1x8192?fit=outside` of a
+  3000x600 source was resized to 40960x8192 and encoded. The image processor now
+  refuses such a size with `ErrOutputTooLarge` before resizing, and both image
+  routes answer it 400, as for a size with one side `0`. The other fit modes
+  make the requested size or one within it, so none can go above 8192.
+- 2026-10-13 a size with one side `0` no longer makes an output above 8192 on a
+  side (closes #241): that side is computed from the source's proportions, and
+  nothing held it to the 8192 a requested side is held to, so `0x8192` of a
+  3000x600 source was resized to 40960x8192 and encoded. The image processor now
+  refuses such a size with `ErrOutputTooLarge` once it knows the source's
+  dimensions, before resizing, and both image routes answer it 400 with the JSON
+  error body. The refusal is not cached as a failed fetch, so the same source is
+  still served at other sizes. `imgcache.MaxDimension` is now the image
+  processor's `MaxOutputDimension`, so the two cannot differ.
+- 2026-10-13 the unused cache lifetime is removed (closes #69):
+  `CacheConfig.CacheTTL`, which pixad set to `DefaultCacheTTL` (24 hours), was
+  never read, so cached images never expired. The field and the constant are
+  removed. Cached source and transformed images stay until eviction removes them
+  to keep the cache within `cache_max_bytes`.
+- 2026-10-13 SVG sources are refused, and `orig` is never saved as JPEG in place
+  of the source's own format (closes #68): the upstream fetch no longer accepts
+  `image/svg+xml`, which libvips would render. Both image routes answer 415 for
+  an upstream `Content-Type` that is not one of the source formats, SVG
+  included, where they answered 500. Before decoding, the image processor
+  refuses with `ErrUnsupportedInputFormat`, also answered 415, any input that
+  govips' `DetermineImageType`, which picks the libvips loader from the first
+  bytes, does not name as a source format. That catches an SVG whose first bytes
+  pass the first-bytes check for another type, as an XML comment can make them
+  do for AVIF; an SVG whose first bytes do not pass that check answers 500, as
+  before, and so does a damaged image in a source format, whose first bytes
+  `DetermineImageType` names. `orig` is always the source's own format, where a
+  source the processor could not name was saved as JPEG. `README.md` no longer
+  lists SVG. The SVG detection in `internal/magic` stays: the request path does
+  not call it, and it goes with the unused code of
+  https://git.eeqj.de/sneak/pixa/issues/168.
+- 2026-10-12 per-client limit on the image routes (closes #236): the new setting
+  `image_requests_per_minute` (`PIXA_IMAGE_REQUESTS_PER_MINUTE`), default 600,
+  is how many `GET` and `HEAD` requests one client may send to `/v1/image/` and
+  `/v1/e/` together per minute, cache hits included; the next is answered 429
+  with `Retry-After`. It uses the login limit's `RateLimit` middleware, so
+  clients are told apart the same way, and runs after the CORS middleware, so a
+  preflight is not counted. `0` turns the limit off; a negative or non-integer
+  value aborts startup.
 - 2026-10-08 a request past its deadline no longer waits for the database to
   count it (closes #224). On a busy host, `TestService_Get_ReturnsByItsDeadline`
   failed because its request, past its deadline, still waited for its miss count
@@ -431,11 +608,11 @@ P2: security: per-IP rate limiting on the image routes
   cancellation but keeps its deadline, and the others wait for its image or
   error holding no upstream connection or processing slot, and stop waiting when
   their own context ends; the request doing the processing waits for it even
-  then, up to its deadline; a request whose context has already ended starts
-  nothing; each request counts one miss, and the processing counts its fetch and
-  transcode once; a panic while processing is reported to Sentry when
-  `sentry_dsn` is set and becomes an error for every waiting request instead of
-  stopping pixad; documented in `README.md`.
+  then, until it ends, which can be after its deadline; a request whose context
+  has already ended starts nothing; each request counts one miss, and the
+  processing counts its fetch and transcode once; a panic while processing is
+  reported to Sentry when `sentry_dsn` is set and becomes an error for every
+  waiting request instead of stopping pixad; documented in `README.md`.
 - 2026-09-29 only the image routes send CORS headers (closes #98): the CORS
   middleware, with the `access_control_allow_origin` origin, moved from the
   router root onto a `/v1` subrouter holding `/v1/image/` and `/v1/e/`, where it
@@ -749,8 +926,6 @@ P2: security: per-IP rate limiting on the image routes
 
 # Future Steps
 
-- P2: security
-    - per-origin rate limiting
 - P2: HTTP response handling
     - Last-Modified headers
 - P2: configuration

configs/config.example.yml

@@ -45,15 +45,14 @@ state_dir: ./data
 signing_key: "CHANGE_ME_generate_with_openssl_rand_base64_32"
 
 # Hosts that don't require signatures (default: none)
+# Every image on an allowlisted host is served to anyone without a
+# signature, at any size and in any format, so list only hosts you
+# control, where nobody else can put images.
 # Use "." prefix for wildcard subdomain matching (e.g., ".example.com" matches "cdn.example.com")
 # An entry that is neither a host name nor an IP address (IPv6 without
 # brackets), such as one with a port or a "*." wildcard, aborts startup.
-allowlist_hosts:
-  - s3.sneak.cloud
-  - static.sneak.cloud
-  - sneak.berlin
-  - github.com
-  - user-images.githubusercontent.com
+# allowlist_hosts:
+#   - images.example.com
 
 # Hosts whose pages may not show pixa's images, written as for
 # allowlist_hosts. A request to /v1/image/ or /v1/e/ whose Referer header
@@ -109,12 +108,21 @@ upstream_connections_per_host: 20
 # downstream_timeout has ended first.
 upstream_connections: 64
 
-# 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.
+# 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.
 # max_concurrent_processing: 4
 
+# How many requests to the image routes (/v1/image/ and /v1/e/) one client
+# may send per minute; a request over the limit is answered 429 with a
+# Retry-After header. 0 turns the limit off. Clients are told apart as for
+# the login limit, so behind a CDN or proxy whose addresses are not in
+# trusted_proxies all requests arriving from one proxy or CDN address count
+# as one client: raise the limit there, or set 0. (default: 600)
+image_requests_per_minute: 600
+
 # Time allowed for one fetch from an upstream host (default: 30s)
 upstream_fetch_timeout: 30s
 

internal/config/concurrency_limits_internal_test.go

@@ -1,6 +1,7 @@
 package config
 
 import (
+	"fmt"
 	"runtime"
 	"testing"
 )
@@ -12,8 +13,8 @@ const (
 )
 
 // TestOmittedConcurrencyLimitsUseDefaults checks that an omitted
-// max_concurrent_processing is the number of CPUs Go uses and an omitted
-// upstream_connections is 64.
+// max_concurrent_processing is half the CPUs Go uses, and at least 1, and
+// an omitted upstream_connections is 64.
 func TestOmittedConcurrencyLimitsUseDefaults(t *testing.T) {
 	t.Parallel()
 
@@ -22,9 +23,9 @@ func TestOmittedConcurrencyLimitsUseDefaults(t *testing.T) {
 		t.Fatalf("minimal config should be valid, got error: %v", err)
 	}
 
-	if c.MaxConcurrentProcessing != runtime.GOMAXPROCS(0) {
-		t.Errorf("MaxConcurrentProcessing = %d, want %d, one per CPU",
-			c.MaxConcurrentProcessing, runtime.GOMAXPROCS(0))
+	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.UpstreamConnections != 64 {
@@ -32,6 +33,36 @@ 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) {
@@ -160,3 +191,19 @@ 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

@@ -34,8 +34,17 @@ const (
 	DefaultUpstreamFetchTimeout       = 30 * time.Second
 	DefaultUpstreamMaxResponseSize    = 50 << 20 // 50 MiB
 	DefaultDownstreamTimeout          = 60 * time.Second
+	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"
@@ -61,6 +70,7 @@ const (
 	keyUpstreamFetchTimeout       = "upstream_fetch_timeout"
 	keyUpstreamMaxResponseSize    = "upstream_max_response_size"
 	keyDownstreamTimeout          = "downstream_timeout"
+	keyImageRequestsPerMinute     = "image_requests_per_minute"
 )
 
 // placeholderSigningKey is the dummy signing_key shipped in
@@ -158,6 +168,11 @@ type Config struct {
 	// of the per-request timeout middleware.
 	DownstreamTimeout time.Duration
 
+	// ImageRequestsPerMinute is how many requests one client may make to
+	// the image routes per minute; the next is refused with 429 Too Many
+	// Requests. 0 turns the limit off.
+	ImageRequestsPerMinute int
+
 	// BlockedNetworks are operator-supplied CIDR ranges to refuse in
 	// addition to the built-in SSRF blocklist. Enforced by the upstream
 	// fetcher's dialer; the built-in ranges always apply.
@@ -290,10 +305,8 @@ 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, runtime.GOMAXPROCS(0)),
+			keyMaxConcurrentProcessing, DefaultMaxConcurrentProcessing()),
 		UpstreamFetchTimeout: loader.durationVal(
 			keyUpstreamFetchTimeout, DefaultUpstreamFetchTimeout),
 		UpstreamMaxResponseSize: loader.int64Val(
@@ -302,6 +315,8 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
 			keyAccessControlAllowOrigin, DefaultAccessControlAllowOrigin),
 		DownstreamTimeout: loader.durationVal(
 			keyDownstreamTimeout, DefaultDownstreamTimeout),
+		ImageRequestsPerMinute: loader.intVal(
+			keyImageRequestsPerMinute, DefaultImageRequestsPerMinute),
 		CacheMaxBytes:    loader.int64Val(keyCacheMaxBytes, 0),
 		BlockedNetworks:  blockedNetworks,
 		TrustedProxies:   trustedProxies,
@@ -311,9 +326,7 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
 	// The default for an omitted cache_max_bytes is worked out when
 	// the cache opens; here we only record whether the operator set
 	// the key explicitly.
-	if _, present := lookupValue(sc, keyCacheMaxBytes); present {
-		c.CacheMaxBytesExplicit = true
-	}
+	_, c.CacheMaxBytesExplicit = lookupValue(sc, keyCacheMaxBytes)
 
 	// Build DBURL from StateDir if not explicitly set. The derived URL
 	// is a default: it applies only when db_url is omitted, never to an
@@ -426,7 +439,7 @@ func isKnownConfigKey(key string) bool {
 		keyMaxConcurrentProcessing, keyCacheMaxBytes, keyBlockedNetworks,
 		keyTrustedProxies, keyAccessControlAllowOrigin, keyUpstreamFetchTimeout,
 		keyUpstreamMaxResponseSize, keyDownstreamTimeout, keyRefererBlocklist,
-		"env":
+		keyImageRequestsPerMinute, "env":
 		return true
 	}
 
@@ -461,6 +474,7 @@ func envVarNames() map[string]string {
 		keyUpstreamFetchTimeout:       "PIXA_UPSTREAM_FETCH_TIMEOUT",
 		keyUpstreamMaxResponseSize:    "PIXA_UPSTREAM_MAX_RESPONSE_SIZE",
 		keyDownstreamTimeout:          "PIXA_DOWNSTREAM_TIMEOUT",
+		keyImageRequestsPerMinute:     "PIXA_IMAGE_REQUESTS_PER_MINUTE",
 	}
 }
 
@@ -613,6 +627,14 @@ func (c *Config) validate() error {
 			settingName(keyCacheMaxBytes), c.CacheMaxBytes, errMustNotBeNegative)
 	}
 
+	// Zero is valid (it turns the limit off); only negative values are
+	// rejected.
+	if c.ImageRequestsPerMinute < 0 {
+		return fmt.Errorf("%s: value %d %w",
+			settingName(keyImageRequestsPerMinute), c.ImageRequestsPerMinute,
+			errMustNotBeNegative)
+	}
+
 	err = c.validateUpstreamMaxResponseSize()
 	if err != nil {
 		return err
@@ -625,12 +647,9 @@ func (c *Config) validate() error {
 		}
 	}
 
-	if c.SentryDSN != "" {
-		parsed, err := url.Parse(c.SentryDSN)
-		if err != nil || parsed.Scheme == "" || parsed.Host == "" {
-			return fmt.Errorf("%s: value %q is %w",
-				settingName(keySentryDSN), c.SentryDSN, errNotAValidURL)
-		}
+	err = c.validateSentryDSN()
+	if err != nil {
+		return err
 	}
 
 	if (c.MetricsUsername == "") != (c.MetricsPassword == "") {
@@ -642,6 +661,22 @@ func (c *Config) validate() error {
 	return c.validateAccessControlAllowOrigin()
 }
 
+// validateSentryDSN checks that sentry_dsn, when set, is a URL with a scheme
+// and a host.
+func (c *Config) validateSentryDSN() error {
+	if c.SentryDSN == "" {
+		return nil
+	}
+
+	parsed, err := url.Parse(c.SentryDSN)
+	if err != nil || parsed.Scheme == "" || parsed.Host == "" {
+		return fmt.Errorf("%s: value %q is %w",
+			settingName(keySentryDSN), c.SentryDSN, errNotAValidURL)
+	}
+
+	return nil
+}
+
 // validateUpstreamMaxResponseSize checks that upstream_max_response_size
 // is from 1 byte to 1 GiB. An upstream response is read whole into
 // memory, and the image processor reads one byte past this limit, which

internal/config/config_internal_test.go

@@ -92,6 +92,32 @@ func TestGetStringSlice_Empty(t *testing.T) {
 	}
 }
 
+// TestExampleConfigAllowlistsNoHost loads configs/config.example.yml
+// with a real signing key, as Getting Started in README.md has it copied
+// and edited, and checks that it allowlists no host: every image on an
+// allowlisted host is served to anyone without a signature.
+func TestExampleConfigAllowlistsNoHost(t *testing.T) {
+	t.Setenv("PIXA_SIGNING_KEY", validTestSigningKey)
+
+	examplePath := filepath.Join("..", "..", "configs", "config.example.yml")
+
+	sc, err := loadTestConfig(examplePath)
+	if err != nil {
+		t.Fatalf("failed to load %s: %v", examplePath, err)
+	}
+
+	c, err := newFromSmartConfig(sc)
+	if err != nil {
+		t.Fatalf("%s is not a valid config: %v", examplePath, err)
+	}
+
+	t.Logf("allowlist_hosts in %s: %v", examplePath, c.AllowlistHosts)
+
+	if len(c.AllowlistHosts) != 0 {
+		t.Errorf("%s allowlists %v, want no host", examplePath, c.AllowlistHosts)
+	}
+}
+
 // loadTestConfig is a helper to load a config file for testing.
 func loadTestConfig(path string) (*smartconfig.Config, error) {
 	return smartconfig.NewFromConfigPath(path)

internal/config/env_internal_test.go

@@ -70,6 +70,7 @@ func TestEnvironmentSetsEveryKey(t *testing.T) {
 	t.Setenv("PIXA_UPSTREAM_CONNECTIONS_PER_HOST", "5")
 	t.Setenv("PIXA_UPSTREAM_CONNECTIONS", "10")
 	t.Setenv("PIXA_MAX_CONCURRENT_PROCESSING", "3")
+	t.Setenv("PIXA_IMAGE_REQUESTS_PER_MINUTE", "30")
 	t.Setenv("PIXA_CACHE_MAX_BYTES", "1024")
 	t.Setenv("PIXA_BLOCKED_NETWORKS", "203.0.113.0/24")
 	t.Setenv("PIXA_TRUSTED_PROXIES", "192.0.2.0/24")
@@ -99,6 +100,7 @@ func TestEnvironmentSetsEveryKey(t *testing.T) {
 		UpstreamConnectionsPerHost: 5,
 		UpstreamConnections:        10,
 		MaxConcurrentProcessing:    3,
+		ImageRequestsPerMinute:     30,
 		CacheMaxBytes:              1024,
 		CacheMaxBytesExplicit:      true,
 		BlockedNetworks:            []netip.Prefix{netip.MustParsePrefix("203.0.113.0/24")},

internal/config/image_requests_per_minute_internal_test.go

@@ -0,0 +1,107 @@
+package config
+
+import "testing"
+
+// testImageRequestsPerMinuteVar is the variable that sets
+// image_requests_per_minute.
+const testImageRequestsPerMinuteVar = "PIXA_IMAGE_REQUESTS_PER_MINUTE"
+
+// TestImageRequestsPerMinuteValues checks that an omitted
+// image_requests_per_minute is 600, and that 0, which turns the limit off,
+// and any larger whole number are used as given.
+func TestImageRequestsPerMinuteValues(t *testing.T) {
+	t.Parallel()
+
+	c, err := configFromYAML(t, signingKeyLine)
+	if err != nil {
+		t.Fatalf("minimal config should be valid, got error: %v", err)
+	}
+
+	if c.ImageRequestsPerMinute != 600 {
+		t.Errorf("ImageRequestsPerMinute = %d, want 600", c.ImageRequestsPerMinute)
+	}
+
+	explicit := []struct {
+		value string
+		want  int
+	}{
+		{"0", 0},
+		{"30", 30},
+	}
+
+	for _, tc := range explicit {
+		c, err := configFromYAML(t,
+			signingKeyLine+"image_requests_per_minute: "+tc.value+"\n")
+		if err != nil {
+			t.Fatalf("image_requests_per_minute: %s should load, got error: %v",
+				tc.value, err)
+		}
+
+		if c.ImageRequestsPerMinute != tc.want {
+			t.Errorf("ImageRequestsPerMinute = %d, want %d",
+				c.ImageRequestsPerMinute, tc.want)
+		}
+	}
+}
+
+// TestInvalidImageRequestsPerMinuteAbortsStartup checks that a value that is
+// negative, not a whole number, or null aborts startup naming the key and
+// the value, and the variable too where the value could have come from it.
+func TestInvalidImageRequestsPerMinuteAbortsStartup(t *testing.T) {
+	t.Parallel()
+
+	key := keyImageRequestsPerMinute
+
+	runAbortCases(t, []abortCase{
+		{
+			name: "negative",
+			yaml: signingKeyLine + key + ": -1\n",
+			wantErrSubstrings: []string{
+				key, testImageRequestsPerMinuteVar, "value -1",
+			},
+		},
+		{
+			name: "not a number",
+			yaml: signingKeyLine + key + ": plenty\n",
+			wantErrSubstrings: []string{
+				key, testImageRequestsPerMinuteVar, "plenty",
+			},
+		},
+		{
+			name:              "fractional",
+			yaml:              signingKeyLine + key + ": 2.5\n",
+			wantErrSubstrings: []string{key, "2.5"},
+		},
+		{
+			name:              "explicit null",
+			yaml:              signingKeyLine + key + ": null\n",
+			wantErrSubstrings: []string{key, nullValueText},
+		},
+	})
+}
+
+// TestImageRequestsPerMinuteFromEnvironment checks that the variable sets the
+// limit over the config file, and that an invalid value in it aborts startup
+// naming the variable and the value.
+func TestImageRequestsPerMinuteFromEnvironment(t *testing.T) {
+	t.Setenv(testImageRequestsPerMinuteVar, "0")
+
+	c, err := configFromYAML(t, signingKeyLine+"image_requests_per_minute: 30\n")
+	if err != nil {
+		t.Fatalf("limit from the environment should load: %v", err)
+	}
+
+	if c.ImageRequestsPerMinute != 0 {
+		t.Errorf("ImageRequestsPerMinute = %d, want 0 from the environment",
+			c.ImageRequestsPerMinute)
+	}
+
+	for _, value := range []string{"-5", "ten"} {
+		t.Run(value, func(t *testing.T) {
+			t.Setenv(testImageRequestsPerMinuteVar, value)
+
+			_, err := configFromYAML(t, signingKeyLine)
+			wantStartupError(t, err, testImageRequestsPerMinuteVar, value)
+		})
+	}
+}

internal/db/migrations/001_schema.sql

@@ -176,3 +176,11 @@ CREATE TABLE IF NOT EXISTS cache_stats (
     last_updated_at DATETIME DEFAULT CURRENT_TIMESTAMP
 );
 INSERT OR IGNORE INTO cache_stats (id) VALUES (1);
+
+-- A random value that a login session carries from its login. pixa makes it
+-- when the table has no row, and each logout replaces it; a login session
+-- carrying another value is refused, so logging out ends every login session.
+CREATE TABLE IF NOT EXISTS logout (
+    id INTEGER PRIMARY KEY CHECK (id = 1),
+    logout_value TEXT NOT NULL
+);

internal/handlers/auth.go

@@ -89,10 +89,18 @@ func (s *Handlers) handleLoginPost(w http.ResponseWriter, r *http.Request) {
 	http.Redirect(w, r, "/", http.StatusSeeOther)
 }
 
-// HandleLogout clears the session and redirects to login.
+// HandleLogout logs out, which ends every login session (see
+// session.Manager.Logout), and redirects to login.
 func (s *Handlers) HandleLogout() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
-		s.sessMgr.ClearSession(w)
+		err := s.sessMgr.Logout(w, r)
+		if err != nil {
+			s.log.Error("failed to log out", "error", err)
+			http.Error(w, "Failed to log out", http.StatusInternalServerError)
+
+			return
+		}
+
 		http.Redirect(w, r, "/", http.StatusSeeOther)
 	}
 }

internal/handlers/auth_session_internal_test.go

@@ -185,6 +185,98 @@ func TestHandleLogout_ClearsSessionCookie(t *testing.T) {
 	}
 }
 
+// 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) {
+	t.Parallel()
+
+	h, srv := newCSRFTestRouter(t)
+
+	copied := newSessionCookie(t, h)
+
+	req := httptest.NewRequestWithContext(
+		t.Context(), http.MethodPost, "/logout", nil)
+	req.AddCookie(copied)
+
+	rec := httptest.NewRecorder()
+	h.HandleLogout().ServeHTTP(rec, req)
+
+	if rec.Code != http.StatusSeeOther {
+		t.Fatalf("POST /logout status = %d, want %d",
+			rec.Code, http.StatusSeeOther)
+	}
+
+	req = httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
+	req.AddCookie(copied)
+
+	rec = httptest.NewRecorder()
+	srv.ServeHTTP(rec, req)
+
+	body := rec.Body.String()
+	t.Logf("GET / with the copied cookie: status %d, login page %v",
+		rec.Code, strings.Contains(body, loginForm))
+
+	if !strings.Contains(body, loginForm) || strings.Contains(body, generatorForm) {
+		t.Error("a cookie copied before /logout still opens the generator page")
+	}
+}
+
+// TestHandleLogout_SaveFails_Answers500 verifies that when the logout cannot
+// 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()
+
+	h, srv := newCSRFTestRouter(t)
+
+	db := setupTestDB(t)
+
+	sessMgr, err := session.NewManagerWithDatabase(
+		t.Context(), testSigningKey, db)
+	if err != nil {
+		t.Fatalf("session.NewManagerWithDatabase() error = %v", err)
+	}
+
+	h.sessMgr = sessMgr
+	cookie := newSessionCookie(t, h)
+
+	// Saving the logout fails on a closed database.
+	err = db.Close()
+	if err != nil {
+		t.Fatalf("closing the database: %v", err)
+	}
+
+	req := httptest.NewRequestWithContext(
+		t.Context(), http.MethodPost, "/logout", nil)
+	req.AddCookie(cookie)
+
+	rec := httptest.NewRecorder()
+	h.HandleLogout().ServeHTTP(rec, req)
+
+	t.Logf("POST /logout: status %d, Set-Cookie %q",
+		rec.Code, rec.Header().Get("Set-Cookie"))
+
+	if rec.Code != http.StatusInternalServerError {
+		t.Errorf("POST /logout status = %d, want %d",
+			rec.Code, http.StatusInternalServerError)
+	}
+
+	if c := findSessionCookie(rec); c != nil {
+		t.Errorf("a logout that failed set the session cookie: %s", c)
+	}
+
+	req = httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
+	req.AddCookie(cookie)
+
+	rec = httptest.NewRecorder()
+	srv.ServeHTTP(rec, req)
+
+	if !strings.Contains(rec.Body.String(), generatorForm) {
+		t.Error("a logout that failed ended the session")
+	}
+}
+
 // TestGeneratePost_NoSession_RedirectsToLogin verifies that POST /generate
 // with a valid CSRF token but no login session answers 303 to / and makes no
 // URL.

internal/handlers/handlers.go

@@ -106,7 +106,6 @@ func (s *Handlers) WaitForProcessing(ctx context.Context) int {
 func newCacheConfig(cfg *config.Config, log *slog.Logger) imgcache.CacheConfig {
 	return imgcache.CacheConfig{
 		StateDir:           cfg.StateDir,
-		CacheTTL:           imgcache.DefaultCacheTTL,
 		NegativeTTL:        imgcache.DefaultNegativeTTL,
 		MaxBytes:           cfg.CacheMaxBytes,
 		UseDefaultMaxBytes: !cfg.CacheMaxBytesExplicit,
@@ -125,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
@@ -164,7 +164,10 @@ func (s *Handlers) initImageService() error {
 
 	// Initialize session manager (signing key is validated at config load
 	// time). Session cookies are always Secure/HttpOnly/SameSite=Strict.
-	sessMgr, err := session.NewManager(s.config.SigningKey)
+	// The random value that each logout replaces is saved in the database,
+	// so it outlasts a restart.
+	sessMgr, err := session.NewManagerWithDatabase(
+		context.Background(), s.config.SigningKey, s.db.DB())
 	if err != nil {
 		return err
 	}

internal/handlers/handlers_internal_test.go

@@ -47,7 +47,6 @@ func setupTestHandler(t *testing.T) *testFixtures {
 
 	cache, err := imgcache.NewCache(db, imgcache.CacheConfig{
 		StateDir:    tmpDir,
-		CacheTTL:    time.Hour,
 		NegativeTTL: 5 * time.Minute,
 	})
 	if err != nil {

internal/handlers/image.go

@@ -206,6 +206,16 @@ func parseExpires(query url.Values) (time.Time, error) {
 	return time.Unix(exp, 0), nil
 }
 
+// HandleTooManyRequests answers 429 Too Many Requests with the JSON error
+// body the image routes send for their other errors. The image routes' rate
+// limit calls it for a request over the limit.
+func (s *Handlers) HandleTooManyRequests() http.HandlerFunc {
+	return func(w http.ResponseWriter, _ *http.Request) {
+		s.respondError(w, "too many requests, try again later",
+			http.StatusTooManyRequests)
+	}
+}
+
 // respondImageError maps image retrieval errors to HTTP responses.
 func (s *Handlers) respondImageError(
 	w http.ResponseWriter, req *imgcache.ImageRequest, err error,
@@ -230,13 +240,30 @@ func (s *Handlers) respondImageError(
 	}
 
 	if errors.Is(err, httpfetcher.ErrTooManyConnections) ||
-		errors.Is(err, imageprocessor.ErrTooManyImages) {
+		errors.Is(err, imageprocessor.ErrTooManyImages) ||
+		errors.Is(err, imgcache.ErrProcessingTimedOut) {
 		s.respondError(w, "server busy, try again later",
 			http.StatusServiceUnavailable)
 
 		return
 	}
 
+	if errors.Is(err, httpfetcher.ErrInvalidContentType) ||
+		errors.Is(err, imageprocessor.ErrUnsupportedInputFormat) {
+		s.respondError(w, "unsupported source format",
+			http.StatusUnsupportedMediaType)
+
+		return
+	}
+
+	if errors.Is(err, imageprocessor.ErrOutputTooLarge) {
+		s.respondError(w, fmt.Sprintf("size too large for the source: the side "+
+			"computed from its proportions is above %d",
+			imageprocessor.MaxOutputDimension), http.StatusBadRequest)
+
+		return
+	}
+
 	s.respondError(w, "internal error", http.StatusInternalServerError)
 }
 

internal/handlers/image_cache_control_internal_test.go

@@ -31,7 +31,6 @@ func newSignedHostServer(
 
 	cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{
 		StateDir:    t.TempDir(),
-		CacheTTL:    time.Hour,
 		NegativeTTL: 5 * time.Minute,
 	})
 	if err != nil {

internal/handlers/image_errors_internal_test.go

@@ -29,7 +29,6 @@ func newImageRoute(t *testing.T, fetcher httpfetcher.Fetcher) http.Handler {
 
 	cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{
 		StateDir:    t.TempDir(),
-		CacheTTL:    time.Hour,
 		NegativeTTL: 5 * time.Minute,
 	})
 	if err != nil {

internal/handlers/imageenc.go

@@ -158,9 +158,18 @@ func (s *Handlers) handleImageError(w http.ResponseWriter, err error) {
 	case errors.Is(err, httpfetcher.ErrUpstreamTimeout):
 		s.respondError(w, "upstream timeout", http.StatusGatewayTimeout)
 	case errors.Is(err, httpfetcher.ErrTooManyConnections),
-		errors.Is(err, imageprocessor.ErrTooManyImages):
+		errors.Is(err, imageprocessor.ErrTooManyImages),
+		errors.Is(err, imgcache.ErrProcessingTimedOut):
 		s.respondError(w, "server busy, try again later",
 			http.StatusServiceUnavailable)
+	case errors.Is(err, httpfetcher.ErrInvalidContentType),
+		errors.Is(err, imageprocessor.ErrUnsupportedInputFormat):
+		s.respondError(w, "unsupported source format",
+			http.StatusUnsupportedMediaType)
+	case errors.Is(err, imageprocessor.ErrOutputTooLarge):
+		s.respondError(w, fmt.Sprintf("size too large for the source: the side "+
+			"computed from its proportions is above %d",
+			imageprocessor.MaxOutputDimension), http.StatusBadRequest)
 	default:
 		s.log.Error("image request failed", "error", err)
 		s.respondError(w, "internal error", http.StatusInternalServerError)

internal/handlers/referer_blocklist_internal_test.go

@@ -49,7 +49,6 @@ func newRefererRoutes(t *testing.T) (http.Handler, *Handlers, *countingFetcher)
 
 	cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{
 		StateDir:    t.TempDir(),
-		CacheTTL:    time.Hour,
 		NegativeTTL: 5 * time.Minute,
 	})
 	if err != nil {

internal/handlers/server_busy_internal_test.go

@@ -1,6 +1,7 @@
 package handlers
 
 import (
+	"context"
 	"fmt"
 	"log/slog"
 	"net/http"
@@ -47,3 +48,40 @@ func TestServerBusyAnswers503(t *testing.T) {
 		}
 	}
 }
+
+// TestProcessingTimedOutAnswers503 checks that respondImageError and
+// handleImageError, which answer the errors of the two image routes, answer
+// 503 with the server busy error when the image service reports that
+// processing stopped at the deadline of the request doing it, wrapped as the
+// service wraps it. A request that waited for that processing gets this error
+// while its own deadline may have time left. The request's source only goes
+// into a log line, so it is left empty.
+func TestProcessingTimedOutAnswers503(t *testing.T) {
+	t.Parallel()
+
+	h := &Handlers{log: slog.New(slog.DiscardHandler)}
+	err := fmt.Errorf("%w: failed to encode: %w",
+		imgcache.ErrProcessingTimedOut, context.DeadlineExceeded)
+
+	plain := httptest.NewRecorder()
+	h.respondImageError(plain, &imgcache.ImageRequest{}, err)
+
+	encrypted := httptest.NewRecorder()
+	h.handleImageError(encrypted, err)
+
+	for name, rec := range map[string]*httptest.ResponseRecorder{
+		"respondImageError": plain, "handleImageError": encrypted,
+	} {
+		t.Logf("%s: status = %d, body = %q", name, rec.Code, rec.Body.String())
+
+		if rec.Code != http.StatusServiceUnavailable {
+			t.Errorf("%s: status = %d, want %d",
+				name, rec.Code, http.StatusServiceUnavailable)
+		}
+
+		if !strings.Contains(rec.Body.String(), "server busy, try again later") {
+			t.Errorf("%s: body = %q, want the server busy error",
+				name, rec.Body.String())
+		}
+	}
+}

internal/handlers/unsupported_source_internal_test.go

@@ -0,0 +1,54 @@
+package handlers
+
+import (
+	"fmt"
+	"log/slog"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"sneak.berlin/go/pixa/internal/httpfetcher"
+	"sneak.berlin/go/pixa/internal/imageprocessor"
+	"sneak.berlin/go/pixa/internal/imgcache"
+)
+
+// TestUnsupportedSourceFormatAnswers415 checks that both image routes answer
+// 415 with a clear error when the upstream answers with a content type pixa
+// does not accept, such as SVG, and when orig is asked of a source in a
+// format the processor cannot name, wrapped as the service wraps them.
+func TestUnsupportedSourceFormatAnswers415(t *testing.T) {
+	t.Parallel()
+
+	h := &Handlers{log: slog.New(slog.DiscardHandler)}
+	req := &imgcache.ImageRequest{SourceHost: "img.example.com", SourcePath: "/a.svg"}
+
+	for _, err := range []error{
+		fmt.Errorf("upstream fetch failed: %w: image/svg+xml",
+			httpfetcher.ErrInvalidContentType),
+		fmt.Errorf("image processing failed: %w: unknown",
+			imageprocessor.ErrUnsupportedInputFormat),
+	} {
+		plain := httptest.NewRecorder()
+		h.respondImageError(plain, req, err)
+
+		encrypted := httptest.NewRecorder()
+		h.handleImageError(encrypted, err)
+
+		for route, rec := range map[string]*httptest.ResponseRecorder{
+			"HandleImage": plain, "HandleImageEnc": encrypted,
+		} {
+			t.Logf("%s for %v: %d %s", route, err, rec.Code, rec.Body.String())
+
+			if rec.Code != http.StatusUnsupportedMediaType {
+				t.Errorf("%s for %v: status = %d, want %d",
+					route, err, rec.Code, http.StatusUnsupportedMediaType)
+			}
+
+			if !strings.Contains(rec.Body.String(), "unsupported source format") {
+				t.Errorf("%s for %v: body = %q, want the unsupported source format error",
+					route, err, rec.Body.String())
+			}
+		}
+	}
+}

internal/httpfetcher/fetch_internal_test.go

@@ -9,6 +9,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"slices"
+	"strconv"
 	"strings"
 	"sync"
 	"testing"
@@ -297,6 +298,139 @@ func TestFetchMaxResponseSizeEnforced(t *testing.T) {
 	}
 }
 
+// fetchWithMaxResponseSize serves handler as the fake upstream and fetches
+// its /image route with MaxResponseSize set to maxResponseSize.
+func fetchWithMaxResponseSize(
+	t *testing.T,
+	maxResponseSize int64,
+	handler http.HandlerFunc,
+) *FetchResult {
+	t.Helper()
+
+	srv := httptest.NewServer(handler)
+	t.Cleanup(srv.Close)
+
+	cfg := DefaultConfig()
+	cfg.MaxResponseSize = maxResponseSize
+
+	f, _ := newServerFetcher(t, srv, cfg)
+
+	res := fetchImage(t, f, "/image")
+	t.Cleanup(func() { _ = res.Content.Close() })
+
+	return res
+}
+
+// TestFetchContentLengthBodyAtMaxResponseSize checks that a body sent with a
+// Content-Length is read whole at exactly MaxResponseSize bytes and refused
+// at one byte more.
+func TestFetchContentLengthBodyAtMaxResponseSize(t *testing.T) {
+	t.Parallel()
+
+	const maxResponseSize = 8
+
+	for _, tc := range []struct {
+		size    int
+		wantErr error
+	}{
+		{maxResponseSize, nil},
+		{maxResponseSize + 1, ErrResponseTooLarge},
+	} {
+		t.Run(strconv.Itoa(tc.size)+" bytes", func(t *testing.T) {
+			t.Parallel()
+
+			body := strings.Repeat("x", tc.size)
+
+			res := fetchWithMaxResponseSize(t, maxResponseSize,
+				func(w http.ResponseWriter, _ *http.Request) {
+					w.Header().Set("Content-Type", contentTypeJPEG)
+					w.Header().Set("Content-Length", strconv.Itoa(tc.size))
+					_, _ = io.WriteString(w, body)
+				})
+
+			if res.ContentLength != int64(tc.size) {
+				t.Fatalf("ContentLength = %d, want %d", res.ContentLength, tc.size)
+			}
+
+			data, err := io.ReadAll(res.Content)
+			t.Logf("body of %d bytes: read %d, error %v", tc.size, len(data), err)
+
+			if !errors.Is(err, tc.wantErr) {
+				t.Fatalf("read error = %v, want %v", err, tc.wantErr)
+			}
+
+			if err == nil && string(data) != body {
+				t.Errorf("read %q, want %q", data, body)
+			}
+		})
+	}
+}
+
+// TestFetchChunkedBodyAtMaxResponseSize checks that a chunked body is read
+// whole at exactly MaxResponseSize bytes and refused at one byte more. The
+// upstream sends the end of the body only once the test has read
+// MaxResponseSize bytes, so the read after them comes before the end of the
+// body has arrived, as it can from a real upstream.
+func TestFetchChunkedBodyAtMaxResponseSize(t *testing.T) {
+	t.Parallel()
+
+	const maxResponseSize = 8
+
+	for _, tc := range []struct {
+		size    int
+		wantErr error
+	}{
+		{maxResponseSize, nil},
+		{maxResponseSize + 1, ErrResponseTooLarge},
+	} {
+		t.Run(strconv.Itoa(tc.size)+" bytes", func(t *testing.T) {
+			t.Parallel()
+
+			body := strings.Repeat("x", tc.size)
+			endBody := make(chan struct{})
+
+			res := fetchWithMaxResponseSize(t, maxResponseSize,
+				func(w http.ResponseWriter, r *http.Request) {
+					w.Header().Set("Content-Type", contentTypeJPEG)
+					_, _ = io.WriteString(w, body)
+					// With no Content-Length, flushing sends the body
+					// chunked; its end is sent when the handler returns.
+					_ = http.NewResponseController(w).Flush()
+
+					select {
+					case <-endBody:
+					case <-r.Context().Done():
+					}
+				})
+
+			if res.ContentLength != -1 {
+				t.Fatalf("ContentLength = %d, want -1 (chunked)", res.ContentLength)
+			}
+
+			head := make([]byte, maxResponseSize)
+
+			_, err := io.ReadFull(res.Content, head)
+			if err != nil {
+				t.Fatalf("read the first %d bytes: %v", maxResponseSize, err)
+			}
+
+			close(endBody)
+
+			rest, err := io.ReadAll(res.Content)
+			data := string(head) + string(rest)
+			t.Logf("body of %d bytes: read %d, error %v", tc.size, len(data), err)
+
+			if !errors.Is(err, tc.wantErr) {
+				t.Fatalf("read error = %v, want %v", err, tc.wantErr)
+			}
+
+			if err == nil && data != body {
+				t.Errorf("read %q, want %q", data, body)
+			}
+		})
+	}
+}
+
 func TestFetchSemaphoreReleasedOnError(t *testing.T) {
 	t.Parallel()
 

internal/httpfetcher/httpfetcher.go

@@ -158,7 +158,6 @@ func DefaultConfig() *Config {
 			contentTypeWebP,
 			contentTypeAVIF,
 			contentTypeJXL,
-			contentTypeSVG,
 		},
 		AllowHTTP:             false,
 		MaxConnectionsPerHost: DefaultMaxConnectionsPerHost,
@@ -661,14 +660,33 @@ func dialSSRFSafe(
 	return nil, fmt.Errorf("%w to %s", errConnectFailed, host)
 }
 
-// limitedReader wraps a reader and limits the number of bytes read.
+// limitedReader wraps a reader and refuses a body longer than remaining
+// bytes with ErrResponseTooLarge. remaining goes below 0 once the body is
+// found to be longer.
 type limitedReader struct {
 	reader    io.Reader
 	remaining int64
 }
 
+// Read reads at most the remaining bytes. Once they are read, it reads one
+// more byte to tell the end of the body from more data: at the end it
+// returns the wrapped reader's error, io.EOF; with more data it returns
+// ErrResponseTooLarge, as does every later Read.
 func (r *limitedReader) Read(p []byte) (int, error) {
-	if r.remaining <= 0 {
+	if r.remaining < 0 {
+		return 0, ErrResponseTooLarge
+	}
+
+	if r.remaining == 0 {
+		var extra [1]byte
+
+		n, err := r.reader.Read(extra[:])
+		r.remaining -= int64(n)
+
+		if n == 0 {
+			return 0, err
+		}
+
 		return 0, ErrResponseTooLarge
 	}
 

internal/imageprocessor/computed_side_internal_test.go

@@ -0,0 +1,52 @@
+package imageprocessor
+
+import (
+	"bytes"
+	"errors"
+	"testing"
+)
+
+// TestImageProcessor_RefusesComputedSideAboveMaximum verifies that a size
+// with one side 0, which Process computes from the input's proportions, is
+// served when that side comes out at MaxOutputDimension and refused with
+// ErrOutputTooLarge when it would be above. A 4096x1 or 1x4096 input given
+// 2 for its short side has 8192 for its long side.
+func TestImageProcessor_RefusesComputedSideAboveMaximum(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name                    string
+		inputWidth, inputHeight int
+		size                    Size
+		wantErr                 error
+	}{
+		{"width at the maximum", 4096, 1, Size{Height: 2}, nil},
+		{"width above the maximum", 4096, 1, Size{Height: 3}, ErrOutputTooLarge},
+		{"height at the maximum", 1, 4096, Size{Width: 2}, nil},
+		{"height above the maximum", 1, 4096, Size{Width: 3}, ErrOutputTooLarge},
+	}
+
+	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 result != nil {
+				t.Logf("Process() made %dx%d", result.Width, result.Height)
+
+				if max(result.Width, result.Height) != MaxOutputDimension {
+					t.Errorf("Process() made %dx%d, want a side of %d",
+						result.Width, result.Height, MaxOutputDimension)
+				}
+			}
+
+			if !errors.Is(err, tt.wantErr) {
+				t.Errorf("Process() error = %v, want %v", err, tt.wantErr)
+			}
+		})
+	}
+}

internal/imageprocessor/deadline_internal_test.go

@@ -0,0 +1,106 @@
+package imageprocessor
+
+import (
+	"bytes"
+	"context"
+	"errors"
+	"io"
+	"testing"
+	"time"
+)
+
+// readerAfterDeadline is a Process input whose first Read waits until done
+// is closed, so the deadline passes after Process has taken its slot and
+// before it encodes.
+type readerAfterDeadline struct {
+	data   *bytes.Reader
+	done   <-chan struct{}
+	waited bool
+}
+
+func (r *readerAfterDeadline) Read(p []byte) (int, error) {
+	if !r.waited {
+		r.waited = true
+
+		<-r.done
+	}
+
+	return r.data.Read(p)
+}
+
+// inputNotToBeRead is a Process input that fails the test when it is read.
+type inputNotToBeRead struct {
+	t *testing.T
+}
+
+func (r inputNotToBeRead) Read([]byte) (int, error) {
+	r.t.Error("Process read its input although its deadline had passed")
+
+	return 0, io.EOF
+}
+
+// TestProcessStopsWhenDeadlinePassed checks that Process returns
+// context.DeadlineExceeded, and holds no slot afterwards, when its context's
+// deadline has passed before it starts, then without reading its input, or
+// while it reads its input. govips gives pixa no way to stop libvips once an
+// encode has begun, so Process must not begin one for a request that has
+// ended.
+func TestProcessStopsWhenDeadlinePassed(t *testing.T) {
+	t.Parallel()
+
+	input := createTestJPEG(t, 50, 50)
+
+	t.Run("before Process starts", func(t *testing.T) {
+		t.Parallel()
+
+		ctx, cancel := context.WithDeadline(t.Context(),
+			time.Now().Add(-time.Second))
+		defer cancel()
+
+		// With a slot free, the select in acquireSlot takes the slot or sees
+		// the ended context at random, so Process is called many times
+		for range 100 {
+			checkProcessStopsAtDeadline(ctx, t, inputNotToBeRead{t: t})
+		}
+	})
+
+	t.Run("while Process reads its input", func(t *testing.T) {
+		t.Parallel()
+
+		ctx, cancel := context.WithTimeout(t.Context(), 100*time.Millisecond)
+		defer cancel()
+
+		checkProcessStopsAtDeadline(ctx, t, &readerAfterDeadline{
+			data: bytes.NewReader(input), done: ctx.Done(),
+		})
+	})
+}
+
+// checkProcessStopsAtDeadline runs Process on input with ctx, whose deadline
+// has passed by the time Process could encode, and fails the test unless
+// Process returns context.DeadlineExceeded and gives its slot back.
+func checkProcessStopsAtDeadline(
+	ctx context.Context, t *testing.T, input io.Reader,
+) {
+	t.Helper()
+
+	proc := New(Params{MaxConcurrentProcessing: 1})
+
+	result, err := proc.Process(ctx, input, smallJPEGRequest())
+	if result != nil {
+		t.Logf("Process() made a %dx%d image after its deadline passed",
+			result.Width, result.Height)
+
+		_ = result.Content.Close()
+	}
+
+	t.Logf("Process() error = %v", err)
+
+	if !errors.Is(err, context.DeadlineExceeded) {
+		t.Errorf("Process() error = %v, want context.DeadlineExceeded", err)
+	}
+
+	if held := len(proc.processingSemaphore); held != 0 {
+		t.Errorf("%d slots still held after Process() returned", held)
+	}
+}

internal/imageprocessor/fit_outside_internal_test.go

@@ -0,0 +1,62 @@
+package imageprocessor
+
+import (
+	"bytes"
+	"errors"
+	"testing"
+)
+
+// TestImageProcessor_RefusesFitOutsideAboveMaximum verifies that FitOutside,
+// which scales the input to cover the requested size and so past it on one
+// side, is refused with ErrOutputTooLarge when that side would come out above
+// MaxOutputDimension, and otherwise makes the scaled size. A 1024x256 or
+// 256x1024 input scaled by 8 has 8192 for its long side.
+func TestImageProcessor_RefusesFitOutsideAboveMaximum(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name                    string
+		inputWidth, inputHeight int
+		size                    Size
+		// wantWidth and wantHeight are the output's, 0 for a refusal
+		wantWidth, wantHeight int
+	}{
+		{"width above the maximum", 3000, 600, Size{Width: 1, Height: 8192}, 0, 0},
+		{"height above the maximum", 600, 3000, Size{Width: 8192, Height: 1}, 0, 0},
+		{"width at the maximum", 1024, 256, Size{Width: 1, Height: 2048}, 8192, 2048},
+		{"height at the maximum", 256, 1024, Size{Width: 2048, Height: 1}, 2048, 8192},
+		{"within the maximum", 3000, 600, Size{Width: 1, Height: 1200}, 6000, 1200},
+	}
+
+	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: FitOutside})
+			if result != nil {
+				t.Logf("Process() made %dx%d", result.Width, result.Height)
+			}
+
+			if tt.wantWidth == 0 {
+				if !errors.Is(err, ErrOutputTooLarge) {
+					t.Errorf("Process() error = %v, want %v", err, ErrOutputTooLarge)
+				}
+
+				return
+			}
+
+			if err != nil {
+				t.Fatalf("Process() error = %v", err)
+			}
+
+			if result.Width != tt.wantWidth || result.Height != tt.wantHeight {
+				t.Errorf("Process() made %dx%d, want %dx%d",
+					result.Width, result.Height, tt.wantWidth, tt.wantHeight)
+			}
+		})
+	}
+}

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,11 +20,10 @@ import (
 var vipsOnce sync.Once
 
 // initVips initializes libvips with quiet logging, one worker thread per
-// 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.
+// 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.
 func initVips() {
 	vipsOnce.Do(func() {
 		vips.LoggingSettings(nil, vips.LogLevelError)
@@ -133,10 +132,26 @@ var ErrInputTooLarge = errors.New("input image dimensions exceed maximum")
 // configured byte limit.
 var ErrInputDataTooLarge = errors.New("input data exceeds maximum allowed size")
 
+// MaxOutputDimension is the maximum width or height of an output image, and
+// so of a requested size (imgcache.MaxDimension).
+const MaxOutputDimension = 8192
+
+// ErrOutputTooLarge is returned, before resizing, when the output would have a
+// side above MaxOutputDimension: the side of a size with one side 0, computed
+// from the input's proportions, or with FitOutside, the side scaled past the
+// requested size.
+var ErrOutputTooLarge = errors.New("output image dimensions exceed maximum")
+
 // ErrUnsupportedOutputFormat is returned when the requested output format is
 // not supported.
 var ErrUnsupportedOutputFormat = errors.New("unsupported output format")
 
+// ErrUnsupportedInputFormat is returned, before the input is decoded, when
+// libvips reads the input's first bytes as another format than the source
+// formats, or as none: for example an SVG whose first bytes pass the
+// first-bytes check for AVIF.
+var ErrUnsupportedInputFormat = errors.New("unsupported input format")
+
 // ErrTooManyImages is returned when MaxConcurrentProcessing images are being
 // processed and none finishes within ProcessingWaitTimeout.
 var ErrTooManyImages = errors.New("too many images being processed at once")
@@ -163,7 +178,7 @@ type Params struct {
 	// If <= 0, DefaultMaxInputBytes is used.
 	MaxInputBytes int64
 	// MaxConcurrentProcessing is the most images processed at once.
-	// If <= 0, the number of CPUs Go uses (runtime.GOMAXPROCS(0)) is used.
+	// If <= 0, config.DefaultMaxConcurrentProcessing() is used.
 	MaxConcurrentProcessing int
 }
 
@@ -179,7 +194,7 @@ func New(params Params) *ImageProcessor {
 
 	maxConcurrentProcessing := params.MaxConcurrentProcessing
 	if maxConcurrentProcessing <= 0 {
-		maxConcurrentProcessing = runtime.GOMAXPROCS(0)
+		maxConcurrentProcessing = config.DefaultMaxConcurrentProcessing()
 	}
 
 	return &ImageProcessor{
@@ -192,7 +207,10 @@ func New(params Params) *ImageProcessor {
 // Process transforms an image according to the request. When
 // MaxConcurrentProcessing images are already being processed, it waits up
 // to ProcessingWaitTimeout for one to finish, then fails with
-// ErrTooManyImages.
+// ErrTooManyImages. When ctx has ended once Process has a slot, or before it
+// encodes, it returns ctx's error and gives the slot back. govips gives pixa
+// no way to stop libvips once an encode has begun, so an encode keeps its
+// slot until it finishes.
 func (p *ImageProcessor) Process(
 	ctx context.Context,
 	input io.Reader,
@@ -219,6 +237,17 @@ func (p *ImageProcessor) Process(
 		return nil, ErrInputDataTooLarge
 	}
 
+	// govips picks the libvips loader from the input's first bytes with
+	// DetermineImageType. Input whose first bytes it reads as no source
+	// format, such as SVG, which librsvg would render, is refused before
+	// libvips reads it.
+	inputFormat := p.detectFormat(vips.DetermineImageType(data))
+
+	sourceFormat, err := p.formatFromString(inputFormat)
+	if err != nil {
+		return nil, err
+	}
+
 	// Decode image
 	img, err := vips.NewImageFromBuffer(data)
 	if err != nil {
@@ -237,16 +266,16 @@ func (p *ImageProcessor) Process(
 	origWidth := img.Width()
 	origHeight := img.Height()
 
-	// Detect input format
-	inputFormat := p.detectFormat(img)
-
 	// Validate input dimensions to prevent DoS via decompression bombs
 	if origWidth > MaxInputDimension || origHeight > MaxInputDimension {
 		return nil, ErrInputTooLarge
 	}
 
-	// Determine target dimensions
-	targetWidth, targetHeight := targetDimensions(req.Size, origWidth, origHeight)
+	// Determine target dimensions; too large ones are refused before resizing
+	targetWidth, targetHeight, err := targetDimensions(req.Size, origWidth, origHeight)
+	if err != nil {
+		return nil, err
+	}
 
 	// Resize if needed
 	if targetWidth != origWidth || targetHeight != origHeight {
@@ -259,11 +288,11 @@ func (p *ImageProcessor) Process(
 	// orig is the source's own format; encode refuses an empty format
 	outputFormat := req.Format
 	if outputFormat == FormatOriginal {
-		outputFormat = p.formatFromString(inputFormat)
+		outputFormat = sourceFormat
 	}
 
 	// Encode to target format
-	output, err := p.encode(img, outputFormat, req.Quality)
+	output, err := p.encode(ctx, img, outputFormat, req.Quality)
 	if err != nil {
 		return nil, fmt.Errorf("failed to encode: %w", err)
 	}
@@ -282,21 +311,29 @@ func (p *ImageProcessor) Process(
 
 // targetDimensions calculates the output dimensions for a requested size,
 // scaling proportionally when only one dimension is given and keeping the
-// original dimensions when both are zero.
-func targetDimensions(size Size, origWidth, origHeight int) (int, int) {
+// original dimensions when both are zero. A side scaled proportionally can
+// come out above MaxOutputDimension, which is ErrOutputTooLarge.
+func targetDimensions(size Size, origWidth, origHeight int) (int, int, error) {
+	width, height := size.Width, size.Height
+
 	switch {
-	case size.Width == 0 && size.Height == 0:
+	case width == 0 && height == 0:
 		// Both are 0: keep original size
-		return origWidth, origHeight
-	case size.Width == 0:
+		return origWidth, origHeight, nil
+	case width == 0:
 		// Only height specified: calculate width proportionally
-		return origWidth * size.Height / origHeight, size.Height
-	case size.Height == 0:
+		width = origWidth * height / origHeight
+	case height == 0:
 		// Only width specified: calculate height proportionally
-		return size.Width, origHeight * size.Width / origWidth
-	default:
-		return size.Width, size.Height
+		height = origHeight * width / origWidth
 	}
+
+	if width > MaxOutputDimension || height > MaxOutputDimension {
+		return 0, 0, fmt.Errorf("%w: %dx%d is above %d", ErrOutputTooLarge,
+			width, height, MaxOutputDimension)
+	}
+
+	return width, height, nil
 }
 
 // MIME types for the supported image formats.
@@ -382,31 +419,33 @@ func (p *ImageProcessor) WaitForProcessing(ctx context.Context) int {
 
 // acquireSlot takes a slot in processingSemaphore, waiting at most
 // processingWaitTimeout for one to free up, and returns the func that gives
-// it back. A free slot is taken even when ctx has ended; only the wait for
-// one stops when ctx ends, as the rest of Process does not check ctx.
+// it back. When ctx has ended by the time it has a slot, it gives the slot
+// back and returns ctx's error.
 func (p *ImageProcessor) acquireSlot(ctx context.Context) (func(), error) {
-	release := func() { <-p.processingSemaphore }
-
 	select {
 	case p.processingSemaphore <- struct{}{}:
-		return release, nil
-	default:
-	}
-
-	select {
-	case p.processingSemaphore <- struct{}{}:
-		return release, nil
 	case <-time.After(p.processingWaitTimeout):
 		return nil, ErrTooManyImages
 	case <-ctx.Done():
 		return nil, ctx.Err()
 	}
+
+	release := func() { <-p.processingSemaphore }
+
+	// select takes the slot even when ctx has ended at the same moment
+	err := ctx.Err()
+	if err != nil {
+		release()
+
+		return nil, err
+	}
+
+	return release, nil
 }
 
-// detectFormat returns the format string from a vips image.
-func (p *ImageProcessor) detectFormat(img *vips.ImageRef) string {
-	format := img.Format()
-	switch format {
+// detectFormat returns the format string for a govips image type.
+func (p *ImageProcessor) detectFormat(imageType vips.ImageType) string {
+	switch imageType {
 	case vips.ImageTypeJPEG:
 		return "jpeg"
 	case vips.ImageTypePNG:
@@ -428,7 +467,9 @@ func (p *ImageProcessor) detectFormat(img *vips.ImageRef) string {
 	}
 }
 
-// resize resizes the image according to the fit mode.
+// resize resizes the image according to the fit mode. With FitOutside one
+// side can come out larger than requested; when it would be above
+// MaxOutputDimension, resize returns ErrOutputTooLarge without resizing.
 func (p *ImageProcessor) resize(
 	img *vips.ImageRef, width, height int, fit FitMode,
 ) error {
@@ -476,6 +517,12 @@ func (p *ImageProcessor) resize(
 		newW := int(float64(imgW) * scale)
 		newH := int(float64(imgH) * scale)
 
+		// The dimension that exceeds the target is held to the maximum too
+		if newW > MaxOutputDimension || newH > MaxOutputDimension {
+			return fmt.Errorf("%w: %dx%d is above %d", ErrOutputTooLarge,
+				newW, newH, MaxOutputDimension)
+		}
+
 		return img.Thumbnail(newW, newH, vips.InterestingNone)
 
 	default:
@@ -485,10 +532,17 @@ func (p *ImageProcessor) resize(
 
 const defaultQuality = 85
 
-// encode encodes an image to the specified format.
+// encode encodes an image to the specified format. govips gives pixa no way
+// to stop libvips once an encode has begun, so when ctx has ended encode
+// returns ctx's error without beginning one.
 func (p *ImageProcessor) encode(
-	img *vips.ImageRef, format Format, quality int,
+	ctx context.Context, img *vips.ImageRef, format Format, quality int,
 ) ([]byte, error) {
+	err := ctx.Err()
+	if err != nil {
+		return nil, err
+	}
+
 	if quality <= 0 {
 		quality = defaultQuality
 	}
@@ -498,7 +552,7 @@ func (p *ImageProcessor) encode(
 	// libvips' built-in profile; govips' own sRGB path variable is set on
 	// first use but read without a lock, so concurrent requests race on it.
 	if img.HasICCProfile() {
-		err := img.TransformICCProfileWithFallback("srgb", "srgb")
+		err = img.TransformICCProfileWithFallback("srgb", "srgb")
 		if err != nil {
 			return nil, fmt.Errorf("failed to convert to sRGB: %w", err)
 		}
@@ -590,10 +644,11 @@ func exportWebP(img *vips.ImageRef, quality int) ([]byte, error) {
 
 // avifEffort is the AVIF effort, from 0 (fastest) to 9; 1 is the lowest
 // govips can set. With one thread, as pixad runs libvips, 1 takes about 51
-// seconds to save an 8192x8192 image of random pixels, the worst case,
-// against the default downstream_timeout of 60 seconds. On an image of
-// milder noise, which 1 saves in about 12 seconds, 2 takes nearly a minute
-// and libvips' default, 4, takes minutes.
+// seconds to save an 8192x8192 RGB image of random pixels, and about 110
+// with an alpha channel, the worst case, against the default
+// downstream_timeout of 60 seconds. On an image of milder noise, which 1
+// saves in about 12 seconds, 2 takes nearly a minute and libvips' default,
+// 4, takes minutes.
 const avifEffort = 1
 
 // avifBitdepth is the AVIF bit depth, 8 bits per sample for every image.
@@ -687,22 +742,23 @@ func jxlDistance(quality int) float64 {
 	return 53.0/3000.0*q*q - 23.0/20.0*q + 25.0
 }
 
-// formatFromString converts a format string to Format.
-func (p *ImageProcessor) formatFromString(format string) Format {
+// formatFromString converts a format string from detectFormat to Format. A
+// format detectFormat could not name is ErrUnsupportedInputFormat.
+func (p *ImageProcessor) formatFromString(format string) (Format, error) {
 	switch format {
 	case "jpeg":
-		return FormatJPEG
+		return FormatJPEG, nil
 	case "png":
-		return FormatPNG
+		return FormatPNG, nil
 	case "gif":
-		return FormatGIF
+		return FormatGIF, nil
 	case "webp":
-		return FormatWebP
+		return FormatWebP, nil
 	case string(FormatAVIF):
-		return FormatAVIF
+		return FormatAVIF, nil
 	case string(FormatJXL):
-		return FormatJXL
+		return FormatJXL, nil
 	default:
-		return FormatJPEG
+		return "", fmt.Errorf("%w: %s", ErrUnsupportedInputFormat, format)
 	}
 }

internal/imageprocessor/max_concurrent_processing_internal_test.go

@@ -115,14 +115,14 @@ func waitForEntries(t *testing.T, entered <-chan struct{}, count int) {
 	}
 }
 
-func TestNewDefaultsMaxConcurrentProcessingToCPUs(t *testing.T) {
+func TestNewDefaultsMaxConcurrentProcessingToHalfTheCPUs(t *testing.T) {
 	t.Parallel()
 
 	for _, limit := range []int{0, -1} {
 		proc := New(Params{MaxConcurrentProcessing: limit})
-		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))
+		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))
 		}
 	}
 
@@ -132,6 +132,30 @@ func TestNewDefaultsMaxConcurrentProcessingToCPUs(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/imageprocessor/orig_format_internal_test.go

@@ -0,0 +1,39 @@
+package imageprocessor
+
+import (
+	"bytes"
+	"errors"
+	"testing"
+
+	"github.com/davidbyttow/govips/v2/vips"
+)
+
+// TestImageProcessor_OrigOfUnnamedFormatRefused verifies that orig of a TIFF
+// input, which is not a source format, is refused with
+// ErrUnsupportedInputFormat before it is decoded, and not saved as JPEG.
+func TestImageProcessor_OrigOfUnnamedFormatRefused(t *testing.T) {
+	t.Parallel()
+
+	img, err := vips.NewImageFromBuffer(createTestPNG(t, 20, 20))
+	if err != nil {
+		t.Fatalf("failed to decode test PNG: %v", err)
+	}
+
+	t.Cleanup(img.Close)
+
+	tiff, _, err := img.ExportTiff(vips.NewTiffExportParams())
+	if err != nil {
+		t.Fatalf("failed to save test TIFF: %v", err)
+	}
+
+	result, err := New(Params{}).Process(
+		t.Context(), bytes.NewReader(tiff), &Request{Format: FormatOriginal},
+	)
+	if result != nil {
+		t.Logf("Process() returned %s", result.ContentType)
+	}
+
+	if !errors.Is(err, ErrUnsupportedInputFormat) {
+		t.Errorf("Process() error = %v, want %v", err, ErrUnsupportedInputFormat)
+	}
+}

internal/imageprocessor/source_formats_internal_test.go

@@ -0,0 +1,46 @@
+package imageprocessor
+
+import (
+	"bytes"
+	"io"
+	"testing"
+)
+
+// TestImageProcessor_LoadsEachSourceFormat verifies that an image in each of
+// the six source formats, saved by the processor itself, is loaded and saved
+// again in its own format (orig).
+func TestImageProcessor_LoadsEachSourceFormat(t *testing.T) {
+	t.Parallel()
+
+	proc := New(Params{})
+
+	for _, format := range []Format{
+		FormatJPEG, FormatPNG, FormatGIF, FormatWebP, FormatAVIF, FormatJXL,
+	} {
+		saved, err := proc.Process(t.Context(),
+			bytes.NewReader(createTestPNG(t, 20, 20)), &Request{Format: format})
+		if err != nil {
+			t.Fatalf("saving the test image as %s: %v", format, err)
+		}
+
+		source, err := io.ReadAll(saved.Content)
+		if err != nil {
+			t.Fatalf("reading the %s image: %v", format, err)
+		}
+
+		result, err := proc.Process(t.Context(), bytes.NewReader(source),
+			&Request{Format: FormatOriginal})
+		if err != nil {
+			t.Errorf("orig of the %s image: %v", format, err)
+
+			continue
+		}
+
+		t.Logf("orig of the %s image: %s", format, result.ContentType)
+
+		if result.ContentType != FormatToMIME(format) {
+			t.Errorf("orig of the %s image is %s, want %s",
+				format, result.ContentType, FormatToMIME(format))
+		}
+	}
+}

internal/imgcache/cache.go

@@ -32,10 +32,15 @@ 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
-	CacheTTL    time.Duration
 	NegativeTTL time.Duration
 
 	// MaxBytes is the disk cache size limit in bytes that eviction
@@ -51,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
@@ -98,7 +104,8 @@ type Cache struct {
 	pendingCountsDone         chan struct{}
 	pendingCountsCancel       context.CancelFunc
 
-	// Held through each write of the pending counts, and by Stats to read them
+	// Held through each write of the pending counts, and by
+	// readHitAndMissCounts to read them
 	pendingCountsWriteMutex sync.Mutex
 
 	// metaCache holds the content types of the variants most recently
@@ -482,38 +489,34 @@ 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.
 func (c *Cache) Stats(ctx context.Context) (*CacheStats, error) {
 	var stats CacheStats
 
-	// So that no write of the pending counts falls between the two reads
-	c.pendingCountsWriteMutex.Lock()
-
-	// Fetch hit/miss counts from the stats table
-	err := c.db.QueryRowContext(ctx, `
-		SELECT hit_count, miss_count
-		FROM cache_stats WHERE id = 1
-	`).Scan(&stats.HitCount, &stats.MissCount)
-
-	// Hits and misses not yet written count too (see StartPendingCountWrites)
-	stats.HitCount += c.pendingHits.Load()
-	stats.MissCount += c.pendingMisses.Load()
-
-	c.pendingCountsWriteMutex.Unlock()
-
+	err := c.readHitAndMissCounts(ctx, &stats)
 	if err != nil && !errors.Is(err, sql.ErrNoRows) {
 		return nil, fmt.Errorf("failed to get cache stats: %w", err)
 	}
@@ -634,14 +637,33 @@ func (c *Cache) IncrementTransformCount(ctx context.Context) {
 	}
 }
 
+// readHitAndMissCounts sets the hit and miss counts of stats to those of the
+// cache_stats row plus the pending hits and misses (see
+// StartPendingCountWrites). It holds pendingCountsWriteMutex, so that no
+// write of the pending counts falls between the two reads.
+func (c *Cache) readHitAndMissCounts(ctx context.Context, stats *CacheStats) error {
+	c.pendingCountsWriteMutex.Lock()
+	defer c.pendingCountsWriteMutex.Unlock()
+
+	err := c.db.QueryRowContext(ctx, `
+		SELECT hit_count, miss_count
+		FROM cache_stats WHERE id = 1
+	`).Scan(&stats.HitCount, &stats.MissCount)
+
+	stats.HitCount += c.pendingHits.Load()
+	stats.MissCount += c.pendingMisses.Load()
+
+	return err
+}
+
 // loadVariantWithMeta is GetVariant for a variant metaCache does not
 // hold: it reads the content type from the variant's .meta file and
 // keeps it in metaCache, unless a StoreVariant has put one there
 // meanwhile, as the store's is newer. A read that finds no .meta file,
-// as one can between a store's writing of the variant file and of its
-// .meta file, serves application/octet-stream and keeps nothing, so
-// metaCache only ever holds a type read from a .meta file or passed to
-// StoreVariant.
+// as one can when the store's .meta write failed or an eviction removed
+// the file after the read opened the variant, serves
+// application/octet-stream and keeps nothing, so metaCache only ever
+// holds a type read from a .meta file or passed to StoreVariant.
 func (c *Cache) loadVariantWithMeta(
 	cacheKey VariantKey,
 ) (io.ReadCloser, int64, string, error) {
@@ -751,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/cache_internal_test.go

@@ -102,7 +102,6 @@ func setupTestCache(t *testing.T) *Cache {
 
 	cache, err := NewCache(db, CacheConfig{
 		StateDir:    tmpDir,
-		CacheTTL:    time.Hour,
 		NegativeTTL: 5 * time.Minute,
 	})
 	if err != nil {
@@ -241,7 +240,6 @@ func TestCache_NegativeCacheExpiry(t *testing.T) {
 	// Very short negative TTL for testing
 	cache, err := NewCache(db, CacheConfig{
 		StateDir:    tmpDir,
-		CacheTTL:    time.Hour,
 		NegativeTTL: 1 * time.Millisecond,
 	})
 	if err != nil {

internal/imgcache/concurrent_misses_internal_test.go

@@ -15,6 +15,7 @@ import (
 
 	"github.com/getsentry/sentry-go"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
+	"sneak.berlin/go/pixa/internal/imageprocessor"
 	"sneak.berlin/go/pixa/internal/magic"
 )
 
@@ -435,6 +436,47 @@ func TestService_Get_ReturnsByItsDeadline(t *testing.T) {
 	}
 }
 
+// TestService_Get_WaiterGetsProcessingTimedOut has a request with no deadline
+// join the held fetch of a first request with a short one. With the only
+// processing slot held, the first request's transcode waits for it until the
+// first request's deadline ends, and stops then. The waiting request, whose
+// own context has not ended, must get ErrProcessingTimedOut, which the image
+// routes answer 503 as when pixa is busy.
+func TestService_Get_WaiterGetsProcessingTimedOut(t *testing.T) {
+	t.Parallel()
+
+	svc, fixtures, fetcher := setupHeldFetchService(t)
+	svc.processor = imageprocessor.New(
+		imageprocessor.Params{MaxConcurrentProcessing: 1},
+	)
+
+	holdProcessingSlot(t, svc.processor)
+
+	const firstTimeout = 500 * time.Millisecond
+
+	firstCtx, cancelFirst := context.WithTimeout(t.Context(), firstTimeout)
+	defer cancelFirst()
+
+	first := startGet(firstCtx, svc, photoVariant(fixtures, 85, FitCover))
+
+	<-fetcher.started
+
+	waiter := startGet(t.Context(), svc, photoVariant(fixtures, 85, FitCover))
+
+	time.Sleep(arrivalWait)
+	fetcher.releaseFetches()
+
+	t.Logf("first request: Get() error = %v", (<-first).err)
+
+	got := <-waiter
+	t.Logf("waiting request: Get() error = %v", got.err)
+
+	if !errors.Is(got.err, ErrProcessingTimedOut) {
+		t.Errorf("waiting request: Get() error = %v, want %v",
+			got.err, ErrProcessingTimedOut)
+	}
+}
+
 // panickingFetcher panics on every fetch.
 type panickingFetcher struct{}
 

internal/imgcache/eviction.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"database/sql"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io/fs"
 	"os"
@@ -437,14 +438,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 +483,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 +504,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 +540,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
@@ -542,10 +561,11 @@ func (c *Cache) runReconciliationPass(ctx context.Context) {
 // (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), 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
+// 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.
@@ -577,8 +597,9 @@ func (c *Cache) reconcileAccounting(ctx context.Context) error {
 	return c.reconcileUsageTotal(ctx)
 }
 
-// reconcileVariantFiles walks the variant storage directory, adopting
-// files without accounting rows and sweeping stale temp files.
+// reconcileVariantFiles walks the variant storage directory, checking
+// each variant file against its accounting row (reconcileVariantFile)
+// and sweeping stale temp files.
 func (c *Cache) reconcileVariantFiles(ctx context.Context) error {
 	return filepath.WalkDir(
 		c.variants.baseDir,
@@ -603,38 +624,55 @@ func (c *Cache) reconcileVariantFiles(ctx context.Context) error {
 				return nil
 			}
 
-			return c.adoptVariantFile(ctx, path, entry, VariantKey(name))
+			return c.reconcileVariantFile(ctx, path, entry, VariantKey(name))
 		},
 	)
 }
 
-// adoptVariantFile inserts an accounting row for a variant file that
-// has none, using the file's size and modification time.
-func (c *Cache) adoptVariantFile(
+// reconcileVariantFile checks one variant file against its accounting
+// row. A file without a row is adopted into the accounting, unless it is
+// empty. A file that is empty, or not the size its row records, as a
+// crash or a restored snapshot can leave it, is removed with its row and
+// a warning naming it, so the variant is converted again.
+func (c *Cache) reconcileVariantFile(
 	ctx context.Context, path string, entry fs.DirEntry, cacheKey VariantKey,
 ) error {
-	var rowExists int
-
-	err := c.db.QueryRowContext(ctx,
-		`SELECT COUNT(*) FROM variant_content WHERE cache_key = ?`, string(cacheKey),
-	).Scan(&rowExists)
-	if err != nil {
-		return fmt.Errorf("failed to check variant accounting row: %w", err)
-	}
-
-	if rowExists > 0 {
-		return nil
-	}
-
 	info, err := entry.Info()
 	if err != nil {
 		return fmt.Errorf("failed to stat variant file: %w", err)
 	}
 
+	var recordedSize int64
+
+	err = c.db.QueryRowContext(ctx,
+		`SELECT size_bytes FROM variant_content WHERE cache_key = ?`, string(cacheKey),
+	).Scan(&recordedSize)
+
+	switch {
+	case errors.Is(err, sql.ErrNoRows) && info.Size() > 0:
+		return c.adoptVariantFile(ctx, path, info, cacheKey)
+	case err != nil && !errors.Is(err, sql.ErrNoRows):
+		return fmt.Errorf("failed to check variant accounting row: %w", err)
+	case info.Size() > 0 && info.Size() == recordedSize:
+		return nil
+	}
+
+	// The file is empty, or not the size its row records
+	c.log.Warn("removing damaged cached variant file", "path", path,
+		"file_bytes", info.Size(), "recorded_bytes", recordedSize)
+
+	return c.evictVariant(ctx, cacheKey)
+}
+
+// adoptVariantFile inserts an accounting row for a variant file that
+// has none, using the file's size and modification time.
+func (c *Cache) adoptVariantFile(
+	ctx context.Context, path string, info fs.FileInfo, cacheKey VariantKey,
+) error {
 	modTime := info.ModTime().UTC().Format(sqliteTimestampLayout)
 	contentType := c.variantContentTypeFromSidecar(path)
 
-	_, err = c.db.ExecContext(ctx, `
+	_, err := c.db.ExecContext(ctx, `
 		INSERT INTO variant_content
 			(cache_key, size_bytes, content_type, created_at, last_accessed_at)
 		VALUES (?, ?, ?, ?, ?)
@@ -753,10 +791,9 @@ func queryStringColumn[T ~string](
 	return values, nil
 }
 
-// reconcileSourceFiles walks the source content directory, removing
-// blob files the database does not track (they are unreachable: source
-// lookups always go through source_metadata) and sweeping stale temp
-// files.
+// reconcileSourceFiles walks the source content directory, checking
+// each blob file against its source_content row (reconcileSourceFile)
+// and sweeping stale temp files.
 func (c *Cache) reconcileSourceFiles(ctx context.Context) error {
 	return filepath.WalkDir(
 		c.srcContent.baseDir,
@@ -777,31 +814,55 @@ func (c *Cache) reconcileSourceFiles(ctx context.Context) error {
 				return nil
 			}
 
-			return c.removeUntrackedSourceFile(ctx, path, ContentHash(name))
+			return c.reconcileSourceFile(ctx, path, entry, ContentHash(name))
 		},
 	)
 }
 
+// reconcileSourceFile checks one source blob file against its
+// source_content row. A file without a row is removed: it is
+// unreachable, as source lookups always go through source_metadata. A
+// file that is empty, or not the size its row records, as a crash or a
+// restored snapshot can leave it, is removed with its rows and a warning
+// naming it, so the source is fetched again.
+func (c *Cache) reconcileSourceFile(
+	ctx context.Context, path string, entry fs.DirEntry, contentHash ContentHash,
+) error {
+	info, err := entry.Info()
+	if err != nil {
+		return fmt.Errorf("failed to stat source content file: %w", err)
+	}
+
+	var recordedSize int64
+
+	err = c.db.QueryRowContext(ctx,
+		`SELECT size_bytes FROM source_content WHERE content_hash = ?`,
+		string(contentHash),
+	).Scan(&recordedSize)
+
+	switch {
+	case errors.Is(err, sql.ErrNoRows):
+		return c.removeUntrackedSourceFile(ctx, path, contentHash)
+	case err != nil:
+		return fmt.Errorf("failed to check source content row: %w", err)
+	case info.Size() > 0 && info.Size() == recordedSize:
+		return nil
+	}
+
+	// The file is empty, or not the size its row records
+	c.log.Warn("removing damaged cached source file", "path", path,
+		"file_bytes", info.Size(), "recorded_bytes", recordedSize)
+
+	return c.evictSourceBlob(ctx, contentHash)
+}
+
 // removeUntrackedSourceFile deletes a source blob file that has no
 // source_content row. Any source_metadata rows referencing the hash
 // are removed first so no row ever points at a deleted file.
 func (c *Cache) removeUntrackedSourceFile(
 	ctx context.Context, path string, contentHash ContentHash,
 ) error {
-	var rowExists int
-
-	err := c.db.QueryRowContext(ctx,
-		`SELECT COUNT(*) FROM source_content WHERE content_hash = ?`, string(contentHash),
-	).Scan(&rowExists)
-	if err != nil {
-		return fmt.Errorf("failed to check source content row: %w", err)
-	}
-
-	if rowExists > 0 {
-		return nil
-	}
-
-	_, err = c.db.ExecContext(ctx,
+	_, err := c.db.ExecContext(ctx,
 		`DELETE FROM source_metadata WHERE content_hash = ?`, string(contentHash))
 	if err != nil {
 		return fmt.Errorf("failed to delete metadata rows for untracked blob: %w", err)

internal/imgcache/eviction_internal_test.go

@@ -70,7 +70,6 @@ func newEvictionTestCache(t *testing.T, maxBytes int64) (*Cache, string) {
 	// backwards compatibility with existing fixtures.
 	cache, err := NewCache(db, CacheConfig{
 		StateDir:         tmpDir,
-		CacheTTL:         time.Hour,
 		NegativeTTL:      5 * time.Minute,
 		MaxBytes:         maxBytes,
 		DisableDiskCache: maxBytes == 0,

internal/imgcache/module.go

@@ -4,6 +4,5 @@ import "time"
 
 // CacheConfig defaults.
 const (
-	DefaultCacheTTL    = 24 * time.Hour
 	DefaultNegativeTTL = 5 * time.Minute
 )

internal/imgcache/negative_cache_internal_test.go

@@ -3,6 +3,7 @@ package imgcache
 import (
 	"context"
 	"errors"
+	"strconv"
 	"testing"
 	"time"
 )
@@ -15,7 +16,6 @@ func TestNegativeCache_StoreAndCheck(t *testing.T) {
 
 	cache, err := NewCache(db, CacheConfig{
 		StateDir:    dir,
-		CacheTTL:    time.Hour,
 		NegativeTTL: 5 * time.Minute,
 	})
 	if err != nil {
@@ -63,7 +63,6 @@ func TestNegativeCache_Expired(t *testing.T) {
 
 	cache, err := NewCache(db, CacheConfig{
 		StateDir:    dir,
-		CacheTTL:    time.Hour,
 		NegativeTTL: 1 * time.Millisecond, // very short TTL
 	})
 	if err != nil {
@@ -96,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/partial_cache_files_internal_test.go

@@ -0,0 +1,359 @@
+package imgcache
+
+import (
+	"bytes"
+	"crypto/sha256"
+	"encoding/hex"
+	"io"
+	"log/slog"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// getImage calls Get for req and returns the image it answered with and
+// its cache status.
+func getImage(t *testing.T, svc *Service, req *ImageRequest) ([]byte, CacheStatus) {
+	t.Helper()
+
+	resp, err := svc.Get(t.Context(), req)
+	if err != nil {
+		t.Fatalf("Get() of quality %d error = %v", req.Quality, err)
+	}
+
+	defer func() { _ = resp.Content.Close() }()
+
+	body, err := io.ReadAll(resp.Content)
+	if err != nil {
+		t.Fatalf("reading the image of quality %d: %v", req.Quality, err)
+	}
+
+	return body, resp.CacheStatus
+}
+
+// fileSize returns the size of the file at path.
+func fileSize(t *testing.T, path string) int64 {
+	t.Helper()
+
+	info, err := os.Stat(path)
+	if err != nil {
+		t.Fatalf("stat %s: %v", path, err)
+	}
+
+	return info.Size()
+}
+
+// truncateFile cuts the file at path down to size bytes.
+func truncateFile(t *testing.T, path string, size int64) {
+	t.Helper()
+
+	t.Logf("cutting %s from %d to %d bytes",
+		filepath.Base(path), fileSize(t, path), size)
+
+	err := os.Truncate(path, size)
+	if err != nil {
+		t.Fatalf("truncating %s: %v", path, err)
+	}
+}
+
+// cachedSourcePath returns the path of the cached source file req uses.
+func cachedSourcePath(t *testing.T, svc *Service, req *ImageRequest) string {
+	t.Helper()
+
+	contentHash, _, err := svc.cache.LookupSource(t.Context(), req)
+	if err != nil || contentHash == "" {
+		t.Fatalf("LookupSource() = %q, %v; want the cached source", contentHash, err)
+	}
+
+	return svc.cache.srcContent.hashToPath(contentHash)
+}
+
+// reconcileAsAtStartup runs the cache's reconciliation pass, as pixa does
+// when it starts, and returns what the pass logged.
+func reconcileAsAtStartup(t *testing.T, cache *Cache) string {
+	t.Helper()
+
+	var logBuf bytes.Buffer
+
+	cache.log = slog.New(slog.NewJSONHandler(&logBuf, nil))
+
+	err := cache.reconcileAccounting(t.Context())
+	if err != nil {
+		t.Fatalf("reconcileAccounting() error = %v", err)
+	}
+
+	t.Logf("reconciliation log: %s", logBuf.String())
+
+	return logBuf.String()
+}
+
+// assertWarningNames checks that logs hold a warning naming the file at
+// path.
+func assertWarningNames(t *testing.T, logs, path string) {
+	t.Helper()
+
+	for line := range strings.Lines(logs) {
+		if strings.Contains(line, `"level":"WARN"`) &&
+			strings.Contains(line, `"path":"`+path+`"`) {
+			return
+		}
+	}
+
+	t.Errorf("reconciliation logged no warning naming %s", path)
+}
+
+// TestService_Get_FetchesAgainForEmptyCachedSource empties the cached source
+// file, as a crash or a restored snapshot could, and runs the reconciliation
+// pass, which must remove it with a warning naming it. It then asks for two
+// new sizes of the source: the first must fetch it again and cache it whole,
+// so that the second is made from the cache without a third fetch.
+func TestService_Get_FetchesAgainForEmptyCachedSource(t *testing.T) {
+	t.Parallel()
+
+	svc, fixtures := SetupTestService(t)
+
+	getImage(t, svc, photoVariant(fixtures, 85, FitCover))
+
+	path := cachedSourcePath(t, svc, photoVariant(fixtures, 85, FitCover))
+	wholeSize := fileSize(t, path)
+
+	truncateFile(t, path, 0)
+	assertWarningNames(t, reconcileAsAtStartup(t, svc.cache), path)
+
+	getImage(t, svc, photoVariant(fixtures, 40, FitCover))
+	getImage(t, svc, photoVariant(fixtures, 30, FitCover))
+
+	fetches := readCacheStatsCounters(t, svc.cache).upstreamFetchCount
+	t.Logf("upstream fetches = %d", fetches)
+
+	if fetches != 2 {
+		t.Errorf("upstream fetches = %d, want 2: the empty cached source "+
+			"was kept and fetched again on every new size", fetches)
+	}
+
+	size := fileSize(t, path)
+	if size != wholeSize {
+		t.Errorf("cached source is %d bytes, want %d", size, wholeSize)
+	}
+}
+
+// TestService_Get_FetchesAgainForPartialCachedSource cuts the cached source
+// file to half its size, as a crash or a restored snapshot could, and runs
+// the reconciliation pass, which must remove it with a warning naming it.
+// It then asks for two new sizes of the source: the first must fetch it
+// again rather than fail to decode the partial file, and the second is
+// made from the cache.
+func TestService_Get_FetchesAgainForPartialCachedSource(t *testing.T) {
+	t.Parallel()
+
+	svc, fixtures := SetupTestService(t)
+
+	getImage(t, svc, photoVariant(fixtures, 85, FitCover))
+
+	path := cachedSourcePath(t, svc, photoVariant(fixtures, 85, FitCover))
+	wholeSize := fileSize(t, path)
+
+	truncateFile(t, path, wholeSize/2)
+	assertWarningNames(t, reconcileAsAtStartup(t, svc.cache), path)
+
+	getImage(t, svc, photoVariant(fixtures, 40, FitCover))
+	getImage(t, svc, photoVariant(fixtures, 30, FitCover))
+
+	fetches := readCacheStatsCounters(t, svc.cache).upstreamFetchCount
+	t.Logf("upstream fetches = %d", fetches)
+
+	if fetches != 2 {
+		t.Errorf("upstream fetches = %d, want 2", fetches)
+	}
+
+	size := fileSize(t, path)
+	if size != wholeSize {
+		t.Errorf("cached source is %d bytes, want %d", size, wholeSize)
+	}
+}
+
+// assertConvertedAgain caches a variant of the photo, cuts its file to size
+// bytes and runs the reconciliation pass, which must remove it with a
+// warning naming it. It then checks that Get converts the image again
+// instead of serving the file, and that the next Get serves the whole
+// variant from the cache.
+func assertConvertedAgain(t *testing.T, size int64) {
+	t.Helper()
+
+	svc, fixtures := SetupTestService(t)
+	req := photoVariant(fixtures, 85, FitCover)
+
+	want, _ := getImage(t, svc, req)
+
+	path := svc.cache.variants.keyToPath(CacheKey(req))
+
+	truncateFile(t, path, size)
+	assertWarningNames(t, reconcileAsAtStartup(t, svc.cache), path)
+
+	got, status := getImage(t, svc, req)
+	t.Logf("after the cut: %d bytes, %s", len(got), status)
+
+	if status != CacheMiss || !bytes.Equal(got, want) {
+		t.Errorf("after the cut: Get() = %d bytes, %s; want the %d bytes "+
+			"converted again, %s", len(got), status, len(want), CacheMiss)
+	}
+
+	got, status = getImage(t, svc, req)
+	t.Logf("next request: %d bytes, %s", len(got), status)
+
+	if status != CacheHit || !bytes.Equal(got, want) {
+		t.Errorf("next request: Get() = %d bytes, %s; want the %d bytes, %s",
+			len(got), status, len(want), CacheHit)
+	}
+}
+
+// TestService_Get_ConvertsAgainForEmptyCachedVariant empties a cached
+// variant file, as a crash or a restored snapshot could, and checks that it
+// is converted again rather than served empty.
+func TestService_Get_ConvertsAgainForEmptyCachedVariant(t *testing.T) {
+	t.Parallel()
+
+	assertConvertedAgain(t, 0)
+}
+
+// TestService_Get_ConvertsAgainForPartialCachedVariant cuts a cached
+// variant file to ten bytes, as a crash or a restored snapshot could, and
+// checks that it is converted again rather than served cut short.
+func TestService_Get_ConvertsAgainForPartialCachedVariant(t *testing.T) {
+	t.Parallel()
+
+	assertConvertedAgain(t, 10)
+}
+
+// TestReconciliationRemovesEmptyVariantFileWithoutRow checks that the
+// reconciliation pass removes an empty variant file that has no
+// accounting row, with a warning naming it, instead of adding a row for
+// it that would have it served empty.
+func TestReconciliationRemovesEmptyVariantFileWithoutRow(t *testing.T) {
+	t.Parallel()
+
+	cache, _ := newEvictionTestCache(t, 1<<30)
+
+	_, err := cache.variants.Store(testVariantKeyOne, bytes.NewReader(nil),
+		testContentTypeWebP)
+	if err != nil {
+		t.Fatalf("failed to store an empty variant file: %v", err)
+	}
+
+	logs := reconcileAsAtStartup(t, cache)
+
+	assertWarningNames(t, logs, cache.variants.keyToPath(testVariantKeyOne))
+
+	if cache.variants.Exists(testVariantKeyOne) {
+		t.Error("the empty variant file is still on disk")
+	}
+
+	rows := countRows(t, cache,
+		`SELECT COUNT(*) FROM variant_content WHERE cache_key = ?`,
+		string(testVariantKeyOne))
+	if rows != 0 {
+		t.Errorf("the empty variant file got %d accounting rows, want 0", rows)
+	}
+}
+
+// TestVariantStorage_StoreReplacesMetaFile stores a variant over one
+// already stored and checks that Store puts a new .meta file in place of
+// the old one, as it does the image, rather than writing into the old
+// file, which a crash could leave empty. The old .meta file is given a
+// second name before the store, so that it can be told apart from the new
+// one after it.
+func TestVariantStorage_StoreReplacesMetaFile(t *testing.T) {
+	t.Parallel()
+
+	storage, key := storeTestVariant(t)
+	metaPath := storage.keyToPath(key) + variantMetaSuffix
+	oldMetaPath := metaPath + ".old"
+
+	err := os.Link(metaPath, oldMetaPath)
+	if err != nil {
+		t.Fatalf("giving the .meta file a second name: %v", err)
+	}
+
+	_, err = storage.Store(key, bytes.NewReader([]byte("new variant data")),
+		testContentTypeJPEG)
+	if err != nil {
+		t.Fatalf("Store() error = %v", err)
+	}
+
+	oldMeta, err := os.Stat(oldMetaPath)
+	if err != nil {
+		t.Fatalf("stat of the old .meta file: %v", err)
+	}
+
+	newMeta, err := os.Stat(metaPath)
+	if err != nil {
+		t.Fatalf("stat of the new .meta file: %v", err)
+	}
+
+	if os.SameFile(oldMeta, newMeta) {
+		t.Error("Store() wrote into the old .meta file instead of putting " +
+			"a new one in its place")
+	}
+
+	reader, _, contentType, err := storage.LoadWithMeta(key)
+	if err != nil {
+		t.Fatalf("LoadWithMeta() error = %v", err)
+	}
+
+	_ = reader.Close()
+
+	t.Logf("content type after the store: %s", contentType)
+
+	if contentType != testContentTypeJPEG {
+		t.Errorf("content type = %q, want %q", contentType, testContentTypeJPEG)
+	}
+}
+
+// TestContentStorage_StoreHashedReplacesFileOfAnotherSize checks that a
+// store over a file of another size at the content's path, such as a
+// partial file a crash left, replaces it with the content.
+func TestContentStorage_StoreHashedReplacesFileOfAnotherSize(t *testing.T) {
+	t.Parallel()
+
+	storage, err := NewContentStorage(t.TempDir())
+	if err != nil {
+		t.Fatalf("NewContentStorage() error = %v", err)
+	}
+
+	content := []byte("the whole source image")
+	sum := sha256.Sum256(content)
+	hash := ContentHash(hex.EncodeToString(sum[:]))
+	path := storage.hashToPath(hash)
+
+	err = os.MkdirAll(filepath.Dir(path), StorageDirPerm)
+	if err != nil {
+		t.Fatalf("creating the content's directory: %v", err)
+	}
+
+	err = os.WriteFile(path, content[:5], StorageFilePerm)
+	if err != nil {
+		t.Fatalf("writing a partial file: %v", err)
+	}
+
+	_, err = storage.StoreHashed(hash, content)
+	if err != nil {
+		t.Fatalf("StoreHashed() error = %v", err)
+	}
+
+	stored, err := storage.Load(hash)
+	if err != nil {
+		t.Fatalf("Load() error = %v", err)
+	}
+
+	defer func() { _ = stored.Close() }()
+
+	got, err := io.ReadAll(stored)
+	if err != nil {
+		t.Fatalf("reading the stored content: %v", err)
+	}
+
+	if !bytes.Equal(got, content) {
+		t.Errorf("stored file = %q, want %q", got, content)
+	}
+}

internal/imgcache/pendingcounts.go

@@ -41,8 +41,10 @@ func (c *Cache) StopPendingCountWrites(ctx context.Context) error {
 		select {
 		case <-c.pendingCountsDone:
 		case <-ctx.Done():
-			// The write under way is left to finish: the database's
-			// close waits for a query under way.
+			// The write given up on here may not reach the database:
+			// database/sql closes without waiting for a query, and a
+			// write still waiting for the connection fails when the
+			// database closes.
 			err = fmt.Errorf("pending counts still being written: %w", ctx.Err())
 		}
 	}
@@ -68,9 +70,9 @@ func (c *Cache) StopPendingCountWrites(ctx context.Context) error {
 }
 
 // pendingCountWriteLoop is the body of the goroutine
-// StartPendingCountWrites starts. It returns when ctx is cancelled, but
-// does not cut off a write under way then. A write that fails leaves the
-// counts pending, for the next write.
+// StartPendingCountWrites starts. Once ctx is cancelled it starts no write
+// and returns, but does not cut off a write under way then. A write that
+// fails leaves the counts pending, for the next write.
 func (c *Cache) pendingCountWriteLoop(ctx context.Context) {
 	defer close(c.pendingCountsDone)
 
@@ -81,6 +83,12 @@ func (c *Cache) pendingCountWriteLoop(ctx context.Context) {
 		case <-c.pendingCountsAdded:
 		}
 
+		// select picks at random when ctx is cancelled and a wakeup
+		// waits too.
+		if ctx.Err() != nil {
+			return
+		}
+
 		err := c.writePendingCounts(context.WithoutCancel(ctx))
 		if err != nil {
 			c.log.Warn("failed to write pending counts", "error", err)

internal/imgcache/pendingcounts_internal_test.go

@@ -10,9 +10,14 @@ import (
 
 // pendingCountsWait is how long a test waits for the database's connection,
 // for a count to reach the database, for a write to start waiting for the
-// connection, or for StopPendingCountWrites to return.
+// connection, for a request to return past its deadline, or for
+// StopPendingCountWrites to return.
 const pendingCountsWait = 5 * time.Second
 
+// otherWritesWait is how long a test gives other writes, once one write waits
+// for the database's connection, to start waiting for it too.
+const otherWritesWait = 100 * time.Millisecond
+
 // holdDatabase takes the one connection of cache's database, so that every
 // other query waits for it, and returns the func that frees it.
 func holdDatabase(t *testing.T, cache *Cache) func() {
@@ -68,9 +73,9 @@ func waitForCounters(t *testing.T, cache *Cache, want cacheStatsCounters) {
 
 // TestService_Get_ReturnsByItsDeadlineWhileTheDatabaseIsBusy holds the
 // database's one connection while a request whose fetch is held reaches its
-// deadline. The request must still return by its deadline with the
-// deadline's error, and its miss must reach the database once the
-// connection is free.
+// deadline. The request must return with the deadline's error while the
+// connection is still held, as it waits for no database write past its
+// deadline, and its miss must reach the database once the connection is free.
 func TestService_Get_ReturnsByItsDeadlineWhileTheDatabaseIsBusy(t *testing.T) {
 	t.Parallel()
 
@@ -107,8 +112,8 @@ func TestService_Get_ReturnsByItsDeadlineWhileTheDatabaseIsBusy(t *testing.T) {
 		if !errors.Is(got.err, context.DeadlineExceeded) {
 			t.Errorf("Get() error = %v, want %v", got.err, context.DeadlineExceeded)
 		}
-	case <-time.After(time.Until(deadline) + time.Second):
-		t.Fatal("request did not return by its deadline while the database was busy")
+	case <-time.After(time.Until(deadline) + pendingCountsWait):
+		t.Fatal("request did not return while the database was busy")
 	}
 
 	releaseDatabase()
@@ -145,7 +150,7 @@ func TestIncrementStats_ManyCountsPastTheirDeadlineLeaveOneWriteWaiting(t *testi
 
 	// Once one write waits for the connection, any others start waiting too.
 	waitForConnectionWaits(t, cache.db, waitsBefore+1)
-	time.Sleep(arrivalWait)
+	time.Sleep(otherWritesWait)
 
 	waits := cache.db.Stats().WaitCount - waitsBefore
 	t.Logf("%d writes waited for the database", waits)
@@ -200,8 +205,8 @@ func TestStats_IncludesPendingCounts(t *testing.T) {
 // TestStopPendingCountWrites_WritesPendingCounts holds the database's one
 // connection while the goroutine that writes the pending counts waits for it,
 // counts more, then stops that goroutine. The stop must leave the write under
-// way to finish rather than cut it off, and every count must reach the
-// database once the connection is free.
+// way to finish rather than cut it off, and once the connection is free it
+// must write the counts that write left.
 func TestStopPendingCountWrites_WritesPendingCounts(t *testing.T) {
 	t.Parallel()
 
@@ -229,6 +234,12 @@ func TestStopPendingCountWrites_WritesPendingCounts(t *testing.T) {
 	cache.IncrementUpstreamFetch(ended, 1024)
 	cache.IncrementTransformCount(ended)
 
+	// The goroutine is told to finish here as well as by the stop, so that it
+	// is told before its write under way ends, however late the stop runs.
+	// The counts above then reach the database only through the stop's own
+	// write.
+	cache.pendingCountsCancel()
+
 	stopped := make(chan error, 1)
 
 	go func() {
@@ -237,7 +248,7 @@ func TestStopPendingCountWrites_WritesPendingCounts(t *testing.T) {
 
 	// Had the stop cut off the write under way, its own write would wait for
 	// the connection too.
-	time.Sleep(arrivalWait)
+	time.Sleep(otherWritesWait)
 
 	waits := cache.db.Stats().WaitCount - waitsBefore
 	t.Logf("%d writes waited for the database", waits)

internal/imgcache/service.go

@@ -9,6 +9,7 @@ import (
 	"log/slog"
 	"net/url"
 	"runtime/debug"
+	"strings"
 	"time"
 
 	"github.com/dustin/go-humanize"
@@ -52,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, one per CPU
+	// uses the image processor's default, half the CPUs
 	MaxConcurrentProcessing int
 	// Logger for logging
 	Logger *slog.Logger
@@ -128,26 +129,20 @@ func NewService(cfg *ServiceConfig) (*Service, error) {
 // ErrNegativeCached is returned when a URL is in the negative cache (recently failed).
 var ErrNegativeCached = errors.New("request is in negative cache (recently failed)")
 
+// ErrProcessingTimedOut is returned when processing an image stopped because
+// the downstream_timeout of the request doing it ended, to that request and to
+// every request that waited for its image.
+var ErrProcessingTimedOut = errors.New(
+	"image processing stopped at the deadline of the request doing it")
+
 // 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 +213,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 +427,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()
 
@@ -535,7 +555,13 @@ func (s *Service) processAndStore(
 		FitMode: imageprocessor.FitMode(req.FitMode),
 	}
 
+	// ctx has the deadline of the request doing the processing, which can
+	// end while a request waiting for its image has time left
 	processResult, err := s.processor.Process(ctx, source, processReq)
+	if errors.Is(err, context.DeadlineExceeded) {
+		return nil, fmt.Errorf("%w: %w", ErrProcessingTimedOut, err)
+	}
+
 	if err != nil {
 		return nil, fmt.Errorf("image processing failed: %w", err)
 	}

internal/imgcache/service_internal_test.go

@@ -2,7 +2,9 @@ package imgcache
 
 import (
 	"context"
+	"encoding/json"
 	"io"
+	"os"
 	"testing"
 	"time"
 
@@ -248,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()
 
@@ -430,6 +523,172 @@ func TestService_Get_Caching(t *testing.T) {
 	}
 }
 
+// TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo caches a source and
+// a variant of it and sets back two days every time pixa stored for them.
+// For the source: fetched_at in source_metadata, fetched_at and
+// last_accessed_at in source_content, the fetched_at in its metadata file,
+// and the access and modification times of its file and of its metadata
+// file. For the variant: created_at and last_accessed_at in variant_content,
+// the created_at in its .meta file, and the access and modification times of
+// its file and of its .meta file. It then checks that the variant is still
+// served from the cache and that a new variant is made from the cached source
+// without fetching it again.
+//
+// Left at the present: the files' status change and birth times, which a
+// program cannot set, and the times of the directories holding the files,
+// which other entries share. The source's stored response headers are empty,
+// as the mock fetcher sends none. expires_at, etag and last_modified in
+// source_metadata and in the metadata file are never written, nor are the
+// request_cache and output_content tables.
+func TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo(t *testing.T) {
+	t.Parallel()
+
+	svc, fixtures := SetupTestService(t)
+	variant := photoVariant(fixtures, 85, FitCover)
+
+	resp, err := svc.Get(t.Context(), variant)
+	if err != nil {
+		t.Fatalf("Get() first request error = %v", err)
+	}
+
+	_ = resp.Content.Close()
+
+	twoDaysAgo := time.Now().AddDate(0, 0, -2)
+	backdateSource(t, svc.cache, variant, twoDaysAgo)
+	backdateVariant(t, svc.cache, variant, twoDaysAgo)
+
+	resp, err = svc.Get(t.Context(), variant)
+	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)
+	}
+
+	// Another quality is a new variant, made from the cached source
+	resp, err = svc.Get(t.Context(), photoVariant(fixtures, 40, FitCover))
+	if err != nil {
+		t.Fatalf("Get() of a new variant error = %v", err)
+	}
+
+	_ = resp.Content.Close()
+
+	fetches := readCacheStatsCounters(t, svc.cache).upstreamFetchCount
+	if fetches != 1 {
+		t.Errorf("upstream fetches = %d, want 1", fetches)
+	}
+}
+
+// backdateSource sets every time pixa stored for req's cached source to when,
+// as listed on TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo.
+func backdateSource(
+	t *testing.T, cache *Cache, req *ImageRequest, when time.Time,
+) {
+	t.Helper()
+
+	pathHash := HashPath(req.SourcePath + "?" + req.SourceQuery)
+
+	meta, err := cache.srcMetadata.Load(req.SourceHost, pathHash)
+	if err != nil {
+		t.Fatalf("reading the source's metadata file: %v", err)
+	}
+
+	meta.FetchedAt = when.Unix()
+
+	err = cache.srcMetadata.Store(req.SourceHost, pathHash, meta)
+	if err != nil {
+		t.Fatalf("writing the source's metadata file: %v", err)
+	}
+
+	dbTime := when.UTC().Format(sqliteTimestampFormat)
+
+	updateOneRow(t, cache, `
+		UPDATE source_metadata SET fetched_at = ?
+		WHERE source_host = ? AND source_path = ? AND source_query = ?
+	`, dbTime, req.SourceHost, req.SourcePath, req.SourceQuery)
+	updateOneRow(t, cache,
+		`UPDATE source_content SET fetched_at = ? WHERE content_hash = ?`,
+		dbTime, meta.ContentHash)
+	setSourceLastAccessed(t, cache, ContentHash(meta.ContentHash), when)
+
+	for _, path := range []string{
+		cache.srcContent.hashToPath(ContentHash(meta.ContentHash)),
+		cache.srcMetadata.metaPath(req.SourceHost, pathHash),
+	} {
+		err = os.Chtimes(path, when, when)
+		if err != nil {
+			t.Fatalf("setting back the times of %s: %v", path, err)
+		}
+	}
+}
+
+// backdateVariant sets every time pixa stored for the cached variant req asks
+// for to when, as listed on
+// TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo.
+func backdateVariant(
+	t *testing.T, cache *Cache, req *ImageRequest, when time.Time,
+) {
+	t.Helper()
+
+	key := CacheKey(req)
+
+	content, size, contentType, err := cache.variants.LoadWithMeta(key)
+	if err != nil || contentType == "" {
+		t.Fatalf("reading the variant's .meta file: content type %q, error %v",
+			contentType, err)
+	}
+
+	_ = content.Close()
+
+	meta, err := json.Marshal(VariantMeta{
+		ContentType: contentType,
+		Size:        size,
+		CreatedAt:   when.Unix(),
+	})
+	if err != nil {
+		t.Fatalf("encoding the variant's .meta file: %v", err)
+	}
+
+	path := cache.variants.keyToPath(key)
+
+	err = os.WriteFile(path+variantMetaSuffix, meta, StorageFilePerm)
+	if err != nil {
+		t.Fatalf("writing the variant's .meta file: %v", err)
+	}
+
+	updateOneRow(t, cache,
+		`UPDATE variant_content SET created_at = ? WHERE cache_key = ?`,
+		when.UTC().Format(sqliteTimestampFormat), string(key))
+	setVariantLastAccessed(t, cache, key, when)
+
+	for _, file := range []string{path, path + variantMetaSuffix} {
+		err = os.Chtimes(file, when, when)
+		if err != nil {
+			t.Fatalf("setting back the times of %s: %v", file, err)
+		}
+	}
+}
+
+// updateOneRow runs the UPDATE statement query and fails the test unless it
+// changed exactly one row.
+func updateOneRow(t *testing.T, cache *Cache, query string, args ...any) {
+	t.Helper()
+
+	result, err := cache.db.ExecContext(t.Context(), query, args...)
+	if err != nil {
+		t.Fatalf("%s: %v", query, err)
+	}
+
+	rows, err := result.RowsAffected()
+	if err != nil || rows != 1 {
+		t.Fatalf("%s: rows = %d, error = %v, want 1 row", query, rows, err)
+	}
+}
+
 func TestService_Get_DifferentSizes(t *testing.T) {
 	t.Parallel()
 

internal/imgcache/stats_internal_test.go

@@ -43,7 +43,6 @@ func TestStats_HitRateIsRatio(t *testing.T) {
 
 	cache, err := NewCache(db, CacheConfig{
 		StateDir:    dir,
-		CacheTTL:    time.Hour,
 		NegativeTTL: 5 * time.Minute,
 	})
 	if err != nil {
@@ -91,7 +90,6 @@ func TestStats_ZeroCounts(t *testing.T) {
 
 	cache, err := NewCache(db, CacheConfig{
 		StateDir:    dir,
-		CacheTTL:    time.Hour,
 		NegativeTTL: 5 * time.Minute,
 	})
 	if err != nil {
@@ -120,7 +118,6 @@ func TestStats_LogsFailedCountQueries(t *testing.T) {
 
 	cache, err := NewCache(db, CacheConfig{
 		StateDir:    t.TempDir(),
-		CacheTTL:    time.Hour,
 		NegativeTTL: 5 * time.Minute,
 		Logger:      slog.New(slog.NewJSONHandler(&logBuf, nil)),
 	})
@@ -162,7 +159,6 @@ func TestIncrementStats_LogsFailedUpdates(t *testing.T) {
 
 	cache, err := NewCache(db, CacheConfig{
 		StateDir:    t.TempDir(),
-		CacheTTL:    time.Hour,
 		NegativeTTL: 5 * time.Minute,
 		Logger:      slog.New(slog.NewJSONHandler(&logBuf, nil)),
 	})
@@ -232,7 +228,6 @@ func TestStats_DisabledCacheReportsNoItems(t *testing.T) {
 
 	disabled, err := NewCache(enabled.db, CacheConfig{
 		StateDir:         t.TempDir(),
-		CacheTTL:         time.Hour,
 		NegativeTTL:      5 * time.Minute,
 		DisableDiskCache: true,
 	})

internal/imgcache/storage.go

@@ -10,6 +10,7 @@ import (
 	"log/slog"
 	"os"
 	"path/filepath"
+	"strings"
 	"time"
 )
 
@@ -79,8 +80,8 @@ func (s *ContentStorage) Store(r io.Reader) (ContentHash, int64, error) {
 // from hash, without recomputing it. Callers that already know the
 // hash before writing (e.g. because they must hold a hash-keyed lock
 // across the whole store operation) use this instead of Store. Like
-// Store, it is idempotent: content already on disk at that path is
-// left untouched.
+// Store, it is idempotent: a file of the content's size already at that
+// path is left untouched, and a file of another size is replaced.
 func (s *ContentStorage) StoreHashed(
 	hash ContentHash, data []byte,
 ) (int64, error) {
@@ -151,28 +152,35 @@ func (s *ContentStorage) Exists(hash ContentHash) bool {
 	return err == nil
 }
 
-// writeIfAbsent writes data to the path derived from hash, unless
-// content already exists there, via a temp-file-plus-rename so
-// concurrent readers never observe a partial file.
+// writeIfAbsent writes data with writeFileSynced to the path derived
+// from hash, unless a file of the data's size is already there.
 func (s *ContentStorage) writeIfAbsent(hash ContentHash, data []byte) error {
 	// Build path: <basedir>/<ab>/<cd>/<hash>
 	path := s.hashToPath(hash)
 
-	// Check if already exists
-	_, statErr := os.Stat(path)
-	if statErr == nil {
+	// A file of another size there is damaged, and is replaced
+	info, statErr := os.Stat(path)
+	if statErr == nil && info.Size() == int64(len(data)) {
 		return nil
 	}
 
-	// Create directory structure
-	dir := filepath.Dir(path)
-
-	err := os.MkdirAll(dir, StorageDirPerm)
+	err := os.MkdirAll(filepath.Dir(path), StorageDirPerm)
 	if err != nil {
 		return fmt.Errorf("failed to create directory: %w", err)
 	}
 
-	// Write to temp file first, then rename for atomicity
+	return writeFileSynced(path, data)
+}
+
+// writeFileSynced writes data to path through a temp file in the same
+// directory: the temp file is synced to disk, renamed to path, and the
+// directory synced after, so that neither a concurrent reader nor a
+// crash finds a partial file under path. When the directory sync fails,
+// the file is removed from path, so that a failed write leaves no file
+// there.
+func writeFileSynced(path string, data []byte) error {
+	dir := filepath.Dir(path)
+
 	tmpFile, err := os.CreateTemp(dir, ".tmp-*")
 	if err != nil {
 		return fmt.Errorf("failed to create temp file: %w", err)
@@ -180,18 +188,20 @@ func (s *ContentStorage) writeIfAbsent(hash ContentHash, data []byte) error {
 
 	tmpPath := tmpFile.Name()
 
-	// Each failure path below unlinks the temp file explicitly. This
-	// replaces a deferred cleanup that read a named result, which the
-	// canonical config does not permit; the set of paths that remove
-	// tmpPath, and the order relative to Close, is unchanged. This
-	// mirrors how MetadataStorage.Store and VariantStorage.Store below
-	// already express the same cleanup.
 	_, err = tmpFile.Write(data)
 	if err != nil {
 		_ = tmpFile.Close()
 		_ = os.Remove(tmpPath)
 
-		return fmt.Errorf("failed to write content: %w", err)
+		return fmt.Errorf("failed to write temp file: %w", err)
+	}
+
+	err = tmpFile.Sync()
+	if err != nil {
+		_ = tmpFile.Close()
+		_ = os.Remove(tmpPath)
+
+		return fmt.Errorf("failed to sync temp file: %w", err)
 	}
 
 	err = tmpFile.Close()
@@ -201,7 +211,6 @@ func (s *ContentStorage) writeIfAbsent(hash ContentHash, data []byte) error {
 		return fmt.Errorf("failed to close temp file: %w", err)
 	}
 
-	// Atomic rename
 	err = os.Rename(filepath.Clean(tmpPath), filepath.Clean(path))
 	if err != nil {
 		_ = os.Remove(tmpPath)
@@ -209,6 +218,31 @@ func (s *ContentStorage) writeIfAbsent(hash ContentHash, data []byte) error {
 		return fmt.Errorf("failed to rename temp file: %w", err)
 	}
 
+	err = syncDirectory(dir)
+	if err != nil {
+		_ = os.Remove(path)
+
+		return err
+	}
+
+	return nil
+}
+
+// syncDirectory syncs the directory dir to disk, so that a file just
+// renamed into it is still there after a crash.
+func syncDirectory(dir string) error {
+	d, err := os.Open(filepath.Clean(dir))
+	if err != nil {
+		return fmt.Errorf("failed to open directory: %w", err)
+	}
+
+	err = d.Sync()
+	_ = d.Close()
+
+	if err != nil {
+		return fmt.Errorf("failed to sync directory: %w", err)
+	}
+
 	return nil
 }
 
@@ -371,11 +405,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 +423,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
@@ -416,7 +460,9 @@ func NewVariantStorage(baseDir string, log *slog.Logger) (*VariantStorage, error
 	return &VariantStorage{baseDir: baseDir, log: log}, nil
 }
 
-// Store writes content and metadata to storage at the given key.
+// Store writes content and metadata to storage at the given key, each
+// with writeFileSynced. The .meta file is written first, so that a crash
+// cannot leave the content in place without it.
 func (s *VariantStorage) Store(
 	key VariantKey, r io.Reader, contentType string,
 ) (int64, error) {
@@ -429,46 +475,11 @@ func (s *VariantStorage) Store(
 	path := s.keyToPath(key)
 	metaPath := path + ".meta"
 
-	// Create directory structure
-	dir := filepath.Dir(path)
-
-	err = os.MkdirAll(dir, StorageDirPerm)
+	err = os.MkdirAll(filepath.Dir(path), StorageDirPerm)
 	if err != nil {
 		return 0, fmt.Errorf("failed to create directory: %w", err)
 	}
 
-	// Write content to temp file first, then rename for atomicity
-	tmpFile, err := os.CreateTemp(dir, ".tmp-*")
-	if err != nil {
-		return 0, fmt.Errorf("failed to create temp file: %w", err)
-	}
-
-	tmpPath := tmpFile.Name()
-
-	_, err = tmpFile.Write(data)
-	if err != nil {
-		_ = tmpFile.Close()
-		_ = os.Remove(tmpPath)
-
-		return 0, fmt.Errorf("failed to write content: %w", err)
-	}
-
-	err = tmpFile.Close()
-	if err != nil {
-		_ = os.Remove(tmpPath)
-
-		return 0, fmt.Errorf("failed to close temp file: %w", err)
-	}
-
-	// Atomic rename content
-	err = os.Rename(filepath.Clean(tmpPath), filepath.Clean(path))
-	if err != nil {
-		_ = os.Remove(tmpPath)
-
-		return 0, fmt.Errorf("failed to rename temp file: %w", err)
-	}
-
-	// Write metadata
 	meta := VariantMeta{
 		ContentType: contentType,
 		Size:        size,
@@ -480,13 +491,19 @@ func (s *VariantStorage) Store(
 		return 0, fmt.Errorf("failed to marshal metadata: %w", err)
 	}
 
-	// Metadata write failure is non-fatal; content is already stored.
-	err = os.WriteFile(metaPath, metaData, StorageFilePerm)
+	// Metadata write failure is non-fatal: the content is then served as
+	// application/octet-stream.
+	err = writeFileSynced(metaPath, metaData)
 	if err != nil {
 		s.log.Warn("failed to write variant metadata sidecar",
 			"path", metaPath, "error", err)
 	}
 
+	err = writeFileSynced(path, data)
+	if err != nil {
+		return 0, err
+	}
+
 	return size, nil
 }
 

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/testutil_internal_test.go

@@ -185,7 +185,6 @@ func SetupTestService(
 
 	cache, err := NewCache(db, CacheConfig{
 		StateDir:    tmpDir,
-		CacheTTL:    time.Hour,
 		NegativeTTL: 5 * time.Minute,
 	})
 	if err != nil {

internal/imgcache/urlparser.go

@@ -7,12 +7,15 @@ import (
 	"regexp"
 	"strconv"
 	"strings"
+
+	"sneak.berlin/go/pixa/internal/imageprocessor"
 )
 
 // URL parsing errors.
 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")
@@ -20,8 +23,9 @@ var (
 	ErrPathTraversal     = errors.New("path traversal detected")
 )
 
-// MaxDimension is the maximum allowed width or height.
-const MaxDimension = 8192
+// MaxDimension is the maximum allowed width or height, the same as the image
+// processor's maximum for an output image.
+const MaxDimension = imageprocessor.MaxOutputDimension
 
 // ValidateDimension checks one requested width or height; name ("width" or
 // "height") appears in the error. 0 means "original size" and is valid.
@@ -140,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/middleware/middleware.go

@@ -8,12 +8,15 @@ import (
 	"net/http"
 	"net/netip"
 	"regexp"
+	"strings"
 	"time"
 
 	basicauth "github.com/99designs/basicauth-go"
+	"github.com/go-chi/chi/v5"
 	"github.com/go-chi/chi/v5/middleware"
 	"github.com/go-chi/cors"
 	"github.com/go-chi/httprate"
+	"github.com/prometheus/client_golang/prometheus"
 	metrics "github.com/slok/go-http-metrics/metrics/prometheus"
 	ghmm "github.com/slok/go-http-metrics/middleware"
 	"github.com/slok/go-http-metrics/middleware/std"
@@ -63,6 +66,10 @@ type Middleware struct {
 	log      *slog.Logger
 	config   *config.Config
 	clientIP *clientip.Resolver
+
+	// metricsRegistry is the registry Metrics registers its metrics with.
+	// nil is Prometheus' default registry, the one /metrics serves.
+	metricsRegistry prometheus.Registerer
 }
 
 // New creates a new Middleware instance.
@@ -98,9 +105,10 @@ func (s *Middleware) ClientIP() func(http.Handler) http.Handler {
 // holds whole; an IPv4-mapped address (::ffff:a.b.c.d) is counted as the
 // IPv4 address it carries, since every such address falls in the same /64.
 // Counts are kept only for the current and the previous window, so memory
-// stays bounded.
+// stays bounded. limitHandler writes the 429 after Retry-After is set; nil
+// keeps the rate limit library's plain-text "Too Many Requests".
 func (s *Middleware) RateLimit(
-	requestLimit int, window time.Duration,
+	requestLimit int, window time.Duration, limitHandler http.HandlerFunc,
 ) func(http.Handler) http.Handler {
 	return httprate.LimitBy(requestLimit, window,
 		func(r *http.Request) (string, error) {
@@ -112,7 +120,8 @@ func (s *Middleware) RateLimit(
 			}
 
 			return httprate.CanonicalizeIP(ip), nil
-		})
+		},
+		httprate.WithLimitHandler(limitHandler))
 }
 
 // requestIDPattern is what a request's own X-Request-Id must look like to be
@@ -207,17 +216,65 @@ func (s *Middleware) CORS() func(http.Handler) http.Handler {
 	})
 }
 
-// Metrics returns a Prometheus metrics middleware.
+// Metrics returns a Prometheus metrics middleware. Its handler label is the
+// pattern of the chi route a request matches, such as /v1/image/*, or
+// "unmatched" for a request that matches none, so the routes fix how many
+// values the label has. It is never the path, with which any client could
+// add series to the registry without limit.
 func (s *Middleware) Metrics() func(http.Handler) http.Handler {
 	mdlw := ghmm.New(ghmm.Config{
-		Recorder: metrics.NewRecorder(metrics.Config{}),
+		Recorder: metrics.NewRecorder(metrics.Config{
+			Registry: s.metricsRegistry,
+		}),
 	})
 
 	return func(next http.Handler) http.Handler {
-		return std.Handler("", mdlw, next)
+		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			std.Handler(routePattern(r), mdlw, next).ServeHTTP(w, r)
+		})
 	}
 }
 
+// routePattern returns the pattern of the chi route r matches, or
+// "unmatched" when it matches none or is not served by a chi router. The
+// metrics library takes the label before the request is routed, so the
+// router is asked which route matches, with the path it routes on:
+// r.URL.RawPath when set, else r.URL.Path, else /.
+func routePattern(r *http.Request) string {
+	pattern := ""
+
+	rctx := chi.RouteContext(r.Context())
+	if rctx != nil {
+		path := r.URL.RawPath
+		if path == "" {
+			path = r.URL.Path
+		}
+
+		if path == "" {
+			path = "/"
+		}
+
+		pattern = rctx.Routes.Find(chi.NewRouteContext(), r.Method, path)
+
+		// For the path a subrouter is mounted at, such as /v1 or /v1/, Find
+		// answers that path without asking the subrouter, which chi hands
+		// the request as /.
+		for _, route := range rctx.Routes.Routes() {
+			if route.SubRoutes != nil &&
+				route.Pattern == strings.TrimSuffix(pattern, "/")+"/*" &&
+				!route.SubRoutes.Match(chi.NewRouteContext(), r.Method, "/") {
+				pattern = ""
+			}
+		}
+	}
+
+	if pattern == "" {
+		return "unmatched"
+	}
+
+	return pattern
+}
+
 // MetricsAuth returns a basic auth middleware for the metrics endpoint.
 func (s *Middleware) MetricsAuth() func(http.Handler) http.Handler {
 	return basicauth.New(

internal/middleware/middleware_internal_test.go

@@ -3,12 +3,15 @@ package middleware
 import (
 	"bytes"
 	"log/slog"
+	"maps"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
 	"strings"
 	"testing"
 
+	"github.com/go-chi/chi/v5"
+	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 
 	"sneak.berlin/go/pixa/internal/config"
@@ -198,7 +201,7 @@ func TestMetricsRecordsServedRequest(t *testing.T) {
 
 	// The line /metrics shows once one GET /test has been served.
 	const want = `http_request_duration_seconds_count{` +
-		`code="200",handler="/test",method="GET",service=""} 1`
+		`code="200",handler="unmatched",method="GET",service=""} 1`
 
 	mw := &Middleware{log: slog.Default(), config: &config.Config{}}
 
@@ -220,6 +223,97 @@ func TestMetricsRecordsServedRequest(t *testing.T) {
 	}
 }
 
+// TestMetricsLabelsRequestsByRoute checks that the metrics middleware labels
+// each request with the pattern of the route that answers it, never with its
+// path: two paths one route answers share that route's pattern, and requests
+// no route answers, a 404 or a 405, share "unmatched", among them requests for
+// the path the subrouter is mounted at, /v1 and /v1/. A request with an empty
+// path is answered by the / route, as chi routes it as /, and is labelled /.
+// With a label per path, any client could add series to the registry without
+// limit. The middleware registers with a registry of its own here, as another
+// test in this package registers with Prometheus' default registry, which can
+// be done only once.
+func TestMetricsLabelsRequestsByRoute(t *testing.T) {
+	t.Parallel()
+
+	registry := prometheus.NewRegistry()
+	mw := &Middleware{
+		log:             slog.Default(),
+		config:          &config.Config{},
+		metricsRegistry: registry,
+	}
+
+	// The image route is in a subrouter, as in pixa's router.
+	router := chi.NewRouter()
+	router.Use(mw.Metrics())
+	router.Get("/", func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	})
+	router.Route("/v1", func(r chi.Router) {
+		r.Get("/image/*", func(w http.ResponseWriter, _ *http.Request) {
+			w.WriteHeader(http.StatusOK)
+		})
+	})
+
+	for _, req := range []struct{ method, path string }{
+		{http.MethodGet, "/v1/image/a.example.com/one.jpg/1x1.png"},
+		{http.MethodGet, "/v1/image/b.example.com/two.jpg/1x1.png"},
+		{http.MethodGet, "/no-such-route"},
+		{http.MethodGet, "/v1/no-such-route"},
+		{http.MethodGet, "/v1"},
+		{http.MethodGet, "/v1/"},
+		{http.MethodPost, "/v1/image/a.example.com/one.jpg/1x1.png"},
+		// The request line GET http://pixa.example HTTP/1.1 leaves both
+		// r.URL.Path and r.URL.RawPath empty.
+		{http.MethodGet, "http://pixa.example"},
+	} {
+		r := httptest.NewRequestWithContext(
+			t.Context(), req.method, req.path, nil)
+		t.Logf("%s %s: r.URL.Path %q, r.URL.RawPath %q",
+			req.method, req.path, r.URL.Path, r.URL.RawPath)
+		router.ServeHTTP(httptest.NewRecorder(), r)
+	}
+
+	got := requestCountsByHandler(t, registry)
+	t.Logf("requests counted by handler label: %v", got)
+
+	want := map[string]uint64{"/": 1, "/v1/image/*": 2, "unmatched": 5}
+	if !maps.Equal(got, want) {
+		t.Errorf("requests counted by handler label = %v, want %v", got, want)
+	}
+}
+
+// requestCountsByHandler returns how many requests the request duration
+// histogram in registry counted under each value of its handler label.
+func requestCountsByHandler(
+	t *testing.T, registry *prometheus.Registry,
+) map[string]uint64 {
+	t.Helper()
+
+	families, err := registry.Gather()
+	if err != nil {
+		t.Fatalf("registry.Gather() error = %v", err)
+	}
+
+	counts := map[string]uint64{}
+
+	for _, family := range families {
+		if family.GetName() != "http_request_duration_seconds" {
+			continue
+		}
+
+		for _, metric := range family.GetMetric() {
+			for _, label := range metric.GetLabel() {
+				if label.GetName() == "handler" {
+					counts[label.GetValue()] += metric.GetHistogram().GetSampleCount()
+				}
+			}
+		}
+	}
+
+	return counts
+}
+
 // TestLoggingLeavesOutSubmittedSigningKey checks that a login, a POST /
 // whose form carries the signing key, leaves no trace of the key in the
 // request's log line.

internal/server/computed_side_internal_test.go

@@ -0,0 +1,153 @@
+package server
+
+import (
+	"bytes"
+	"encoding/json"
+	"image/jpeg"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"sneak.berlin/go/pixa/internal/encurl"
+	"sneak.berlin/go/pixa/internal/imgcache"
+)
+
+// TestImageProxyRefusesComputedSideAboveMaximum requests sizes of a 3000x600
+// and a 600x3000 PNG through both image routes. A size with one side 0, whose
+// side computed from the source's proportions would be above 8192, is refused
+// with 400 and the JSON error body. The sizes after those refusals are served
+// as before, which also shows that a refusal is not cached as a failed fetch of
+// the source: orig, a size with both sides given, and a size with one side 0
+// whose computed side stays within 8192.
+func TestImageProxyRefusesComputedSideAboveMaximum(t *testing.T) {
+	t.Parallel()
+
+	// The upstream paths of the two sources
+	const (
+		widePath = "/wide.png"
+		tallPath = "/tall.png"
+	)
+
+	wide := encodeTestPNG(t, 3000, 600)
+	tall := encodeTestPNG(t, 600, 3000)
+
+	upstream := httptest.NewServer(http.HandlerFunc(
+		func(w http.ResponseWriter, r *http.Request) {
+			w.Header().Set("Content-Type", "image/png")
+
+			if r.URL.Path == tallPath {
+				_, _ = w.Write(tall)
+
+				return
+			}
+
+			_, _ = w.Write(wide)
+		}))
+	t.Cleanup(upstream.Close)
+
+	s, _, _ := startImageProxy(t, upstream)
+
+	generator, err := encurl.NewGenerator(testSigningKey)
+	if err != nil {
+		t.Fatalf("encurl.NewGenerator: %v", err)
+	}
+
+	tests := []struct {
+		path string
+		// size is the size of the /v1/image/ URL, and width and height are
+		// those of the encrypted URL
+		size          string
+		width, height int
+		// wantWidth and wantHeight are the served image's, 0 for a refusal
+		wantWidth, wantHeight int
+	}{
+		{widePath, "0x8192", 0, 8192, 0, 0},
+		{tallPath, "8192x0", 8192, 0, 0, 0},
+		{widePath, "orig", 0, 0, 3000, 600},
+		{widePath, "300x200", 300, 200, 300, 200},
+		{widePath, "0x300", 0, 300, 1500, 300},
+		{tallPath, "300x0", 300, 0, 300, 1500},
+	}
+
+	for _, tt := range tests {
+		token, err := generator.Generate(&encurl.Payload{
+			SourceHost: upstreamHost, SourcePath: tt.path,
+			Width: tt.width, Height: tt.height, Format: imgcache.FormatJPEG,
+		})
+		if err != nil {
+			t.Fatalf("encurl Generate: %v", err)
+		}
+
+		for _, target := range []string{
+			"/v1/image/" + upstreamHost + tt.path + "/" + tt.size + ".jpeg",
+			"/v1/e/" + token + "/img.jpg",
+		} {
+			rec := httptest.NewRecorder()
+			s.ServeHTTP(rec, httptest.NewRequestWithContext(
+				t.Context(), http.MethodGet, target, nil))
+			t.Logf("GET %s: %d, Content-Type %s",
+				target, rec.Code, rec.Header().Get("Content-Type"))
+
+			if tt.wantWidth == 0 {
+				checkSizeRefused(t, rec)
+			} else {
+				checkServedSize(t, rec, tt.wantWidth, tt.wantHeight)
+			}
+		}
+	}
+}
+
+// checkSizeRefused checks that rec answers 400 with the JSON error body that
+// refuses a size whose side computed from the source's proportions is above
+// 8192.
+func checkSizeRefused(t *testing.T, rec *httptest.ResponseRecorder) {
+	t.Helper()
+
+	const wantError = "size too large for the source: " +
+		"the side computed from its proportions is above 8192"
+
+	var body struct {
+		Error  string `json:"error"`
+		Status int    `json:"status"`
+	}
+
+	err := json.Unmarshal(rec.Body.Bytes(), &body)
+	if err != nil {
+		t.Errorf("status %d, body is not JSON: %v", rec.Code, err)
+
+		return
+	}
+
+	if rec.Code != http.StatusBadRequest || body.Status != http.StatusBadRequest ||
+		body.Error != wantError {
+		t.Errorf("status %d, body %+v, want %d and the error %q",
+			rec.Code, body, http.StatusBadRequest, wantError)
+	}
+}
+
+// checkServedSize checks that rec answers 200 with a JPEG of wantWidth by
+// wantHeight.
+func checkServedSize(
+	t *testing.T, rec *httptest.ResponseRecorder, wantWidth, wantHeight int,
+) {
+	t.Helper()
+
+	if rec.Code != http.StatusOK {
+		t.Errorf("status %d, want %d; body %s",
+			rec.Code, http.StatusOK, rec.Body.String())
+
+		return
+	}
+
+	served, err := jpeg.DecodeConfig(bytes.NewReader(rec.Body.Bytes()))
+	if err != nil {
+		t.Errorf("decoding the served JPEG: %v", err)
+
+		return
+	}
+
+	if served.Width != wantWidth || served.Height != wantHeight {
+		t.Errorf("served %dx%d, want %dx%d",
+			served.Width, served.Height, wantWidth, wantHeight)
+	}
+}

internal/server/fit_outside_internal_test.go

@@ -0,0 +1,91 @@
+package server
+
+import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"sneak.berlin/go/pixa/internal/encurl"
+	"sneak.berlin/go/pixa/internal/imgcache"
+)
+
+// TestImageProxyRefusesFitOutsideAboveMaximum requests sizes with fit=outside
+// through both image routes. fit=outside scales the source to cover the size,
+// and so past it on one side: a size for which that side would be above 8192
+// is refused with 400 and the JSON error body, whether it is the width or the
+// height, and a size for which it stays within 8192 is served at the scaled
+// size.
+func TestImageProxyRefusesFitOutsideAboveMaximum(t *testing.T) {
+	t.Parallel()
+
+	// The upstream paths of the sources
+	const (
+		widePath   = "/wide.png"
+		rowPath    = "/row.png"
+		columnPath = "/column.png"
+	)
+
+	sources := map[string][]byte{
+		widePath:   encodeTestPNG(t, 3000, 600),
+		rowPath:    encodeTestPNG(t, 8192, 1),
+		columnPath: encodeTestPNG(t, 1, 8192),
+	}
+
+	upstream := httptest.NewServer(http.HandlerFunc(
+		func(w http.ResponseWriter, r *http.Request) {
+			w.Header().Set("Content-Type", "image/png")
+			_, _ = w.Write(sources[r.URL.Path])
+		}))
+	t.Cleanup(upstream.Close)
+
+	s, _, _ := startImageProxy(t, upstream)
+
+	generator, err := encurl.NewGenerator(testSigningKey)
+	if err != nil {
+		t.Fatalf("encurl.NewGenerator: %v", err)
+	}
+
+	tests := []struct {
+		path          string
+		width, height int
+		// wantWidth and wantHeight are the served image's, 0 for a refusal
+		wantWidth, wantHeight int
+	}{
+		{widePath, 1, 8192, 0, 0},
+		{rowPath, 1, 8192, 0, 0},
+		{columnPath, 8192, 1, 0, 0},
+		{widePath, 1, 1200, 6000, 1200},
+	}
+
+	for _, tt := range tests {
+		token, err := generator.Generate(&encurl.Payload{
+			SourceHost: upstreamHost, SourcePath: tt.path,
+			Width: tt.width, Height: tt.height, Format: imgcache.FormatJPEG,
+			FitMode: imgcache.FitOutside,
+		})
+		if err != nil {
+			t.Fatalf("encurl Generate: %v", err)
+		}
+
+		size := fmt.Sprintf("%dx%d", tt.width, tt.height)
+
+		for _, target := range []string{
+			"/v1/image/" + upstreamHost + tt.path + "/" + size +
+				".jpeg?fit=outside",
+			"/v1/e/" + token + "/img.jpg",
+		} {
+			rec := httptest.NewRecorder()
+			s.ServeHTTP(rec, httptest.NewRequestWithContext(
+				t.Context(), http.MethodGet, target, nil))
+			t.Logf("GET %s: %d, Content-Type %s",
+				target, rec.Code, rec.Header().Get("Content-Type"))
+
+			if tt.wantWidth == 0 {
+				checkSizeRefused(t, rec)
+			} else {
+				checkServedSize(t, rec, tt.wantWidth, tt.wantHeight)
+			}
+		}
+	}
+}

internal/server/image_rate_limit_internal_test.go

@@ -0,0 +1,204 @@
+package server
+
+import (
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"strconv"
+	"testing"
+
+	"sneak.berlin/go/pixa/internal/config"
+)
+
+// requestImageFrom sends method path to s from remoteAddr and returns the
+// response. The image handlers refuse an unsigned image URL with 401 and a
+// token they cannot decrypt with 400, so either status shows the limit let
+// the request through.
+func requestImageFrom(
+	t *testing.T, s *Server, method, path, remoteAddr string,
+) *httptest.ResponseRecorder {
+	t.Helper()
+
+	req := httptest.NewRequestWithContext(t.Context(), method, path, nil)
+	req.RemoteAddr = remoteAddr
+
+	rec := httptest.NewRecorder()
+	s.ServeHTTP(rec, req)
+
+	return rec
+}
+
+// TestImageRequestLimitRefusesRequestOverLimit verifies that GET and HEAD on
+// both image routes count toward one limit per client, and that the request
+// after image_requests_per_minute of them is refused with 429 and a
+// Retry-After header.
+func TestImageRequestLimitRefusesRequestOverLimit(t *testing.T) {
+	t.Parallel()
+
+	requests := []struct {
+		method string
+		path   string
+		want   int
+	}{
+		{http.MethodGet, unsignedImagePath, http.StatusUnauthorized},
+		{http.MethodHead, unsignedImagePath, http.StatusUnauthorized},
+		{http.MethodGet, encryptedImagePath, http.StatusBadRequest},
+	}
+
+	s := newTestServer(t)
+	s.config.ImageRequestsPerMinute = len(requests)
+	s.SetupRoutes()
+
+	for _, tc := range requests {
+		rec := requestImageFrom(t, s, tc.method, tc.path, firstClient)
+		if rec.Code != tc.want {
+			t.Fatalf("%s %s within the limit status = %d, want %d",
+				tc.method, tc.path, rec.Code, tc.want)
+		}
+	}
+
+	rec := requestImageFrom(t, s, http.MethodHead, encryptedImagePath, firstClient)
+	if rec.Code != http.StatusTooManyRequests {
+		t.Fatalf("request over the limit status = %d, want %d",
+			rec.Code, http.StatusTooManyRequests)
+	}
+
+	retryAfter := rec.Header().Get("Retry-After")
+
+	seconds, err := strconv.Atoi(retryAfter)
+	if err != nil || seconds <= 0 {
+		t.Errorf("Retry-After = %q, want a positive number of seconds",
+			retryAfter)
+	}
+}
+
+// TestImageRequestLimitAnswersWithJSONError verifies that the 429 for a
+// request over the limit carries Retry-After and the JSON error body the
+// image routes send for their other errors.
+func TestImageRequestLimitAnswersWithJSONError(t *testing.T) {
+	t.Parallel()
+
+	s := newTestServer(t)
+	s.config.ImageRequestsPerMinute = 1
+	s.SetupRoutes()
+
+	requestImageFrom(t, s, http.MethodGet, unsignedImagePath, firstClient)
+
+	rec := requestImageFrom(t, s, http.MethodGet, unsignedImagePath, firstClient)
+	t.Logf("status %d, body %s", rec.Code, rec.Body.String())
+
+	if rec.Code != http.StatusTooManyRequests {
+		t.Fatalf("request over the limit status = %d, want %d",
+			rec.Code, http.StatusTooManyRequests)
+	}
+
+	retryAfter := rec.Header().Get("Retry-After")
+
+	seconds, err := strconv.Atoi(retryAfter)
+	if err != nil || seconds <= 0 {
+		t.Errorf("Retry-After = %q, want a positive number of seconds",
+			retryAfter)
+	}
+
+	contentType := rec.Header().Get("Content-Type")
+	if contentType != "application/json" {
+		t.Errorf("Content-Type = %q, want %q", contentType, "application/json")
+	}
+
+	var body struct {
+		Error     string `json:"error"`
+		Status    int    `json:"status"`
+		Timestamp string `json:"timestamp"`
+	}
+
+	err = json.NewDecoder(rec.Body).Decode(&body)
+	if err != nil {
+		t.Fatalf("body is not JSON: %v", err)
+	}
+
+	if body.Error == "" || body.Status != http.StatusTooManyRequests ||
+		body.Timestamp == "" {
+		t.Errorf("body = %+v, want an error, status %d and a timestamp",
+			body, http.StatusTooManyRequests)
+	}
+}
+
+// TestImageRequestLimitCountsClientsSeparately verifies that one client going
+// over the limit does not limit another.
+func TestImageRequestLimitCountsClientsSeparately(t *testing.T) {
+	t.Parallel()
+
+	s := newTestServer(t)
+	s.config.ImageRequestsPerMinute = 1
+	s.SetupRoutes()
+
+	rec := requestImageFrom(t, s, http.MethodGet, unsignedImagePath, firstClient)
+	if rec.Code != http.StatusUnauthorized {
+		t.Fatalf("first client's first request status = %d, want %d",
+			rec.Code, http.StatusUnauthorized)
+	}
+
+	rec = requestImageFrom(t, s, http.MethodGet, unsignedImagePath, firstClient)
+	if rec.Code != http.StatusTooManyRequests {
+		t.Fatalf("first client's second request status = %d, want %d",
+			rec.Code, http.StatusTooManyRequests)
+	}
+
+	rec = requestImageFrom(t, s, http.MethodGet, unsignedImagePath, secondClient)
+	if rec.Code != http.StatusUnauthorized {
+		t.Errorf("second client's first request status = %d, want %d",
+			rec.Code, http.StatusUnauthorized)
+	}
+}
+
+// TestImageRequestLimitZeroLetsEveryRequestThrough verifies that with
+// image_requests_per_minute set to 0 no image request is refused with 429,
+// not even past the default limit.
+func TestImageRequestLimitZeroLetsEveryRequestThrough(t *testing.T) {
+	t.Parallel()
+
+	s := newTestServer(t)
+	s.config.ImageRequestsPerMinute = 0
+	s.SetupRoutes()
+
+	for request := range config.DefaultImageRequestsPerMinute + 1 {
+		rec := requestImageFrom(t, s, http.MethodGet, unsignedImagePath, firstClient)
+		if rec.Code != http.StatusUnauthorized {
+			t.Fatalf("request %d status = %d, want %d",
+				request+1, rec.Code, http.StatusUnauthorized)
+		}
+	}
+}
+
+// TestImageRequestLimitDoesNotCountPreflight verifies that a browser's CORS
+// preflight request to an image route is answered without being counted, so
+// a client that has sent several still gets its image request through.
+func TestImageRequestLimitDoesNotCountPreflight(t *testing.T) {
+	t.Parallel()
+
+	s := newTestServer(t)
+	s.config.AccessControlAllowOrigin = config.DefaultAccessControlAllowOrigin
+	s.config.ImageRequestsPerMinute = 1
+	s.SetupRoutes()
+
+	for range 3 {
+		req := httptest.NewRequestWithContext(
+			t.Context(), http.MethodOptions, unsignedImagePath, nil)
+		req.RemoteAddr = firstClient
+		req.Header.Set("Origin", "https://app.example.com")
+		req.Header.Set("Access-Control-Request-Method", http.MethodGet)
+
+		rec := httptest.NewRecorder()
+		s.ServeHTTP(rec, req)
+
+		if rec.Code != http.StatusOK {
+			t.Fatalf("preflight status = %d, want %d", rec.Code, http.StatusOK)
+		}
+	}
+
+	rec := requestImageFrom(t, s, http.MethodGet, unsignedImagePath, firstClient)
+	if rec.Code != http.StatusUnauthorized {
+		t.Errorf("image request after preflights status = %d, want %d",
+			rec.Code, http.StatusUnauthorized)
+	}
+}

internal/server/logout_internal_test.go

@@ -0,0 +1,269 @@
+package server
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"path/filepath"
+	"regexp"
+	"strings"
+	"testing"
+
+	"go.uber.org/fx"
+	"go.uber.org/fx/fxtest"
+
+	"sneak.berlin/go/pixa/internal/config"
+	"sneak.berlin/go/pixa/internal/database"
+	"sneak.berlin/go/pixa/internal/globals"
+	"sneak.berlin/go/pixa/internal/handlers"
+	"sneak.berlin/go/pixa/internal/healthcheck"
+	"sneak.berlin/go/pixa/internal/logger"
+	"sneak.berlin/go/pixa/internal/middleware"
+	"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 {
+	t.Helper()
+
+	var (
+		h  *handlers.Handlers
+		mw *middleware.Middleware
+	)
+
+	app := fxtest.New(t,
+		fx.Supply(cfg),
+		fx.Provide(
+			globals.New,
+			logger.New,
+			database.New,
+			healthcheck.New,
+			handlers.New,
+			middleware.New,
+		),
+		fx.Populate(&h, &mw),
+	)
+	app.RequireStart()
+	t.Cleanup(app.RequireStop)
+
+	s := &Server{config: cfg, mw: mw, h: h}
+	s.SetupRoutes()
+
+	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 {
+	t.Helper()
+
+	req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
+	req.AddCookie(cookie)
+
+	rec := httptest.NewRecorder()
+	s.ServeHTTP(rec, req)
+
+	return rec.Code == http.StatusOK &&
+		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 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)
+
+	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)
+	}
+
+	restarted := startPixa(t, s.config)
+
+	opens := opensGeneratorPage(t, restarted, copied)
+	t.Logf("after the restart, the copied cookie opens the generator page: %v",
+		opens)
+
+	if opens {
+		t.Error("after a restart, a cookie copied before the logout " +
+			"still opens the generator page")
+	}
+}
+
+// TestLostStateDirectoryEndsEveryLoginSession verifies that a pixa started on
+// an empty state directory, as pixad is after a restart that lost its state
+// directory, refuses a session cookie from before, and that a login on it
+// opens the generator page.
+func TestLostStateDirectoryEndsEveryLoginSession(t *testing.T) {
+	t.Parallel()
+
+	s := newTestServer(t)
+
+	before := logIn(t, s)
+	if !opensGeneratorPage(t, s, before) {
+		t.Fatal("logging in did not open the generator page")
+	}
+
+	emptyStateDir := *s.config
+	emptyStateDir.StateDir = t.TempDir()
+	emptyStateDir.DBURL = "file:" +
+		filepath.Join(emptyStateDir.StateDir, "state.sqlite3")
+
+	restarted := startPixa(t, &emptyStateDir)
+
+	opens := opensGeneratorPage(t, restarted, before)
+	t.Logf("on an empty state directory, the cookie from before opens the "+
+		"generator page: %v", opens)
+
+	if opens {
+		t.Error("on an empty state directory, a cookie from before still " +
+			"opens the generator page")
+	}
+
+	if !opensGeneratorPage(t, restarted, logIn(t, restarted)) {
+		t.Error("a login on an empty state directory does not open the " +
+			"generator page")
+	}
+}

internal/server/routes.go

@@ -66,13 +66,12 @@ func (s *Server) SetupRoutes() {
 		r.Use(s.h.LimitBody(handlers.MaxFormBytes))
 		r.Use(s.h.CSRF())
 		r.Get("/", s.h.HandleRoot())
-		r.With(s.mw.RateLimit(LoginAttemptsPerMinute, time.Minute)).
+		r.With(s.mw.RateLimit(LoginAttemptsPerMinute, time.Minute, nil)).
 			Post("/", s.h.HandleRoot())
 		r.Post("/generate", s.h.HandleGenerateURL())
+		r.Post("/logout", s.h.HandleLogout())
 	})
 
-	s.router.Get("/logout", s.h.HandleLogout())
-
 	// Image routes, the only ones that send CORS headers, as pages on other
 	// sites read them. They are a subrouter rather than a group: a group's
 	// middleware runs only for a request that matches one of its routes,
@@ -88,6 +87,15 @@ func (s *Server) SetupRoutes() {
 		r.Group(func(r chi.Router) {
 			r.Use(s.refuseDuringMaintenance)
 
+			// Limited per client, as the login POST is, but the 429 has
+			// the image routes' JSON error body; 0 turns the limit off.
+			// The CORS middleware answers a preflight before it gets
+			// here, so a preflight is not counted.
+			if s.config.ImageRequestsPerMinute > 0 {
+				r.Use(s.mw.RateLimit(s.config.ImageRequestsPerMinute,
+					time.Minute, s.h.HandleTooManyRequests()))
+			}
+
 			// Main image proxy route
 			// /v1/image/<host>/<path>/<width>x<height>.<format>, or with no
 			// format /v1/image/<host>/<path>/<width>x<height>

internal/server/svg_as_avif_internal_test.go

@@ -0,0 +1,71 @@
+package server
+
+import (
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"sneak.berlin/go/pixa/internal/encurl"
+	"sneak.berlin/go/pixa/internal/imgcache"
+)
+
+// svgServedAsAVIF is an SVG whose first bytes pass the first-bytes check for
+// AVIF: its XML comment puts "ftypavis" at bytes 4 to 11, where an AVIF file
+// has its ftyp box and brand. libvips would load it as SVG.
+const svgServedAsAVIF = `<!--ftypavis--><svg xmlns="http://www.w3.org/2000/svg" ` +
+	`width="64" height="48"><rect width="64" height="48"/></svg>`
+
+// TestImageProxyRefusesSVGServedAsAVIF requests svgServedAsAVIF, which the
+// upstream answers as image/avif, through both image routes, resized as PNG
+// and in its own format (orig). Each request is answered 415, as for an SVG
+// source, never with an image.
+func TestImageProxyRefusesSVGServedAsAVIF(t *testing.T) {
+	t.Parallel()
+
+	upstream := httptest.NewServer(http.HandlerFunc(
+		func(w http.ResponseWriter, _ *http.Request) {
+			w.Header().Set("Content-Type", "image/avif")
+			_, _ = io.WriteString(w, svgServedAsAVIF)
+		}))
+	t.Cleanup(upstream.Close)
+
+	s, _, _ := startImageProxy(t, upstream)
+
+	generator, err := encurl.NewGenerator(testSigningKey)
+	if err != nil {
+		t.Fatalf("encurl.NewGenerator: %v", err)
+	}
+
+	encryptedURL := func(format imgcache.ImageFormat) string {
+		token, err := generator.Generate(&encurl.Payload{
+			SourceHost: upstreamHost, SourcePath: "/drawing.avif",
+			Width: 32, Height: 24, Format: format,
+		})
+		if err != nil {
+			t.Fatalf("encurl Generate: %v", err)
+		}
+
+		return "/v1/e/" + token + "/drawing." + string(format)
+	}
+
+	for _, target := range []string{
+		"/v1/image/" + upstreamHost + "/drawing.avif/32x24.png",
+		"/v1/image/" + upstreamHost + "/drawing.avif/64x48.orig",
+		encryptedURL(imgcache.FormatPNG),
+		encryptedURL(imgcache.FormatOriginal),
+	} {
+		rec := httptest.NewRecorder()
+		s.ServeHTTP(rec, httptest.NewRequestWithContext(
+			t.Context(), http.MethodGet, target, nil))
+		t.Logf("GET %s: %d, Content-Type %s, body %q",
+			target, rec.Code, rec.Header().Get("Content-Type"), rec.Body.String())
+
+		if rec.Code != http.StatusUnsupportedMediaType ||
+			!strings.Contains(rec.Body.String(), "unsupported source format") {
+			t.Errorf("GET %s: %d %q, want %d unsupported source format",
+				target, rec.Code, rec.Body.String(), http.StatusUnsupportedMediaType)
+		}
+	}
+}

internal/server/svg_internal_test.go

@@ -0,0 +1,41 @@
+package server
+
+import (
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
+
+// TestImageProxyRefusesSVG requests an image the upstream answers as SVG,
+// in its own format (orig) and resized as PNG, through pixa's router,
+// upstream fetcher and image processor. pixa does not accept SVG as a
+// source, so each request is answered 415, never with an image.
+func TestImageProxyRefusesSVG(t *testing.T) {
+	t.Parallel()
+
+	upstream := httptest.NewServer(http.HandlerFunc(
+		func(w http.ResponseWriter, _ *http.Request) {
+			w.Header().Set("Content-Type", "image/svg+xml")
+			_, _ = io.WriteString(w, `<svg xmlns="http://www.w3.org/2000/svg" `+
+				`width="64" height="48"><rect width="64" height="48"/></svg>`)
+		}))
+	t.Cleanup(upstream.Close)
+
+	s, _, _ := startImageProxy(t, upstream)
+
+	for _, sizeFormat := range []string{"64x48.orig", "32x24.png"} {
+		target := "/v1/image/" + upstreamHost + "/drawing.svg/" + sizeFormat
+
+		rec := httptest.NewRecorder()
+		s.ServeHTTP(rec, httptest.NewRequestWithContext(
+			t.Context(), http.MethodGet, target, nil))
+		t.Logf("GET %s: %d, Content-Type %s, body %s",
+			target, rec.Code, rec.Header().Get("Content-Type"), rec.Body.String())
+
+		if rec.Code != http.StatusUnsupportedMediaType {
+			t.Errorf("GET %s status = %d, want %d",
+				target, rec.Code, http.StatusUnsupportedMediaType)
+		}
+	}
+}

internal/session/session.go

@@ -1,9 +1,16 @@
-// Package session provides encrypted session cookie management.
+// Package session provides encrypted session cookie management, and keeps in
+// the database a random value that each logout replaces, so that a logout
+// ends every session made before it.
 package session
 
 import (
+	"context"
+	"crypto/rand"
+	"database/sql"
 	"errors"
+	"fmt"
 	"net/http"
+	"sync"
 	"time"
 
 	"github.com/gorilla/securecookie"
@@ -32,14 +39,36 @@ type Data struct {
 	Authenticated bool      `json:"auth"`
 	CreatedAt     time.Time `json:"created"`
 	ExpiresAt     time.Time `json:"expires"`
+
+	// LogoutValue is the Manager's random value, which each logout
+	// replaces, when the session was made.
+	LogoutValue string `json:"logoutValue"`
 }
 
 // Manager handles session creation and validation using encrypted cookies.
+//
+// Every login is made with the one signing key, so a logout ends every
+// session made before it, in every browser. The Manager keeps a random
+// value, which each logout replaces: a session carries the value current at
+// its login, and a session carrying another value is refused. The check for
+// a logout compares no time, so a login after a logout works whatever the
+// host's clock does; the session's expiry is still checked against the
+// clock. The Manager keeps the value in memory, so checking a session reads
+// no database; a Manager from NewManagerWithDatabase also saves it in the
+// database, so that it outlasts a restart.
 type Manager struct {
 	sc *securecookie.SecureCookie
+	db *sql.DB
+
+	// mu guards logoutValue, and is held while a logout is saved, so that
+	// the database and logoutValue agree.
+	mu          sync.Mutex
+	logoutValue string
 }
 
 // NewManager creates a session manager with keys derived from the signing key.
+// It makes a new random value and keeps it in memory only, so a restart ends
+// every session; NewManagerWithDatabase keeps the value in the database.
 //
 // Session cookies always carry the Secure, HttpOnly, and SameSite=Strict
 // attributes; this cannot be configured. Browsers treat http://localhost as a
@@ -63,10 +92,44 @@ func NewManager(signingKey string) (*Manager, error) {
 	sc.MaxAge(int(SessionTTL.Seconds()))
 
 	return &Manager{
-		sc: sc,
+		sc:          sc,
+		logoutValue: rand.Text(),
 	}, nil
 }
 
+// NewManagerWithDatabase creates a session manager like NewManager that keeps
+// its random value in db, which must have pixa's schema: it saves the value
+// NewManager made when db has none, and otherwise starts from the one saved
+// there.
+func NewManagerWithDatabase(
+	ctx context.Context, signingKey string, db *sql.DB,
+) (*Manager, error) {
+	m, err := NewManager(signingKey)
+	if err != nil {
+		return nil, err
+	}
+
+	_, err = db.ExecContext(ctx,
+		"INSERT OR IGNORE INTO logout (id, logout_value) VALUES (1, ?)",
+		m.logoutValue)
+	if err != nil {
+		return nil, fmt.Errorf(
+			"failed to save the random value in the logout table: %w", err)
+	}
+
+	err = db.QueryRowContext(ctx,
+		"SELECT logout_value FROM logout WHERE id = 1",
+	).Scan(&m.logoutValue)
+	if err != nil {
+		return nil, fmt.Errorf(
+			"failed to read the random value in the logout table: %w", err)
+	}
+
+	m.db = db
+
+	return m, nil
+}
+
 // CreateSession creates a new authenticated session and sets the cookie.
 func (m *Manager) CreateSession(w http.ResponseWriter) error {
 	now := time.Now()
@@ -74,6 +137,7 @@ func (m *Manager) CreateSession(w http.ResponseWriter) error {
 		Authenticated: true,
 		CreatedAt:     now,
 		ExpiresAt:     now.Add(SessionTTL),
+		LogoutValue:   m.currentLogoutValue(),
 	}
 
 	encoded, err := m.sc.Encode(CookieName, data)
@@ -122,9 +186,30 @@ func (m *Manager) ValidateSession(r *http.Request) (*Data, error) {
 		return nil, ErrInvalidSession
 	}
 
+	// A logout since the session was made has ended it.
+	if data.LogoutValue != m.currentLogoutValue() {
+		return nil, ErrInvalidSession
+	}
+
 	return &data, nil
 }
 
+// Logout removes the session cookie and, when r has a valid session, ends
+// every session made until now. A request without one ends no session, so
+// someone who has not logged in cannot end the sessions of those who have.
+func (m *Manager) Logout(w http.ResponseWriter, r *http.Request) error {
+	if m.IsAuthenticated(r) {
+		err := m.saveLogout(r.Context())
+		if err != nil {
+			return err
+		}
+	}
+
+	m.ClearSession(w)
+
+	return nil
+}
+
 // ClearSession removes the session cookie.
 func (m *Manager) ClearSession(w http.ResponseWriter) {
 	http.SetCookie(w, &http.Cookie{
@@ -145,3 +230,32 @@ func (m *Manager) IsAuthenticated(r *http.Request) bool {
 
 	return err == nil && data != nil && data.Authenticated
 }
+
+// saveLogout replaces the random value with a new one, saving it in the
+// database first when the Manager has one.
+func (m *Manager) saveLogout(ctx context.Context) error {
+	value := rand.Text()
+
+	m.mu.Lock()
+	defer m.mu.Unlock()
+
+	if m.db != nil {
+		_, err := m.db.ExecContext(ctx,
+			"UPDATE logout SET logout_value = ? WHERE id = 1", value)
+		if err != nil {
+			return fmt.Errorf("failed to save the logout: %w", err)
+		}
+	}
+
+	m.logoutValue = value
+
+	return nil
+}
+
+// currentLogoutValue returns the Manager's random value.
+func (m *Manager) currentLogoutValue() string {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+
+	return m.logoutValue
+}

internal/session/session_logout_test.go

@@ -0,0 +1,148 @@
+package session_test
+
+import (
+	"database/sql"
+	"errors"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"sneak.berlin/go/pixa/internal/database"
+	"sneak.berlin/go/pixa/internal/session"
+)
+
+const logoutTestSigningKey = "test-signing-key-12345"
+
+// newTestDatabase opens an empty in-memory database with pixa's schema. It
+// has one connection, as pixad's has: each connection to ":memory:" would
+// open a database of its own.
+func newTestDatabase(t *testing.T) *sql.DB {
+	t.Helper()
+
+	db, err := sql.Open("sqlite", ":memory:")
+	if err != nil {
+		t.Fatalf("sql.Open() error = %v", err)
+	}
+
+	db.SetMaxOpenConns(1)
+	t.Cleanup(func() { _ = db.Close() })
+
+	err = database.ApplyMigrations(t.Context(), db, nil)
+	if err != nil {
+		t.Fatalf("ApplyMigrations() error = %v", err)
+	}
+
+	return db
+}
+
+// createSessionCookie logs in with mgr and returns the session cookie it
+// sets.
+func createSessionCookie(t *testing.T, mgr *session.Manager) *http.Cookie {
+	t.Helper()
+
+	rec := httptest.NewRecorder()
+
+	err := mgr.CreateSession(rec)
+	if err != nil {
+		t.Fatalf("CreateSession() error = %v", err)
+	}
+
+	for _, c := range rec.Result().Cookies() {
+		if c.Name == session.CookieName {
+			return c
+		}
+	}
+
+	t.Fatalf("CreateSession() did not set a %q cookie", session.CookieName)
+
+	return nil
+}
+
+// requestWithCookie returns a GET request carrying cookie, or no cookie when
+// cookie is nil.
+func requestWithCookie(t *testing.T, cookie *http.Cookie) *http.Request {
+	t.Helper()
+
+	req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
+	if cookie != nil {
+		req.AddCookie(cookie)
+	}
+
+	return req
+}
+
+// TestLogout_EndsEverySessionMadeBeforeIt verifies that after a logout a
+// copy of the session cookie taken before it is refused and a login after it
+// works, and that both still hold for a new Manager on the same database, as
+// after a restart.
+func TestLogout_EndsEverySessionMadeBeforeIt(t *testing.T) {
+	t.Parallel()
+
+	db := newTestDatabase(t)
+
+	mgr, err := session.NewManagerWithDatabase(
+		t.Context(), logoutTestSigningKey, db)
+	if err != nil {
+		t.Fatalf("NewManagerWithDatabase() error = %v", err)
+	}
+
+	copied := createSessionCookie(t, mgr)
+
+	err = mgr.Logout(httptest.NewRecorder(), requestWithCookie(t, copied))
+	if err != nil {
+		t.Fatalf("Logout() error = %v", err)
+	}
+
+	_, err = mgr.ValidateSession(requestWithCookie(t, copied))
+	t.Logf("cookie copied before the logout: ValidateSession() error = %v", err)
+
+	if !errors.Is(err, session.ErrInvalidSession) {
+		t.Errorf("cookie copied before the logout: error = %v, want %v",
+			err, session.ErrInvalidSession)
+	}
+
+	loginAfter := createSessionCookie(t, mgr)
+
+	_, err = mgr.ValidateSession(requestWithCookie(t, loginAfter))
+	if err != nil {
+		t.Errorf("login after the logout: ValidateSession() error = %v", err)
+	}
+
+	restarted, err := session.NewManagerWithDatabase(
+		t.Context(), logoutTestSigningKey, db)
+	if err != nil {
+		t.Fatalf("NewManagerWithDatabase() after restart error = %v", err)
+	}
+
+	if restarted.IsAuthenticated(requestWithCookie(t, copied)) {
+		t.Error("after a restart, a cookie copied before the logout is accepted")
+	}
+
+	if !restarted.IsAuthenticated(requestWithCookie(t, loginAfter)) {
+		t.Error("after a restart, a login made after the logout is refused")
+	}
+}
+
+// TestLogout_WithoutSession_EndsNoSession verifies that a logout from a
+// 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()
+
+	mgr, err := session.NewManagerWithDatabase(
+		t.Context(), logoutTestSigningKey, newTestDatabase(t))
+	if err != nil {
+		t.Fatalf("NewManagerWithDatabase() error = %v", err)
+	}
+
+	cookie := createSessionCookie(t, mgr)
+
+	err = mgr.Logout(httptest.NewRecorder(), requestWithCookie(t, nil))
+	if err != nil {
+		t.Fatalf("Logout() error = %v", err)
+	}
+
+	if !mgr.IsAuthenticated(requestWithCookie(t, cookie)) {
+		t.Error("a logout without a session cookie ended a session")
+	}
+}

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 == "" {

internal/static/style.css

@@ -122,12 +122,20 @@ header {
     margin-bottom: 2rem;
 }
 
-header a {
+/* The logout button, drawn as a link. */
+
+header button {
+    width: auto;
+    margin: 0;
+    padding: 0;
+    background: none;
     font-size: 0.875rem;
     color: #4b5563;
+    text-decoration: underline;
 }
 
-header a:hover {
+header button:hover {
+    background: none;
     color: #1f2937;
 }
 

internal/templates/generator.html

@@ -10,9 +10,12 @@
     <div class="page">
         <header>
             <h1>Pixa URL Generator</h1>
-            <a href="/logout">
-                Logout
-            </a>
+            <form method="POST" action="/logout">
+                {{ .CSRFField }}
+                <button type="submit">
+                    Logout
+                </button>
+            </form>
         </header>
 
         {{if .GeneratedURL}}

script/loadtest

@@ -94,7 +94,8 @@ main() {
 }
 
 # start_containers starts a new origin and a new pixad, and waits up to 30
-# seconds for pixad's health check to pass.
+# seconds for pixad's health check to pass. pixad sees all of vegeta's
+# requests come from one address, so its image request limit is off.
 start_containers() {
     docker run -d --name "$run-origin" \
         --network "$run" --network-alias origin \
@@ -104,6 +105,7 @@ start_containers() {
         --network "$run" -p 127.0.0.1::8080 --health-interval=1s \
         -e PIXA_SIGNING_KEY="$(head -c 32 /dev/urandom | base64)" \
         -e PIXA_ALLOWLIST_HOSTS=origin -e PIXA_ALLOW_HTTP=true \
+        -e PIXA_IMAGE_REQUESTS_PER_MINUTE=0 \
         "$image" >/dev/null
 
     waited=0