Scale both sides of a fit=cover image by the same amount (part of #282)

cropCentreAndResize cut the centre to whole source pixels and forced
that to the requested size, so the two sides were scaled by different
amounts: a 16x16 source at 100x60 came out squashed by 4%. It now
widens the centre to whole source pixels and lets thumbnail with a
centre crop scale that by the one amount next uses and cut the output
from it. thumbnail copies the scaled image into memory, which is larger
than the output by less than one scaled source pixel on each side.
TODO.md says so.

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
@@ -288,6 +316,20 @@ once, is refused with 400.
   both optional (values under Signature Specification). Both are part of what is
   cached, so each value of either is a separate cached image.
 
+A source URL's query follows its path after `%3F`, the encoded `?`:
+`/v1/image/<host>/<path>%3F<query>/<size>.<format>`. pixa decodes the
+percent-escapes in the image URL's path once, then reads the host up to the
+first `/`, the size and format after the last `/`, and between them the source
+path up to the first `?` and the source query after it. So the `?` before the
+query, any `?` in the query, and every `%` in the source path or query must be
+percent-encoded, as `%3F` and `%25`. Any other character may be written as
+itself or percent-encoded: `/`, `;`, `,`, `=` and `&` work either way. For
+example,
+`https://cdn.example.com/img?url=https://origin.example.com/my%20cat.jpg` at
+800x600 WebP is
+`/v1/image/cdn.example.com/img%3Furl=https://origin.example.com/my%2520cat.jpg/800x600.webp`,
+signed as shown under Signature Specification.
+
 A size with one side `0` has that side scaled to keep the source's proportions,
 and is refused with 400 when that side would come out above 8192, the most
 either side may be; so is a size with `fit=outside`, which scales the image to
@@ -507,6 +549,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
@@ -566,7 +619,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 +705,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
+  default half the CPUs pixa can use (`GOMAXPROCS`, which follows a container's
-  container's CPU limit. A request that finds all of them in use waits up to 10
+  CPU limit), rounded down, and at least 1. A value that is not a whole number
-  seconds for one to free up; if none does, and `downstream_timeout` has not
+  of at least 1 aborts startup. A request that finds all of them in use waits up
-  ended first, it is answered 503 the same way
+  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,6 +30,44 @@ P2: security: per-origin rate limiting
 
 # Completed Steps
 
+- 2026-10-15 a thin source requested with `fit=cover` no longer makes libvips
+  build a huge image (closes #282): libvips' thumbnail scaled the whole source
+  to cover the requested size and copied that into memory before it cut out the
+  centre, so an 8192x64 source at 8192x8192 made it allocate about 24 GiB, and
+  an 8192x1 source answered 500. The image processor now first cuts the centre
+  of the source, widened to whole source pixels, and thumbnail scales and cuts
+  only that, by the same amount on both sides as before. What it copies into
+  memory is then larger than the output by less than one scaled source pixel on
+  each side. With the runtime image's libvips and JPEG output at 8192x8192,
+  resident memory peaked at about 242 MiB for the 8192x64 source and 431 MiB for
+  the 8192x1 one. The output can sit a fraction of an output pixel from where it
+  was.
+- 2026-10-15 the README says how a source URL with a query is written into an
+  image URL, and the image route accepts it (closes #255): the query follows the
+  source path after `%3F`. pixa now reads the image URL's path decoded once, so
+  the `?` before the query, any `?` in it and every `%` in the source path or
+  query must be percent-encoded, and any other character may be. The route used
+  to parse chi's wildcard, which holds the path as the client wrote it whenever
+  that is not Go's own encoding of it; a `/`, `;` or `,` in the query written as
+  `%2F`, `%3B` or `%2C`, as `GenerateSignedURL` wrote them, kept the `%3F` from
+  being read as `?`, and the request failed with 401. `GenerateSignedURL` now
+  writes Go's own encoding of the path. The README has an example with a query
+  and its signature. The path traversal check no longer reads the source query,
+  and keeps a `%` that does not start a percent-escape as it is instead of
+  refusing the URL, so a source path or query holding `%25` is accepted, while a
+  double-encoded `..` next to such a `%` is still refused.
+- 2026-10-15 `max_concurrent_processing` defaults to half the CPUs pixa sees,
+  rounded down, and at least 1 (closes #242): it defaulted to one image per CPU,
+  and a container with no CPU limit sees every CPU of the host, so on a 48-CPU
+  host pixa processed 48 images at once, each of which can take about 4.2 GiB.
+  `config.DefaultMaxConcurrentProcessing` gives the default to the config and to
+  `imageprocessor.New`; `PIXA_MAX_CONCURRENT_PROCESSING` still overrides it.
+  "Running under upaas" in `README.md` gives the worst-case memory, measured,
+  and tells the operator to set `PIXA_MAX_CONCURRENT_PROCESSING` and
+  `PIXA_UPSTREAM_CONNECTIONS` to fit the container's memory. It also gives the
+  file libvips writes to `/tmp` for each large source, up to 1 GiB, measured,
+  which is outside `/var/lib/pixa` and `cache_max_bytes` and is memory where
+  `/tmp` is a tmpfs.
 - 2026-10-15 the canonical files are re-vendored from `sneak/prompts` commit
   `e3f79c4` (closes #221): `.dockerignore`, `.editorconfig`,
   `.gitea/workflows/check.yml`, `.gitignore`, `.golangci.yml`,

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
+# Maximum number of images decoded and encoded at once (default: half the
-# number of CPUs pixa can use, which follows a container's CPU limit). A
+# number of CPUs pixa can use, at least 1; the number of CPUs follows a
-# request that finds none free waits up to 10 seconds for one, and if none
+# container's CPU limit). A request that finds none free waits up to 10
-# frees up it is answered 503, unless downstream_timeout has ended first.
+# 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
+// max_concurrent_processing is half the CPUs Go uses, and at least 1, and
-// upstream_connections is 64.
+// 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) {
+	if c.MaxConcurrentProcessing != max(runtime.GOMAXPROCS(0)/2, 1) {
-		t.Errorf("MaxConcurrentProcessing = %d, want %d, one per CPU",
+		t.Errorf("MaxConcurrentProcessing = %d, want %d, half the CPUs",
-			c.MaxConcurrentProcessing, runtime.GOMAXPROCS(0))
+			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/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
+	// The path is parsed decoded once, as r.URL.Path holds it, and not from
-	pathParam := chi.URLParam(r, "*")
+	// 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
-	// Parse the URL path
+	// the %3F before a source query when, say, a "/" in the query is written
-	parsed, err := imgcache.ParseImagePath(pathParam)
+	// 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_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
+// image and no operation cache. Each request decodes different source
-// by default, so more threads per image would only compete for the CPUs.
+// bytes, so the operation cache would rarely be hit and would hold memory
-// Each request decodes different source bytes, so the operation cache
+// outside MaxConcurrentProcessing; repeated requests are served from
-// would rarely be hit and would hold memory outside MaxConcurrentProcessing;
+// pixa's disk cache instead.
-// 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)
+		// Crop the centre and resize to fill exact dimensions (default)
-		return img.Thumbnail(width, height, vips.InterestingCentre)
+		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) {
+		if got := cap(proc.processingSemaphore); got != max(runtime.GOMAXPROCS(0)/2, 1) {
-			t.Errorf("MaxConcurrentProcessing %d: %d slots, want %d, one per CPU",
+			t.Errorf("MaxConcurrentProcessing %d: %d slots, want %d, half the CPUs",
-				limit, got, runtime.GOMAXPROCS(0))
+				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/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

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

internal/imgcache/urlparser_internal_test.go

@@ -407,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/signature/signature.go

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