Give the login rate limit tests the default request timeout (closes #61)

newTestServer left downstream_timeout at zero, so every request in those
tests ran with an already expired per-request timeout. It now uses
config.DefaultDownstreamTimeout, as a real config would. Approved by the
owner on the issue.

Model: opus-5-5

Dockerfile

@@ -3,14 +3,13 @@
 # golangci/golangci-lint:v2.12.2-alpine, 2026-08-07
 FROM golangci/golangci-lint:v2.12.2-alpine@sha256:91b27804074a0bacea298707f016911e60cf0cdbc6c7bf5ccacb5f0606d18d60 AS lint
 
+RUN apk add --no-cache make build-base vips-dev libheif-dev pkgconfig
+
 WORKDIR /src
 
-# script/bootstrap installs the build dependencies and downloads the Go
+# Copy go mod files first for better layer caching
-# modules. Only script/, go.mod and go.sum are copied first, so this
-# layer is reused until one of them changes.
-COPY script/ ./script/
 COPY go.mod go.sum ./
-RUN script/bootstrap
+RUN go mod download
 
 # Copy source code
 COPY . .
@@ -29,12 +28,20 @@ FROM golang:1.25.4-alpine@sha256:d3f0cf7723f3429e3f9ed846243970b20a2de7bae6a5b66
 # Depend on lint stage passing
 COPY --from=lint /src/go.sum /dev/null
 
+ARG VERSION=dev
+
+# Install build dependencies for CGO image libraries
+RUN apk add --no-cache \
+    build-base \
+    vips-dev \
+    libheif-dev \
+    pkgconfig
+
 WORKDIR /src
 
-# Build dependencies and Go modules, as in the lint stage
+# Copy go mod files first for better layer caching
-COPY script/ ./script/
 COPY go.mod go.sum ./
-RUN script/bootstrap
+RUN GOTOOLCHAIN=auto go mod download
 
 # Copy source code
 COPY . .
@@ -42,14 +49,8 @@ COPY . .
 # Run tests
 RUN make test
 
-# VERSION is declared here, not earlier: a new value reruns only the
+# Build with CGO enabled
-# build, not script/bootstrap or the tests. CGO stays enabled for
+RUN CGO_ENABLED=1 GOTOOLCHAIN=auto go build -ldflags "-X main.Version=${VERSION}" -o /pixad ./cmd/pixad
-# govips; -trimpath keeps build paths out of the binary, and -s -w
-# leave out the symbol table and debug information.
-ARG VERSION=dev
-RUN CGO_ENABLED=1 GOTOOLCHAIN=auto go build -trimpath \
-    -ldflags "-s -w -X main.Version=${VERSION}" \
-    -o /pixad ./cmd/pixad
 
 # Runtime stage
 # alpine:3.21, 2026-02-25

Dockerfile.lint

@@ -6,16 +6,16 @@
 # golangci/golangci-lint:v2.12.2-alpine, 2026-08-07
 FROM golangci/golangci-lint:v2.12.2-alpine@sha256:91b27804074a0bacea298707f016911e60cf0cdbc6c7bf5ccacb5f0606d18d60
 
+# pixa is CGO/libvips: the type-aware linters compile every package, so
+# this image needs the same C libraries the build does.
+RUN apk add --no-cache build-base vips-dev libheif-dev pkgconfig
+
 WORKDIR /src
 
-# pixa is CGO/libvips: the type-aware linters compile every package, so
+# Modules first for layer caching; go.mod/go.sum settle this layer's
-# this image needs the same C libraries the build does. script/bootstrap
+# result, so it may safely be reused between runs.
-# installs them and downloads the Go modules. Only script/, go.mod and
-# go.sum are copied first; they settle this layer's result, so it may
-# safely be reused between runs.
-COPY script/ ./script/
 COPY go.mod go.sum ./
-RUN script/bootstrap
+RUN go mod download
 
 COPY . .
 

README.md

@@ -40,8 +40,9 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for pixa needs:
 
 - **Port:** pixa listens on container port `8080`.
 - **Volume:** container path `/var/lib/pixa`, where pixa keeps its
-  database and cache. Creating the host directory when it is missing is
+  database and cache. upaas bind-mounts the host path it is given and
-  upaas's job, tracked in https://git.eeqj.de/sneak/upaas/issues/235.
+  does not create it, so the host directory must exist before the first
+  deploy.
 - **Environment variables:**
     - `PIXA_SIGNING_KEY` (required): secret for signed and encrypted URLs
       and login, 32+ characters, for example from
@@ -57,6 +58,10 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for pixa needs:
   `healthy`. The probe uses the port from `PORT` (default `8080`), so a
   port changed only in a mounted config file is not seen by it: change
   the port with `PORT`.
+- **First run:** create the host directory, owned by root or by uid
+  `65532` and gid `65532`. The server runs as the container's `pixad`
+  user, which has that uid and gid, and the container gives the
+  directory to `pixad` when it starts.
 
 ## Rationale
 
@@ -81,10 +86,7 @@ prevent abuse, and allowlisted source hosts for open access.
 
 Multiple source paths may reference the same content blob; the
 database tracks references rather than using filesystem refcounting.
-Toward a target of 1-5k r/s, pixa keeps in memory the content types of
+In-process caching of request-to-output mappings targets 1-5k r/s.
-the 10,000 transformed images most recently cached or served, so a
-cache hit on one of them reads only the image file from disk and not
-the metadata file stored beside it.
 
 ### Routes
 
@@ -233,23 +235,20 @@ variables set by the file's `env:` section are checked the same way.
 | `PIXA_TRUSTED_PROXIES`               | `trusted_proxies`               | CIDR ranges of proxies whose `X-Forwarded-For` is believed; default RFC 1918 |
 | `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_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`                 |
-| `PIXA_ACCESS_CONTROL_ALLOW_ORIGIN`   | `access_control_allow_origin`   | CORS origin allowed to read image responses: `*` or one origin; default `*`  |
+| `PIXA_ACCESS_CONTROL_ALLOW_ORIGIN`   | `access_control_allow_origin`   | CORS origin allowed to read responses: `*` or one origin; default `*`        |
 | `PIXA_METRICS_USERNAME`              | `metrics.username`              | Username for `/metrics`, which is served only when both are set              |
 | `PIXA_METRICS_PASSWORD`              | `metrics.password`              | Password for `/metrics`; set together with the username                      |
 | `PIXA_SENTRY_DSN`                    | `sentry_dsn`                    | Sentry DSN for error reporting; empty disables it                            |
 | `PIXA_DEBUG`                         | `debug`                         | Debug logging and plain-HTTP local development; default `false`              |
-| `PIXA_MAINTENANCE_MODE`              | `maintenance_mode`              | Answer image requests with 503; the health check stays 200; default `false`  |
+| `PIXA_MAINTENANCE_MODE`              | `maintenance_mode`              | Maintenance flag reported by the health check; default `false`               |
 
 Key settings in more detail:
 
-- `access_control_allow_origin` — the origin a browser lets read the responses
+- `access_control_allow_origin` — the origin a browser lets read pixa's
-  of the image routes, `/v1/image/` and `/v1/e/`, sent as the CORS
+  responses, sent as the CORS `Access-Control-Allow-Origin` header: `*`, the
-  `Access-Control-Allow-Origin` header; no other route sends it. `*`, the
   default, is any site; otherwise one `http` or `https` origin such as
   `https://example.com`, whose host is a lowercase host name (letters,
   digits, hyphens and dots, with a letter in its last part) or an IP address
@@ -287,31 +286,12 @@ Key settings in more detail:
   bytes; default `52428800` (50 MiB). It also limits the image data pixa
   decodes
 - `downstream_timeout` — time allowed for answering one client request, as a
-  duration; default `60s`. The upstream fetch counts toward it, and so do the
+  duration; default `60s`. The upstream fetch counts toward it, so keep it
-  waits for an upstream connection and for a processing slot (up to 10 seconds
+  longer than `upstream_fetch_timeout`
-  each), so keep it longer than `upstream_fetch_timeout` plus 20 seconds
 - `signing_key` — HMAC secret for URL signatures
 - `cache_max_bytes` — disk cache size limit in bytes; `0` disables the
   disk cache entirely; omitted defaults to 75% of the free space on
   the filesystem containing `<state_dir>/cache/` (minimum 500 MiB)
-- `upstream_connections` — the most connections to upstream hosts at once, all
-  hosts together, on top of `upstream_connections_per_host`; default `64`. A
-  fetch holds its connection until its image has been processed. A fetch 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, 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
-- `maintenance_mode` — while `true`, the image routes (`/v1/image/` and
-  `/v1/e/`) answer every request with 503, a `Retry-After` header and a JSON
-  error body. The health check (`/.well-known/healthcheck.json`) still answers
-  200 and reports `"maintenance_mode": true`. It stays 200 because the image's
-  Docker `HEALTHCHECK` requests it: a 503 there would make the container
-  unhealthy, and upaas marks a deploy failed when its container is unhealthy.
-  The login and URL generator pages and `/metrics` keep working
 
 See `config.example.yml` for all options with defaults.
 

TODO.md

@@ -25,60 +25,11 @@ The disk cache is now size-bounded with LRU eviction
 
 # Next Step
 
-P2: security: referer blacklist
+P1: rate limit global concurrent upstream fetches to prevent resource
+exhaustion
 
 # Completed Steps
 
-- 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
-  still answers a preflight `OPTIONS` request; the login and URL generator
-  pages, `/metrics` and the other routes send no `Access-Control-Allow-Origin`;
-  documented in `README.md` and `config.example.yml`.
-- 2026-09-29 the container makes `/var/lib/pixa` usable by itself (closes
-  #159): `deploy/docker-entrypoint.sh` creates the directory if it is missing,
-  gives the directory and everything in it to `pixad` when the directory or one
-  of its top-level entries belongs to another user or group, sets its mode to
-  `750`, then runs the server as `pixad`; data left by an earlier run under
-  another uid is taken over this way; "Running under upaas" in `README.md` no
-  longer tells the operator to create or chown the host directory.
-- 2026-09-29 variant content types kept in memory (closes #70):
-  `Cache.metaCache` holds the content types of up to 10,000 variants in an LRU
-  (`github.com/hashicorp/golang-lru/v2`), filled by `StoreVariant` and by
-  `GetVariant` after it reads a `.meta` file, where a type `StoreVariant` added
-  meanwhile is kept over the one read, and never with the
-  `application/octet-stream` served for a variant without one; for a variant it
-  holds, `GetVariant` skips the `.meta` read, still opening the variant file and
-  taking the size from it; eviction removes the entry before deleting the files,
-  and `GetVariant` removes it when the file will not open; the cap is a
-  constant, not a setting; the unused `variantMeta` type is gone; `README.md`
-  describes it.
-- 2026-09-29 maintenance mode refuses image requests (closes #71): while
-  `maintenance_mode` is on, `/v1/image/` and `/v1/e/` answer 503 with a
-  `Retry-After` header and the JSON error body, from one middleware in
-  `internal/server/routes.go`; the health check stays 200 and reports
-  `maintenance_mode`, as the image's Docker `HEALTHCHECK` requests it and upaas
-  marks a deploy failed when its container is unhealthy; the login and URL
-  generator pages and `/metrics` keep working; documented in `README.md` and
-  `config.example.yml`.
-- 2026-09-29 bound concurrent image processing and upstream fetches (closes
-  #64): `max_concurrent_processing` (default the number of CPUs pixa can use)
-  limits the images decoded and encoded at once, and `upstream_connections`
-  (default 64) the connections to all upstream hosts together, on top of
-  `upstream_connections_per_host`; a fetch holds its connection until its image
-  has been processed, and a request whose source is cached reads it only once it
-  has a processing slot; a request that finds either limit reached waits up to
-  10 seconds for a free one, then gets 503 `server busy, try again later`;
-  libvips runs one worker thread per image with its operation cache off;
-  documented in `README.md` and `config.example.yml`.
-- 2026-09-29 Dockerfiles install through `script/bootstrap` (closes #95): the
-  `Dockerfile` lint and build stages and `Dockerfile.lint` copy `script/`,
-  `go.mod` and `go.sum`, then run `script/bootstrap` in place of their own
-  `apk add` lines, so the build dependencies are listed in one place;
-  `script/bootstrap` now also installs a C compiler when `gcc` is missing; the
-  build uses `-trimpath` and `-s -w` and keeps `CGO_ENABLED=1` for govips;
-  `ARG VERSION` sits just above the build, so a new version reruns neither
-  `script/bootstrap` nor the tests.
 - 2026-09-29 migrations at the path `REPO_POLICIES.md` sets (closes #96): the
   migration files moved, contents unchanged, from `internal/database/schema/`
   to `internal/db/migrations/` as `000_migration.sql` and `001_schema.sql`; the
@@ -357,6 +308,7 @@ P2: security: referer blacklist
 # Future Steps
 
 - P2: security
+    - referer blacklist
     - per-IP rate limiting on the image routes
     - per-origin rate limiting
 - P2: HTTP response handling

config.example.yml

@@ -16,12 +16,6 @@
 # Server settings
 port: 8080
 debug: false
-
-# While true, the image routes (/v1/image/ and /v1/e/) answer every request
-# with 503 and a Retry-After header. The health check keeps answering 200 and
-# reports maintenance_mode as true. It stays 200 because the image's Docker
-# HEALTHCHECK requests it: a 503 there would make the container unhealthy, and
-# upaas marks a deploy failed when its container is unhealthy.
 maintenance_mode: false
 
 # Data directory for SQLite database and cache files
@@ -77,19 +71,6 @@ allow_http: false
 # Maximum concurrent connections per upstream host (default: 20)
 upstream_connections_per_host: 20
 
-# Maximum concurrent connections to all upstream hosts together, on top of
-# the per-host limit (default: 64). A fetch holds its connection until its
-# image has been processed. A fetch that finds none free waits up to 10
-# seconds for one, and if none frees up the request is answered 503, unless
-# 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.
-# max_concurrent_processing: 4
-
 # Time allowed for one fetch from an upstream host (default: 30s)
 upstream_fetch_timeout: 30s
 
@@ -97,15 +78,12 @@ upstream_fetch_timeout: 30s
 # (1 GiB) (default: 52428800, 50 MiB)
 upstream_max_response_size: 52428800
 
-# Time allowed for answering one client request (default: 60s). The
+# Time allowed for answering one client request, the upstream fetch
-# upstream fetch counts toward it, and so do the waits for an upstream
+# included, so keep it longer than upstream_fetch_timeout (default: 60s)
-# connection and for a processing slot (up to 10 seconds each), so keep it
-# longer than upstream_fetch_timeout plus 20 seconds.
 downstream_timeout: 60s
 
-# The origin a browser lets read the responses of the image routes,
+# The origin a browser lets read pixa's responses, sent as the CORS
-# /v1/image/ and /v1/e/, sent as the CORS Access-Control-Allow-Origin
+# Access-Control-Allow-Origin header: "*" (the default) is any site;
-# header; no other route sends it. "*" (the default) is any site;
 # otherwise one http or https origin such as https://example.com, whose
 # host is a lowercase host name (letters, digits, hyphens and dots, with a
 # letter in its last part) or an IP address (IPv6 in brackets, in its

deploy/docker-entrypoint.sh

@@ -1,22 +1,14 @@
 #!/bin/sh
 # deploy/docker-entrypoint.sh: the Docker image's ENTRYPOINT. It runs as
-# root only to make /var/lib/pixa usable by pixad: a host directory
+# root only to give /var/lib/pixa to pixad: a host directory
-# bind-mounted there keeps its host owner, often root, and data from an
+# bind-mounted there keeps its host owner, often root, and pixad could
-# earlier run may belong to another uid. The server itself always runs
+# not write to it. The server itself always runs as pixad.
-# as pixad.
 set -eu
 
 main() {
-    mkdir -p /var/lib/pixa
+    if [ "$(stat -c %U /var/lib/pixa)" != pixad ]; then
-    # Only the directory and its top-level entries are checked, so a
+        chown pixad:pixad /var/lib/pixa
-    # normal start does not walk the cache. -depth gives each directory
-    # to pixad after its contents, so a start stopped part way leaves
-    # something at the top for the next start to find; -h changes a
-    # symlink itself, never the file it points to.
-    if [ -n "$(find /var/lib/pixa -maxdepth 1 \( ! -user pixad -o ! -group pixad \))" ]; then
-        find /var/lib/pixa -depth -exec chown -h pixad:pixad {} +
     fi
-    chmod 750 /var/lib/pixa
     exec su-exec pixad /usr/local/bin/pixad "$@"
 }
 

go.mod

@@ -14,7 +14,6 @@ require (
 	github.com/go-chi/httprate v0.16.0
 	github.com/gorilla/csrf v1.7.3
 	github.com/gorilla/securecookie v1.1.2
-	github.com/hashicorp/golang-lru/v2 v2.0.7
 	github.com/prometheus/client_golang v1.23.2
 	github.com/slok/go-http-metrics v0.13.0
 	github.com/spf13/cobra v1.10.2

go.sum

@@ -228,8 +228,6 @@ github.com/hashicorp/go-version v1.2.1/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09
 github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8=
 github.com/hashicorp/golang-lru v0.5.4 h1:YDjusn29QI/Das2iO9M0BHnIbxPeyuCHsjMW+lJfyTc=
 github.com/hashicorp/golang-lru v0.5.4/go.mod h1:iADmTwqILo4mZ8BN3D2Q6+9jd8WM5uGBxy+E8yxSoD4=
-github.com/hashicorp/golang-lru/v2 v2.0.7 h1:a+bsQ5rvGLjzHuww6tVxozPZFVghXaHOwFs4luLUK2k=
-github.com/hashicorp/golang-lru/v2 v2.0.7/go.mod h1:QeFd9opnmA6QUJc5vARoKUSoFhyfM2/ZepoAG6RGpeM=
 github.com/hashicorp/hcl v1.0.1-vault-7 h1:ag5OxFVy3QYTFTJODRzTKVZ6xvdfLLCA1cy/Y6xGI0I=
 github.com/hashicorp/hcl v1.0.1-vault-7/go.mod h1:XYhtn6ijBSAj6n4YqAaf7RBPS4I06AItNorpy+MoQNM=
 github.com/hashicorp/logutils v1.0.0/go.mod h1:QIAnNjmIWmVIIkWDTG1z5v++HQmx9WQRO+LraFDTW64=

internal/config/concurrency_limits_internal_test.go

@@ -1,162 +0,0 @@
-package config
-
-import (
-	"runtime"
-	"testing"
-)
-
-// The variables that set the two concurrency limits.
-const (
-	testMaxConcurrentProcessingVar = "PIXA_MAX_CONCURRENT_PROCESSING"
-	testUpstreamConnectionsVar     = "PIXA_UPSTREAM_CONNECTIONS"
-)
-
-// TestOmittedConcurrencyLimitsUseDefaults checks that an omitted
-// max_concurrent_processing is the number of CPUs Go uses and an omitted
-// upstream_connections is 64.
-func TestOmittedConcurrencyLimitsUseDefaults(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.MaxConcurrentProcessing != runtime.GOMAXPROCS(0) {
-		t.Errorf("MaxConcurrentProcessing = %d, want %d, one per CPU",
-			c.MaxConcurrentProcessing, runtime.GOMAXPROCS(0))
-	}
-
-	if c.UpstreamConnections != 64 {
-		t.Errorf("UpstreamConnections = %d, want 64", c.UpstreamConnections)
-	}
-}
-
-// TestExplicitConcurrencyLimitsAreUsed checks that valid values for the
-// two limits are used as given.
-func TestExplicitConcurrencyLimitsAreUsed(t *testing.T) {
-	t.Parallel()
-
-	c, err := configFromYAML(t, signingKeyLine+
-		"max_concurrent_processing: 3\nupstream_connections: 10\n")
-	if err != nil {
-		t.Fatalf("valid config should load, got error: %v", err)
-	}
-
-	if c.MaxConcurrentProcessing != 3 {
-		t.Errorf("MaxConcurrentProcessing = %d, want 3", c.MaxConcurrentProcessing)
-	}
-
-	if c.UpstreamConnections != 10 {
-		t.Errorf("UpstreamConnections = %d, want 10", c.UpstreamConnections)
-	}
-}
-
-// TestInvalidConcurrencyLimitAbortsStartup checks that a limit that is
-// not a whole number of at least 1, or is null, aborts startup naming the
-// key and the value, and the variable too where the value could have come
-// from it.
-func TestInvalidConcurrencyLimitAbortsStartup(t *testing.T) {
-	t.Parallel()
-
-	processing := keyMaxConcurrentProcessing
-	connections := keyUpstreamConnections
-
-	runAbortCases(t, []abortCase{
-		{
-			name: "max_concurrent_processing zero",
-			yaml: signingKeyLine + processing + ": 0\n",
-			wantErrSubstrings: []string{
-				processing, testMaxConcurrentProcessingVar, "value 0",
-			},
-		},
-		{
-			name: "max_concurrent_processing negative",
-			yaml: signingKeyLine + processing + ": -2\n",
-			wantErrSubstrings: []string{
-				processing, testMaxConcurrentProcessingVar, "value -2",
-			},
-		},
-		{
-			name: "max_concurrent_processing not a number",
-			yaml: signingKeyLine + processing + ": lots\n",
-			wantErrSubstrings: []string{
-				processing, testMaxConcurrentProcessingVar, "lots",
-			},
-		},
-		{
-			name:              "max_concurrent_processing fractional",
-			yaml:              signingKeyLine + processing + ": 1.5\n",
-			wantErrSubstrings: []string{processing, "1.5"},
-		},
-		{
-			name:              "max_concurrent_processing null",
-			yaml:              signingKeyLine + processing + ": null\n",
-			wantErrSubstrings: []string{processing, nullValueText},
-		},
-		{
-			name: "upstream_connections zero",
-			yaml: signingKeyLine + connections + ": 0\n",
-			wantErrSubstrings: []string{
-				connections, testUpstreamConnectionsVar, "value 0",
-			},
-		},
-		{
-			name: "upstream_connections negative",
-			yaml: signingKeyLine + connections + ": -5\n",
-			wantErrSubstrings: []string{
-				connections, testUpstreamConnectionsVar, "value -5",
-			},
-		},
-		{
-			name: "upstream_connections not a number",
-			yaml: signingKeyLine + connections + ": many\n",
-			wantErrSubstrings: []string{
-				connections, testUpstreamConnectionsVar, "many",
-			},
-		},
-		{
-			name:              "upstream_connections null",
-			yaml:              signingKeyLine + connections + ": null\n",
-			wantErrSubstrings: []string{connections, nullValueText},
-		},
-	})
-}
-
-// TestConcurrencyLimitsFromEnvironment checks that the two variables set
-// the limits over the config file, and that an invalid value in either
-// aborts startup naming the variable and the value.
-func TestConcurrencyLimitsFromEnvironment(t *testing.T) {
-	t.Setenv(testMaxConcurrentProcessingVar, "3")
-	t.Setenv(testUpstreamConnectionsVar, "10")
-
-	c, err := configFromYAML(t, signingKeyLine+
-		"max_concurrent_processing: 5\nupstream_connections: 50\n")
-	if err != nil {
-		t.Fatalf("limits from the environment should load: %v", err)
-	}
-
-	if c.MaxConcurrentProcessing != 3 || c.UpstreamConnections != 10 {
-		t.Errorf("limits = %d and %d, want 3 and 10 from the environment",
-			c.MaxConcurrentProcessing, c.UpstreamConnections)
-	}
-
-	cases := []struct {
-		variable string
-		value    string
-	}{
-		{testMaxConcurrentProcessingVar, "lots"},
-		{testMaxConcurrentProcessingVar, "0"},
-		{testUpstreamConnectionsVar, "-1"},
-		{testUpstreamConnectionsVar, "ten"},
-	}
-
-	for _, tc := range cases {
-		t.Run(tc.variable+"="+tc.value, func(t *testing.T) {
-			t.Setenv(tc.variable, tc.value)
-
-			_, err := configFromYAML(t, signingKeyLine)
-			wantStartupError(t, err, tc.variable, tc.value)
-		})
-	}
-}

internal/config/config.go

@@ -10,7 +10,6 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
-	"runtime"
 	"sort"
 	"strconv"
 	"strings"
@@ -26,7 +25,6 @@ const (
 	DefaultPort                       = 8080
 	DefaultStateDir                   = "/var/lib/pixa"
 	DefaultUpstreamConnectionsPerHost = 20
-	DefaultUpstreamConnections        = 64
 	DefaultAccessControlAllowOrigin   = "*"
 	DefaultUpstreamFetchTimeout       = 30 * time.Second
 	DefaultUpstreamMaxResponseSize    = 50 << 20 // 50 MiB
@@ -48,8 +46,6 @@ const (
 	keyAllowlistHosts             = "allowlist_hosts"
 	keyAllowHTTP                  = "allow_http"
 	keyUpstreamConnectionsPerHost = "upstream_connections_per_host"
-	keyUpstreamConnections        = "upstream_connections"
-	keyMaxConcurrentProcessing    = "max_concurrent_processing"
 	keyCacheMaxBytes              = "cache_max_bytes"
 	keyBlockedNetworks            = "blocked_networks"
 	keyTrustedProxies             = "trusted_proxies"
@@ -83,7 +79,7 @@ var (
 	errNotAValidURL      = errors.New("not a valid URL")
 	errPortOutOfRange    = errors.New("outside the valid port range")
 	errSizeOutOfRange    = errors.New("outside the accepted range")
-	errMustBeAtLeastOne  = errors.New("must be at least 1")
+	errTooFewConnections = errors.New("must be at least 1")
 	errValueTooShort     = errors.New("value too short")
 	errPlaceholderKey    = errors.New(
 		"is the placeholder from config.example.yml; " +
@@ -130,12 +126,6 @@ type Config struct {
 	AllowHTTP                  bool     // Allow non-TLS upstream (testing only)
 	UpstreamConnectionsPerHost int      // Max concurrent connections per upstream host
 
-	// UpstreamConnections is the most concurrent connections to all
-	// upstream hosts together, on top of the per-host limit.
-	// MaxConcurrentProcessing is the most images processed at once.
-	UpstreamConnections     int
-	MaxConcurrentProcessing int
-
 	// UpstreamFetchTimeout is the time allowed for one fetch from an
 	// upstream host. UpstreamMaxResponseSize is the largest upstream
 	// response accepted, in bytes, and also the image processor's input
@@ -236,12 +226,14 @@ func New(_ fx.Lifecycle, params Params) (*Config, error) {
 // unparseable or invalid is an error: defaults apply only to omitted
 // keys, never to invalid explicit values.
 func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
-	err := validateKnownKeys(sc)
+	if sc != nil {
-	if err != nil {
+		err := validateKnownKeys(sc)
-		return nil, err
+		if err != nil {
+			return nil, err
+		}
 	}
 
-	err = validateAllowlistHostsValue(sc)
+	err := validateAllowlistHostsValue(sc)
 	if err != nil {
 		return nil, err
 	}
@@ -279,12 +271,6 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
 		AllowHTTP:       loader.boolVal(keyAllowHTTP, false),
 		UpstreamConnectionsPerHost: loader.intVal(
 			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)),
 		UpstreamFetchTimeout: loader.durationVal(
 			keyUpstreamFetchTimeout, DefaultUpstreamFetchTimeout),
 		UpstreamMaxResponseSize: loader.int64Val(
@@ -335,13 +321,8 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
 // being silently ignored, and rejects keys that are explicitly set to
 // null: a null is a SET value, never an omission, so it must not
 // silently take the default. The env section is permitted because
-// smartconfig consumes it for environment variable injection. A nil sc
+// smartconfig consumes it for environment variable injection.
-// means no config file, which has no keys to check.
 func validateKnownKeys(sc *smartconfig.Config) error {
-	if sc == nil {
-		return nil
-	}
-
 	var unknown, nullKeys []string
 
 	for key, value := range sc.Data() {
@@ -411,8 +392,7 @@ func isKnownConfigKey(key string) bool {
 	switch key {
 	case keyDebug, keyMaintenanceMode, keyPort, keyStateDir, keySentryDSN,
 		keyDBURL, keyMetrics, keySigningKey, keyAllowlistHosts, keyAllowHTTP,
-		keyUpstreamConnectionsPerHost, keyUpstreamConnections,
+		keyUpstreamConnectionsPerHost, keyCacheMaxBytes, keyBlockedNetworks,
-		keyMaxConcurrentProcessing, keyCacheMaxBytes, keyBlockedNetworks,
 		keyTrustedProxies, keyAccessControlAllowOrigin, keyUpstreamFetchTimeout,
 		keyUpstreamMaxResponseSize, keyDownstreamTimeout, "env":
 		return true
@@ -439,8 +419,6 @@ func envVarNames() map[string]string {
 		keyAllowlistHosts:             "PIXA_ALLOWLIST_HOSTS",
 		keyAllowHTTP:                  "PIXA_ALLOW_HTTP",
 		keyUpstreamConnectionsPerHost: "PIXA_UPSTREAM_CONNECTIONS_PER_HOST",
-		keyUpstreamConnections:        "PIXA_UPSTREAM_CONNECTIONS",
-		keyMaxConcurrentProcessing:    "PIXA_MAX_CONCURRENT_PROCESSING",
 		keyCacheMaxBytes:              "PIXA_CACHE_MAX_BYTES",
 		keyBlockedNetworks:            "PIXA_BLOCKED_NETWORKS",
 		keyTrustedProxies:             "PIXA_TRUSTED_PROXIES",
@@ -584,9 +562,10 @@ func (c *Config) validate() error {
 			settingName(keyPort), c.Port, errPortOutOfRange, maxPort)
 	}
 
-	err = c.validateConcurrencyLimits()
+	if c.UpstreamConnectionsPerHost < 1 {
-	if err != nil {
+		return fmt.Errorf("%s: value %d %w",
-		return err
+			settingName(keyUpstreamConnectionsPerHost),
+			c.UpstreamConnectionsPerHost, errTooFewConnections)
 	}
 
 	if c.StateDir == "" {
@@ -705,30 +684,6 @@ func (c *Config) validateAccessControlAllowOrigin() error {
 	return nil
 }
 
-// validateConcurrencyLimits checks that the two upstream connection limits
-// and the image processing limit are at least 1.
-func (c *Config) validateConcurrencyLimits() error {
-	if c.UpstreamConnectionsPerHost < 1 {
-		return fmt.Errorf("%s: value %d %w",
-			settingName(keyUpstreamConnectionsPerHost),
-			c.UpstreamConnectionsPerHost, errMustBeAtLeastOne)
-	}
-
-	if c.UpstreamConnections < 1 {
-		return fmt.Errorf("%s: value %d %w",
-			settingName(keyUpstreamConnections),
-			c.UpstreamConnections, errMustBeAtLeastOne)
-	}
-
-	if c.MaxConcurrentProcessing < 1 {
-		return fmt.Errorf("%s: value %d %w",
-			settingName(keyMaxConcurrentProcessing),
-			c.MaxConcurrentProcessing, errMustBeAtLeastOne)
-	}
-
-	return nil
-}
-
 // validateAllowlistHost checks that an allowlist_hosts entry is a bare
 // hostname, optionally with a leading dot for suffix matching. URLs,
 // paths, and whitespace indicate a misconfigured entry. An entry with

internal/config/env_internal_test.go

@@ -67,8 +67,6 @@ func TestEnvironmentSetsEveryKey(t *testing.T) {
 	t.Setenv("PIXA_ALLOWLIST_HOSTS", "s3.sneak.cloud,.example.com")
 	t.Setenv("PIXA_ALLOW_HTTP", "true")
 	t.Setenv("PIXA_UPSTREAM_CONNECTIONS_PER_HOST", "5")
-	t.Setenv("PIXA_UPSTREAM_CONNECTIONS", "10")
-	t.Setenv("PIXA_MAX_CONCURRENT_PROCESSING", "3")
 	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")
@@ -95,8 +93,6 @@ func TestEnvironmentSetsEveryKey(t *testing.T) {
 		AllowlistHosts:             []string{testHostS3, ".example.com"},
 		AllowHTTP:                  true,
 		UpstreamConnectionsPerHost: 5,
-		UpstreamConnections:        10,
-		MaxConcurrentProcessing:    3,
 		CacheMaxBytes:              1024,
 		cacheMaxBytesExplicit:      true,
 		BlockedNetworks:            []netip.Prefix{netip.MustParsePrefix("203.0.113.0/24")},

internal/handlers/handlers.go

@@ -113,17 +113,15 @@ func (s *Handlers) initImageService() error {
 		fetcherCfg.MaxConnectionsPerHost = s.config.UpstreamConnectionsPerHost
 	}
 
-	fetcherCfg.MaxConnections = s.config.UpstreamConnections
 	fetcherCfg.BlockedNetworks = s.config.BlockedNetworks
 
 	// Create the service
 	svc, err := imgcache.NewService(&imgcache.ServiceConfig{
-		Cache:                   cache,
+		Cache:         cache,
-		FetcherConfig:           fetcherCfg,
+		FetcherConfig: fetcherCfg,
-		SigningKey:              s.config.SigningKey,
+		SigningKey:    s.config.SigningKey,
-		Allowlist:               s.config.AllowlistHosts,
+		Allowlist:     s.config.AllowlistHosts,
-		MaxConcurrentProcessing: s.config.MaxConcurrentProcessing,
+		Logger:        s.log,
-		Logger:                  s.log,
 	})
 	if err != nil {
 		return err

internal/handlers/image.go

@@ -12,7 +12,6 @@ import (
 	"github.com/go-chi/chi/v5"
 	"sneak.berlin/go/pixa/internal/encurl"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
-	"sneak.berlin/go/pixa/internal/imageprocessor"
 	"sneak.berlin/go/pixa/internal/imgcache"
 )
 
@@ -218,14 +217,6 @@ func (s *Handlers) respondImageError(
 		return
 	}
 
-	if errors.Is(err, httpfetcher.ErrTooManyConnections) ||
-		errors.Is(err, imageprocessor.ErrTooManyImages) {
-		s.respondError(w, "server busy, try again later",
-			http.StatusServiceUnavailable)
-
-		return
-	}
-
 	s.respondError(w, "internal error", http.StatusInternalServerError)
 }
 

internal/handlers/imageenc.go

@@ -12,7 +12,6 @@ import (
 
 	"sneak.berlin/go/pixa/internal/encurl"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
-	"sneak.berlin/go/pixa/internal/imageprocessor"
 	"sneak.berlin/go/pixa/internal/imgcache"
 )
 
@@ -125,10 +124,6 @@ func (s *Handlers) handleImageError(w http.ResponseWriter, err error) {
 		s.respondError(w, "upstream error", http.StatusBadGateway)
 	case errors.Is(err, httpfetcher.ErrUpstreamTimeout):
 		s.respondError(w, "upstream timeout", http.StatusGatewayTimeout)
-	case errors.Is(err, httpfetcher.ErrTooManyConnections),
-		errors.Is(err, imageprocessor.ErrTooManyImages):
-		s.respondError(w, "server busy, try again later",
-			http.StatusServiceUnavailable)
 	default:
 		s.log.Error("image request failed", "error", err)
 		s.respondError(w, "internal error", http.StatusInternalServerError)

internal/handlers/server_busy_internal_test.go

@@ -1,49 +0,0 @@
-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"
-)
-
-// TestServerBusyAnswers503 checks that both image routes answer 503 with
-// a clear error when the image service gives up waiting for a free upstream
-// connection or processing slot, wrapped as the service wraps them.
-func TestServerBusyAnswers503(t *testing.T) {
-	t.Parallel()
-
-	h := &Handlers{log: slog.New(slog.DiscardHandler)}
-	req := &imgcache.ImageRequest{SourceHost: "img.example.com", SourcePath: "/a.jpg"}
-
-	for _, err := range []error{
-		fmt.Errorf("upstream fetch failed: %w", httpfetcher.ErrTooManyConnections),
-		fmt.Errorf("image processing failed: %w", imageprocessor.ErrTooManyImages),
-	} {
-		plain := httptest.NewRecorder()
-		h.respondImageError(plain, req, err)
-
-		encrypted := httptest.NewRecorder()
-		h.handleImageError(encrypted, err)
-
-		for route, rec := range map[string]*httptest.ResponseRecorder{
-			"/v1/image/": plain, "/v1/e/": encrypted,
-		} {
-			if rec.Code != http.StatusServiceUnavailable {
-				t.Errorf("%s for %v: status = %d, want %d",
-					route, err, rec.Code, http.StatusServiceUnavailable)
-			}
-
-			if !strings.Contains(rec.Body.String(), "server busy, try again later") {
-				t.Errorf("%s for %v: body = %q, want the server busy error",
-					route, err, rec.Body.String())
-			}
-		}
-	}
-}

internal/httpfetcher/httpfetcher.go

@@ -1,6 +1,5 @@
 // Package httpfetcher fetches content from upstream HTTP origins with SSRF
-// protection, connection limits per host and for all hosts together, and
+// protection, per-host connection limits, and content-type validation.
-// content-type validation.
 package httpfetcher
 
 import (
@@ -29,13 +28,8 @@ const (
 	DefaultIdleConnTimeout       = 90 * time.Second
 	DefaultMaxRedirects          = 10
 	DefaultMaxConnectionsPerHost = 20
-	DefaultMaxConnections        = 64
 )
 
-// ConnectionWaitTimeout is how long Fetch waits for a free connection when
-// MaxConnections fetches are already in progress.
-const ConnectionWaitTimeout = 10 * time.Second
-
 // MIME content types.
 const (
 	contentTypeJPEG        = "image/jpeg"
@@ -76,7 +70,6 @@ var (
 	ErrInvalidContentType = errors.New("invalid or unsupported content type")
 	ErrUpstreamError      = errors.New("upstream server error")
 	ErrUpstreamTimeout    = errors.New("upstream request timeout")
-	ErrTooManyConnections = errors.New("too many concurrent upstream connections")
 )
 
 // Internal fetcher errors.
@@ -129,9 +122,6 @@ type Config struct {
 	AllowHTTP bool
 	// MaxConnectionsPerHost limits concurrent connections to each upstream host.
 	MaxConnectionsPerHost int
-	// MaxConnections limits concurrent connections to all upstream hosts
-	// together.
-	MaxConnections int
 	// BlockedNetworks are operator-supplied CIDR ranges refused by the
 	// dialer, in addition to the always-enforced built-in ranges.
 	BlockedNetworks []netip.Prefix
@@ -153,22 +143,15 @@ func DefaultConfig() *Config {
 		},
 		AllowHTTP:             false,
 		MaxConnectionsPerHost: DefaultMaxConnectionsPerHost,
-		MaxConnections:        DefaultMaxConnections,
 	}
 }
 
-// HTTPFetcher implements Fetcher with SSRF protection and connection limits
+// HTTPFetcher implements Fetcher with SSRF protection and per-host connection limits.
-// per host and for all hosts together.
 type HTTPFetcher struct {
 	client    *http.Client
 	config    *Config
 	hostSems  map[string]chan struct{} // per-host semaphores
 	hostSemMu sync.Mutex               // protects hostSems map
-	// allHostsSemaphore has one slot per connection allowed to all hosts
-	// together (config.MaxConnections).
-	allHostsSemaphore chan struct{}
-	// connectionWaitTimeout is ConnectionWaitTimeout; tests shorten it.
-	connectionWaitTimeout time.Duration
 }
 
 // New creates a new HTTPFetcher with SSRF protection.
@@ -209,18 +192,13 @@ func New(config *Config) *HTTPFetcher {
 	}
 
 	return &HTTPFetcher{
-		client:                client,
+		client:   client,
-		config:                config,
+		config:   config,
-		hostSems:              make(map[string]chan struct{}),
+		hostSems: make(map[string]chan struct{}),
-		allHostsSemaphore:     make(chan struct{}, config.MaxConnections),
-		connectionWaitTimeout: ConnectionWaitTimeout,
 	}
 }
 
-// Fetch retrieves content from the given URL with SSRF protection. When
+// Fetch retrieves content from the given URL with SSRF protection.
-// MaxConnections fetches are already in progress, it waits up to
-// ConnectionWaitTimeout for one to finish, then fails with
-// ErrTooManyConnections.
 func (f *HTTPFetcher) Fetch(ctx context.Context, url string) (*FetchResult, error) {
 	// Validate URL before making request
 	err := validateURL(ctx, url, f.config.AllowHTTP)
@@ -228,17 +206,24 @@ func (f *HTTPFetcher) Fetch(ctx context.Context, url string) (*FetchResult, erro
 		return nil, err
 	}
 
-	release, err := f.acquireConnection(ctx, extractHost(url))
+	// Extract host for rate limiting
-	if err != nil {
+	host := extractHost(url)
-		return nil, err
+
+	// Acquire semaphore slot for this host
+	sem := f.getHostSemaphore(host)
+	select {
+	case sem <- struct{}{}:
+		// Acquired slot
+	case <-ctx.Done():
+		return nil, ctx.Err()
 	}
 
-	// If we fail before returning a result, release the connection
+	// If we fail before returning a result, release the slot
 	success := false
 
 	defer func() {
 		if !success {
-			release()
+			<-sem
 		}
 	}()
 
@@ -282,52 +267,17 @@ func (f *HTTPFetcher) Fetch(ctx context.Context, url string) (*FetchResult, erro
 		return nil, fmt.Errorf("upstream request failed: %w", err)
 	}
 
-	result, err := f.buildResult(resp, remoteAddr, fetchDuration, release)
+	result, err := f.buildResult(resp, remoteAddr, fetchDuration, sem)
 	if err != nil {
 		return nil, err
 	}
 
-	// Mark success so defer doesn't release the connection; closing the
+	// Mark success so defer doesn't release the semaphore
-	// result's Content does
 	success = true
 
 	return result, nil
 }
 
-// acquireConnection takes a slot for host, then one of the slots shared by
-// all hosts, and returns the func that gives both back. The host's slot
-// comes first, so fetches queued for one busy host hold no shared slot.
-// Only the wait for a shared slot is bounded: after connectionWaitTimeout
-// it fails with ErrTooManyConnections.
-func (f *HTTPFetcher) acquireConnection(
-	ctx context.Context, host string,
-) (func(), error) {
-	hostSem := f.getHostSemaphore(host)
-
-	select {
-	case hostSem <- struct{}{}:
-	case <-ctx.Done():
-		return nil, ctx.Err()
-	}
-
-	select {
-	case f.allHostsSemaphore <- struct{}{}:
-	case <-time.After(f.connectionWaitTimeout):
-		<-hostSem
-
-		return nil, ErrTooManyConnections
-	case <-ctx.Done():
-		<-hostSem
-
-		return nil, ctx.Err()
-	}
-
-	return func() {
-		<-hostSem
-		<-f.allHostsSemaphore
-	}, nil
-}
-
 // getHostSemaphore returns the semaphore for a host, creating it if necessary.
 func (f *HTTPFetcher) getHostSemaphore(host string) chan struct{} {
 	f.hostSemMu.Lock()
@@ -343,12 +293,12 @@ func (f *HTTPFetcher) getHostSemaphore(host string) chan struct{} {
 }
 
 // buildResult validates the upstream response and assembles a FetchResult
-// whose Content calls release when closed.
+// whose Content releases the host semaphore slot when closed.
 func (f *HTTPFetcher) buildResult(
 	resp *http.Response,
 	remoteAddr string,
 	fetchDuration time.Duration,
-	release func(),
+	sem chan struct{},
 ) (*FetchResult, error) {
 	// Extract HTTP version (strip "HTTP/" prefix)
 	httpVersion := strings.TrimPrefix(resp.Proto, "HTTP/")
@@ -383,7 +333,7 @@ func (f *HTTPFetcher) buildResult(
 	}
 
 	return &FetchResult{
-		Content:         &semaphoreReleasingReadCloser{limitedBody, resp.Body, release},
+		Content:         &semaphoreReleasingReadCloser{limitedBody, resp.Body, sem},
 		ContentLength:   resp.ContentLength,
 		ContentType:     contentType,
 		Headers:         resp.Header,
@@ -624,18 +574,17 @@ func (r *limitedReader) Read(p []byte) (int, error) {
 	return n, err
 }
 
-// semaphoreReleasingReadCloser releases the fetch's connection slots when
+// semaphoreReleasingReadCloser releases a semaphore slot when closed.
-// closed.
 type semaphoreReleasingReadCloser struct {
 	*limitedReader
 
-	closer  io.Closer
+	closer io.Closer
-	release func()
+	sem    chan struct{}
 }
 
 func (r *semaphoreReleasingReadCloser) Close() error {
 	err := r.closer.Close()
-	r.release()
+	<-r.sem // Release semaphore slot
 
 	return err
 }

internal/httpfetcher/max_connections_internal_test.go

@@ -1,165 +0,0 @@
-package httpfetcher
-
-import (
-	"context"
-	"errors"
-	"net"
-	"strconv"
-	"testing"
-	"time"
-)
-
-// imageURLOnPort is the fake upstream's image route on testPublicHost at
-// port. Each port is a different host to the per-host limit, while the test
-// dialer sends every port to the one test server.
-func imageURLOnPort(port int) string {
-	return "http://" + net.JoinHostPort(testPublicHost, strconv.Itoa(port)) +
-		"/image"
-}
-
-func TestDefaultConfigMaxConnections(t *testing.T) {
-	t.Parallel()
-
-	if got := DefaultConfig().MaxConnections; got != DefaultMaxConnections {
-		t.Errorf("MaxConnections = %d, want %d", got, DefaultMaxConnections)
-	}
-}
-
-// TestFetchLimitsConnectionsToAllHostsTogether checks that MaxConnections
-// counts the fetches to every host together, apart from the per-host
-// limit: with MaxConnections at 2 and two responses open from two hosts, a
-// fetch from a third host, which has nothing open, waits the whole wait
-// timeout and fails with ErrTooManyConnections. Closing one response lets
-// it through.
-func TestFetchLimitsConnectionsToAllHostsTogether(t *testing.T) {
-	t.Parallel()
-
-	srv := startUpstream(t)
-
-	cfg := DefaultConfig()
-	cfg.MaxConnections = 2
-
-	f, _ := newServerFetcher(t, srv, cfg)
-	f.connectionWaitTimeout = 100 * time.Millisecond
-
-	first, err := f.Fetch(testContext(t), imageURLOnPort(81))
-	if err != nil {
-		t.Fatalf("first Fetch() error = %v", err)
-	}
-
-	second, err := f.Fetch(testContext(t), imageURLOnPort(82))
-	if err != nil {
-		t.Fatalf("second Fetch() error = %v", err)
-	}
-
-	defer func() { _ = second.Content.Close() }()
-
-	start := time.Now()
-
-	_, err = f.Fetch(testContext(t), imageURLOnPort(83))
-	if !errors.Is(err, ErrTooManyConnections) {
-		t.Fatalf("third Fetch() error = %v, want ErrTooManyConnections", err)
-	}
-
-	if waited := time.Since(start); waited < f.connectionWaitTimeout {
-		t.Errorf("third Fetch() failed after %v, before waiting %v",
-			waited, f.connectionWaitTimeout)
-	}
-
-	if held := semLen(f, testPublicHost+":83"); held != 0 {
-		t.Errorf("the refused fetch kept its host's slot: %d held", held)
-	}
-
-	err = first.Content.Close()
-	if err != nil {
-		t.Fatalf("close first body: %v", err)
-	}
-
-	third, err := f.Fetch(testContext(t), imageURLOnPort(83))
-	if err != nil {
-		t.Fatalf("Fetch() after a response was closed: error = %v", err)
-	}
-
-	_ = third.Content.Close()
-}
-
-// TestFetchFreesHostSlotWhenContextEndsWaitingForConnection checks that a
-// fetch whose request context ends while it waits for a connection shared
-// by all hosts gives its host's slot back. With MaxConnections at 1 and one
-// response open, a fetch from another host takes that host's slot and waits;
-// its context ends long before the 10 second wait timeout.
-func TestFetchFreesHostSlotWhenContextEndsWaitingForConnection(t *testing.T) {
-	t.Parallel()
-
-	srv := startUpstream(t)
-
-	cfg := DefaultConfig()
-	cfg.MaxConnections = 1
-
-	f, _ := newServerFetcher(t, srv, cfg)
-
-	first, err := f.Fetch(testContext(t), imageURLOnPort(81))
-	if err != nil {
-		t.Fatalf("first Fetch() error = %v", err)
-	}
-
-	defer func() { _ = first.Content.Close() }()
-
-	ctx, cancel := context.WithTimeout(t.Context(), 100*time.Millisecond)
-	defer cancel()
-
-	_, err = f.Fetch(ctx, imageURLOnPort(82))
-	if !errors.Is(err, context.DeadlineExceeded) {
-		t.Fatalf("second Fetch() error = %v, want context.DeadlineExceeded", err)
-	}
-
-	if held := semLen(f, testPublicHost+":82"); held != 0 {
-		t.Errorf("the fetch kept its host's slot after its context ended: "+
-			"%d held", held)
-	}
-}
-
-// TestFetchReleasesConnectionOnError checks that a fetch that fails after
-// taking its connection gives it back: with MaxConnections at 1, the slot
-// must be free after the failure and the next fetch must succeed.
-func TestFetchReleasesConnectionOnError(t *testing.T) {
-	t.Parallel()
-
-	cases := []struct {
-		name string
-		url  string
-		want error
-	}{
-		{"upstream answers 500", upstreamURL("/status/500"), ErrUpstreamError},
-		{"upstream sends HTML", upstreamURL("/html"), ErrInvalidContentType},
-		// 198.51.100.7 (TEST-NET-2) passes the SSRF checks, and the test
-		// dialer refuses every host but testPublicHost.
-		{"connecting fails", "http://198.51.100.7/image", errUnexpectedDial},
-	}
-
-	for _, tc := range cases {
-		t.Run(tc.name, func(t *testing.T) {
-			t.Parallel()
-
-			srv := startUpstream(t)
-
-			cfg := DefaultConfig()
-			cfg.MaxConnections = 1
-
-			f, _ := newServerFetcher(t, srv, cfg)
-			f.connectionWaitTimeout = 100 * time.Millisecond
-
-			_, err := f.Fetch(testContext(t), tc.url)
-			if !errors.Is(err, tc.want) {
-				t.Fatalf("Fetch() error = %v, want %v", err, tc.want)
-			}
-
-			if held := len(f.allHostsSemaphore); held != 0 {
-				t.Fatalf("connection still held after the error: %d held", held)
-			}
-
-			res := fetchImage(t, f, "/image")
-			_ = res.Content.Close()
-		})
-	}
-}

internal/imageprocessor/imageprocessor.go

@@ -7,9 +7,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"runtime"
 	"sync"
-	"time"
 
 	"github.com/davidbyttow/govips/v2/vips"
 )
@@ -19,21 +17,11 @@ import (
 //nolint:gochecknoglobals // package-level sync.Once for one-time vips init
 var vipsOnce sync.Once
 
-// initVips initializes libvips with quiet logging, one worker thread per
+// initVips initializes libvips with quiet logging.
-// image and no operation cache. Process already works on one image per CPU
-// by default, so more threads per image would only compete for the CPUs.
-// Each request decodes different source bytes, so the operation cache
-// would rarely be hit and would hold memory outside MaxConcurrentProcessing;
-// repeated requests are served from pixa's disk cache instead.
 func initVips() {
 	vipsOnce.Do(func() {
 		vips.LoggingSettings(nil, vips.LogLevelError)
-		vips.Startup(&vips.Config{
+		vips.Startup(nil)
-			ConcurrencyLevel: 1,
-			MaxCacheSize:     0,
-			MaxCacheMem:      0,
-			MaxCacheFiles:    0,
-		})
 	})
 }
 
@@ -118,23 +106,9 @@ var ErrInputDataTooLarge = errors.New("input data exceeds maximum allowed size")
 // not supported.
 var ErrUnsupportedOutputFormat = errors.New("unsupported output 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")
-
-// ProcessingWaitTimeout is how long Process waits for a free slot when
-// MaxConcurrentProcessing images are already being processed.
-const ProcessingWaitTimeout = 10 * time.Second
-
 // ImageProcessor implements image transformation using libvips via govips.
 type ImageProcessor struct {
 	maxInputBytes int64
-	// processingSemaphore has one slot per image that may be processed at
-	// once. Process holds a slot from before it reads its input until it
-	// returns, so the input, the decoded image and the output all count.
-	processingSemaphore chan struct{}
-	// processingWaitTimeout is ProcessingWaitTimeout; tests shorten it.
-	processingWaitTimeout time.Duration
 }
 
 // Params holds configuration for creating an ImageProcessor.
@@ -143,9 +117,6 @@ type Params struct {
 	// MaxInputBytes is the maximum allowed input size in bytes.
 	// 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.
-	MaxConcurrentProcessing int
 }
 
 // New creates a new image processor with the given parameters.
@@ -158,34 +129,17 @@ func New(params Params) *ImageProcessor {
 		maxInputBytes = DefaultMaxInputBytes
 	}
 
-	maxConcurrentProcessing := params.MaxConcurrentProcessing
-	if maxConcurrentProcessing <= 0 {
-		maxConcurrentProcessing = runtime.GOMAXPROCS(0)
-	}
-
 	return &ImageProcessor{
-		maxInputBytes:         maxInputBytes,
+		maxInputBytes: maxInputBytes,
-		processingSemaphore:   make(chan struct{}, maxConcurrentProcessing),
-		processingWaitTimeout: ProcessingWaitTimeout,
 	}
 }
 
-// Process transforms an image according to the request. When
+// Process transforms an image according to the request.
-// MaxConcurrentProcessing images are already being processed, it waits up
-// to ProcessingWaitTimeout for one to finish, then fails with
-// ErrTooManyImages.
 func (p *ImageProcessor) Process(
-	ctx context.Context,
+	_ context.Context,
 	input io.Reader,
 	req *Request,
 ) (*Result, error) {
-	release, err := p.acquireSlot(ctx)
-	if err != nil {
-		return nil, err
-	}
-
-	defer release()
-
 	// Read input with a size limit to prevent unbounded memory consumption.
 	// We read at most maxInputBytes+1 so we can detect if the input exceeds
 	// the limit without consuming additional memory.
@@ -331,29 +285,6 @@ func FormatToMIME(format Format) string {
 	}
 }
 
-// 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.
-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()
-	}
-}
-
 // detectFormat returns the format string from a vips image.
 func (p *ImageProcessor) detectFormat(img *vips.ImageRef) string {
 	format := img.Format()

internal/imageprocessor/max_concurrent_processing_internal_test.go

@@ -1,302 +0,0 @@
-package imageprocessor
-
-import (
-	"bytes"
-	"context"
-	"errors"
-	"io"
-	"runtime"
-	"strings"
-	"sync"
-	"testing"
-	"testing/iotest"
-	"time"
-)
-
-// errTestReadFailed is the error the unreadable test input returns.
-var errTestReadFailed = errors.New("test input cannot be read")
-
-// readingCounter counts the Process calls reading their input at the same
-// time and remembers the most there ever were.
-type readingCounter struct {
-	mu      sync.Mutex
-	reading int
-	most    int
-}
-
-func (c *readingCounter) start() {
-	c.mu.Lock()
-	defer c.mu.Unlock()
-
-	c.reading++
-	c.most = max(c.most, c.reading)
-}
-
-func (c *readingCounter) stop() {
-	c.mu.Lock()
-	defer c.mu.Unlock()
-
-	c.reading--
-}
-
-func (c *readingCounter) mostReading() int {
-	c.mu.Lock()
-	defer c.mu.Unlock()
-
-	return c.most
-}
-
-// gatedReader is a Process input. Its first Read counts the call in,
-// reports it on entered and blocks until gate is closed; it counts the call
-// out when it returns io.EOF. Process reads its input only while it holds a
-// processing slot, so the count never goes above MaxConcurrentProcessing.
-type gatedReader struct {
-	data    *bytes.Reader
-	gate    <-chan struct{}
-	entered chan<- struct{}
-	counter *readingCounter
-	started bool
-}
-
-func (r *gatedReader) Read(p []byte) (int, error) {
-	if !r.started {
-		r.started = true
-		r.counter.start()
-
-		r.entered <- struct{}{}
-
-		<-r.gate
-	}
-
-	n, err := r.data.Read(p)
-	if errors.Is(err, io.EOF) {
-		r.counter.stop()
-	}
-
-	return n, err
-}
-
-// smallJPEGRequest asks for a 5x5 JPEG.
-func smallJPEGRequest() *Request {
-	return &Request{
-		Size:    Size{Width: 5, Height: 5},
-		Format:  FormatJPEG,
-		Quality: 85,
-		FitMode: FitCover,
-	}
-}
-
-// processInBackground runs Process on reader in a new goroutine and sends
-// its error on results.
-func processInBackground(
-	proc *ImageProcessor, reader *gatedReader, results chan<- error,
-) {
-	go func() {
-		result, err := proc.Process(context.Background(), reader, smallJPEGRequest())
-		if err == nil {
-			_ = result.Content.Close()
-		}
-
-		results <- err
-	}()
-}
-
-// waitForEntries fails the test unless count Process calls report on
-// entered within a few seconds.
-func waitForEntries(t *testing.T, entered <-chan struct{}, count int) {
-	t.Helper()
-
-	for range count {
-		select {
-		case <-entered:
-		case <-time.After(5 * time.Second):
-			t.Fatal("Process calls did not start reading their input")
-		}
-	}
-}
-
-func TestNewDefaultsMaxConcurrentProcessingToCPUs(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))
-		}
-	}
-
-	proc := New(Params{MaxConcurrentProcessing: 3})
-	if got := cap(proc.processingSemaphore); got != 3 {
-		t.Errorf("MaxConcurrentProcessing 3: %d slots, want 3", got)
-	}
-}
-
-// 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
-// working at once, and the calls held back must wait for a slot and then
-// succeed.
-func TestProcessNeverExceedsMaxConcurrentProcessing(t *testing.T) {
-	t.Parallel()
-
-	const (
-		limit = 2
-		calls = 6
-	)
-
-	proc := New(Params{MaxConcurrentProcessing: limit})
-	input := createTestJPEG(t, 50, 50)
-
-	counter := &readingCounter{}
-	gate := make(chan struct{})
-	entered := make(chan struct{}, calls)
-	results := make(chan error, calls)
-
-	openGate := sync.OnceFunc(func() { close(gate) })
-	t.Cleanup(openGate)
-
-	for range calls {
-		processInBackground(proc, &gatedReader{
-			data: bytes.NewReader(input), gate: gate, entered: entered,
-			counter: counter,
-		}, results)
-	}
-
-	waitForEntries(t, entered, limit)
-
-	// A call beyond the limit would start reading its input now.
-	select {
-	case <-entered:
-		t.Fatalf("a Process call started while %d were already working", limit)
-	case <-time.After(100 * time.Millisecond):
-	}
-
-	openGate()
-
-	for range calls {
-		err := <-results
-		if err != nil {
-			t.Errorf("Process() error = %v, want nil once a slot is free", err)
-		}
-	}
-
-	if most := counter.mostReading(); most > limit {
-		t.Errorf("%d Process calls worked at once, want at most %d", most, limit)
-	}
-}
-
-// TestProcessWaitsThenFailsWhenNoSlotFrees holds the only slot and checks
-// that another call waits the whole wait timeout, then fails with
-// ErrTooManyImages instead of processing anyway.
-func TestProcessWaitsThenFailsWhenNoSlotFrees(t *testing.T) {
-	t.Parallel()
-
-	proc := New(Params{MaxConcurrentProcessing: 1})
-	proc.processingWaitTimeout = 100 * time.Millisecond
-
-	input := createTestJPEG(t, 10, 10)
-
-	gate := make(chan struct{})
-	entered := make(chan struct{}, 1)
-	held := make(chan error, 1)
-
-	openGate := sync.OnceFunc(func() { close(gate) })
-	t.Cleanup(openGate)
-
-	processInBackground(proc, &gatedReader{
-		data: bytes.NewReader(input), gate: gate, entered: entered,
-		counter: &readingCounter{},
-	}, held)
-	waitForEntries(t, entered, 1)
-
-	start := time.Now()
-
-	_, err := proc.Process(context.Background(), bytes.NewReader(input),
-		smallJPEGRequest())
-	if !errors.Is(err, ErrTooManyImages) {
-		t.Fatalf("Process() error = %v, want ErrTooManyImages", err)
-	}
-
-	if waited := time.Since(start); waited < proc.processingWaitTimeout {
-		t.Errorf("Process() failed after %v, before waiting %v",
-			waited, proc.processingWaitTimeout)
-	}
-
-	openGate()
-
-	err = <-held
-	if err != nil {
-		t.Errorf("Process() holding the slot: error = %v, want nil", err)
-	}
-}
-
-// TestProcessReleasesSlotOnError checks that Process gives its slot back
-// when it fails, whether it fails early or late: with one slot, the slot
-// must be free after the failure and the next call must succeed.
-func TestProcessReleasesSlotOnError(t *testing.T) {
-	t.Parallel()
-
-	valid := createTestJPEG(t, 10, 10)
-
-	unsupported := smallJPEGRequest()
-	unsupported.Format = "bmp"
-
-	cases := []struct {
-		name  string
-		input io.Reader
-		req   *Request
-		// want is the error Process must return; nil means any error.
-		want error
-	}{
-		{
-			name:  "input cannot be read",
-			input: iotest.ErrReader(errTestReadFailed),
-			req:   smallJPEGRequest(),
-			want:  errTestReadFailed,
-		},
-		{
-			name:  "input over the byte limit",
-			input: bytes.NewReader(createTestJPEG(t, 800, 600)),
-			req:   smallJPEGRequest(),
-			want:  ErrInputDataTooLarge,
-		},
-		{
-			name:  "input not an image",
-			input: strings.NewReader("not an image"),
-			req:   smallJPEGRequest(),
-		},
-		{
-			name:  "output format not supported",
-			input: bytes.NewReader(valid),
-			req:   unsupported,
-			want:  ErrUnsupportedOutputFormat,
-		},
-	}
-
-	for _, tc := range cases {
-		t.Run(tc.name, func(t *testing.T) {
-			t.Parallel()
-
-			proc := New(Params{MaxInputBytes: 4096, MaxConcurrentProcessing: 1})
-			proc.processingWaitTimeout = 100 * time.Millisecond
-
-			_, err := proc.Process(context.Background(), tc.input, tc.req)
-			if err == nil || (tc.want != nil && !errors.Is(err, tc.want)) {
-				t.Fatalf("Process() error = %v, want %v", err, tc.want)
-			}
-
-			if held := len(proc.processingSemaphore); held != 0 {
-				t.Fatalf("slot still held after the error: %d held", held)
-			}
-
-			result, err := proc.Process(context.Background(), bytes.NewReader(valid),
-				smallJPEGRequest())
-			if err != nil {
-				t.Fatalf("Process() after the error = %v, want nil", err)
-			}
-
-			_ = result.Content.Close()
-		})
-	}
-}

internal/imgcache/cache.go

@@ -14,7 +14,6 @@ import (
 	"sync"
 	"time"
 
-	lru "github.com/hashicorp/golang-lru/v2"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
 )
 
@@ -27,10 +26,6 @@ var (
 // HTTP status code for successful fetch.
 const httpStatusOK = 200
 
-// metaCacheSize is how many variants' content types metaCache holds. A
-// variant not among them is served as before, reading its .meta file.
-const metaCacheSize = 10000
-
 // CacheConfig holds cache configuration.
 type CacheConfig struct {
 	StateDir    string
@@ -54,6 +49,12 @@ type CacheConfig struct {
 	Logger *slog.Logger
 }
 
+// variantMeta stores content type for fast cache hits without reading .meta file.
+type variantMeta struct {
+	ContentType string
+	Size        int64
+}
+
 // Cache implements the caching layer for the image proxy.
 type Cache struct {
 	db          *sql.DB
@@ -75,10 +76,9 @@ type Cache struct {
 	evictionStarted  bool
 	evictionStopOnce sync.Once
 
-	// metaCache holds the content types of the variants most recently
+	// In-memory cache of variant metadata (content type, size) to avoid
-	// stored or served, so a hit does not read the variant's .meta file.
+	// reading .meta files
-	// It never stands in for the variant file, which is always opened.
+	metaCache map[VariantKey]variantMeta
-	metaCache *lru.Cache[VariantKey, string]
 
 	// contentLocks serializes StoreSource and evictSourceBlob per
 	// content hash, closing the race window between an eviction's row
@@ -101,11 +101,6 @@ func NewCache(db *sql.DB, config CacheConfig) (*Cache, error) {
 		log = slog.Default()
 	}
 
-	metaCache, err := lru.New[VariantKey, string](metaCacheSize)
-	if err != nil {
-		return nil, fmt.Errorf("failed to create variant content type cache: %w", err)
-	}
-
 	c := &Cache{
 		db:               db,
 		config:           config,
@@ -114,7 +109,7 @@ func NewCache(db *sql.DB, config CacheConfig) (*Cache, error) {
 		evictionPressure: make(chan struct{}, 1),
 		evictionStop:     make(chan struct{}),
 		evictionDone:     make(chan struct{}),
-		metaCache:        metaCache,
+		metaCache:        make(map[VariantKey]variantMeta),
 		contentLocks:     newContentLock(),
 	}
 
@@ -182,30 +177,13 @@ func (c *Cache) Lookup(ctx context.Context, req *ImageRequest) (*LookupResult, e
 	}, nil
 }
 
-// GetVariant returns a reader, size, and content type for a cached
+// GetVariant returns a reader, size, and content type for a cached variant.
-// variant. The content type comes from metaCache, or else from the
-// variant's .meta file and is then kept in metaCache. A variant with
-// no .meta file is served as application/octet-stream, which is not
-// kept.
 func (c *Cache) GetVariant(cacheKey VariantKey) (io.ReadCloser, int64, string, error) {
 	if c.disabled {
 		return nil, 0, "", ErrNotFound
 	}
 
-	contentType, known := c.metaCache.Get(cacheKey)
+	return c.variants.LoadWithMeta(cacheKey)
-	if !known {
-		return c.loadVariantWithMeta(cacheKey)
-	}
-
-	reader, size, err := c.variants.LoadWithSize(cacheKey)
-	if err != nil {
-		// The file is gone, e.g. deleted outside pixa
-		c.metaCache.Remove(cacheKey)
-
-		return nil, 0, "", err
-	}
-
-	return reader, size, contentType, nil
 }
 
 // StoreSource stores fetched source content and metadata. On a
@@ -308,8 +286,6 @@ func (c *Cache) StoreVariant(
 		return err
 	}
 
-	c.metaCache.Add(cacheKey, contentType)
-
 	_, err = c.db.ExecContext(ctx, `
 		INSERT INTO variant_content (cache_key, size_bytes, content_type)
 		VALUES (?, ?, ?)
@@ -407,16 +383,13 @@ func (c *Cache) GetSourceMetadataID(
 	return id, nil
 }
 
-// GetSourceContent returns a reader for cached source content by its hash,
+// GetSourceContent returns a reader for cached source content by its hash.
-// and the content's size in bytes.
+func (c *Cache) GetSourceContent(contentHash ContentHash) (io.ReadCloser, error) {
-func (c *Cache) GetSourceContent(
-	contentHash ContentHash,
-) (io.ReadCloser, int64, error) {
 	if c.disabled {
-		return nil, 0, ErrNotFound
+		return nil, ErrNotFound
 	}
 
-	return c.srcContent.LoadWithSize(contentHash)
+	return c.srcContent.Load(contentHash)
 }
 
 // CleanExpired removes expired entries from the cache.
@@ -523,31 +496,6 @@ func (c *Cache) IncrementTransformCount(ctx context.Context) {
 	}
 }
 
-// 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.
-func (c *Cache) loadVariantWithMeta(
-	cacheKey VariantKey,
-) (io.ReadCloser, int64, string, error) {
-	reader, size, contentType, err := c.variants.LoadWithMeta(cacheKey)
-	if err != nil {
-		return nil, 0, "", err
-	}
-
-	if contentType == "" {
-		return reader, size, fallbackContentType, nil
-	}
-
-	c.metaCache.ContainsOrAdd(cacheKey, contentType)
-
-	return reader, size, contentType, nil
-}
-
 // writeMetadataSidecar writes the JSON metadata sidecar of a stored source.
 // A failure is logged and is otherwise non-fatal; the metadata is in the
 // database.

internal/imgcache/eviction.go

@@ -39,8 +39,8 @@ const tempFilePrefix = ".tmp-"
 // to each variant file.
 const variantMetaSuffix = ".meta"
 
-// fallbackContentType is the content type given to a variant file that
+// fallbackContentType is recorded when a reconciled variant file has
-// has no readable .meta sidecar, when it is served or reconciled.
+// no readable .meta sidecar.
 const fallbackContentType = "application/octet-stream"
 
 // UsageBytes returns the total number of bytes of cache content
@@ -271,9 +271,7 @@ func (c *Cache) sourceCandidates(ctx context.Context) ([]evictionCandidate, erro
 
 // evictVariant removes one variant: accounting row first, then the
 // content and .meta files, so the database never references a deleted
-// file. The metaCache entry goes before the files; a GetVariant that
+// file.
-// read them just before may put it back, and the next GetVariant then
-// fails to open the file and removes it again.
 func (c *Cache) evictVariant(ctx context.Context, cacheKey VariantKey) error {
 	_, err := c.db.ExecContext(ctx,
 		`DELETE FROM variant_content WHERE cache_key = ?`, string(cacheKey))
@@ -281,8 +279,6 @@ func (c *Cache) evictVariant(ctx context.Context, cacheKey VariantKey) error {
 		return fmt.Errorf("failed to delete variant accounting row: %w", err)
 	}
 
-	c.metaCache.Remove(cacheKey)
-
 	err = c.variants.DeleteWithMeta(cacheKey)
 	if err != nil {
 		return err

internal/imgcache/max_concurrent_processing_internal_test.go

@@ -1,125 +0,0 @@
-package imgcache
-
-import (
-	"image/color"
-	"image/jpeg"
-	"io"
-	"os"
-	"testing"
-	"time"
-
-	"sneak.berlin/go/pixa/internal/imageprocessor"
-)
-
-// widthOnlyRequest asks for the test photo at width, its height scaled to
-// keep the photo's aspect ratio.
-func widthOnlyRequest(fixtures *TestFixtures, width int) *ImageRequest {
-	return &ImageRequest{
-		SourceHost: fixtures.GoodHost,
-		SourcePath: testPathPhoto,
-		Size:       Size{Width: width},
-		Format:     FormatJPEG,
-		Quality:    85,
-		FitMode:    FitCover,
-	}
-}
-
-// holdProcessingSlot takes one of proc's processing slots and returns the
-// func that gives it back. Process takes its slot before it reads its input,
-// so once it has read a byte from the pipe it holds the slot, until the pipe
-// is closed.
-func holdProcessingSlot(
-	t *testing.T, proc *imageprocessor.ImageProcessor,
-) func() {
-	t.Helper()
-
-	input, feed := io.Pipe()
-
-	go func() {
-		_, _ = proc.Process(t.Context(), input, &imageprocessor.Request{})
-	}()
-
-	_, err := feed.Write([]byte{0})
-	if err != nil {
-		t.Fatalf("Process call to hold the slot did not start: %v", err)
-	}
-
-	release := func() { _ = feed.Close() }
-	t.Cleanup(release)
-
-	return release
-}
-
-// TestService_Get_WaitsForSlotBeforeReadingCachedSource checks that a
-// request whose source is cached holds none of it while it waits for a
-// processing slot: it reads the cached file only once it has a slot. With
-// the only slot held, a request for a new width of the cached 100x100 photo
-// waits; the cached file is then rewritten as a 100x50 image before the slot
-// is freed, so the request must answer with that image scaled to 40x20.
-func TestService_Get_WaitsForSlotBeforeReadingCachedSource(t *testing.T) {
-	t.Parallel()
-
-	svc, fixtures := SetupTestService(t)
-	svc.processor = imageprocessor.New(
-		imageprocessor.Params{MaxConcurrentProcessing: 1},
-	)
-
-	// A first request caches the photo as a source.
-	resp, err := svc.Get(t.Context(), widthOnlyRequest(fixtures, 50))
-	if err != nil {
-		t.Fatalf("first Get() error = %v", err)
-	}
-
-	_ = resp.Content.Close()
-
-	contentHash, _, err := svc.cache.LookupSource(t.Context(),
-		widthOnlyRequest(fixtures, 50))
-	if err != nil || contentHash == "" {
-		t.Fatalf("LookupSource() = %q, %v; want the cached source",
-			contentHash, err)
-	}
-
-	release := holdProcessingSlot(t, svc.processor)
-
-	var (
-		waited    *ImageResponse
-		waitedErr error
-	)
-
-	done := make(chan struct{})
-
-	go func() {
-		defer close(done)
-
-		waited, waitedErr = svc.Get(t.Context(), widthOnlyRequest(fixtures, 40))
-	}()
-
-	// Give the request time to reach the slot: had it read the cached source
-	// before waiting, it would have read it by now.
-	time.Sleep(100 * time.Millisecond)
-
-	err = os.WriteFile(svc.cache.srcContent.hashToPath(contentHash),
-		generateTestJPEG(t, 100, 50, color.RGBA{0, 0, 255, 255}), 0o600)
-	if err != nil {
-		t.Fatalf("failed to rewrite the cached source: %v", err)
-	}
-
-	release()
-	<-done
-
-	if waitedErr != nil {
-		t.Fatalf("Get() error = %v", waitedErr)
-	}
-
-	defer func() { _ = waited.Content.Close() }()
-
-	output, err := jpeg.DecodeConfig(waited.Content)
-	if err != nil {
-		t.Fatalf("failed to decode the response: %v", err)
-	}
-
-	if output.Width != 40 || output.Height != 20 {
-		t.Errorf("response is %dx%d, want 40x20: the request read the cached "+
-			"source before it had a processing slot", output.Width, output.Height)
-	}
-}

internal/imgcache/metacache_internal_test.go

@@ -1,349 +0,0 @@
-package imgcache
-
-import (
-	"bytes"
-	"encoding/json"
-	"errors"
-	"fmt"
-	"io"
-	"os"
-	"sync"
-	"testing"
-	"time"
-)
-
-// webpRequest returns a request for a 100x100 WebP variant of path.
-func webpRequest(path string) *ImageRequest {
-	return &ImageRequest{
-		SourceHost: testHostCDN,
-		SourcePath: path,
-		Size:       Size{Width: 100, Height: 100},
-		Format:     FormatWebP,
-		Quality:    85,
-		FitMode:    FitCover,
-	}
-}
-
-// assertVariantServed checks that GetVariant serves key with the given
-// content and the image/webp content type storeEvictionTestVariant stores.
-func assertVariantServed(t *testing.T, cache *Cache, key VariantKey, content []byte) {
-	t.Helper()
-
-	reader, size, contentType, err := cache.GetVariant(key)
-	if err != nil {
-		t.Fatalf("GetVariant(%s) error = %v", key, err)
-	}
-
-	defer func() { _ = reader.Close() }()
-
-	got, err := io.ReadAll(reader)
-	if err != nil {
-		t.Fatalf("reading variant %s: %v", key, err)
-	}
-
-	if !bytes.Equal(got, content) {
-		t.Errorf("GetVariant(%s) content = %q, want %q", key, got, content)
-	}
-
-	if size != int64(len(content)) {
-		t.Errorf("GetVariant(%s) size = %d, want %d", key, size, len(content))
-	}
-
-	if contentType != testContentTypeWebP {
-		t.Errorf("GetVariant(%s) content type = %q, want %q",
-			key, contentType, testContentTypeWebP)
-	}
-}
-
-// assertVariantNotFound checks that GetVariant refuses key with
-// ErrNotFound.
-func assertVariantNotFound(t *testing.T, cache *Cache, key VariantKey) {
-	t.Helper()
-
-	reader, _, _, err := cache.GetVariant(key)
-	if err == nil {
-		_ = reader.Close()
-	}
-
-	if !errors.Is(err, ErrNotFound) {
-		t.Errorf("GetVariant(%s) error = %v, want ErrNotFound", key, err)
-	}
-}
-
-// assertLookupMisses checks that Lookup reports request as a miss.
-func assertLookupMisses(t *testing.T, cache *Cache, request *ImageRequest) {
-	t.Helper()
-
-	lookup, err := cache.Lookup(t.Context(), request)
-	if err != nil {
-		t.Fatalf("Lookup(%s) error = %v", request.SourcePath, err)
-	}
-
-	if lookup.Hit {
-		t.Errorf("Lookup(%s) is a hit, want a miss", request.SourcePath)
-	}
-}
-
-// renameFile renames the file at from to to.
-func renameFile(t *testing.T, from, to string) {
-	t.Helper()
-
-	err := os.Rename(from, to)
-	if err != nil {
-		t.Fatalf("renaming %s: %v", from, err)
-	}
-}
-
-// TestSecondHitDoesNotReadMetaFile checks that once a variant has been
-// stored or read, a hit takes its content type from memory: with the
-// .meta file deleted, GetVariant must still return the stored content
-// type rather than the application/octet-stream it uses without one.
-func TestSecondHitDoesNotReadMetaFile(t *testing.T) {
-	t.Parallel()
-
-	cache, _ := newEvictionTestCache(t, 1<<20)
-	content := []byte("webp variant bytes")
-
-	storeEvictionTestVariant(t, cache, testVariantKeyOne, content)
-
-	// A second Cache on the same state directory starts with nothing in
-	// memory, as pixad does after a restart, so its first read uses the
-	// .meta file.
-	restarted, err := NewCache(cache.db, cache.config)
-	if err != nil {
-		t.Fatalf("NewCache() error = %v", err)
-	}
-
-	assertVariantServed(t, restarted, testVariantKeyOne, content)
-
-	err = os.Remove(cache.variants.keyToPath(testVariantKeyOne) + ".meta")
-	if err != nil {
-		t.Fatalf("removing .meta file: %v", err)
-	}
-
-	assertVariantServed(t, cache, testVariantKeyOne, content)
-	assertVariantServed(t, restarted, testVariantKeyOne, content)
-}
-
-// TestReadDuringStoreKeepsStoredContentType checks that a GetVariant
-// which began before StoreVariant finished cannot replace the content
-// type the store kept in memory. Such a read can find the variant file
-// but not yet its .meta file, and so gets application/octet-stream. The
-// test deletes the .meta file after the store, then runs the part of
-// GetVariant that comes after its check of memory.
-func TestReadDuringStoreKeepsStoredContentType(t *testing.T) {
-	t.Parallel()
-
-	cache, _ := newEvictionTestCache(t, 1<<20)
-	content := []byte("webp variant bytes")
-
-	storeEvictionTestVariant(t, cache, testVariantKeyOne, content)
-
-	err := os.Remove(cache.variants.keyToPath(testVariantKeyOne) + ".meta")
-	if err != nil {
-		t.Fatalf("removing .meta file: %v", err)
-	}
-
-	reader, _, contentType, err := cache.loadVariantWithMeta(testVariantKeyOne)
-	if err != nil {
-		t.Fatalf("loadVariantWithMeta(%s) error = %v", testVariantKeyOne, err)
-	}
-
-	_ = reader.Close()
-
-	t.Logf("the read without a .meta file got content type %q", contentType)
-
-	assertVariantServed(t, cache, testVariantKeyOne, content)
-}
-
-// TestReadOfOlderMetaFileKeepsStoredContentType checks that a read
-// which got its content type from a .meta file that StoreVariant had
-// not yet rewritten cannot replace the type the store kept in memory.
-// The test writes such a .meta file, with a different content type,
-// after the store, then runs the part of GetVariant that comes after
-// its check of memory.
-func TestReadOfOlderMetaFileKeepsStoredContentType(t *testing.T) {
-	t.Parallel()
-
-	cache, _ := newEvictionTestCache(t, 1<<20)
-	content := []byte("webp variant bytes")
-
-	storeEvictionTestVariant(t, cache, testVariantKeyOne, content)
-
-	olderMeta, err := json.Marshal(VariantMeta{
-		ContentType: testContentTypeJPEG,
-		Size:        int64(len(content)),
-	})
-	if err != nil {
-		t.Fatalf("encoding .meta file: %v", err)
-	}
-
-	metaPath := cache.variants.keyToPath(testVariantKeyOne) + ".meta"
-
-	err = os.WriteFile(metaPath, olderMeta, StorageFilePerm)
-	if err != nil {
-		t.Fatalf("writing .meta file: %v", err)
-	}
-
-	reader, _, contentType, err := cache.loadVariantWithMeta(testVariantKeyOne)
-	if err != nil {
-		t.Fatalf("loadVariantWithMeta(%s) error = %v", testVariantKeyOne, err)
-	}
-
-	_ = reader.Close()
-
-	if contentType != testContentTypeJPEG {
-		t.Fatalf("loadVariantWithMeta(%s) content type = %q, want %q from the .meta file",
-			testVariantKeyOne, contentType, testContentTypeJPEG)
-	}
-
-	kept, _ := cache.metaCache.Get(testVariantKeyOne)
-	if kept != testContentTypeWebP {
-		t.Errorf("content type in memory = %q, want the stored %q",
-			kept, testContentTypeWebP)
-	}
-}
-
-// TestFailedReadDuringStoreKeepsStoredContentType checks that a read
-// which found no .meta file cannot leave application/octet-stream in
-// memory, even when another read has removed the content type
-// StoreVariant kept there. In this order: the store; a read that finds
-// the variant in memory but cannot open its file, and so removes it
-// from memory; a read that opened the variant file before the store
-// wrote its .meta file. Later hits must get the stored content type.
-func TestFailedReadDuringStoreKeepsStoredContentType(t *testing.T) {
-	t.Parallel()
-
-	cache, _ := newEvictionTestCache(t, 1<<20)
-	content := []byte("webp variant bytes")
-	variantPath := cache.variants.keyToPath(testVariantKeyOne)
-	metaPath := variantPath + ".meta"
-
-	storeEvictionTestVariant(t, cache, testVariantKeyOne, content)
-
-	renameFile(t, variantPath, variantPath+".hidden")
-	assertVariantNotFound(t, cache, testVariantKeyOne)
-	renameFile(t, variantPath+".hidden", variantPath)
-
-	renameFile(t, metaPath, metaPath+".hidden")
-
-	reader, _, contentType, err := cache.GetVariant(testVariantKeyOne)
-	if err != nil {
-		t.Fatalf("GetVariant(%s) error = %v", testVariantKeyOne, err)
-	}
-
-	_ = reader.Close()
-
-	t.Logf("the read without a .meta file got content type %q", contentType)
-
-	renameFile(t, metaPath+".hidden", metaPath)
-
-	assertVariantServed(t, cache, testVariantKeyOne, content)
-}
-
-// TestEvictedVariantIsNotServed checks that a variant the evictor
-// removed is a miss and cannot be read, although it had been stored
-// and served before.
-func TestEvictedVariantIsNotServed(t *testing.T) {
-	t.Parallel()
-
-	cache, _ := newEvictionTestCache(t, 1500)
-
-	oldRequest := webpRequest("/old.jpg")
-	newRequest := webpRequest("/new.jpg")
-	oldKey := CacheKey(oldRequest)
-	newKey := CacheKey(newRequest)
-	oldContent := bytes.Repeat([]byte{0x01}, 1000)
-	newContent := bytes.Repeat([]byte{0x02}, 1000)
-
-	storeEvictionTestVariant(t, cache, oldKey, oldContent)
-	storeEvictionTestVariant(t, cache, newKey, newContent)
-	assertVariantServed(t, cache, oldKey, oldContent)
-	setVariantLastAccessed(t, cache, oldKey, time.Now().Add(-time.Hour))
-
-	err := cache.EvictToLimit(t.Context())
-	if err != nil {
-		t.Fatalf("EvictToLimit() error = %v", err)
-	}
-
-	assertLookupMisses(t, cache, oldRequest)
-	assertVariantNotFound(t, cache, oldKey)
-	assertVariantServed(t, cache, newKey, newContent)
-}
-
-// TestVariantDeletedFromDiskIsNotServed checks that a variant whose
-// file was deleted by something other than the evictor cannot be read,
-// and is a miss afterwards.
-func TestVariantDeletedFromDiskIsNotServed(t *testing.T) {
-	t.Parallel()
-
-	cache, _ := newEvictionTestCache(t, 1<<20)
-
-	request := webpRequest("/deleted.jpg")
-	key := CacheKey(request)
-	content := []byte("webp variant bytes")
-
-	storeEvictionTestVariant(t, cache, key, content)
-	assertVariantServed(t, cache, key, content)
-
-	err := os.Remove(cache.variants.keyToPath(key))
-	if err != nil {
-		t.Fatalf("removing variant file: %v", err)
-	}
-
-	assertVariantNotFound(t, cache, key)
-	assertLookupMisses(t, cache, request)
-}
-
-// TestConcurrentVariantStoreReadAndEvict stores, reads and evicts
-// variants from several goroutines at once, for the race detector.
-func TestConcurrentVariantStoreReadAndEvict(t *testing.T) {
-	t.Parallel()
-
-	cache, _ := newEvictionTestCache(t, 1<<20)
-	ctx := t.Context()
-
-	var wg sync.WaitGroup
-
-	for goroutine := range 8 {
-		wg.Go(func() {
-			key := VariantKey(fmt.Sprintf("aabbccdd01%02d", goroutine))
-			content := []byte(key)
-
-			for range 20 {
-				err := cache.StoreVariant(
-					ctx, key, bytes.NewReader(content), testContentTypeWebP)
-				if err != nil {
-					t.Errorf("StoreVariant(%s) error = %v", key, err)
-
-					return
-				}
-
-				reader, _, contentType, err := cache.GetVariant(key)
-				if err != nil {
-					t.Errorf("GetVariant(%s) error = %v", key, err)
-
-					return
-				}
-
-				_ = reader.Close()
-
-				if contentType != testContentTypeWebP {
-					t.Errorf("GetVariant(%s) content type = %q, want %q",
-						key, contentType, testContentTypeWebP)
-				}
-
-				err = cache.evictVariant(ctx, key)
-				if err != nil {
-					t.Errorf("evictVariant(%s) error = %v", key, err)
-
-					return
-				}
-
-				assertVariantNotFound(t, cache, key)
-			}
-		})
-	}
-
-	wg.Wait()
-}

internal/imgcache/service.go

@@ -43,9 +43,6 @@ type ServiceConfig struct {
 	SigningKey string
 	// 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
-	MaxConcurrentProcessing int
 	// Logger for logging
 	Logger *slog.Logger
 }
@@ -94,10 +91,9 @@ func NewService(cfg *ServiceConfig) (*Service, error) {
 	}
 
 	maxResponseSize := fetcherCfg.MaxResponseSize
-	processor := imageprocessor.New(imageprocessor.Params{
+	processor := imageprocessor.New(
-		MaxInputBytes:           maxResponseSize,
+		imageprocessor.Params{MaxInputBytes: maxResponseSize},
-		MaxConcurrentProcessing: cfg.MaxConcurrentProcessing,
+	)
-	})
 
 	return &Service{
 		cache:           cfg.Cache,
@@ -241,37 +237,38 @@ func (s *Service) GenerateSignedURL(
 		baseURL, path, sig, exp, req.Quality, req.FitMode), nil
 }
 
-// loadCachedSource opens source content from cache, without reading it, and
+// loadCachedSource attempts to load source content from cache, returning nil
-// returns it with its size; nil if the cached data is unavailable, empty or
+// if the cached data is unavailable or exceeds maxResponseSize.
-// exceeds maxResponseSize.
+func (s *Service) loadCachedSource(contentHash ContentHash) []byte {
-func (s *Service) loadCachedSource(
+	reader, err := s.cache.GetSourceContent(contentHash)
-	contentHash ContentHash,
-) (io.ReadCloser, int64) {
-	reader, size, err := s.cache.GetSourceContent(contentHash)
 	if err != nil {
 		s.log.Warn("failed to load cached source, fetching", "error", err)
 
-		return nil, 0
+		return nil
 	}
 
-	if size > s.maxResponseSize {
+	// Bound the read to maxResponseSize to prevent unbounded memory use
-		_ = reader.Close()
+	// from unexpectedly large cached files.
+	limited := io.LimitReader(reader, s.maxResponseSize+1)
+	data, err := io.ReadAll(limited)
+	_ = reader.Close()
 
+	if err != nil {
+		s.log.Warn("failed to read cached source, fetching", "error", err)
+
+		return nil
+	}
+
+	if int64(len(data)) > s.maxResponseSize {
 		s.log.Warn("cached source exceeds max response size, discarding",
 			"hash", contentHash,
 			"max_bytes", s.maxResponseSize,
 		)
 
-		return nil, 0
+		return nil
 	}
 
-	if size == 0 {
+	return data
-		_ = reader.Close()
-
-		return nil, 0
-	}
-
-	return reader, size
 }
 
 // processFromSourceOrFetch processes an image, using cached source content
@@ -288,27 +285,22 @@ func (s *Service) processFromSourceOrFetch(
 		s.log.Warn("source lookup failed", "error", err)
 	}
 
-	var (
+	var sourceData []byte
-		source     io.ReadCloser
-		sourceSize int64
-	)
 
 	if contentHash != "" {
 		s.log.Debug("using cached source", "hash", contentHash)
-		source, sourceSize = s.loadCachedSource(contentHash)
+		sourceData = s.loadCachedSource(contentHash)
 	}
 
 	// Fetch from upstream if we don't have source data or it's empty
-	if source == nil {
+	if len(sourceData) == 0 {
 		return s.fetchAndProcess(ctx, req, cacheKey)
 	}
 
-	defer func() { _ = source.Close() }()
+	// Process using cached source; nothing was fetched from upstream
-
+	resp, err := s.processAndStore(
-	// Process using cached source; nothing was fetched from upstream. The
+		ctx, req, cacheKey, sourceData, int64(len(sourceData)),
-	// image processor reads the source only once it has a processing slot,
+	)
-	// so a request waiting for one holds none of it in memory.
-	resp, err := s.processAndStore(ctx, req, cacheKey, source, sourceSize)
 
 	return resp, 0, err
 }
@@ -342,10 +334,6 @@ func (s *Service) fetchAndProcess(
 		return nil, 0, fmt.Errorf("upstream fetch failed: %w", err)
 	}
 
-	// Closing the body frees the upstream connection. It is closed only
-	// after processing, so the fetcher's connection limit also bounds the
-	// fetched sources held in memory while their requests wait for a
-	// processing slot.
 	defer func() { _ = fetchResult.Content.Close() }()
 
 	// Read and validate the source content
@@ -391,20 +379,17 @@ func (s *Service) fetchAndProcess(
 		// Continue even if caching fails
 	}
 
-	resp, err := s.processAndStore(
+	resp, err := s.processAndStore(ctx, req, cacheKey, sourceData, fetchBytes)
-		ctx, req, cacheKey, bytes.NewReader(sourceData), fetchBytes,
-	)
 
 	return resp, fetchBytes, err
 }
 
-// processAndStore processes the image read from source and stores the
+// processAndStore processes an image and stores the result.
-// result.
 func (s *Service) processAndStore(
 	ctx context.Context,
 	req *ImageRequest,
 	cacheKey VariantKey,
-	source io.Reader,
+	sourceData []byte,
 	fetchBytes int64,
 ) (*ImageResponse, error) {
 	// Process the image
@@ -417,7 +402,7 @@ func (s *Service) processAndStore(
 		FitMode: imageprocessor.FitMode(req.FitMode),
 	}
 
-	processResult, err := s.processor.Process(ctx, source, processReq)
+	processResult, err := s.processor.Process(ctx, bytes.NewReader(sourceData), processReq)
 	if err != nil {
 		return nil, fmt.Errorf("image processing failed: %w", err)
 	}

internal/imgcache/storage.go

@@ -506,44 +506,32 @@ func (s *VariantStorage) Load(key VariantKey) (io.ReadCloser, error) {
 	return f, nil
 }
 
-// LoadWithSize returns a reader and file size for the content at the
+// LoadWithMeta returns a reader, size, and content type for the content at
-// given key.
+// the given key.
-func (s *VariantStorage) LoadWithSize(key VariantKey) (io.ReadCloser, int64, error) {
+func (s *VariantStorage) LoadWithMeta(
+	key VariantKey,
+) (io.ReadCloser, int64, string, error) {
 	path := s.keyToPath(key)
+	metaPath := path + ".meta"
 
 	f, err := os.Open(path) //nolint:gosec // path derived from cache key
 	if err != nil {
 		if os.IsNotExist(err) {
-			return nil, 0, ErrNotFound
+			return nil, 0, "", ErrNotFound
 		}
 
-		return nil, 0, fmt.Errorf("failed to open content: %w", err)
+		return nil, 0, "", fmt.Errorf("failed to open content: %w", err)
 	}
 
 	stat, err := f.Stat()
 	if err != nil {
 		_ = f.Close()
 
-		return nil, 0, fmt.Errorf("failed to stat content: %w", err)
+		return nil, 0, "", fmt.Errorf("failed to stat content: %w", err)
 	}
 
-	return f, stat.Size(), nil
+	// Load metadata for content type
-}
+	contentType := "application/octet-stream" // fallback
-
-// LoadWithMeta returns a reader, size, and content type for the content at
-// the given key. The content type is read from the .meta file, and is
-// empty when that file is missing or unreadable.
-func (s *VariantStorage) LoadWithMeta(
-	key VariantKey,
-) (io.ReadCloser, int64, string, error) {
-	f, size, err := s.LoadWithSize(key)
-	if err != nil {
-		return nil, 0, "", err
-	}
-
-	var contentType string
-
-	metaPath := s.keyToPath(key) + ".meta"
 
 	metaData, err := os.ReadFile(metaPath) //nolint:gosec // path derived from cache key
 	if err == nil {
@@ -553,7 +541,7 @@ func (s *VariantStorage) LoadWithMeta(
 		}
 	}
 
-	return f, size, contentType, nil
+	return f, stat.Size(), contentType, nil
 }
 
 // Exists checks if content exists at the given key.

internal/imgcache/testutil_internal_test.go

@@ -24,7 +24,6 @@ const (
 	testHostExample       = "example.com"
 	testPathCat           = "/photos/cat.jpg"
 	testContentTypeJPEG   = "image/jpeg"
-	testContentTypeWebP   = "image/webp"
 	testHeaderContentType = "Content-Type"
 )
 

internal/server/cors_internal_test.go

@@ -1,64 +0,0 @@
-package server
-
-import (
-	"net/http"
-	"net/http/httptest"
-	"testing"
-)
-
-// TestCORSOnlyOnImageRoutes verifies that the image routes answer with the
-// configured access_control_allow_origin, a preflight request included, and
-// that the login and URL generator pages send no Access-Control-Allow-Origin,
-// so no other site can read them. /metrics is left out: its middleware
-// registers with the process-wide Prometheus registry, which only one test
-// in this package can do.
-func TestCORSOnlyOnImageRoutes(t *testing.T) {
-	t.Parallel()
-
-	const appOrigin = "https://app.example.com"
-
-	s := newTestServer(t)
-	s.config.AccessControlAllowOrigin = appOrigin
-	s.SetupRoutes()
-
-	requests := []struct {
-		method string
-		path   string
-		want   string
-	}{
-		{http.MethodGet, unsignedImagePath, appOrigin},
-		{http.MethodHead, unsignedImagePath, appOrigin},
-		{http.MethodOptions, unsignedImagePath, appOrigin},
-		{http.MethodGet, encryptedImagePath, appOrigin},
-		{http.MethodGet, "/", ""},
-		{http.MethodOptions, "/", ""},
-		{http.MethodPost, "/generate", ""},
-		{http.MethodGet, "/logout", ""},
-	}
-
-	for _, tc := range requests {
-		t.Run(tc.method+" "+tc.path, func(t *testing.T) {
-			t.Parallel()
-
-			req := httptest.NewRequestWithContext(
-				t.Context(), tc.method, tc.path, nil)
-			req.Header.Set("Origin", appOrigin)
-
-			// An OPTIONS request naming the method it asks about is the
-			// preflight a browser sends before some cross-origin requests.
-			if tc.method == http.MethodOptions {
-				req.Header.Set("Access-Control-Request-Method", http.MethodGet)
-			}
-
-			rec := httptest.NewRecorder()
-			s.ServeHTTP(rec, req)
-			t.Logf("status %d", rec.Code)
-
-			got := rec.Header().Get("Access-Control-Allow-Origin")
-			if got != tc.want {
-				t.Errorf("Access-Control-Allow-Origin = %q, want %q",
-					got, tc.want)
-			}
-		})
-	}
-}

internal/server/login_rate_limit_internal_test.go

@@ -18,7 +18,6 @@ import (
 	"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"
 )
@@ -72,15 +71,8 @@ func newTestServer(t *testing.T) *Server {
 		t.Fatalf("database.New() error = %v", err)
 	}
 
-	hc, err := healthcheck.New(lc, healthcheck.Params{
-		Globals: &globals.Globals{}, Config: cfg, Logger: log, Database: db,
-	})
-	if err != nil {
-		t.Fatalf("healthcheck.New() error = %v", err)
-	}
-
 	h, err := handlers.New(lc, handlers.Params{
-		Logger: log, Healthcheck: hc, Database: db, Config: cfg,
+		Logger: log, Database: db, Config: cfg,
 	})
 	if err != nil {
 		t.Fatalf("handlers.New() error = %v", err)

internal/server/maintenance_internal_test.go

@@ -1,173 +0,0 @@
-package server
-
-import (
-	"encoding/json"
-	"net/http"
-	"net/http/httptest"
-	"strconv"
-	"testing"
-
-	"sneak.berlin/go/pixa/internal/healthcheck"
-)
-
-// unsignedImagePath is an image URL that carries no signature.
-const unsignedImagePath = "/v1/image/cdn.example.com/cat.jpg/100x100.jpeg"
-
-// encryptedImagePath is an encrypted image URL whose token cannot be
-// decrypted.
-const encryptedImagePath = "/v1/e/token/cat.jpg"
-
-// TestMaintenanceModeRefusesImageRequests verifies that while maintenance
-// mode is on, both image routes answer 503 Service Unavailable with a
-// Retry-After header and the JSON error body the image handlers send.
-func TestMaintenanceModeRefusesImageRequests(t *testing.T) {
-	t.Parallel()
-
-	s := newTestServer(t)
-	s.config.MaintenanceMode = true
-
-	requests := []struct {
-		method string
-		path   string
-	}{
-		{http.MethodGet, unsignedImagePath},
-		{http.MethodHead, unsignedImagePath},
-		{http.MethodGet, encryptedImagePath},
-	}
-
-	for _, tc := range requests {
-		t.Run(tc.method+" "+tc.path, func(t *testing.T) {
-			t.Parallel()
-
-			rec := httptest.NewRecorder()
-			s.ServeHTTP(rec, httptest.NewRequestWithContext(
-				t.Context(), tc.method, tc.path, nil))
-			t.Logf("status %d, body %s", rec.Code, rec.Body.String())
-
-			if rec.Code != http.StatusServiceUnavailable {
-				t.Fatalf("status = %d, want %d",
-					rec.Code, http.StatusServiceUnavailable)
-			}
-
-			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)
-			}
-
-			// A HEAD response carries no body.
-			if tc.method == http.MethodHead {
-				return
-			}
-
-			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.StatusServiceUnavailable ||
-				body.Timestamp == "" {
-				t.Errorf("body = %+v, want an error, status %d and a timestamp",
-					body, http.StatusServiceUnavailable)
-			}
-		})
-	}
-}
-
-// TestImageRequestsServedWithoutMaintenanceMode verifies that while
-// maintenance mode is off, image requests reach the image handlers instead
-// of the 503. The handlers refuse an unsigned image URL with 401 and a token
-// they cannot decrypt with 400, so either status shows a request got through.
-func TestImageRequestsServedWithoutMaintenanceMode(t *testing.T) {
-	t.Parallel()
-
-	s := newTestServer(t)
-	s.config.MaintenanceMode = false
-
-	requests := []struct {
-		method string
-		path   string
-		want   int
-	}{
-		{http.MethodGet, unsignedImagePath, http.StatusUnauthorized},
-		{http.MethodHead, unsignedImagePath, http.StatusUnauthorized},
-		{http.MethodGet, encryptedImagePath, http.StatusBadRequest},
-	}
-
-	for _, tc := range requests {
-		t.Run(tc.method+" "+tc.path, func(t *testing.T) {
-			t.Parallel()
-
-			rec := httptest.NewRecorder()
-			s.ServeHTTP(rec, httptest.NewRequestWithContext(
-				t.Context(), tc.method, tc.path, nil))
-			t.Logf("status %d, body %s", rec.Code, rec.Body.String())
-
-			if rec.Code != tc.want {
-				t.Errorf("status = %d, want %d from the image handler",
-					rec.Code, tc.want)
-			}
-		})
-	}
-}
-
-// TestMaintenanceModeKeepsOtherRoutes verifies that while maintenance mode
-// is on, the health check still answers 200 and reports it, and the login
-// page and /metrics still answer 200. The image's Docker HEALTHCHECK
-// requests the health check: a 503 there would make the container
-// unhealthy, and upaas marks a deploy failed when its container is
-// unhealthy.
-func TestMaintenanceModeKeepsOtherRoutes(t *testing.T) {
-	t.Parallel()
-
-	s := newTestServer(t)
-	s.config.MaintenanceMode = true
-
-	// /metrics is routed only when its username is set.
-	s.config.MetricsUsername = "metrics"
-	s.config.MetricsPassword = "metrics-password"
-	s.SetupRoutes()
-
-	rec := httptest.NewRecorder()
-	s.ServeHTTP(rec, httptest.NewRequestWithContext(t.Context(),
-		http.MethodGet, "/.well-known/healthcheck.json", nil))
-	t.Logf("health check status %d, body %s", rec.Code, rec.Body.String())
-
-	if rec.Code != http.StatusOK {
-		t.Fatalf("health check status = %d, want %d", rec.Code, http.StatusOK)
-	}
-
-	var health healthcheck.Response
-
-	err := json.NewDecoder(rec.Body).Decode(&health)
-	if err != nil || !health.Maintenance {
-		t.Errorf("health check maintenance_mode = %v (error %v), want true",
-			health.Maintenance, err)
-	}
-
-	rec = httptest.NewRecorder()
-	s.ServeHTTP(rec, clientRequest(t, http.MethodGet, nil, firstClient, ""))
-
-	if rec.Code != http.StatusOK {
-		t.Errorf("login page status = %d, want %d", rec.Code, http.StatusOK)
-	}
-
-	req := httptest.NewRequestWithContext(t.Context(),
-		http.MethodGet, "/metrics", nil)
-	req.SetBasicAuth(s.config.MetricsUsername, s.config.MetricsPassword)
-
-	rec = httptest.NewRecorder()
-	s.ServeHTTP(rec, req)
-
-	if rec.Code != http.StatusOK {
-		t.Errorf("/metrics status = %d, want %d", rec.Code, http.StatusOK)
-	}
-}

internal/server/routes.go

@@ -1,9 +1,7 @@
 package server
 
 import (
-	"encoding/json"
 	"net/http"
-	"strconv"
 	"time"
 
 	sentryhttp "github.com/getsentry/sentry-go/http"
@@ -19,10 +17,6 @@ import (
 // make per minute; the next is refused with 429 Too Many Requests.
 const LoginAttemptsPerMinute = 5
 
-// MaintenanceRetryAfterSeconds is the Retry-After, in seconds, sent with
-// the 503 that the image routes answer while maintenance mode is on.
-const MaintenanceRetryAfterSeconds = 300
-
 // SetupRoutes configures all HTTP routes.
 func (s *Server) SetupRoutes() {
 	s.router = chi.NewRouter()
@@ -38,6 +32,7 @@ func (s *Server) SetupRoutes() {
 		s.router.Use(s.mw.Metrics())
 	}
 
+	s.router.Use(s.mw.CORS())
 	s.router.Use(middleware.Timeout(s.config.DownstreamTimeout))
 
 	if s.sentryEnabled {
@@ -73,32 +68,15 @@ func (s *Server) SetupRoutes() {
 
 	s.router.Get("/logout", s.h.HandleLogout())
 
-	// Image routes, the only ones that send CORS headers, as pages on other
+	// Main image proxy route
-	// sites read them. They are a subrouter rather than a group: a group's
+	// /v1/image/<host>/<path>/<width>x<height>.<format>
-	// middleware runs only for a request that matches one of its routes,
+	s.router.Get("/v1/image/*", s.h.HandleImage())
-	// and a browser's preflight OPTIONS request matches none, so the CORS
+	s.router.Head("/v1/image/*", s.h.HandleImage())
-	// middleware could not answer it.
-	s.router.Route("/v1", func(r chi.Router) {
-		r.Use(s.mw.CORS())
 
-		// Refused while maintenance mode is on. Only these: the image's
+	// Encrypted image URL route
-		// Docker HEALTHCHECK requests the health check, a 503 there would
+	// The trailing filename (e.g., /img.jpg) is ignored but helps
-		// make the container unhealthy, and upaas marks a deploy failed
+	// browsers with content type
-		// when its container is unhealthy.
+	s.router.Get("/v1/e/{token}/*", s.h.HandleImageEnc())
-		r.Group(func(r chi.Router) {
-			r.Use(s.refuseDuringMaintenance)
-
-			// Main image proxy route
-			// /v1/image/<host>/<path>/<width>x<height>.<format>
-			r.Get("/image/*", s.h.HandleImage())
-			r.Head("/image/*", s.h.HandleImage())
-
-			// Encrypted image URL route
-			// The trailing filename (e.g., /img.jpg) is ignored but helps
-			// browsers with content type
-			r.Get("/e/{token}/*", s.h.HandleImageEnc())
-		})
-	})
 
 	// Metrics endpoint with auth
 	if s.config.MetricsUsername != "" {
@@ -108,30 +86,3 @@ func (s *Server) SetupRoutes() {
 		})
 	}
 }
-
-// refuseDuringMaintenance answers a request with 503 Service Unavailable,
-// a Retry-After header and a JSON error body while maintenance mode is on,
-// and passes it on otherwise. The body has the fields of the JSON errors
-// the image handlers send.
-func (s *Server) refuseDuringMaintenance(next http.Handler) http.Handler {
-	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if !s.MaintenanceMode() {
-			next.ServeHTTP(w, r)
-
-			return
-		}
-
-		w.Header().Set("Retry-After", strconv.Itoa(MaintenanceRetryAfterSeconds))
-		w.Header().Set("Content-Type", "application/json")
-		w.WriteHeader(http.StatusServiceUnavailable)
-
-		err := json.NewEncoder(w).Encode(map[string]any{
-			"error":     "down for maintenance, try again later",
-			"status":    http.StatusServiceUnavailable,
-			"timestamp": time.Now().UTC().Format(time.RFC3339),
-		})
-		if err != nil {
-			s.log.Error("json encode error", "error", err)
-		}
-	})
-}

script/bootstrap

@@ -5,10 +5,8 @@
 # or apk (detected in that order); assumes NOTHING is present (not git,
 # make, or go). The linter is never installed on the host: golangci-lint
 # runs only inside a container, Dockerfile.lint or the Dockerfile lint
-# stage (see script/lint). A C compiler and the CGO image libraries
+# stage (see script/lint). CGO image libraries (pkg-config, vips,
-# (pkg-config, vips, libheif) are installed for the govips bindings.
+# libheif) are installed for the govips bindings.
-# Both Dockerfiles run this script too, so their build dependencies are
-# the ones listed here.
 set -eu
 
 ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
@@ -55,11 +53,6 @@ missing() {
 
 # CGO dependencies for govips (image processing)
 ensure_cgo_deps() {
-    # cgo compiles with gcc on Linux; build-base and build-essential
-    # also bring the C library headers.
-    if missing gcc; then
-        pkg_install gcc build-essential gcc build-base
-    fi
     if missing pkg-config; then
         pkg_install pkg-config pkg-config pkg-config pkgconfig
     fi