Re-vendor the canonical files from sneak/prompts at e3f79c4 (closes #221)

The vendored dotfiles, REPO_POLICIES.md and the standard scripts are
fetched from sneak/prompts commit e3f79c4, with pixa's own entries kept
after the standard part. The scripts pass a new CACHEBUST build argument
in place of --no-cache and every Dockerfile stage declares it first, so
every RUN step reruns while Go's build cache, mounted on the steps that
compile, is kept. The lint phase runs golangci-lint v2.14.0, the test
phase runs on the Debian Go image, and the build stage takes the version
as the policy gives it. The two new lint findings are fixed in the code.

Model: opus-5-5

README.md

@@ -71,8 +71,7 @@ The state directory (`state_dir`, `/var/lib/pixa` in the container) holds the
 database and the disk cache:
 
 - It needs a persistent volume: without one, every restart starts with an empty
-  cache. Losing the directory also ends every login session (see Encrypted
-  URLs). In the container, the startup script gives the directory to the user
+  cache. In the container, the startup script gives the directory to the user
   pixa runs as (uid 65532) and sets its mode to `750`; outside it, that user
   must be able to write the directory.
 - `cache_max_bytes` limits the source and transformed images together. The
@@ -131,34 +130,6 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for pixa needs:
       the cache can grow to three quarters or more of the space they all have
       left, and to all of it when that is 500 MiB or less.
     - the rest are in the table under Configuration below
-- **Memory:** pixa can use up to `max_concurrent_processing` times 4.2 GiB for
-  the images it processes at once, plus `upstream_connections` times
-  `upstream_max_response_size` for the source images it has fetched that wait
-  for processing. 4.2 GiB is the most resident memory was measured to grow while
-  pixa, with pixad's libvips settings, processed one 8192x8192 source, the
-  largest it accepts, saving it at full size to each output format in turn, one
-  at a time. The sources were a JPEG, a GIF, a WebP, an 8-bit and a 16-bit PNG,
-  an 8-bit and a 12-bit AVIF, and an 8-bit, a 16-bit and a 32-bit floating-point
-  JPEG XL; memory grew most for the 16-bit JPEG XL, of random pixels, saved as
-  WebP. At the defaults that is half the CPUs pixa sees times 4.2 GiB, plus 64
-  times 50 MiB: on a host where pixa sees 48 CPUs, 24 times 4.2 GiB plus 3.1
-  GiB, about 104 GiB. Set `PIXA_MAX_CONCURRENT_PROCESSING` and
-  `PIXA_UPSTREAM_CONNECTIONS` so that this fits the memory the container may
-  use.
-- **Temporary files:** while pixa processes a large source, libvips writes its
-  decoded pixels to a file in `/tmp`. The file disappears from `/tmp` once it is
-  written, so a listing of `/tmp` does not show it, but it keeps its space, on
-  disk or, where `/tmp` is a tmpfs, in memory, until the image is done. For a
-  GIF, WebP, AVIF or JPEG XL source the decoded pixels are held in memory as
-  well as written to that file, and the 4.2 GiB above includes that memory. For
-  an 8192x8192 source the file was measured on pixad at 192 MiB for an 8-bit RGB
-  JPEG, 512 MiB for a 16-bit RGBA PNG or JPEG XL, and 1 GiB for a JPEG XL of
-  32-bit floating-point RGBA, the largest measured: up to 1 GiB per image
-  processed at once, 24 GiB at the defaults on a host where pixa sees 48 CPUs.
-  This `/tmp` is in the container's own filesystem, not on the volume at
-  `/var/lib/pixa`, and `cache_max_bytes` does not count it. The 4.2 GiB above
-  does not include these files; where `/tmp` is a tmpfs, they are memory on top
-  of it.
 - **Health check:** the image's `HEALTHCHECK` requests
   `/.well-known/healthcheck.json`. upaas reads the container's health 60 seconds
   after a deploy and marks the deploy failed unless it is `healthy`. The probe
@@ -225,11 +196,7 @@ path under `/v1/` answers 200, in maintenance mode too.
   session it answers 303 to `/`. Answers: 200 with the page showing the URL; 400
   with the page naming a field that is not valid; 500 when the URL cannot be
   made.
-- `POST /logout` — log out with the generator page's Logout button: end every
-  login session, in every browser (see Encrypted URLs), and remove the session
-  cookie. Needs: a login session and the generator page's form (below); without
-  a login session it only removes the cookie. Answers: 303 to `/`; 500 when the
-  logout cannot be saved in the database.
+- `GET /logout` — end the login session. Needs: nothing. Answers: 303 to `/`.
 - `GET` or `HEAD` `/v1/image/<host>/<path>/<size>.<format>`, or
   `/v1/image/<host>/<path>/<size>` with no format — an image, fetched, resized
   and converted (below). Needs: a signature, unless the host is allowlisted (see
@@ -277,7 +244,7 @@ the request carries the same ID as `request_id`, and so do the lines it logs
 when it fetches, converts and serves an image; the fetch sends it to the
 upstream host as `X-Request-ID`.
 
-The `POST` routes accept only a form that pixa's own page served: the page puts
+Both `POST` routes accept only a form that pixa's own page served: the page puts
 a token in the form and sets a cookie to match, and a request without both is
 refused with 403, so another site cannot submit the form from a visitor's
 browser. The login and generator pages are meant to be opened over HTTPS: while
@@ -316,32 +283,12 @@ 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
 cover the size and so past it on one side, when that side would come out
 above 8192.
 
-An AVIF image of more than 16,777,216 pixels, the number 4096x4096 and 8192x2048
-have, is refused with 400 before anything is decoded, as its encode could take
-more than 40 seconds (see below). That includes `orig` of an AVIF source, and
-`auto` when it chooses AVIF, so a client whose `Accept` names `image/avif` but
-not `image/jxl` is refused such a size.
-
 The source image may be JPEG, PNG, GIF, WebP, AVIF or JPEG XL. The upstream's
 `Content-Type` must name its format, and the image's first bytes must match it.
 An upstream answering with any other `Content-Type`, SVG included, is refused
@@ -391,18 +338,14 @@ transcode that stops this way is answered 503 with the error
 ended. govips gives pixa no way to stop libvips once an encode has begun, so an
 encode keeps its slot until it finishes, which can be after
 `downstream_timeout`. The limit of 8192 on each side of the source and of the
-output, and for AVIF the limit of 16,777,216 pixels, bound how long that takes.
-With one libvips thread, as pixad runs it, encoding 8192x8192 random pixels
-takes up to about 1 second as JPEG, 15 as PNG and 16 as GIF, the slowest of 8 or
-16 bits per sample, with or without an alpha channel. The WebP and JPEG XL
-encoders run below libvips' default effort, and AVIF at the lowest effort govips
-can set, so that no encode takes more than 40 seconds of the default
-`downstream_timeout` of 60: at any quality and 8 or 16 bits per sample,
-8192x8192 random pixels with an alpha channel take up to about 23 seconds as
-WebP and 27 as JPEG XL, and 4096x4096, 8192x2048 or 2048x8192 up to about 34 as
-AVIF, which at quality 85 would take about 96 at 8192x8192. Without an alpha
-channel they take up to about 16, 13 and 22 seconds. Decoding such a source adds
-up to about 11 seconds.
+output bounds how long that takes. With one libvips thread, as pixad runs it,
+encoding 8192x8192 random pixels takes up to about 1 second as JPEG, 15 as PNG,
+16 as GIF, 63 as WebP, 110 as AVIF and 96 as JPEG XL, the slowest of 8 or 16
+bits per sample, with or without an alpha channel. WebP, AVIF and JPEG XL are
+that slow only with an alpha channel: 8-bit RGB takes about 44, 53 and 28
+seconds. Decoding such a source adds up to about 11 seconds. A WebP, AVIF or
+JPEG XL encode can therefore hold its slot past the default `downstream_timeout`
+of 60 seconds.
 
 The login form (`POST /`) is limited to 5 attempts per minute per client
 address, counting an IPv6 client by its /64; an attempt over the limit is
@@ -439,23 +382,14 @@ nor change what it asks for.
 
 1. Open `/` in a browser over HTTPS (or over plain HTTP while `debug` is on, see
    Routes) and log in with the signing key (`signing_key`). The login session
-   lasts 30 days, or until someone logs out with the generator page's Logout
-   button. Every login is made with the one signing key, so logging out ends
-   every login session, in every browser, a copy of the session cookie included.
-   pixa keeps a random value in its database, which it makes when the database
-   has none and replaces at each logout. A login session carries the value
-   current at its login and is refused once the value has changed, also after a
-   restart (see Deployment). The check for a logout compares no time, so a login
-   after a logout works whatever the host's clock says; the 30 days are still
-   measured by the host's clock.
+   lasts 30 days, or until `/logout`.
 2. On the generator page, give the source image's URL, the width and height, the
    format, quality and fit, and how long the URL lasts, then submit the form
    (`POST /generate`). The format is JPEG XL unless another is chosen; a form
    sent with an empty format, or none, also makes a JPEG XL URL. Width and
    height both empty or `0` keep the original size; if only one of them is empty
    or `0`, that side is scaled to keep the image's proportions. A URL whose
-   scaled side would come out above 8192 answers 400, and so does an AVIF URL of
-   more than 16,777,216 pixels (see Routes).
+   scaled side would come out above 8192 answers 400.
 3. The page shows the URL, `https://<host>/v1/e/<token>/img.<format>`, and when
    it expires. `<host>` is the host the page was opened on, and the URL starts
    with `http` instead while `debug` is on. The name after the token is ignored
@@ -560,17 +494,6 @@ For the same image at quality 40 with fit `contain`, the input ends in
 and the URL is
 `/v1/image/cdn.example.com/photos/cat.jpg/800x600.webp?sig=5IwXUx6vf7yefhaUvFzgXZvG2o0Df4RJxPTK3pKq5VU=&exp=1704067200&q=40&fit=contain`.
 
-For a source URL with a query,
-`https://cdn.example.com/img?url=https://origin.example.com/my%20cat.jpg`, at
-800x600 WebP with the same key and expiration and the default quality and fit:
-
-1. Build input, with the query's `:` written as `%3A` and its `%` as `%25`:
-   `cdn.example.com:/img:url=https%3A//origin.example.com/my%2520cat.jpg:800:600:webp:1704067200:85:cover`
-2. The signature is `37aFa9GSVCZM1kRY9SzMOk2fOg_M7Wt5gKNR4xM-FYA=`
-3. URL, with the `?` before the query written as `%3F` and the query's `%` as
-   `%25` (see Routes):
-   `/v1/image/cdn.example.com/img%3Furl=https://origin.example.com/my%2520cat.jpg/800x600.webp?sig=37aFa9GSVCZM1kRY9SzMOk2fOg_M7Wt5gKNR4xM-FYA=&exp=1704067200`
-
 **Allowlist patterns:**
 
 - **Exact match**: `cdn.example.com` — matches only that host
@@ -630,7 +553,7 @@ tell. With no file, pixa uses the environment and the defaults.
 | `PIXA_ALLOW_HTTP`                    | `allow_http`                    | Allow plain-HTTP upstreams, for testing only; default `false`                |
 | `PIXA_UPSTREAM_CONNECTIONS_PER_HOST` | `upstream_connections_per_host` | Concurrent connections per upstream host; default `20`                       |
 | `PIXA_UPSTREAM_CONNECTIONS`          | `upstream_connections`          | Concurrent connections to all upstream hosts together; default `64`          |
-| `PIXA_MAX_CONCURRENT_PROCESSING`     | `max_concurrent_processing`     | Images processed at once; default half the CPUs, at least 1                  |
+| `PIXA_MAX_CONCURRENT_PROCESSING`     | `max_concurrent_processing`     | Images processed at once; default the number of CPUs                         |
 | `PIXA_IMAGE_REQUESTS_PER_MINUTE`     | `image_requests_per_minute`     | Image requests per client per minute; `0` turns it off; default `600`        |
 | `PIXA_UPSTREAM_FETCH_TIMEOUT`        | `upstream_fetch_timeout`        | Time allowed for one fetch from an upstream host; default `30s`              |
 | `PIXA_UPSTREAM_MAX_RESPONSE_SIZE`    | `upstream_max_response_size`    | Largest upstream response accepted, in bytes; default 50 MiB                 |
@@ -716,12 +639,10 @@ Key settings in more detail:
   does, and `downstream_timeout` has not ended first, the request is answered
   503 with the error `server busy, try again later`
 - `max_concurrent_processing` — the most images decoded and encoded at once;
-  default half the CPUs pixa can use (`GOMAXPROCS`, which follows a container's
-  CPU limit), rounded down, and at least 1. A value that is not a whole number
-  of at least 1 aborts startup. A request that finds all of them in use waits up
-  to 10 seconds for one to free up; if none does, and `downstream_timeout` has
-  not ended first, it is answered 503 the same way. Each one can take a lot of
-  memory: see "Running under upaas"
+  default the number of CPUs pixa can use (`GOMAXPROCS`), which follows a
+  container's CPU limit. A request that finds all of them in use waits up to 10
+  seconds for one to free up; if none does, and `downstream_timeout` has not
+  ended first, it is answered 503 the same way
 - `image_requests_per_minute` — how many requests one client may send to the
   image routes per minute (see the limit on them under Routes); default `600`.
   `0` turns the limit off, and a negative or non-integer value aborts startup.
@@ -751,9 +672,7 @@ See `configs/config.example.yml` for all options with defaults.
   `metrics.username` and `metrics.password` are set. A request's `handler` label
   is the pattern of the route it matches, such as `/v1/image/*`, never its path.
   Requests that match no route share the label `unmatched`: those the router
-  answers 404 or 405, and CORS preflights. A request's `method` label is its
-  method when that is `GET`, `HEAD`, `POST`, `PUT`, `PATCH`, `DELETE`,
-  `OPTIONS`, `CONNECT` or `TRACE`, and `other` for any other method
+  answers 404 or 405, and CORS preflights
 - **Logging**: stdlib slog
 
 ## Entrypoints

TODO.md

@@ -30,44 +30,6 @@ P2: security: per-origin rate limiting
 
 # Completed Steps
 
-- 2026-10-15 a WebP, AVIF or JPEG XL encode at the limits takes at most about 34
-  seconds (closes #272): with one libvips thread, 8192x8192 random pixels with
-  an alpha channel took about 61 seconds to encode as WebP, 96 as AVIF and 81 as
-  JPEG XL, so with the default `downstream_timeout` of 60 seconds such an image
-  was never delivered, yet its encode held a processing slot to the end. WebP is
-  now saved at effort 2 and JPEG XL at effort 4, which take up to about 23 and
-  27 seconds. AVIF was already at the lowest effort govips can set, so an AVIF
-  output of more than 16,777,216 pixels is refused with 400 before anything is
-  decoded; 4096x4096, 8192x2048 or 2048x8192 takes up to about 34 seconds. Each
-  time is the slowest at any quality. Resizing with `fit=cover` already decodes
-  the input, so the output's dimensions are worked out before resizing.
-  `README.md` gives the limit and the new times.
-- 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`,
@@ -86,34 +48,7 @@ P2: security: per-origin rate limiting
   and `script/fmt-check` use the node on the `PATH` only when it has the pinned
   major version. Of golangci-lint v2.14.0's two findings, an unused `nolint`
   directive is removed, and gosec's new path check is marked as not applying to
-  the fixed config file locations. The agent guidance moves unchanged from
-  `CLAUDE.md` to a root `AGENTS.md`, where `REPO_POLICIES.md` keeps it.
-- 2026-10-15 the request metrics record any method that is not a standard one as
-  `other` (closes #274): their `method` label was each request's method as sent,
-  and Go's HTTP server accepts any method name, so a client sending made-up
-  methods added series to the Prometheus registry that were never removed. The
-  label is now the request's method only for `GET`, `HEAD`, `POST`, `PUT`,
-  `PATCH`, `DELETE`, `OPTIONS`, `CONNECT` and `TRACE`. The metrics library reads
-  the method from the request it is handed, so the metrics middleware hands it a
-  copy of the request carrying the label; the handlers still see the method the
-  client sent.
-- 2026-10-15 logging out ends every login session (closes #256): a login session
-  was a cookie with nothing kept on the server, so `/logout` removed it from one
-  browser and a copy of it kept working for its 30 days. Every login is made
-  with the one signing key, so logging out now ends them all: the session
-  manager keeps a random value, made with `crypto/rand` when the new `logout`
-  table has none and replaced by each logout, saves it there so that it outlasts
-  a restart, puts the value at login in each session, and refuses a session
-  carrying another value. The check for a logout compares no time, so a login
-  after a logout works whatever the host's clock does; a session's 30 days are
-  still checked against the clock. Losing the state directory ends every login
-  session, and no logout can make an ended one work again. Logging out is now
-  `POST /logout` from a form on the generator page, behind the CSRF check the
-  other forms have, so a link or image elsewhere on the same site cannot end the
-  sessions. A logout without a valid login session ends none and only removes
-  the cookie, as anyone can take a CSRF token from the login page. A logout that
-  cannot be saved answers 500. `session.NewManager`, which the tests use, keeps
-  the value in memory only; pixad uses `session.NewManagerWithDatabase`.
+  the fixed config file locations.
 - 2026-10-15 a cached file left empty or partial is no longer served, or fetched
   again on every request (closes #252). Source and transformed image files, and
   each transformed image's `.meta` file, are synced to disk before they are

configs/config.example.yml

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

internal/config/concurrency_limits_internal_test.go

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

internal/config/config.go

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

internal/db/migrations/001_schema.sql

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

internal/handlers/auth.go

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

internal/handlers/auth_session_internal_test.go

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

internal/handlers/avif_too_large_internal_test.go

@@ -1,49 +0,0 @@
-package handlers
-
-import (
-	"fmt"
-	"log/slog"
-	"net/http"
-	"net/http/httptest"
-	"strings"
-	"testing"
-
-	"sneak.berlin/go/pixa/internal/imageprocessor"
-	"sneak.berlin/go/pixa/internal/imgcache"
-)
-
-// TestAVIFOutputTooLargeAnswers400 checks that respondImageError and
-// handleImageError, which answer the errors of the two image routes, answer
-// 400 naming the AVIF pixel limit when the image processor refuses an AVIF
-// output above it, wrapped as the service wraps it. The request's source only
-// goes into a log line, so it is left empty.
-func TestAVIFOutputTooLargeAnswers400(t *testing.T) {
-	t.Parallel()
-
-	h := &Handlers{log: slog.New(slog.DiscardHandler)}
-	err := fmt.Errorf("image processing failed: %w: 4097x4096",
-		imageprocessor.ErrAVIFOutputTooLarge)
-	wantBody := fmt.Sprintf("above %d pixels", imageprocessor.MaxAVIFOutputPixels)
-
-	plain := httptest.NewRecorder()
-	h.respondImageError(plain, &imgcache.ImageRequest{}, err)
-
-	encrypted := httptest.NewRecorder()
-	h.handleImageError(encrypted, err)
-
-	for name, rec := range map[string]*httptest.ResponseRecorder{
-		"respondImageError": plain, "handleImageError": encrypted,
-	} {
-		t.Logf("%s: status = %d, body = %q", name, rec.Code, rec.Body.String())
-
-		if rec.Code != http.StatusBadRequest {
-			t.Errorf("%s: status = %d, want %d",
-				name, rec.Code, http.StatusBadRequest)
-		}
-
-		if !strings.Contains(rec.Body.String(), wantBody) {
-			t.Errorf("%s: body = %q, want it to say %q",
-				name, rec.Body.String(), wantBody)
-		}
-	}
-}

internal/handlers/handlers.go

@@ -164,10 +164,7 @@ func (s *Handlers) initImageService() error {
 
 	// Initialize session manager (signing key is validated at config load
 	// time). Session cookies are always Secure/HttpOnly/SameSite=Strict.
-	// The random value that each logout replaces is saved in the database,
-	// so it outlasts a restart.
-	sessMgr, err := session.NewManagerWithDatabase(
-		context.Background(), s.config.SigningKey, s.db.DB())
+	sessMgr, err := session.NewManager(s.config.SigningKey)
 	if err != nil {
 		return err
 	}

internal/handlers/image.go

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

internal/handlers/image_signature_internal_test.go

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

internal/handlers/imageenc.go

@@ -170,10 +170,6 @@ func (s *Handlers) handleImageError(w http.ResponseWriter, err error) {
 		s.respondError(w, fmt.Sprintf("size too large for the source: the side "+
 			"computed from its proportions is above %d",
 			imageprocessor.MaxOutputDimension), http.StatusBadRequest)
-	case errors.Is(err, imageprocessor.ErrAVIFOutputTooLarge):
-		s.respondError(w, fmt.Sprintf("size too large for AVIF: the output "+
-			"would be above %d pixels", imageprocessor.MaxAVIFOutputPixels),
-			http.StatusBadRequest)
 	default:
 		s.log.Error("image request failed", "error", err)
 		s.respondError(w, "internal error", http.StatusInternalServerError)

internal/imageprocessor/avif_too_large_internal_test.go

@@ -1,64 +0,0 @@
-package imageprocessor
-
-import (
-	"bytes"
-	"errors"
-	"testing"
-)
-
-// TestImageProcessor_RefusesAVIFAboveMaxPixels verifies that an AVIF output
-// of more than MaxAVIFOutputPixels is refused with ErrAVIFOutputTooLarge
-// before anything is decoded, whatever size and fit give it, and that an AVIF
-// output of at most that many pixels, or one in another format, is not. The
-// input is a 64x32 PNG cut off in its pixel data: libvips reads its size from
-// its header but fails when it decodes it, so a request that gets that failure
-// was not refused, and a refusal came before any decoding.
-func TestImageProcessor_RefusesAVIFAboveMaxPixels(t *testing.T) {
-	t.Parallel()
-
-	whole := createTestPNG(t, 64, 32)
-	cutOff := whole[:len(whole)/2]
-
-	tests := []struct {
-		name          string
-		format        Format
-		width, height int
-		fit           FitMode
-		refused       bool
-	}{
-		{"AVIF at the limit", FormatAVIF, 4096, 4096, FitCover, false},
-		{"AVIF above the limit", FormatAVIF, 4096, 4097, FitCover, true},
-		{"WebP above the AVIF limit", FormatWebP, 4096, 4097, FitCover, false},
-		// FitInside does not enlarge, so the output stays 64x32
-		{"AVIF inside a larger size", FormatAVIF, 8192, 8192, FitInside, false},
-		// FitOutside scales 64x32 by 128 to cover 8192x1: 8192x4096
-		{"AVIF outside a smaller size", FormatAVIF, 8192, 1, FitOutside, true},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			_, err := New(Params{}).Process(t.Context(), bytes.NewReader(cutOff),
-				&Request{
-					Size:    Size{Width: tt.width, Height: tt.height},
-					Format:  tt.format,
-					FitMode: tt.fit,
-				})
-			t.Logf("Process() error = %v", err)
-
-			if tt.refused {
-				if !errors.Is(err, ErrAVIFOutputTooLarge) {
-					t.Errorf("Process() error = %v, want %v", err, ErrAVIFOutputTooLarge)
-				}
-
-				return
-			}
-
-			if err == nil || errors.Is(err, ErrAVIFOutputTooLarge) {
-				t.Errorf("Process() error = %v, want the failure to decode "+
-					"the cut-off input", err)
-			}
-		})
-	}
-}

internal/imageprocessor/encoder_settings_internal_test.go

@@ -77,10 +77,10 @@ func TestImageProcessor_PNGIsCompressed(t *testing.T) {
 	}
 }
 
-// TestImageProcessor_WebPAtEffort2 verifies that WebP is saved at effort 2,
-// not libvips' default of 4: the output is the size of the same image saved
-// at effort 2.
-func TestImageProcessor_WebPAtEffort2(t *testing.T) {
+// TestImageProcessor_WebPAtDefaultEffort verifies that WebP is saved at
+// libvips' default effort, 4: the output is the size of the same image saved
+// at that effort.
+func TestImageProcessor_WebPAtDefaultEffort(t *testing.T) {
 	t.Parallel()
 
 	input := createTestJPEG(t, 200, 150)
@@ -90,14 +90,14 @@ func TestImageProcessor_WebPAtEffort2(t *testing.T) {
 	want, _, err := decode(t, input).ExportWebp(&vips.WebpExportParams{
 		StripMetadata:   true,
 		Quality:         85,
-		ReductionEffort: 2,
+		ReductionEffort: 4,
 	})
 	if err != nil {
 		t.Fatalf("ExportWebp() error = %v", err)
 	}
 
 	if size != int64(len(want)) {
-		t.Errorf("WebP output is %d bytes, want %d, the size at effort 2",
+		t.Errorf("WebP output is %d bytes, want %d, the size at effort 4",
 			size, len(want))
 	}
 }
@@ -143,29 +143,3 @@ func TestImageProcessor_AVIFAtEffort1(t *testing.T) {
 			"and 8 bits", size, len(want))
 	}
 }
-
-// TestImageProcessor_JPEGXLAtEffort4 verifies that JPEG XL is saved at
-// effort 4, not libvips' default of 7: the output is the size of the same
-// image saved at effort 4, with the distance exportJXL gives quality 85.
-func TestImageProcessor_JPEGXLAtEffort4(t *testing.T) {
-	t.Parallel()
-
-	input := createTestJPEG(t, 200, 150)
-
-	size := processedSize(t, input, &Request{Format: FormatJXL, Quality: 85})
-
-	params := vips.NewJxlExportParams()
-	params.Quality = 85
-	params.Distance = jxlDistance(85)
-	params.Effort = 4
-
-	want, _, err := decode(t, input).ExportJxl(params)
-	if err != nil {
-		t.Fatalf("ExportJxl() error = %v", err)
-	}
-
-	if size != int64(len(want)) {
-		t.Errorf("JPEG XL output is %d bytes, want %d, the size at effort 4",
-			size, len(want))
-	}
-}

internal/imageprocessor/imageprocessor.go

@@ -7,11 +7,11 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"runtime"
 	"sync"
 	"time"
 
 	"github.com/davidbyttow/govips/v2/vips"
-	"sneak.berlin/go/pixa/internal/config"
 )
 
 // vipsOnce ensures vips is initialized exactly once.
@@ -20,10 +20,11 @@ import (
 var vipsOnce sync.Once
 
 // initVips initializes libvips with quiet logging, one worker thread per
-// image and no operation cache. Each request decodes different source
-// bytes, so the operation cache would rarely be hit and would hold memory
-// outside MaxConcurrentProcessing; repeated requests are served from
-// pixa's disk cache instead.
+// image and no operation cache. Process already works on one image per CPU
+// by default, so more threads per image would only compete for the CPUs.
+// Each request decodes different source bytes, so the operation cache
+// would rarely be hit and would hold memory outside MaxConcurrentProcessing;
+// repeated requests are served from pixa's disk cache instead.
 func initVips() {
 	vipsOnce.Do(func() {
 		vips.LoggingSettings(nil, vips.LogLevelError)
@@ -142,19 +143,6 @@ const MaxOutputDimension = 8192
 // requested size.
 var ErrOutputTooLarge = errors.New("output image dimensions exceed maximum")
 
-// MaxAVIFOutputPixels is the most pixels an AVIF output may have, so that an
-// encode takes at most 40 seconds of the default downstream_timeout of 60
-// seconds; AVIF is already at the lowest effort govips can set (avifEffort).
-// With one thread, as pixad runs libvips, saving 4096x4096, 8192x2048 or
-// 2048x8192 random pixels with an alpha channel takes up to about 34
-// seconds, the slowest at any quality and at 8 or 16 bits per sample, and
-// 5120x4096 at 8 bits already takes about 36.
-const MaxAVIFOutputPixels = 4096 * 4096
-
-// ErrAVIFOutputTooLarge is returned, before anything is decoded, when an AVIF
-// output would have more than MaxAVIFOutputPixels pixels.
-var ErrAVIFOutputTooLarge = errors.New("AVIF output has too many pixels")
-
 // ErrUnsupportedOutputFormat is returned when the requested output format is
 // not supported.
 var ErrUnsupportedOutputFormat = errors.New("unsupported output format")
@@ -191,7 +179,7 @@ type Params struct {
 	// If <= 0, DefaultMaxInputBytes is used.
 	MaxInputBytes int64
 	// MaxConcurrentProcessing is the most images processed at once.
-	// If <= 0, config.DefaultMaxConcurrentProcessing() is used.
+	// If <= 0, the number of CPUs Go uses (runtime.GOMAXPROCS(0)) is used.
 	MaxConcurrentProcessing int
 }
 
@@ -207,7 +195,7 @@ func New(params Params) *ImageProcessor {
 
 	maxConcurrentProcessing := params.MaxConcurrentProcessing
 	if maxConcurrentProcessing <= 0 {
-		maxConcurrentProcessing = config.DefaultMaxConcurrentProcessing()
+		maxConcurrentProcessing = runtime.GOMAXPROCS(0)
 	}
 
 	return &ImageProcessor{
@@ -223,8 +211,7 @@ func New(params Params) *ImageProcessor {
 // ErrTooManyImages. When ctx has ended once Process has a slot, or before it
 // encodes, it returns ctx's error and gives the slot back. govips gives pixa
 // no way to stop libvips once an encode has begun, so an encode keeps its
-// slot until it finishes. An AVIF output of more than MaxAVIFOutputPixels is
-// refused with ErrAVIFOutputTooLarge before anything is decoded.
+// slot until it finishes.
 func (p *ImageProcessor) Process(
 	ctx context.Context,
 	input io.Reader,
@@ -237,9 +224,18 @@ func (p *ImageProcessor) Process(
 
 	defer release()
 
-	data, err := p.readInput(input)
+	// Read input with a size limit to prevent unbounded memory consumption.
+	// We read at most maxInputBytes+1 so we can detect if the input exceeds
+	// the limit without consuming additional memory.
+	limited := io.LimitReader(input, p.maxInputBytes+1)
+
+	data, err := io.ReadAll(limited)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to read input: %w", err)
+	}
+
+	if int64(len(data)) > p.maxInputBytes {
+		return nil, ErrInputDataTooLarge
 	}
 
 	// govips picks the libvips loader from the input's first bytes with
@@ -282,28 +278,20 @@ func (p *ImageProcessor) Process(
 		return nil, err
 	}
 
+	// Resize if needed
+	if targetWidth != origWidth || targetHeight != origHeight {
+		err := p.resize(img, targetWidth, targetHeight, req.FitMode)
+		if err != nil {
+			return nil, fmt.Errorf("failed to resize: %w", err)
+		}
+	}
+
 	// orig is the source's own format; encode refuses an empty format
 	outputFormat := req.Format
 	if outputFormat == FormatOriginal {
 		outputFormat = sourceFormat
 	}
 
-	// Determine output dimensions; too large ones are refused before
-	// resizing, which with FitCover already decodes the input
-	outputWidth, outputHeight, err := outputDimensions(origWidth, origHeight,
-		targetWidth, targetHeight, req.FitMode, outputFormat)
-	if err != nil {
-		return nil, err
-	}
-
-	// Resize if needed
-	if targetWidth != origWidth || targetHeight != origHeight {
-		err := p.resize(img, outputWidth, outputHeight, req.FitMode)
-		if err != nil {
-			return nil, fmt.Errorf("failed to resize: %w", err)
-		}
-	}
-
 	// Encode to target format
 	output, err := p.encode(ctx, img, outputFormat, req.Quality)
 	if err != nil {
@@ -456,23 +444,6 @@ func (p *ImageProcessor) acquireSlot(ctx context.Context) (func(), error) {
 	return release, nil
 }
 
-// readInput reads input with a size limit to prevent unbounded memory
-// consumption, refusing more than maxInputBytes with ErrInputDataTooLarge. It
-// reads at most maxInputBytes+1 so it can detect if the input exceeds the
-// limit without consuming additional memory.
-func (p *ImageProcessor) readInput(input io.Reader) ([]byte, error) {
-	data, err := io.ReadAll(io.LimitReader(input, p.maxInputBytes+1))
-	if err != nil {
-		return nil, fmt.Errorf("failed to read input: %w", err)
-	}
-
-	if int64(len(data)) > p.maxInputBytes {
-		return nil, ErrInputDataTooLarge
-	}
-
-	return data, nil
-}
-
 // detectFormat returns the format string for a govips image type.
 func (p *ImageProcessor) detectFormat(imageType vips.ImageType) string {
 	switch imageType {
@@ -497,61 +468,9 @@ func (p *ImageProcessor) detectFormat(imageType vips.ImageType) string {
 	}
 }
 
-// outputDimensions returns the dimensions resize gives an image of imgWidth x
-// imgHeight for the target dimensions and fit mode, without resizing it, and
-// refuses an output too large for format. With FitOutside one side can come
-// out larger than the target; when it would be above MaxOutputDimension,
-// outputDimensions returns ErrOutputTooLarge. An AVIF output of more than
-// MaxAVIFOutputPixels is ErrAVIFOutputTooLarge.
-func outputDimensions(
-	imgWidth, imgHeight, width, height int, fit FitMode, format Format,
-) (int, int, error) {
-	scaleW := float64(width) / float64(imgWidth)
-	scaleH := float64(height) / float64(imgHeight)
-	outputWidth, outputHeight := width, height
-
-	switch fit {
-	case FitCover, FitFill, "":
-		// Cover crops, and fill stretches, to exactly the target dimensions
-
-	case FitContain:
-		// Fit within the target dimensions, maintaining aspect ratio
-		scale := min(scaleW, scaleH)
-		outputWidth = int(float64(imgWidth) * scale)
-		outputHeight = int(float64(imgHeight) * scale)
-
-	case FitInside:
-		// Same as contain, but only shrink
-		scale := min(scaleW, scaleH, 1)
-		outputWidth = int(float64(imgWidth) * scale)
-		outputHeight = int(float64(imgHeight) * scale)
-
-	case FitOutside:
-		// The smallest dimension fits, the other may exceed the target
-		scale := max(scaleW, scaleH)
-		outputWidth = int(float64(imgWidth) * scale)
-		outputHeight = int(float64(imgHeight) * scale)
-
-	default:
-		return 0, 0, fmt.Errorf("%w: %s", ErrInvalidFitMode, fit)
-	}
-
-	// The side FitOutside scales past the target is held to the maximum too
-	if max(outputWidth, outputHeight) > MaxOutputDimension {
-		return 0, 0, fmt.Errorf("%w: %dx%d is above %d", ErrOutputTooLarge,
-			outputWidth, outputHeight, MaxOutputDimension)
-	}
-
-	if format == FormatAVIF && outputWidth*outputHeight > MaxAVIFOutputPixels {
-		return 0, 0, fmt.Errorf("%w: %dx%d is above %d pixels",
-			ErrAVIFOutputTooLarge, outputWidth, outputHeight, MaxAVIFOutputPixels)
-	}
-
-	return outputWidth, outputHeight, nil
-}
-
-// resize resizes the image to width x height, the dimensions
-// outputDimensions gives for the fit mode.
+// resize resizes the image according to the fit mode. With FitOutside one
+// side can come out larger than requested; when it would be above
+// MaxOutputDimension, resize returns ErrOutputTooLarge without resizing.
 func (p *ImageProcessor) resize(
 	img *vips.ImageRef, width, height int, fit FitMode,
 ) error {
@@ -560,20 +479,52 @@ func (p *ImageProcessor) resize(
 		// Resize and crop to fill exact dimensions (default)
 		return img.Thumbnail(width, height, vips.InterestingCentre)
 
+	case FitContain:
+		// Resize to fit within dimensions, maintaining aspect ratio
+		imgW, imgH := img.Width(), img.Height()
+		scaleW := float64(width) / float64(imgW)
+		scaleH := float64(height) / float64(imgH)
+		scale := min(scaleW, scaleH)
+		newW := int(float64(imgW) * scale)
+		newH := int(float64(imgH) * scale)
+
+		return img.Thumbnail(newW, newH, vips.InterestingNone)
+
 	case FitFill:
 		// Resize to exact dimensions (may distort)
 		return img.ThumbnailWithSize(width, height, vips.InterestingNone, vips.SizeForce)
 
 	case FitInside:
-		// outputDimensions keeps the dimensions of an image that already fits
-		if img.Width() == width && img.Height() == height {
-			return nil
+		// Same as contain, but only shrink
+		if img.Width() <= width && img.Height() <= height {
+			return nil // Already fits
 		}
 
-		return img.Thumbnail(width, height, vips.InterestingNone)
+		imgW, imgH := img.Width(), img.Height()
+		scaleW := float64(width) / float64(imgW)
+		scaleH := float64(height) / float64(imgH)
+		scale := min(scaleW, scaleH)
+		newW := int(float64(imgW) * scale)
+		newH := int(float64(imgH) * scale)
 
-	case FitContain, FitOutside:
-		return img.Thumbnail(width, height, vips.InterestingNone)
+		return img.Thumbnail(newW, newH, vips.InterestingNone)
+
+	case FitOutside:
+		// Resize so smallest dimension fits, may exceed target on other dimension
+		imgW, imgH := img.Width(), img.Height()
+		scaleW := float64(width) / float64(imgW)
+		scaleH := float64(height) / float64(imgH)
+		scale := max(scaleW, scaleH)
+		newW := int(float64(imgW) * scale)
+		newH := int(float64(imgH) * scale)
+
+		// The dimension that exceeds the target is held to the maximum too
+		if newW > MaxOutputDimension || newH > MaxOutputDimension {
+			return fmt.Errorf("%w: %dx%d is above %d", ErrOutputTooLarge,
+				newW, newH, MaxOutputDimension)
+		}
+
+		return img.Thumbnail(newW, newH, vips.InterestingNone)
 
 	default:
 		return fmt.Errorf("%w: %s", ErrInvalidFitMode, fit)
@@ -677,16 +628,11 @@ func exportGIF(img *vips.ImageRef) ([]byte, error) {
 	return output, err
 }
 
-// webpEffort is the WebP effort, from 0 (fastest) to 6. It is below libvips'
-// default of 4 so that an encode takes at most 40 seconds of the default
-// downstream_timeout of 60 seconds. With one thread, as pixad runs libvips,
-// saving an 8192x8192 image of random pixels with an alpha channel takes up
-// to about 23 seconds at 2, and up to about 16 without one, at any quality;
-// with an alpha channel, 3 takes about 50 seconds and 4 about 61.
-const webpEffort = 2
+// webpEffort is libvips' default WebP effort, from 0 (fastest) to 6.
+const webpEffort = 4
 
-// exportWebP encodes img as lossy WebP at quality and webpEffort, without
-// metadata.
+// exportWebP encodes img as lossy WebP at quality and libvips' default
+// effort, without metadata.
 func exportWebP(img *vips.ImageRef, quality int) ([]byte, error) {
 	output, _, err := img.ExportWebp(&vips.WebpExportParams{
 		StripMetadata:   true,
@@ -698,13 +644,12 @@ func exportWebP(img *vips.ImageRef, quality int) ([]byte, error) {
 }
 
 // avifEffort is the AVIF effort, from 0 (fastest) to 9; 1 is the lowest
-// govips can set. With one thread, as pixad runs libvips, 1 takes about 52
-// seconds to save an 8192x8192 RGB image of random pixels at quality 85,
-// and about 96 with an alpha channel, against the default
-// downstream_timeout of 60 seconds, so an AVIF output is held to
-// MaxAVIFOutputPixels. On an image of milder noise, which 1 saves in about
-// 12 seconds, 2 takes nearly a minute and libvips' default, 4, takes
-// minutes.
+// govips can set. With one thread, as pixad runs libvips, 1 takes about 51
+// seconds to save an 8192x8192 RGB image of random pixels, and about 110
+// with an alpha channel, the worst case, against the default
+// downstream_timeout of 60 seconds. On an image of milder noise, which 1
+// saves in about 12 seconds, 2 takes nearly a minute and libvips' default,
+// 4, takes minutes.
 const avifEffort = 1
 
 // avifBitdepth is the AVIF bit depth, 8 bits per sample for every image.
@@ -731,18 +676,10 @@ func exportAVIF(img *vips.ImageRef, quality int) ([]byte, error) {
 // JPEG that names none.
 const jxlResolution = 72 / 25.4
 
-// jxlEffort is the JPEG XL effort, from 1 (fastest) to 9. It is below
-// libvips' default of 7 so that an encode takes at most 40 seconds of the
-// default downstream_timeout of 60 seconds. With one thread, as pixad runs
-// libvips, saving an 8192x8192 image of random pixels with an alpha channel
-// takes up to about 27 seconds at 4, and up to about 13 without one, at any
-// quality; with an alpha channel, 5 takes about 43 seconds and 7 about 81.
-const jxlEffort = 4
-
-// exportJXL encodes img as JPEG XL at quality and jxlEffort, without
-// metadata. govips sends libvips a distance, the JPEG XL encoder's own
-// measure of quality, along with the quality, and libvips then uses the
-// distance alone, so the quality is also given as a distance.
+// exportJXL encodes img as JPEG XL at quality, with libvips' default effort
+// and without metadata. govips sends libvips a distance, the JPEG XL
+// encoder's own measure of quality, along with the quality, and libvips then
+// uses the distance alone, so the quality is also given as a distance.
 func exportJXL(img *vips.ImageRef, quality int) ([]byte, error) {
 	// libvips converts a CMYK image to sRGB before it saves WebP, AVIF or
 	// PNG, but cannot save one as JPEG XL. encode has already converted
@@ -781,7 +718,6 @@ func exportJXL(img *vips.ImageRef, quality int) ([]byte, error) {
 	params := vips.NewJxlExportParams()
 	params.Quality = quality
 	params.Distance = jxlDistance(quality)
-	params.Effort = jxlEffort
 
 	output, _, err := toSave.ExportJxl(params)
 	if err != nil {

internal/imageprocessor/max_concurrent_processing_internal_test.go

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

internal/imgcache/service.go

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

internal/imgcache/urlparser.go

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

internal/imgcache/urlparser_internal_test.go

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

internal/middleware/middleware.go

@@ -220,10 +220,7 @@ func (s *Middleware) CORS() func(http.Handler) http.Handler {
 // pattern of the chi route a request matches, such as /v1/image/*, or
 // "unmatched" for a request that matches none, so the routes fix how many
 // values the label has. It is never the path, with which any client could
-// add series to the registry without limit. Its method label is the
-// request's method when that is a standard one, such as GET, and "other"
-// for any other: Go's HTTP server accepts any method name, so a label per
-// method would let a client add series the same way.
+// add series to the registry without limit.
 func (s *Middleware) Metrics() func(http.Handler) http.Handler {
 	mdlw := ghmm.New(ghmm.Config{
 		Recorder: metrics.NewRecorder(metrics.Config{
@@ -233,19 +230,7 @@ func (s *Middleware) Metrics() func(http.Handler) http.Handler {
 
 	return func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-			// The metrics library takes the method label from the request
-			// it is handed, so it is handed a shallow copy of r whose
-			// method is the label. next is served r itself, through the
-			// response writer the library wraps to see the status and size.
-			labelled := r.WithContext(r.Context())
-			labelled.Method = methodLabel(r.Method)
-
-			serveNext := http.HandlerFunc(
-				func(wrapped http.ResponseWriter, _ *http.Request) {
-					next.ServeHTTP(wrapped, r)
-				})
-
-			std.Handler(routePattern(r), mdlw, serveNext).ServeHTTP(w, labelled)
+			std.Handler(routePattern(r), mdlw, next).ServeHTTP(w, r)
 		})
 	}
 }
@@ -290,20 +275,6 @@ func routePattern(r *http.Request) string {
 	return pattern
 }
 
-// methodLabel returns method when it is one of the standard methods net/http
-// names: GET, HEAD, POST, PUT, PATCH, DELETE, OPTIONS, CONNECT and TRACE.
-// For any other method it returns "other".
-func methodLabel(method string) string {
-	switch method {
-	case http.MethodGet, http.MethodHead, http.MethodPost, http.MethodPut,
-		http.MethodPatch, http.MethodDelete, http.MethodOptions,
-		http.MethodConnect, http.MethodTrace:
-		return method
-	default:
-		return "other"
-	}
-}
-
 // MetricsAuth returns a basic auth middleware for the metrics endpoint.
 func (s *Middleware) MetricsAuth() func(http.Handler) http.Handler {
 	return basicauth.New(

internal/middleware/middleware_internal_test.go

@@ -7,7 +7,6 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"net/url"
-	"slices"
 	"strings"
 	"testing"
 
@@ -315,58 +314,6 @@ func requestCountsByHandler(
 	return counts
 }
 
-// TestMetricsGivesMadeUpMethodsOneLabel checks that the metrics middleware
-// gives requests with methods that are not standard ones a single method
-// label, "other", and still serves each request with the method it was sent
-// with. With a label per method, any client could add series to the
-// registry without limit by making up a new method for each request.
-func TestMetricsGivesMadeUpMethodsOneLabel(t *testing.T) {
-	t.Parallel()
-
-	// The line /metrics shows once both made-up methods have been served.
-	const want = `http_request_duration_seconds_count{` +
-		`code="200",handler="unmatched",method="other",service=""} 2`
-
-	registry := prometheus.NewRegistry()
-	mw := &Middleware{
-		log:             slog.Default(),
-		config:          &config.Config{},
-		metricsRegistry: registry,
-	}
-
-	var served []string
-
-	handler := mw.Metrics()(http.HandlerFunc(
-		func(w http.ResponseWriter, r *http.Request) {
-			served = append(served, r.Method)
-
-			w.WriteHeader(http.StatusOK)
-		}))
-
-	methods := []string{"FROBNICATE", "WIBBLE"}
-	for _, method := range methods {
-		handler.ServeHTTP(httptest.NewRecorder(), httptest.NewRequestWithContext(
-			t.Context(), method, "/", nil))
-	}
-
-	t.Logf("methods the handler was served: %v", served)
-
-	if !slices.Equal(served, methods) {
-		t.Errorf("methods the handler was served = %v, want %v",
-			served, methods)
-	}
-
-	rec := httptest.NewRecorder()
-	promhttp.HandlerFor(registry, promhttp.HandlerOpts{}).ServeHTTP(rec,
-		httptest.NewRequestWithContext(
-			t.Context(), http.MethodGet, "/metrics", nil))
-
-	if !strings.Contains(rec.Body.String(), want) {
-		t.Errorf("/metrics does not count both made-up methods as one; "+
-			"want the line %q in:\n%s", want, rec.Body.String())
-	}
-}
-
 // TestLoggingLeavesOutSubmittedSigningKey checks that a login, a POST /
 // whose form carries the signing key, leaves no trace of the key in the
 // request's log line.

internal/server/logout_internal_test.go

@@ -1,269 +0,0 @@
-package server
-
-import (
-	"net/http"
-	"net/http/httptest"
-	"net/url"
-	"path/filepath"
-	"regexp"
-	"strings"
-	"testing"
-
-	"go.uber.org/fx"
-	"go.uber.org/fx/fxtest"
-
-	"sneak.berlin/go/pixa/internal/config"
-	"sneak.berlin/go/pixa/internal/database"
-	"sneak.berlin/go/pixa/internal/globals"
-	"sneak.berlin/go/pixa/internal/handlers"
-	"sneak.berlin/go/pixa/internal/healthcheck"
-	"sneak.berlin/go/pixa/internal/logger"
-	"sneak.berlin/go/pixa/internal/middleware"
-	"sneak.berlin/go/pixa/internal/session"
-)
-
-// logoutFormPattern extracts the CSRF token rendered into the generator
-// page's logout form.
-var logoutFormPattern = regexp.MustCompile(
-	`(?s)action="/logout".*?name="gorilla\.csrf\.Token" value="([^"]+)"`)
-
-// startPixa starts the components pixad's fx app builds, from cfg, and
-// returns the server with its routes, as newTestServer does.
-func startPixa(t *testing.T, cfg *config.Config) *Server {
-	t.Helper()
-
-	var (
-		h  *handlers.Handlers
-		mw *middleware.Middleware
-	)
-
-	app := fxtest.New(t,
-		fx.Supply(cfg),
-		fx.Provide(
-			globals.New,
-			logger.New,
-			database.New,
-			healthcheck.New,
-			handlers.New,
-			middleware.New,
-		),
-		fx.Populate(&h, &mw),
-	)
-	app.RequireStart()
-	t.Cleanup(app.RequireStop)
-
-	s := &Server{config: cfg, mw: mw, h: h}
-	s.SetupRoutes()
-
-	return s
-}
-
-// logIn logs in to s with the signing key, as postLogin does, and returns
-// the session cookie it sets.
-func logIn(t *testing.T, s *Server) *http.Cookie {
-	t.Helper()
-
-	rec := postLogin(t, s, firstClient, "", testSigningKey)
-
-	for _, c := range rec.Result().Cookies() {
-		if c.Name == session.CookieName {
-			return c
-		}
-	}
-
-	t.Fatal("logging in set no session cookie")
-
-	return nil
-}
-
-// opensGeneratorPage reports whether GET / with cookie shows s's generator
-// page, which only a valid login session opens.
-func opensGeneratorPage(t *testing.T, s *Server, cookie *http.Cookie) bool {
-	t.Helper()
-
-	req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
-	req.AddCookie(cookie)
-
-	rec := httptest.NewRecorder()
-	s.ServeHTTP(rec, req)
-
-	return rec.Code == http.StatusOK &&
-		strings.Contains(rec.Body.String(), `action="/generate"`)
-}
-
-// postLogout loads the generator page with GET / and sessionCookie and
-// submits its logout form with POST /logout, as a browser does.
-func postLogout(
-	t *testing.T, s *Server, sessionCookie *http.Cookie,
-) *httptest.ResponseRecorder {
-	t.Helper()
-
-	req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
-	req.AddCookie(sessionCookie)
-
-	page := httptest.NewRecorder()
-	s.ServeHTTP(page, req)
-
-	match := logoutFormPattern.FindStringSubmatch(page.Body.String())
-	if match == nil {
-		t.Fatal("the generator page has no logout form with a CSRF token")
-	}
-
-	form := url.Values{"gorilla.csrf.Token": {match[1]}}
-	req = httptest.NewRequestWithContext(t.Context(), http.MethodPost,
-		"/logout", strings.NewReader(form.Encode()))
-	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
-	req.AddCookie(sessionCookie)
-
-	for _, c := range page.Result().Cookies() {
-		req.AddCookie(c)
-	}
-
-	rec := httptest.NewRecorder()
-	s.ServeHTTP(rec, req)
-
-	return rec
-}
-
-// TestLogoutWithoutCSRFTokenEndsNoSession verifies that a request to /logout
-// carrying the session cookie but not the logout form's CSRF token ends no
-// session: GET /logout, as an image on another page of the same site makes a
-// browser send, answers 405, and POST /logout without the token answers 403.
-func TestLogoutWithoutCSRFTokenEndsNoSession(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		method string
-		want   int
-	}{
-		{http.MethodGet, http.StatusMethodNotAllowed},
-		{http.MethodPost, http.StatusForbidden},
-	}
-
-	for _, tc := range tests {
-		t.Run(tc.method, func(t *testing.T) {
-			t.Parallel()
-
-			s := newTestServer(t)
-			cookie := logIn(t, s)
-
-			req := httptest.NewRequestWithContext(
-				t.Context(), tc.method, "/logout", nil)
-			req.AddCookie(cookie)
-
-			rec := httptest.NewRecorder()
-			s.ServeHTTP(rec, req)
-			t.Logf("%s /logout without the CSRF token: status %d",
-				tc.method, rec.Code)
-
-			if rec.Code != tc.want {
-				t.Errorf("%s /logout without the CSRF token status = %d, "+
-					"want %d", tc.method, rec.Code, tc.want)
-			}
-
-			if !opensGeneratorPage(t, s, cookie) {
-				t.Errorf("%s /logout without the CSRF token ended the "+
-					"login session", tc.method)
-			}
-		})
-	}
-}
-
-// TestLoginAfterLogoutOpensGeneratorPage verifies that after a logout from
-// the generator page a copy of the session cookie taken before it is
-// refused, and that a login after it opens the generator page.
-func TestLoginAfterLogoutOpensGeneratorPage(t *testing.T) {
-	t.Parallel()
-
-	s := newTestServer(t)
-
-	copied := logIn(t, s)
-	if !opensGeneratorPage(t, s, copied) {
-		t.Fatal("logging in did not open the generator page")
-	}
-
-	logout := postLogout(t, s, copied)
-	if logout.Code != http.StatusSeeOther {
-		t.Fatalf("POST /logout status = %d, want %d",
-			logout.Code, http.StatusSeeOther)
-	}
-
-	if opensGeneratorPage(t, s, copied) {
-		t.Error("a cookie copied before the logout still opens the " +
-			"generator page")
-	}
-
-	opens := opensGeneratorPage(t, s, logIn(t, s))
-	t.Logf("a login after the logout opens the generator page: %v", opens)
-
-	if !opens {
-		t.Error("a login after the logout does not open the generator page")
-	}
-}
-
-// TestLogoutOutlastsRestart verifies that a copy of the session cookie taken
-// before a logout is still refused by a pixa started again on the same state
-// directory, as pixad is after a restart.
-func TestLogoutOutlastsRestart(t *testing.T) {
-	t.Parallel()
-
-	s := newTestServer(t)
-
-	copied := logIn(t, s)
-	if !opensGeneratorPage(t, s, copied) {
-		t.Fatal("logging in did not open the generator page")
-	}
-
-	logout := postLogout(t, s, copied)
-	if logout.Code != http.StatusSeeOther {
-		t.Fatalf("POST /logout status = %d, want %d",
-			logout.Code, http.StatusSeeOther)
-	}
-
-	restarted := startPixa(t, s.config)
-
-	opens := opensGeneratorPage(t, restarted, copied)
-	t.Logf("after the restart, the copied cookie opens the generator page: %v",
-		opens)
-
-	if opens {
-		t.Error("after a restart, a cookie copied before the logout " +
-			"still opens the generator page")
-	}
-}
-
-// TestLostStateDirectoryEndsEveryLoginSession verifies that a pixa started on
-// an empty state directory, as pixad is after a restart that lost its state
-// directory, refuses a session cookie from before, and that a login on it
-// opens the generator page.
-func TestLostStateDirectoryEndsEveryLoginSession(t *testing.T) {
-	t.Parallel()
-
-	s := newTestServer(t)
-
-	before := logIn(t, s)
-	if !opensGeneratorPage(t, s, before) {
-		t.Fatal("logging in did not open the generator page")
-	}
-
-	emptyStateDir := *s.config
-	emptyStateDir.StateDir = t.TempDir()
-	emptyStateDir.DBURL = "file:" +
-		filepath.Join(emptyStateDir.StateDir, "state.sqlite3")
-
-	restarted := startPixa(t, &emptyStateDir)
-
-	opens := opensGeneratorPage(t, restarted, before)
-	t.Logf("on an empty state directory, the cookie from before opens the "+
-		"generator page: %v", opens)
-
-	if opens {
-		t.Error("on an empty state directory, a cookie from before still " +
-			"opens the generator page")
-	}
-
-	if !opensGeneratorPage(t, restarted, logIn(t, restarted)) {
-		t.Error("a login on an empty state directory does not open the " +
-			"generator page")
-	}
-}

internal/server/routes.go

@@ -69,9 +69,10 @@ func (s *Server) SetupRoutes() {
 		r.With(s.mw.RateLimit(LoginAttemptsPerMinute, time.Minute, nil)).
 			Post("/", s.h.HandleRoot())
 		r.Post("/generate", s.h.HandleGenerateURL())
-		r.Post("/logout", s.h.HandleLogout())
 	})
 
+	s.router.Get("/logout", s.h.HandleLogout())
+
 	// Image routes, the only ones that send CORS headers, as pages on other
 	// sites read them. They are a subrouter rather than a group: a group's
 	// middleware runs only for a request that matches one of its routes,

internal/session/session.go

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

internal/session/session_logout_test.go

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

internal/signature/signature.go

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

internal/signature/signature_test.go

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

internal/static/style.css

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

internal/templates/generator.html

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