Fetch the source path and query URL-encoded, not joined as text (closes #288)

The source URL was built by joining the decoded path and query as
text, so a space in the query reached the upstream raw and failed with
502, 100%25.png made a URL Go could not parse and answered 401, and
%20 and %2520 fetched the same path under different cache keys. The
source URL is now built with net/url, so each decoded path and query is
one upstream request matching its signature and cache key; the query
encodes the characters a browser encodes, ' included. The generator page
compares what pixa would fetch with the URL as entered, and answers 400
with the URL to use when they differ, or for a user name and password
or a .. segment.

Model: opus-5-5

README.md

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

TODO.md

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

configs/config.example.yml

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

internal/config/concurrency_limits_internal_test.go

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

internal/config/config.go

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

internal/handlers/auth.go

@@ -9,7 +9,9 @@ import (
 	"math"
 	"net/http"
 	"net/url"
+	"slices"
 	"strconv"
+	"strings"
 	"time"
 
 	"sneak.berlin/go/pixa/internal/clientip"
@@ -133,6 +135,16 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc {
 			return
 		}
 
+		// /v1/image/ refuses a ".." segment in the source path, and so does
+		// the generator. The path is decoded, so %2e%2e counts too
+		if slices.Contains(strings.Split(parsed.Path, "/"), "..") {
+			s.renderGeneratorWithForm(w, r,
+				"Source URL path cannot hold a .. segment", r.Form,
+				http.StatusBadRequest)
+
+			return
+		}
+
 		payload, expiresAt, ttl, err := buildGeneratePayload(parsed, r.Form)
 		if err != nil {
 			s.renderGeneratorWithForm(w, r, err.Error(), r.Form,
@@ -141,6 +153,19 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc {
 			return
 		}
 
+		// The image URL must fetch the source URL as it was written
+		sourceRequest := payload.ToImageRequest()
+		sourceRequest.AllowHTTP = s.config.AllowHTTP
+		fetchURL := sourceRequest.SourceURL()
+
+		if !fetchedAsWritten(parsed, fetchURL) {
+			s.renderGeneratorWithForm(w, r, "Source URL cannot be fetched as "+
+				"written: pixa would fetch "+fetchURL,
+				r.Form, http.StatusBadRequest)
+
+			return
+		}
+
 		// Generate encrypted token
 		token, err := s.encGen.Generate(payload)
 		if err != nil {
@@ -173,6 +198,29 @@ func (s *Handlers) HandleGenerateURL() http.HandlerFunc {
 	}
 }
 
+// fetchedAsWritten reports whether fetchURL, the URL pixa fetches for an
+// image URL made from the source URL source, is source as it was written,
+// apart from the scheme, which the token does not keep. It is not when source
+// holds a user name and password, which the token does not keep either, or a
+// "?" with no query after it. The token keeps the source path decoded and
+// pixa fetches it percent-encoded again, so a path written another way, such
+// as with %2F, is fetched as another path. url.URL keeps the path as written
+// in RawPath when it is not the decoded path percent-encoded again.
+// EscapedPath ignores RawPath when it also holds a character net/url encodes,
+// such as a space, so RawPath is compared itself.
+func fetchedAsWritten(source *url.URL, fetchURL string) bool {
+	writtenPath := source.RawPath
+	if writtenPath == "" {
+		writtenPath = source.EscapedPath()
+	}
+
+	fetched, err := url.Parse(fetchURL)
+
+	return err == nil && source.User == nil && !source.ForceQuery &&
+		fetched.EscapedPath() == writtenPath &&
+		fetched.RawQuery == source.RawQuery
+}
+
 // buildGeneratePayload parses the numeric form fields and assembles the
 // encrypted URL payload. ttl=0 means never expires (ExpiresAt stays 0). A
 // non-numeric or out-of-range width, height, quality or ttl, or an

internal/handlers/image.go

@@ -9,7 +9,6 @@ import (
 	"strconv"
 	"time"
 
-	"github.com/go-chi/chi/v5"
 	"github.com/go-chi/chi/v5/middleware"
 	"sneak.berlin/go/pixa/internal/encurl"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
@@ -80,20 +79,21 @@ func (s *Handlers) HandleRobotsTxt() http.HandlerFunc {
 	}
 }
 
-// parseImageRequest parses the wildcard path and query parameters into
+// parseImageRequest parses the image URL's path and query parameters into
 // an ImageRequest. On invalid input it writes an error response and
 // returns false.
 func (s *Handlers) parseImageRequest(
 	w http.ResponseWriter, r *http.Request,
 ) (*imgcache.ImageRequest, bool) {
-	// Get the wildcard path from chi
-	pathParam := chi.URLParam(r, "*")
-
-	// Parse the URL path
-	parsed, err := imgcache.ParseImagePath(pathParam)
+	// The path is parsed decoded once, as r.URL.Path holds it, and not from
+	// chi's wildcard: chi routes on the path as the client wrote it whenever
+	// that is not Go's own encoding of it, so its wildcard would still hold
+	// the %3F before a source query when, say, a "/" in the query is written
+	// as %2F
+	parsed, err := imgcache.ParseImageURL(r.URL.Path)
 	if err != nil {
 		s.log.Warn("failed to parse image URL",
-			"path", pathParam,
+			"path", r.URL.Path,
 			"error", err,
 		)
 		s.respondError(w, "invalid image URL: "+err.Error(), http.StatusBadRequest)

internal/handlers/image_errors_internal_test.go

@@ -264,3 +264,127 @@ func TestHandleImage_AllowlistOrSignature(t *testing.T) {
 		})
 	}
 }
+
+// TestHandleImage_InvalidHost_Returns400 sends image URLs whose host no URL
+// host can be, and checks that each is refused with 400 naming the host as
+// decoded: allowlistedHost with a character after it that a URL host cannot
+// hold, percent-encoded, an IPv6 address without brackets, and brackets
+// around something that is not an IPv6 address. A "?" or "#" after
+// allowlistedHost would otherwise end the host when the source URL is read,
+// so the request would pass the allowlist and be fetched and cached under a
+// host that is not the one fetched.
+func TestHandleImage_InvalidHost_Returns400(t *testing.T) {
+	t.Parallel()
+
+	photos := newPhotoFetcher(t, allowlistedHost)
+
+	tests := []struct {
+		name        string
+		writtenHost string
+		decodedHost string
+	}{
+		{"? written as %3F", allowlistedHost + "%3Fx=1", allowlistedHost + "?x=1"},
+		{"# written as %23", allowlistedHost + "%23x", allowlistedHost + "#x"},
+		{"% written as %25", allowlistedHost + "%253F", allowlistedHost + "%3F"},
+		{`\ written as %5C`, allowlistedHost + "%5Cx", allowlistedHost + `\x`},
+		{"space written as %20", allowlistedHost + "%20", allowlistedHost + " "},
+		{"IPv6 address without brackets", "2001:db8::1", "2001:db8::1"},
+		{"IPv4 address in brackets", "[203.0.113.5]", "[203.0.113.5]"},
+		{"no address in brackets", "[abc]", "[abc]"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			rec := sendGet(t, newImageRoute(t, photos), photoURL(tt.writtenHost))
+			checkErrorBody(t, rec, http.StatusBadRequest,
+				fmt.Sprintf("invalid image URL: invalid source host: %q",
+					tt.decodedHost))
+		})
+	}
+}
+
+// TestHandleImage_HostForms_Served checks that the image route serves, with
+// a valid signature, the image at photoPath on a host name, an IPv4 address,
+// an IPv6 address in brackets, and a host followed by ":" with or without a
+// port.
+func TestHandleImage_HostForms_Served(t *testing.T) {
+	t.Parallel()
+
+	hosts := []string{
+		allowlistedHost,
+		"img_1.example.com",
+		"203.0.113.5",
+		"[2001:db8::1]",
+		allowlistedHost + ":8443",
+		"[2001:db8::1]:8443",
+		allowlistedHost + ":",
+	}
+
+	photos := newPhotoFetcher(t, hosts...)
+	exp := time.Now().Add(time.Hour)
+
+	for _, host := range hosts {
+		t.Run(host, func(t *testing.T) {
+			t.Parallel()
+
+			sig := photoSignature(testSigningKey, host, exp)
+
+			rec := sendGet(t, newImageRoute(t, photos),
+				photoURLWithSig(host, sig, exp))
+			if rec.Code != http.StatusOK {
+				t.Errorf("status = %d, want %d: %s",
+					rec.Code, http.StatusOK, rec.Body)
+			}
+		})
+	}
+}
+
+// TestHandleImage_HashInSourcePathOrQuery_Returns400 sends image URLs whose
+// source path or source query holds a "#", written as %23, and checks that
+// each is refused with 400, the one signed for that source path included. The
+// source URL would otherwise end at the "#" when it is fetched, so each text
+// after it would fetch the image at photoPath and cache it under keys of its
+// own.
+func TestHandleImage_HashInSourcePathOrQuery_Returns400(t *testing.T) {
+	t.Parallel()
+
+	photos := newPhotoFetcher(t, allowlistedHost, signedHost)
+	exp := time.Now().Add(time.Hour)
+
+	sig := signature.New(testSigningKey).Sign(&signature.Request{
+		SourceHost: signedHost,
+		SourcePath: photoPath + "#x",
+		Width:      50,
+		Height:     50,
+		Format:     string(imgcache.FormatJPEG),
+		Quality:    85,
+		FitMode:    string(imgcache.FitCover),
+		Expires:    exp,
+	})
+
+	tests := []struct {
+		name   string
+		target string
+	}{
+		{"# in the source path",
+			"/v1/image/" + allowlistedHost + photoPath + "%23x/50x50.jpeg"},
+		{"# in the source query",
+			"/v1/image/" + allowlistedHost + photoPath + "%3Fv=1%23x/50x50.jpeg"},
+		{"# in the source path, signed",
+			fmt.Sprintf("/v1/image/%s%s%%23x/50x50.jpeg?sig=%s&exp=%d",
+				signedHost, photoPath, sig, exp.Unix())},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			rec := sendGet(t, newImageRoute(t, photos), tt.target)
+			checkErrorBody(t, rec, http.StatusBadRequest,
+				`invalid image URL: invalid image path: `+
+					`the source path or query holds a "#"`)
+		})
+	}
+}

internal/handlers/image_signature_internal_test.go

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

internal/imageprocessor/fit_cover_internal_test.go

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

internal/imageprocessor/imageprocessor.go

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

internal/imageprocessor/max_concurrent_processing_internal_test.go

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

internal/imgcache/eviction.go

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

internal/imgcache/eviction_internal_test.go

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

internal/imgcache/imgcache.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"errors"
 	"io"
+	"net/url"
 	"time"
 )
 
@@ -86,9 +87,11 @@ func ValidateImageRequest(req *ImageRequest) error {
 type ImageRequest struct {
 	// SourceHost is the origin host (e.g., "cdn.example.com")
 	SourceHost string
-	// SourcePath is the path on the origin (e.g., "/photos/cat.jpg")
+	// SourcePath is the path on the origin with its percent-escapes decoded,
+	// as url.URL's Path holds it (e.g., "/photos/my cat.jpg")
 	SourcePath string
-	// SourceQuery is the optional query string for the origin URL
+	// SourceQuery is the optional query string for the origin URL as the URL
+	// writes it, percent-escapes included, as url.URL's RawQuery holds it
 	SourceQuery string
 	// Size is the requested output dimensions
 	Size Size
@@ -107,20 +110,25 @@ type ImageRequest struct {
 	AllowHTTP bool
 }
 
-// SourceURL returns the full upstream URL to fetch.
-// Uses http:// scheme when AllowHTTP is true, otherwise https://.
+// SourceURL returns the full upstream URL to fetch: the source path
+// percent-encoded again, as net/url writes a path, so each source path is one
+// upstream path, and the source query as it is, with the characters
+// escapeQuery encodes percent-encoded. Uses http:// scheme when AllowHTTP is
+// true, otherwise https://.
 func (r *ImageRequest) SourceURL() string {
 	scheme := "https"
 	if r.AllowHTTP {
 		scheme = "http"
 	}
 
-	url := scheme + "://" + r.SourceHost + r.SourcePath
-	if r.SourceQuery != "" {
-		url += "?" + r.SourceQuery
+	sourceURL := url.URL{
+		Scheme:   scheme,
+		Host:     r.SourceHost,
+		Path:     r.SourcePath,
+		RawQuery: escapeQuery(r.SourceQuery),
 	}
 
-	return url
+	return sourceURL.String()
 }
 
 // ImageResponse represents a processed image ready to serve

internal/imgcache/service.go

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

internal/imgcache/urlparser.go

@@ -3,6 +3,7 @@ package imgcache
 import (
 	"errors"
 	"fmt"
+	"net/netip"
 	"net/url"
 	"regexp"
 	"strconv"
@@ -47,6 +48,24 @@ func ValidateDimension(name string, value int) error {
 var sizeFormatRegex = regexp.MustCompile(
 	`^(\d+)x(\d+)(?:\.(\w+))?$|^(orig)(?:\.(\w+))?$`)
 
+// percentEscapeRegex matches one percent-escape, such as "%2e"
+var percentEscapeRegex = regexp.MustCompile(`%[0-9A-Fa-f]{2}`)
+
+// queryEscapeRegex matches a percent-escape, or one character that a browser
+// percent-encodes in the query of an http or https URL: one outside the
+// printable ASCII characters "!" to "~", such as a space, a control character
+// or a character outside ASCII, or one of `"#'<>`. A "%" that does not begin a
+// percent-escape matches too.
+var queryEscapeRegex = regexp.MustCompile(`%[0-9A-Fa-f]{2}|[^!-~]|["#'<>%]`)
+
+// hostRegex matches the forms the host of an image URL may take: a host name
+// or an IPv4 address (letters, digits, "-", "_" and "."), or an IPv6 address
+// in brackets (hex digits, ":" and "."), either one optionally followed by ":"
+// and a port, which may be empty. Its one group holds the text in brackets,
+// which parseImageComponents also checks is an IPv6 address.
+var hostRegex = regexp.MustCompile(
+	`^(?:[A-Za-z0-9_.-]+|\[([0-9A-Fa-f:.]+)\])(?::[0-9]*)?$`)
+
 // ParsedURL contains the parsed components of an image proxy URL.
 type ParsedURL struct {
 	// Host is the source origin host (e.g., "cdn.example.com")
@@ -61,9 +80,8 @@ type ParsedURL struct {
 	Format ImageFormat
 }
 
-// ParseImagePath parses the path captured by chi's wildcard:
-// <host>/<path>/<size>.<format>, or <host>/<path>/<size> for JPEG XL
-// This is the primary entry point when using chi routing.
+// ParseImagePath parses an image URL's path after /v1/image/, decoded once:
+// <host>/<path>/<size>.<format>, or <host>/<path>/<size> for JPEG XL.
 // Examples:
 //   - cdn.example.com/photos/cat.jpg/800x600.webp
 //   - cdn.example.com/photos/cat.jpg/0x0.jpeg
@@ -79,9 +97,10 @@ func ParseImagePath(path string) (*ParsedURL, error) {
 	return parseImageComponents(path)
 }
 
-// ParseImageURL parses a full URL path like /v1/image/<host>/<path>/<size>.<format>,
-// or /v1/image/<host>/<path>/<size> for JPEG XL
-// Use ParseImagePath instead when working with chi's wildcard capture.
+// ParseImageURL parses an image URL's path, decoded once as r.URL.Path holds
+// it: /v1/image/<host>/<path>/<size>.<format>, or /v1/image/<host>/<path>/<size>
+// for JPEG XL. A source query follows the source path after a "?", which the
+// URL writes as %3F.
 func ParseImageURL(urlPath string) (*ParsedURL, error) {
 	// Remove the /v1/image/ prefix
 	const prefix = "/v1/image/"
@@ -100,12 +119,6 @@ func ParseImageURL(urlPath string) (*ParsedURL, error) {
 // parseImageComponents parses <host>/<path>/<size>.<format>, or
 // <host>/<path>/<size> for JPEG XL.
 func parseImageComponents(remainder string) (*ParsedURL, error) {
-	// Check for path traversal before any other processing
-	err := checkPathTraversal(remainder)
-	if err != nil {
-		return nil, err
-	}
-
 	// Find the last path segment, which holds "size" or "size.format"
 	lastSlash := strings.LastIndex(remainder, "/")
 	if lastSlash == -1 {
@@ -144,10 +157,20 @@ func parseImageComponents(remainder string) (*ParsedURL, error) {
 		return nil, ErrMissingHost
 	}
 
-	// A URL reads what comes before "@" as a user name and password, and the
-	// rest as the host
-	if strings.Contains(host, "@") {
-		return nil, fmt.Errorf("%w: %q holds @", ErrInvalidHost, host)
+	// Only the forms hostRegex describes are accepted. A "?", "#" or "@" in
+	// the host would make Go's URL parser, which the allowlist check and the
+	// fetcher use, read a different host from the one allowlisted and cached
+	hostMatch := hostRegex.FindStringSubmatch(host)
+	if hostMatch == nil {
+		return nil, fmt.Errorf("%w: %q", ErrInvalidHost, host)
+	}
+
+	// Go's URL parser refuses brackets around anything but an IPv6 address
+	if inBrackets := hostMatch[1]; inBrackets != "" {
+		addr, err := netip.ParseAddr(inBrackets)
+		if err != nil || !addr.Is6() {
+			return nil, fmt.Errorf("%w: %q", ErrInvalidHost, host)
+		}
 	}
 
 	// Host names do not depend on letter case, so the allowlist, the upstream
@@ -155,12 +178,30 @@ func parseImageComponents(remainder string) (*ParsedURL, error) {
 	// host
 	host = strings.ToLower(host)
 
-	// Extract query string if present in path
+	// Go's URL parser, which the fetcher uses, reads a "#" as the start of a
+	// fragment and drops the rest, so each text after it would be another
+	// spelling, cached under its own keys, of one upstream request
+	if strings.Contains(path, "#") {
+		return nil, fmt.Errorf(
+			`%w: the source path or query holds a "#"`, ErrInvalidPath)
+	}
+
+	// Extract query string if present in path. It is the source URL's query
+	// as that URL writes it. A character a URL's query cannot hold as it is,
+	// such as a space written %20 in the image URL, is percent-encoded here,
+	// so the query is signed and cached as it is sent upstream
 	if qIndex := strings.Index(path, "?"); qIndex != -1 {
-		query = path[qIndex+1:]
+		query = escapeQuery(path[qIndex+1:])
 		path = path[:qIndex]
 	}
 
+	// The source query names no file on the upstream, so only the host and
+	// the source path are checked for path traversal
+	err = checkPathTraversal(host + path)
+	if err != nil {
+		return nil, err
+	}
+
 	// Ensure path starts with /
 	if !strings.HasPrefix(path, "/") {
 		path = "/" + path
@@ -186,12 +227,7 @@ func checkPathTraversal(path string) error {
 	// Decode multiple times to catch double-encoding
 	decoded := path
 	for range 3 {
-		newDecoded, err := url.PathUnescape(decoded)
-		if err != nil {
-			// Malformed encoding is suspicious
-			return ErrPathTraversal
-		}
-
+		newDecoded := decodePercentEscapes(decoded)
 		if newDecoded == decoded {
 			break
 		}
@@ -232,6 +268,32 @@ func checkPathTraversal(path string) error {
 	return nil
 }
 
+// decodePercentEscapes decodes every percent-escape in s, such as %2e, and
+// keeps a "%" that does not start one as it is, where url.PathUnescape would
+// refuse the whole string. A source path holding %25, a literal "%", has such
+// a "%" after one more decoding.
+func decodePercentEscapes(s string) string {
+	return percentEscapeRegex.ReplaceAllStringFunc(s, func(escape string) string {
+		decoded, _ := url.PathUnescape(escape)
+
+		return decoded
+	})
+}
+
+// escapeQuery returns a source query as a URL's query holds it: each
+// character that queryEscapeRegex names, such as a space, percent-encoded,
+// and the rest, the query's percent-escapes included, kept as they are. The
+// result is unchanged by a second escapeQuery.
+func escapeQuery(query string) string {
+	return queryEscapeRegex.ReplaceAllStringFunc(query, func(match string) string {
+		if percentEscapeRegex.MatchString(match) {
+			return match
+		}
+
+		return url.PathEscape(match)
+	})
+}
+
 // parseSizeFormat parses strings like "800x600.webp", "orig.png" or "800x600"
 func parseSizeFormat(s string) (Size, ImageFormat, error) {
 	matches := sizeFormatRegex.FindStringSubmatch(s)

internal/imgcache/urlparser_internal_test.go

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

internal/server/source_url_internal_test.go

@@ -0,0 +1,371 @@
+package server
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"regexp"
+	"slices"
+	"strings"
+	"sync"
+	"testing"
+
+	"sneak.berlin/go/pixa/internal/imgcache"
+	"sneak.berlin/go/pixa/internal/session"
+)
+
+// percentSignPath is a source path holding a "%", which a URL writes %25.
+const percentSignPath = "/100%25.png"
+
+// generateFormPattern extracts the CSRF token rendered into the generator
+// page's form.
+var generateFormPattern = regexp.MustCompile(
+	`(?s)action="/generate".*?name="gorilla\.csrf\.Token" value="([^"]+)"`)
+
+// generatedURLPattern extracts the path of the encrypted URL the generator
+// page shows.
+var generatedURLPattern = regexp.MustCompile(`value="[^"]*(/v1/e/[^"]+)"`)
+
+// recordingUpstream is a test upstream that answers every request with a PNG
+// and records the request target, the path and query as sent, of each request
+// it receives.
+type recordingUpstream struct {
+	*httptest.Server
+
+	mu       sync.Mutex
+	received []string
+}
+
+// startRecordingUpstream starts a recordingUpstream, which is closed when the
+// test ends.
+func startRecordingUpstream(t *testing.T) *recordingUpstream {
+	t.Helper()
+
+	source := encodeTestPNG(t, 64, 48)
+	upstream := &recordingUpstream{}
+
+	upstream.Server = httptest.NewServer(http.HandlerFunc(
+		func(w http.ResponseWriter, r *http.Request) {
+			upstream.mu.Lock()
+			upstream.received = append(upstream.received, r.RequestURI)
+			upstream.mu.Unlock()
+
+			w.Header().Set("Content-Type", "image/png")
+			_, _ = w.Write(source)
+		}))
+	t.Cleanup(upstream.Close)
+
+	return upstream
+}
+
+// requestTargets returns the request targets the upstream has received, in
+// the order it received them.
+func (u *recordingUpstream) requestTargets() []string {
+	u.mu.Lock()
+	defer u.mu.Unlock()
+
+	return slices.Clone(u.received)
+}
+
+// pageOrigin is the origin of the login and generator pages below, opened
+// over HTTPS; httptest gives every request the host example.com.
+const pageOrigin = "https://example.com"
+
+// openRoot sends GET / to s over HTTPS, with cookie when it is not nil, and
+// returns the page: the login page, or the generator page for a login session.
+func openRoot(
+	t *testing.T, s *Server, cookie *http.Cookie,
+) *httptest.ResponseRecorder {
+	t.Helper()
+
+	req := httptest.NewRequestWithContext(t.Context(), http.MethodGet,
+		pageOrigin+"/", nil)
+	if cookie != nil {
+		req.AddCookie(cookie)
+	}
+
+	page := httptest.NewRecorder()
+	s.ServeHTTP(page, req)
+
+	return page
+}
+
+// submitForm posts form to path on s as a browser submits it from page over
+// HTTPS: with the CSRF token tokenPattern finds in page, the cookies page set,
+// cookie when it is not nil, and a Referer naming the page, which pixa needs
+// while debug is off, as it is in startImageProxy's config.
+func submitForm(
+	t *testing.T, s *Server, page *httptest.ResponseRecorder,
+	tokenPattern *regexp.Regexp, path string, form url.Values,
+	cookie *http.Cookie,
+) *httptest.ResponseRecorder {
+	t.Helper()
+
+	match := tokenPattern.FindStringSubmatch(page.Body.String())
+	if match == nil {
+		t.Fatalf("the page has no form for %s with a CSRF token", path)
+	}
+
+	form.Set("gorilla.csrf.Token", match[1])
+
+	req := httptest.NewRequestWithContext(t.Context(), http.MethodPost,
+		pageOrigin+path, strings.NewReader(form.Encode()))
+	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	req.Header.Set("Referer", pageOrigin+"/")
+
+	for _, c := range page.Result().Cookies() {
+		req.AddCookie(c)
+	}
+
+	if cookie != nil {
+		req.AddCookie(cookie)
+	}
+
+	rec := httptest.NewRecorder()
+	s.ServeHTTP(rec, req)
+
+	return rec
+}
+
+// postGenerate logs in to s with the signing key, submits the generator page's
+// form with sourceURL, and returns the response.
+func postGenerate(
+	t *testing.T, s *Server, sourceURL string,
+) *httptest.ResponseRecorder {
+	t.Helper()
+
+	login := submitForm(t, s, openRoot(t, s, nil), csrfFieldPattern, "/",
+		url.Values{"key": {testSigningKey}}, nil)
+
+	var sessionCookie *http.Cookie
+
+	for _, c := range login.Result().Cookies() {
+		if c.Name == session.CookieName {
+			sessionCookie = c
+		}
+	}
+
+	if sessionCookie == nil {
+		t.Fatalf("logging in answered %d and set no session cookie", login.Code)
+	}
+
+	rec := submitForm(t, s, openRoot(t, s, sessionCookie), generateFormPattern,
+		"/generate", url.Values{"url": {sourceURL}}, sessionCookie)
+	t.Logf("POST /generate with url %s: %d", sourceURL, rec.Code)
+
+	return rec
+}
+
+// TestImageURLFetchesItsSourcePathAndQuery requests image URLs whose source
+// query holds a space, written percent-encoded, or a "'", and whose source
+// path holds a "%", written percent-encoded, and checks the one request the
+// upstream receives: the source path percent-encoded again, and the source
+// query with the space written %20 and the "'" written %27, as a browser
+// writes them in a query.
+func TestImageURLFetchesItsSourcePathAndQuery(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name string
+		// pathAndQuery is the source path and query as the image URL writes
+		// them
+		pathAndQuery string
+		// wantRequest is the path and query the upstream must receive
+		wantRequest string
+	}{
+		{"space in the query", "/photo.png%3Fname=my%20cat",
+			"/photo.png?name=my%20cat"},
+		{"apostrophe in the query", "/photo.png%3Fname=it's",
+			"/photo.png?name=it%27s"},
+		{"percent sign in the path", percentSignPath, percentSignPath},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			upstream := startRecordingUpstream(t)
+			s, _, _ := startImageProxy(t, upstream.Server)
+
+			getImage(t, s, "/v1/image/"+upstreamHost+tt.pathAndQuery+"/orig.png")
+
+			got := upstream.requestTargets()
+			t.Logf("upstream received %q", got)
+
+			if !slices.Equal(got, []string{tt.wantRequest}) {
+				t.Errorf("upstream received %q, want [%q]", got, tt.wantRequest)
+			}
+		})
+	}
+}
+
+// TestSourcePathsHoldingSpaceAndPercent20AreFetchedApart requests twice each
+// of the source paths /my cat.png and /my%20cat.png, which the image URL
+// writes my%20cat.png and my%2520cat.png. Each is fetched once, as a request
+// of its own, and its second request is served from the cache.
+func TestSourcePathsHoldingSpaceAndPercent20AreFetchedApart(t *testing.T) {
+	t.Parallel()
+
+	upstream := startRecordingUpstream(t)
+	s, _, _ := startImageProxy(t, upstream.Server)
+
+	imageURLs := []string{
+		"/v1/image/" + upstreamHost + "/my%20cat.png/orig.png",
+		"/v1/image/" + upstreamHost + "/my%2520cat.png/orig.png",
+	}
+
+	for _, imageURL := range imageURLs {
+		getImage(t, s, imageURL)
+	}
+
+	for _, imageURL := range imageURLs {
+		cacheStatus := getImage(t, s, imageURL).Header().Get("X-Pixa-Cache")
+		if cacheStatus != string(imgcache.CacheHit) {
+			t.Errorf("second GET %s: X-Pixa-Cache = %q, want HIT",
+				imageURL, cacheStatus)
+		}
+	}
+
+	got := upstream.requestTargets()
+	t.Logf("upstream received %q", got)
+
+	want := []string{"/my%20cat.png", "/my%2520cat.png"}
+	if !slices.Equal(got, want) {
+		t.Errorf("upstream received %q, want %q", got, want)
+	}
+}
+
+// TestSourceQueryWrittenTwoWaysIsOneUpstreamRequest requests the source query
+// name=my%20cat written as the README says, name=my%2520cat, and with the
+// space written %20, name=my%20cat. Both are one source query: the upstream
+// receives one request, and the second image URL is served from the cache.
+func TestSourceQueryWrittenTwoWaysIsOneUpstreamRequest(t *testing.T) {
+	t.Parallel()
+
+	upstream := startRecordingUpstream(t)
+	s, _, _ := startImageProxy(t, upstream.Server)
+
+	getImage(t, s, "/v1/image/"+upstreamHost+"/photo.png%3Fname=my%2520cat/orig.png")
+
+	second := getImage(t, s,
+		"/v1/image/"+upstreamHost+"/photo.png%3Fname=my%20cat/orig.png")
+	if got := second.Header().Get("X-Pixa-Cache"); got != string(imgcache.CacheHit) {
+		t.Errorf("second X-Pixa-Cache = %q, want HIT", got)
+	}
+
+	got := upstream.requestTargets()
+	t.Logf("upstream received %q", got)
+
+	want := []string{"/photo.png?name=my%20cat"}
+	if !slices.Equal(got, want) {
+		t.Errorf("upstream received %q, want %q", got, want)
+	}
+}
+
+// TestGeneratedURLFetchesTheSourceURLAsEntered makes an encrypted URL on the
+// generator page for source URLs holding %20, %25 and %23 in the path or the
+// query, requests it, and checks that the upstream receives the path and
+// query exactly as entered.
+func TestGeneratedURLFetchesTheSourceURLAsEntered(t *testing.T) {
+	t.Parallel()
+
+	// The names hold no "%": each test's state directory is named after it
+	// and goes into the database URL, which decodes percent-escapes
+	tests := []struct {
+		name         string
+		pathAndQuery string
+	}{
+		{"space in the path", "/my%20cat.png"},
+		{"percent sign in the path", percentSignPath},
+		{"number sign in the path", "/a%23b.png"},
+		{"all three in the query", "/photo.png?name=my%20cat%25%23"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			upstream := startRecordingUpstream(t)
+			s, _, _ := startImageProxy(t, upstream.Server)
+
+			rec := postGenerate(t, s, "https://"+upstreamHost+tt.pathAndQuery)
+
+			match := generatedURLPattern.FindStringSubmatch(rec.Body.String())
+			if rec.Code != http.StatusOK || match == nil {
+				t.Fatalf("POST /generate: %d with no encrypted URL; body %s",
+					rec.Code, rec.Body.String())
+			}
+
+			getImage(t, s, match[1])
+
+			got := upstream.requestTargets()
+			t.Logf("upstream received %q", got)
+
+			if !slices.Equal(got, []string{tt.pathAndQuery}) {
+				t.Errorf("upstream received %q, want [%q]", got, tt.pathAndQuery)
+			}
+		})
+	}
+}
+
+// TestGeneratorRefusesSourceURLItCannotFetchAsWritten submits source URLs the
+// generator page must make no encrypted URL for, and checks that it answers
+// 400 and shows why. pixa keeps the source path decoded and fetches it
+// percent-encoded again, so a path holding %2F would be fetched with a "/"
+// there, whatever else the path holds. pixa sends no user name or password,
+// and no "?" with no query after it. For these the page shows the URL pixa
+// would fetch. A source path holding a ".." segment is refused, as
+// /v1/image/ refuses it.
+func TestGeneratorRefusesSourceURLItCannotFetchAsWritten(t *testing.T) {
+	t.Parallel()
+
+	// The test config sets allow_http, so pixa would fetch over plain HTTP
+	const fetchedFrom = "http://" + upstreamHost
+
+	tests := []struct {
+		name string
+		// sourceURL is the source URL after "https://"
+		sourceURL string
+		// wantShown is what the page must show: the URL pixa would fetch, or
+		// why it makes no encrypted URL
+		wantShown string
+	}{
+		{"encoded slash", upstreamHost + "/a%2Fb.png",
+			fetchedFrom + "/a/b.png"},
+		{"encoded slash and a space", upstreamHost + "/a%2Fb c.png",
+			fetchedFrom + "/a/b%20c.png"},
+		{"encoded slash and a bar", upstreamHost + "/a%2Fb|c.png",
+			fetchedFrom + "/a/b%7Cc.png"},
+		{"encoded slash and a letter outside ASCII", upstreamHost + "/café%2Fx.png",
+			fetchedFrom + "/caf%C3%A9/x.png"},
+		{"user name and password", "user:pass@" + upstreamHost + "/a.png",
+			fetchedFrom + "/a.png"},
+		{"question mark with no query", upstreamHost + "/a.png?",
+			fetchedFrom + "/a.png"},
+		{"two-dot segment", upstreamHost + "/a/../b.png",
+			"cannot hold a .. segment"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			s, _, _ := startImageProxy(t, startRecordingUpstream(t).Server)
+
+			rec := postGenerate(t, s, "https://"+tt.sourceURL)
+			body := rec.Body.String()
+
+			if rec.Code != http.StatusBadRequest {
+				t.Errorf("status = %d, want %d", rec.Code, http.StatusBadRequest)
+			}
+
+			if strings.Contains(body, "/v1/e/") {
+				t.Error("the page shows an encrypted URL")
+			}
+
+			if !strings.Contains(body, tt.wantShown) {
+				t.Errorf("the page does not show %s; body %s", tt.wantShown, body)
+			}
+		})
+	}
+}

internal/signature/signature.go

@@ -126,31 +126,20 @@ func (s *Signer) GenerateSignedURL(
 		sizeStr = fmt.Sprintf("%dx%d", req.Width, req.Height)
 	}
 
-	// Build the path.
-	// When a source query is present, it is embedded as a path segment
-	// (e.g. /host/path?query/size.fmt) so that the URL parser can extract
-	// it from the last-slash split. The "?" inside a path segment is
-	// percent-encoded by clients but chi delivers it decoded, which is
-	// exactly what the URL parser expects.
-	var path string
+	// Build the path. A source query follows the source path after a "?".
+	// The image route reads the path decoded once, so the whole path is
+	// written in Go's own percent-encoding of a URL path, which writes "?"
+	// as %3F and "%" as %25, and leaves "/", ";", ",", "=" and "&" as they
+	// are.
+	sourcePathAndQuery := req.SourcePath
 	if req.SourceQuery != "" {
-		path = fmt.Sprintf("/v1/image/%s%s%%3F%s/%s.%s",
-			req.SourceHost,
-			req.SourcePath,
-			url.PathEscape(req.SourceQuery),
-			sizeStr,
-			req.Format,
-		)
-	} else {
-		path = fmt.Sprintf("/v1/image/%s%s/%s.%s",
-			req.SourceHost,
-			req.SourcePath,
-			sizeStr,
-			req.Format,
-		)
+		sourcePathAndQuery += "?" + req.SourceQuery
 	}
 
-	return path, sig, exp
+	unescapedPath := fmt.Sprintf("/v1/image/%s%s/%s.%s",
+		req.SourceHost, sourcePathAndQuery, sizeStr, req.Format)
+
+	return (&url.URL{Path: unescapedPath}).EscapedPath(), sig, exp
 }
 
 // buildSignatureData creates the string to be signed.

internal/signature/signature_test.go

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