Refuse a q outside 1-100 on /v1/image/ with 400 (closes #134)

The route ignored a q that was not a number or was outside 1-100 and
used 85, so q=banana or q=500 was served as if q were absent and
verified against a signature made for 85.

It now reads q with the check the URL generator uses for its quality
field (parseFormInt with minQuality and maxQuality, default
encurl.DefaultQuality) and answers anything else with a 400 naming q
and the value. An absent or empty q is still 85. README.md states the
range.

Model: opus-5-5

README.md

@@ -94,38 +94,9 @@ In-process caching of request-to-output mappings targets 1-5k r/s.
 
 Images are only fetched from origins using TLS with valid certificates.
 
-A request whose query string cannot be decoded, or gives any parameter more
-than once, is refused with 400.
-
 - `<format>`: one of `orig`, `png`, `jpeg`, `webp`
 - `<size>`: `orig` or `<width>x<height>` (e.g. `800x600`)
 
-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
-refused with 429 and a `Retry-After` header. Behind a reverse proxy the client
-address comes from `X-Forwarded-For` only when the proxy's address is in
-`trusted_proxies`; otherwise all users behind the proxy are counted as one
-client. With the default `trusted_proxies` (the RFC 1918 ranges), a client
-with a private address can choose the address it is counted by through its own
-`X-Forwarded-For`, whether it connects directly or through the proxy, because
-its own address is trusted too. Setting `trusted_proxies` to the proxy's own
-address closes this.
-
-### Image Metadata
-
-pixa decodes and re-encodes every image it serves, and removes all metadata from
-the output: EXIF (GPS position, camera make, model and serial number, capture
-time, embedded thumbnail), XMP, IPTC and the ICC colour profile. This cannot be
-turned off.
-
-- The `orig` format means the source's own format, not the source's bytes: an
-  `orig` image is re-encoded and stripped like any other.
-- An image with an EXIF orientation is turned upright first, so it displays the
-  same without the tag; a requested size applies to the upright image.
-- An image with an ICC profile is converted to sRGB first, since clients show an
-  image with no profile as sRGB. Colours outside sRGB, such as the most
-  saturated ones in a Display P3 photo, are clipped.
-
 ### Source Hosts
 
 Source hosts may be allowlisted in the configuration. Non-allowlisted
@@ -153,15 +124,12 @@ Where:
 - `width` — requested width in pixels, `0` for original
 - `height` — requested height in pixels, `0` for original
 - `format` — output format (jpeg, png, webp, avif, gif, orig)
-- `expiration` — the URL's `exp` query parameter, the Unix timestamp when
+- `expiration` — Unix timestamp when signature expires
-  the signature expires; a request whose `exp` is not a whole number, an
-  empty `exp=` included, is refused with 400
 - `quality` — the URL's `q` query parameter, a whole number from 1 to 100,
   or `85` when the URL has no `q`; a request whose `q` is anything else is
   refused with 400
 - `fit` — the URL's `fit` query parameter (cover, contain, fill, inside,
-  outside), or `cover` when the URL has no `fit`; a request whose `fit` is
+  outside), or `cover` when the URL has no `fit`
-  anything else, an empty `fit=` included, is refused with 400
 
 **Example:** resize `https://cdn.example.com/photos/cat.jpg` to 800x600
 WebP with expiration 1704067200, default quality and fit:
@@ -231,8 +199,7 @@ Key settings in more detail:
   inside one of these ranges; the logged and login-recorded client
   address is then the rightmost forwarded entry that is not itself a
   trusted proxy. Otherwise the direct peer address is used and the header
-  is ignored, so a client connecting directly from an address outside
+  is ignored, so a client connecting directly cannot spoof its address.
-  these ranges cannot spoof its address.
   An omitted key defaults to the RFC 1918 private ranges (`10.0.0.0/8`,
   `172.16.0.0/12`, `192.168.0.0/16`), since pixa is deployed behind a
   proxy on a private network; an explicitly empty list (`[]`) trusts no

TODO.md

@@ -30,46 +30,12 @@ exhaustion
 
 # Completed Steps
 
-- 2026-09-28 strip metadata from processed images (closes #82): every output is
-  exported with govips' `StripMetadata`, so it carries no EXIF, XMP, IPTC or ICC
-  profile; the image is first turned upright with `AutoRotate` (before sizes are
-  worked out) and, when it has an ICC profile, converted to sRGB; the `orig`
-  format is re-encoded and stripped like any other, as pixa never serves the
-  source bytes; there is no setting to keep metadata; documented in `README.md`.
-- 2026-09-28 rate limit the login form (closes #66): `POST /` is limited to 5
-  attempts per minute per client address, and an attempt over the limit is
-  refused with 429 and a `Retry-After` header; the address is the one
-  `internal/clientip` resolves through `trusted_proxies`, an IPv6 client is
-  counted by its /64, and an IPv4-mapped address as the IPv4 address it
-  carries; the limit is a `RateLimit` middleware in `internal/middleware` on
-  `github.com/go-chi/httprate`, which the image routes can reuse; the library
-  keeps counts for the current and the previous minute only; documented in
-  `README.md`.
-- 2026-09-28 refuse an unparseable `exp` on `/v1/image/` and log swallowed
-  cache errors (closes #72): an `exp` in the URL that is not a whole
-  number, an empty `exp=` included, is a 400 naming `exp` and the value,
-  instead of being ignored and answered with 401 as if the URL had no
-  `exp`; only an `exp` missing from the URL is unchanged; `README.md` says
-  so where it documents `exp`. A failed variant `.meta` write, source
-  metadata JSON write, `Stats` count query, stats counter update, negative
-  cache write or expired negative cache delete is now logged at `warn`
-  with the path or key and the error, and stays non-fatal.
-- 2026-09-28 refuse an empty `fit` on `/v1/image/` (closes #139): a
-  `fit` in the URL with an empty value (`fit=`) is a 400 naming `fit`,
-  instead of being served as `cover` and verified against a signature
-  made for `cover`; only a `fit` missing from the URL is still `cover`;
-  any other value still goes through the existing fit-mode check;
-  `README.md` says so where it documents `fit`.
 - 2026-09-28 refuse an invalid `q` on `/v1/image/` (closes #134): a `q`
-  that is not a whole number from 1 to 100, an empty `q` included, is a
+  that is not a whole number from 1 to 100 is a 400 naming `q` and the
-  400 naming `q` and the value, instead of being served at the default
+  value, instead of being served at the default 85; the route reads `q`
-  85; the route reads `q` with the generator's quality check
+  with the generator's quality check (`parseFormInt` with `minQuality`
-  (`parseFormInt` with `minQuality` and `maxQuality`); only a `q` missing
+  and `maxQuality`); an absent or empty `q` is still 85; `README.md`
-  from the URL is still 85; a query string that cannot be decoded, such
+  states the range.
-  as `q=80%`, is a 400 showing it; any query parameter given more than
-  once (`q`, `fit`, `sig`, `exp` alike) is a 400 naming it, so none is
-  read from its first value only; `README.md` states the range and both
-  query-string rules.
 - 2026-09-28 unknown `PIXA_` environment variables abort startup (closes
   #133): a variable whose name starts with `PIXA_` but is neither a
   setting's variable nor `PIXA_CONFIG_PATH` aborts startup naming it, as
@@ -257,9 +223,10 @@ exhaustion
 
 # Future Steps
 
+- P1: strip EXIF and other metadata from processed images (privacy)
 - P2: security
     - referer blacklist
-    - per-IP rate limiting on the image routes
+    - per-IP rate limiting
     - per-origin rate limiting
 - P2: HTTP response handling
     - Last-Modified headers

go.mod

@@ -11,7 +11,6 @@ require (
 	github.com/getsentry/sentry-go v0.40.0
 	github.com/go-chi/chi/v5 v5.2.3
 	github.com/go-chi/cors v1.2.2
-	github.com/go-chi/httprate v0.16.0
 	github.com/gorilla/csrf v1.7.3
 	github.com/gorilla/securecookie v1.1.2
 	github.com/prometheus/client_golang v1.23.2
@@ -92,7 +91,6 @@ require (
 	github.com/inconshreveable/mousetrap v1.1.0 // indirect
 	github.com/josharian/intern v1.0.0 // indirect
 	github.com/json-iterator/go v1.1.12 // indirect
-	github.com/klauspost/cpuid/v2 v2.2.10 // indirect
 	github.com/kylelemons/godebug v1.1.0 // indirect
 	github.com/mailru/easyjson v0.7.7 // indirect
 	github.com/mattn/go-colorable v0.1.13 // indirect
@@ -115,7 +113,6 @@ require (
 	github.com/tidwall/match v1.1.1 // indirect
 	github.com/tidwall/pretty v1.2.0 // indirect
 	github.com/x448/float16 v0.8.4 // indirect
-	github.com/zeebo/xxh3 v1.0.2 // indirect
 	go.etcd.io/etcd/api/v3 v3.6.2 // indirect
 	go.etcd.io/etcd/client/pkg/v3 v3.6.2 // indirect
 	go.etcd.io/etcd/client/v3 v3.6.2 // indirect

go.sum

@@ -114,8 +114,6 @@ github.com/go-chi/chi/v5 v5.2.3 h1:WQIt9uxdsAbgIYgid+BpYc+liqQZGMHRaUwp0JUcvdE=
 github.com/go-chi/chi/v5 v5.2.3/go.mod h1:L2yAIGWB3H+phAw1NxKwWM+7eUH/lU8pOMm5hHcoops=
 github.com/go-chi/cors v1.2.2 h1:Jmey33TE+b+rB7fT8MUy1u0I4L+NARQlK6LhzKPSyQE=
 github.com/go-chi/cors v1.2.2/go.mod h1:sSbTewc+6wYHBBCW7ytsFSn836hqM7JxpglAy2Vzc58=
-github.com/go-chi/httprate v0.16.0 h1:8V5DH9j6pSK6UQoBsTpvMyFxycqaKEIToyPKzHJjUa8=
-github.com/go-chi/httprate v0.16.0/go.mod h1:A8lo+qRhk+s9LiuP5saS7XCGDXRXMcrueq0NfIuCa/I=
 github.com/go-errors/errors v1.4.2 h1:J6MZopCL4uSllY1OfXM374weqZFFItUbrImctkmUxIA=
 github.com/go-errors/errors v1.4.2/go.mod h1:sIVyrIiJhuEF+Pj9Ebtd6P/rEYROXFi3BopGUQ5a5Og=
 github.com/go-jose/go-jose/v4 v4.0.5 h1:M6T8+mKZl/+fNNuFHvGIzDz7BTLQPIounk/b9dw3AaE=
@@ -253,8 +251,6 @@ github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI
 github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
 github.com/klauspost/compress v1.18.0 h1:c/Cqfb0r+Yi+JtIEq73FWXVkRonBlf0CRNYc8Zttxdo=
 github.com/klauspost/compress v1.18.0/go.mod h1:2Pp+KzxcywXVXMr50+X0Q/Lsb43OQHYWRCY2AiWywWQ=
-github.com/klauspost/cpuid/v2 v2.2.10 h1:tBs3QSyvjDyFTq3uoc/9xFpCuOsJQFNPiAhYdw2skhE=
-github.com/klauspost/cpuid/v2 v2.2.10/go.mod h1:hqwkgyIinND0mEev00jJYCxPNVRVXFQeu1XKlok6oO0=
 github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ=
 github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFBFZlji/RkVcI2GknAs/DXo4wKdlNEc=
 github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
@@ -400,10 +396,6 @@ github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcY
 github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
-github.com/zeebo/assert v1.3.0 h1:g7C04CbJuIDKNPFHmsk4hwZDO5O+kntRxzaUoNXj+IQ=
-github.com/zeebo/assert v1.3.0/go.mod h1:Pq9JiuJQpG8JLJdtkwrJESF0Foym2/D9XMU5ciN/wJ0=
-github.com/zeebo/xxh3 v1.0.2 h1:xZmwmqxHZA8AI603jOQ0tMqmBr9lPeFwGg6d+xy9DC0=
-github.com/zeebo/xxh3 v1.0.2/go.mod h1:5NWz9Sef7zIDm2JHfFlcQvNekmcEl9ekUZQQKCYaDcA=
 go.etcd.io/etcd/api/v3 v3.6.2 h1:25aCkIMjUmiiOtnBIp6PhNj4KdcURuBak0hU2P1fgRc=
 go.etcd.io/etcd/api/v3 v3.6.2/go.mod h1:eFhhvfR8Px1P6SEuLT600v+vrhdDTdcfMzmnxVXXSbk=
 go.etcd.io/etcd/client/pkg/v3 v3.6.2 h1:zw+HRghi/G8fKpgKdOcEKpnBTE4OO39T6MegA0RopVU=

internal/handlers/auth.go

@@ -18,9 +18,9 @@ import (
 	"sneak.berlin/go/pixa/internal/templates"
 )
 
-// errInvalidFormField reports a generator form field, or the q or exp
+// errInvalidFormField reports a generator form field whose value is
-// parameter of /v1/image/, whose value is non-numeric or out of range. The
+// non-numeric or out of range. The offending field name is wrapped in so the
-// offending field name is wrapped in so the response can name it.
+// response can name it.
 var errInvalidFormField = errors.New("invalid")
 
 // Bounds for the generator's quality and ttl fields; the quality bounds also

internal/handlers/handlers_internal_test.go

@@ -293,35 +293,22 @@ func TestHandleImage_InvalidFitMode_Returns400(t *testing.T) {
 	}
 }
 
-// TestHandleImage_InvalidQuery_Returns400 verifies that the plain image route
+// TestHandleImage_InvalidQuality_Returns400 verifies that the plain image
-// answers a q that is not a whole number from 1 to 100, an empty one
+// route answers a q that is not a whole number from 1 to 100 with 400 naming
-// included, with 400 naming q and the value, a parameter given more than once
+// q and the value, instead of serving the image at the default quality 85.
-// with 400 naming it, and a query string that cannot be decoded with 400
+func TestHandleImage_InvalidQuality_Returns400(t *testing.T) {
-// showing it, instead of serving the image at the default quality 85 or at
-// the first value given.
-func TestHandleImage_InvalidQuery_Returns400(t *testing.T) {
 	t.Parallel()
 
 	tests := []struct {
-		query, wantError string
+		q, wantError string
 	}{
-		{"q=banana", `invalid q: not a number, got "banana"`},
+		{"banana", `invalid q: not a number, got "banana"`},
-		{"q=0", `invalid q: must be from 1 to 100, got "0"`},
+		{"0", `invalid q: must be from 1 to 100, got "0"`},
-		{"q=101", `invalid q: must be from 1 to 100, got "101"`},
+		{"101", `invalid q: must be from 1 to 100, got "101"`},
-		{"q=", `invalid q: not a number, got ""`},
-		{"q=80&q=500", `invalid q: given more than once`},
-		{"q=80&q=", `invalid q: given more than once`},
-		{"fit=cover&fit=contain", `invalid fit: given more than once`},
-		{"q=80%", `invalid query string "q=80%": invalid URL escape "%"`},
-		{
-			"q=50;fit=contain",
-			`invalid query string "q=50;fit=contain": ` +
-				`invalid semicolon separator in query`,
-		},
 	}
 
 	for _, tt := range tests {
-		t.Run(tt.query, func(t *testing.T) {
+		t.Run("q="+tt.q, func(t *testing.T) {
 			t.Parallel()
 
 			fix := setupTestHandler(t)
@@ -330,7 +317,7 @@ func TestHandleImage_InvalidQuery_Returns400(t *testing.T) {
 			r.Get("/v1/image/*", fix.handler.HandleImage())
 
 			req := httptest.NewRequestWithContext(t.Context(), http.MethodGet,
-				"/v1/image/"+fix.goodHost+"/images/photo.jpg/50x50.jpeg?"+tt.query, nil)
+				"/v1/image/"+fix.goodHost+"/images/photo.jpg/50x50.jpeg?q="+tt.q, nil)
 			rec := httptest.NewRecorder()
 
 			r.ServeHTTP(rec, req)
@@ -356,41 +343,3 @@ func TestHandleImage_InvalidQuery_Returns400(t *testing.T) {
 		})
 	}
 }
-
-// TestHandleImage_EmptyFit_Returns400 verifies that the plain image route
-// answers a fit that is in the URL but empty with 400 naming fit, instead of
-// serving the image as cover. Only a fit missing from the URL means cover.
-func TestHandleImage_EmptyFit_Returns400(t *testing.T) {
-	t.Parallel()
-
-	fix := setupTestHandler(t)
-
-	r := chi.NewRouter()
-	r.Get("/v1/image/*", fix.handler.HandleImage())
-
-	req := httptest.NewRequestWithContext(t.Context(), http.MethodGet,
-		"/v1/image/"+fix.goodHost+"/images/photo.jpg/50x50.jpeg?fit=", nil)
-	rec := httptest.NewRecorder()
-
-	r.ServeHTTP(rec, req)
-
-	if rec.Code != http.StatusBadRequest {
-		t.Fatalf("status = %d, want %d", rec.Code, http.StatusBadRequest)
-	}
-
-	t.Logf("GET %s: %d %s", req.URL, rec.Code, rec.Body)
-
-	var body struct {
-		Error string `json:"error"`
-	}
-
-	err := json.NewDecoder(rec.Body).Decode(&body)
-	if err != nil {
-		t.Fatalf("decoding response body: %v", err)
-	}
-
-	wantError := `invalid fit: not a fit mode, got ""`
-	if body.Error != wantError {
-		t.Errorf("error = %q, want %q", body.Error, wantError)
-	}
-}

internal/handlers/image.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"io"
 	"net/http"
-	"net/url"
 	"strconv"
 	"time"
 
@@ -92,66 +91,31 @@ func (s *Handlers) parseImageRequest(
 	// Convert to ImageRequest
 	req := parsed.ToImageRequest()
 
-	// Parse signature params from query string. r.URL.Query() would silently
+	// Parse signature params from query string
-	// drop a pair it cannot decode, such as q=80%, so that q would be served
+	query := r.URL.Query()
-	// at 85; a query string that cannot be decoded is refused instead. A
+	req.Signature = query.Get("sig")
-	// parameter given more than once is refused too, as only its first value
-	// would be read.
-	query, err := url.ParseQuery(r.URL.RawQuery)
-	if err != nil {
-		s.respondError(w, fmt.Sprintf("invalid query string %q: %v",
-			r.URL.RawQuery, err), http.StatusBadRequest)
 
-		return nil, false
+	if expStr := query.Get("exp"); expStr != "" {
-	}
+		exp, parseErr := strconv.ParseInt(expStr, 10, 64)
-
+		if parseErr == nil {
-	for name, values := range query {
+			req.Expires = time.Unix(exp, 0)
-		if len(values) > 1 {
-			s.respondError(w, fmt.Sprintf("invalid %s: given more than once",
-				name), http.StatusBadRequest)
-
-			return nil, false
 		}
 	}
 
-	req.Signature = query.Get("sig")
+	// Parse optional quality and fit params. An absent q is 85; a q that is
-
+	// not a whole number from 1 to 100 is refused, checked as the generator
-	req.Expires, err = parseExpires(query)
+	// checks its quality field.
-	if err != nil {
-		s.respondError(w, err.Error(), http.StatusBadRequest)
-
-		return nil, false
-	}
-
-	// Parse optional quality and fit params. Only a q missing from the URL is
-	// 85. A q in the URL that is not a whole number from 1 to 100, an empty
-	// one included, is refused, checked as the generator checks its quality
-	// field; that check alone would take an empty q as missing.
-	qStr := query.Get("q")
-	if query.Has("q") && qStr == "" {
-		s.respondError(w, `invalid q: not a number, got ""`,
-			http.StatusBadRequest)
-
-		return nil, false
-	}
-
 	req.Quality, err = parseFormInt(query, "q",
 		encurl.DefaultQuality, minQuality, maxQuality)
 	if err != nil {
-		s.respondError(w, fmt.Sprintf("%v, got %q", err, qStr),
+		s.respondError(w, fmt.Sprintf("%v, got %q", err, query.Get("q")),
 			http.StatusBadRequest)
 
 		return nil, false
 	}
 
-	// Only a fit missing from the URL is cover. A fit in the URL that is not a
+	if fit := query.Get("fit"); fit != "" {
-	// fit mode is refused by the fit-mode check below; that check would take an
+		req.FitMode = imgcache.FitMode(fit)
-	// empty fit as missing, so an empty one is refused here.
-	req.FitMode = imgcache.FitMode(query.Get("fit"))
-	if query.Has("fit") && req.FitMode == "" {
-		s.respondError(w, `invalid fit: not a fit mode, got ""`, http.StatusBadRequest)
-
-		return nil, false
 	}
 
 	// Default fit mode if not set
@@ -174,26 +138,6 @@ func (s *Handlers) parseImageRequest(
 	return req, true
 }
 
-// parseExpires reads the exp query parameter, a Unix time in seconds. An exp
-// missing from the URL gives the zero time, which the signature check takes
-// as no expiration. An exp in the URL that is not a whole number, an empty
-// one included, is an error naming exp and the value.
-func parseExpires(query url.Values) (time.Time, error) {
-	if !query.Has("exp") {
-		return time.Time{}, nil
-	}
-
-	expStr := query.Get("exp")
-
-	exp, err := strconv.ParseInt(expStr, 10, 64)
-	if err != nil {
-		return time.Time{}, fmt.Errorf("%w exp: not a number, got %q",
-			errInvalidFormField, expStr)
-	}
-
-	return time.Unix(exp, 0), nil
-}
-
 // respondImageError maps image retrieval errors to HTTP responses.
 func (s *Handlers) respondImageError(
 	w http.ResponseWriter, req *imgcache.ImageRequest, err error,

internal/handlers/image_signature_internal_test.go

@@ -1,7 +1,6 @@
 package handlers
 
 import (
-	"encoding/json"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
@@ -119,53 +118,3 @@ func TestHandleImage_GeneratedSignedURLVerifies(t *testing.T) {
 		})
 	}
 }
-
-// TestHandleImage_InvalidExp_Returns400 sends a signed-host URL whose exp is
-// not a whole number, and one whose exp is empty. Each is refused with 400
-// naming exp and the value, not with the 401 a URL without exp still gets.
-func TestHandleImage_InvalidExp_Returns400(t *testing.T) {
-	t.Parallel()
-
-	tests := []struct {
-		query      string
-		wantStatus int
-		wantError  string
-	}{
-		{"sig=x&exp=banana", http.StatusBadRequest,
-			`invalid exp: not a number, got "banana"`},
-		{"sig=x&exp=", http.StatusBadRequest, `invalid exp: not a number, got ""`},
-		{"sig=x", http.StatusUnauthorized, "unauthorized"},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.query, func(t *testing.T) {
-			t.Parallel()
-
-			fix := setupTestHandler(t)
-
-			r := chi.NewRouter()
-			r.Get("/v1/image/*", fix.handler.HandleImage())
-
-			req := httptest.NewRequestWithContext(t.Context(), http.MethodGet,
-				"/v1/image/"+signedHost+"/images/photo.jpg/50x50.jpeg?"+tt.query, nil)
-			rec := httptest.NewRecorder()
-
-			r.ServeHTTP(rec, req)
-			t.Logf("GET %s: %d %s", req.URL, rec.Code, rec.Body)
-
-			var body struct {
-				Error string `json:"error"`
-			}
-
-			err := json.NewDecoder(rec.Body).Decode(&body)
-			if err != nil {
-				t.Fatalf("decoding response body: %v", err)
-			}
-
-			if rec.Code != tt.wantStatus || body.Error != tt.wantError {
-				t.Errorf("got %d %q, want %d %q",
-					rec.Code, body.Error, tt.wantStatus, tt.wantError)
-			}
-		})
-	}
-}

internal/imageprocessor/imageprocessor.go

@@ -161,13 +161,6 @@ func (p *ImageProcessor) Process(
 	}
 	defer img.Close()
 
-	// Turn the image upright now: encode strips the EXIF orientation tag,
-	// and sizes below must be worked out on the upright image.
-	err = img.AutoRotate()
-	if err != nil {
-		return nil, fmt.Errorf("failed to auto-rotate: %w", err)
-	}
-
 	// Get original dimensions
 	origWidth := img.Width()
 	origHeight := img.Height()
@@ -411,21 +404,6 @@ func (p *ImageProcessor) encode(
 		return nil, fmt.Errorf("%w: %s", ErrUnsupportedOutputFormat, format)
 	}
 
-	// Stripping drops the ICC profile as well, and clients show an image
-	// with no profile as sRGB, so convert to sRGB first. "srgb" names
-	// libvips' built-in profile; govips' own sRGB path variable is set on
-	// first use but read without a lock, so concurrent requests race on it.
-	if img.HasICCProfile() {
-		err := img.TransformICCProfileWithFallback("srgb", "srgb")
-		if err != nil {
-			return nil, fmt.Errorf("failed to convert to sRGB: %w", err)
-		}
-	}
-
-	// Drop EXIF, XMP, IPTC and the ICC profile. govips ignores this for
-	// GIF, which carries none of them.
-	params.StripMetadata = true
-
 	output, _, err := img.Export(&params)
 	if err != nil {
 		return nil, err

internal/imageprocessor/imageprocessor_internal_test.go

@@ -9,9 +9,7 @@ import (
 	"image/jpeg"
 	"image/png"
 	"io"
-	"math"
 	"os"
-	"slices"
 	"testing"
 
 	"github.com/davidbyttow/govips/v2/vips"
@@ -563,152 +561,3 @@ func TestImageProcessor_EncodeAVIF(t *testing.T) {
 
 	encodeAndCheck(t, FormatAVIF, 85, mimeAVIF)
 }
-
-// processAndDecode runs input through Process and decodes the output with
-// vips, so a test can inspect the image a client would receive.
-func processAndDecode(t *testing.T, input []byte, req *Request) *vips.ImageRef {
-	t.Helper()
-
-	result, err := New(Params{}).Process(
-		context.Background(), bytes.NewReader(input), req,
-	)
-	if err != nil {
-		t.Fatalf("Process() error = %v", err)
-	}
-
-	defer func() { _ = result.Content.Close() }()
-
-	data, err := io.ReadAll(result.Content)
-	if err != nil {
-		t.Fatalf("failed to read result: %v", err)
-	}
-
-	output, err := vips.NewImageFromBuffer(data)
-	if err != nil {
-		t.Fatalf("failed to decode output: %v", err)
-	}
-
-	t.Cleanup(output.Close)
-
-	return output
-}
-
-func TestImageProcessor_StripsEXIF(t *testing.T) {
-	t.Parallel()
-
-	// gps-exif.jpg carries GPS coordinates, a camera make, model and serial
-	// number, and a capture time.
-	input, err := os.ReadFile("testdata/gps-exif.jpg")
-	if err != nil {
-		t.Fatalf("failed to read test JPEG: %v", err)
-	}
-
-	fixture, err := vips.NewImageFromBuffer(input)
-	if err != nil {
-		t.Fatalf("failed to decode test JPEG: %v", err)
-	}
-
-	t.Cleanup(fixture.Close)
-
-	if !slices.Contains(fixture.GetFields(), "exif-ifd3-GPSLatitude") {
-		t.Fatal("testdata/gps-exif.jpg has no GPS latitude")
-	}
-
-	formats := []Format{
-		FormatJPEG, FormatPNG, FormatWebP, FormatAVIF, FormatGIF, FormatOriginal,
-	}
-
-	for _, format := range formats {
-		t.Run(string(format), func(t *testing.T) {
-			t.Parallel()
-
-			output := processAndDecode(t, input, &Request{Format: format})
-
-			if output.HasExif() {
-				t.Errorf("output has EXIF: %v", output.GetExif())
-			}
-		})
-	}
-}
-
-func TestImageProcessor_AppliesEXIFOrientation(t *testing.T) {
-	t.Parallel()
-
-	// orientation-6.jpg is stored 16x8, red on the left and blue on the
-	// right, with EXIF orientation 6 (turn 90 degrees clockwise to view).
-	// Upright it is 8x16, red on top and blue below.
-	input, err := os.ReadFile("testdata/orientation-6.jpg")
-	if err != nil {
-		t.Fatalf("failed to read test JPEG: %v", err)
-	}
-
-	tests := []struct {
-		name  string
-		size  Size
-		wantW int
-		wantH int
-	}{
-		{name: "original size", size: Size{}, wantW: 8, wantH: 16},
-		{name: "width only", size: Size{Width: 4}, wantW: 4, wantH: 8},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			output := processAndDecode(t, input, &Request{
-				Size:   tt.size,
-				Format: FormatPNG,
-			})
-
-			if output.Width() != tt.wantW || output.Height() != tt.wantH {
-				t.Fatalf("output is %dx%d, want %dx%d",
-					output.Width(), output.Height(), tt.wantW, tt.wantH)
-			}
-
-			top, err := output.GetPoint(tt.wantW/2, 0)
-			if err != nil {
-				t.Fatalf("GetPoint() error = %v", err)
-			}
-
-			bottom, err := output.GetPoint(tt.wantW/2, tt.wantH-1)
-			if err != nil {
-				t.Fatalf("GetPoint() error = %v", err)
-			}
-
-			if top[0] <= top[2] || bottom[2] <= bottom[0] {
-				t.Errorf("top pixel = %v, bottom pixel = %v, want red above blue",
-					top, bottom)
-			}
-		})
-	}
-}
-
-func TestImageProcessor_ConvertsWideGamutToSRGB(t *testing.T) {
-	t.Parallel()
-
-	// display-p3.jpg is a flat 8x8 image with the Display P3 profile
-	// embedded, filled with Display P3 (234, 51, 35), which is sRGB red.
-	input, err := os.ReadFile("testdata/display-p3.jpg")
-	if err != nil {
-		t.Fatalf("failed to read test JPEG: %v", err)
-	}
-
-	output := processAndDecode(t, input, &Request{Format: FormatPNG})
-
-	if output.HasICCProfile() {
-		t.Error("output has an ICC profile")
-	}
-
-	pixel, err := output.GetPoint(4, 4)
-	if err != nil {
-		t.Fatalf("GetPoint() error = %v", err)
-	}
-
-	want := []float64{255, 0, 0}
-	for i := range want {
-		if math.Abs(pixel[i]-want[i]) > 5 {
-			t.Fatalf("pixel = %v, want within 5 of %v", pixel, want)
-		}
-	}
-}

internal/imgcache/cache.go

@@ -125,7 +125,7 @@ func NewCache(db *sql.DB, config CacheConfig) (*Cache, error) {
 	}
 
 	variants, err := NewVariantStorage(
-		filepath.Join(config.StateDir, "cache", "variants"), log,
+		filepath.Join(config.StateDir, "cache", "variants"),
 	)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create variant storage: %w", err)
@@ -263,7 +263,23 @@ func (c *Cache) StoreSource(
 		return "", fmt.Errorf("failed to insert source metadata: %w", err)
 	}
 
-	c.writeMetadataSidecar(req, pathHash, contentHash, result)
+	// Store metadata JSON file
+	meta := &SourceMetadata{
+		Host:            req.SourceHost,
+		Path:            req.SourcePath,
+		Query:           req.SourceQuery,
+		ContentHash:     string(contentHash),
+		StatusCode:      result.StatusCode,
+		ContentType:     result.ContentType,
+		ContentLength:   result.ContentLength,
+		ResponseHeaders: result.Headers,
+		FetchedAt:       time.Now().UTC().Unix(),
+		FetchDurationMs: result.FetchDurationMs,
+		RemoteAddr:      result.RemoteAddr,
+	}
+
+	// A failure here is non-fatal; the metadata is in the database.
+	_ = c.srcMetadata.Store(req.SourceHost, pathHash, meta)
 
 	c.notifyWritePressure()
 
@@ -420,19 +436,12 @@ func (c *Cache) Stats(ctx context.Context) (*CacheStats, error) {
 	}
 
 	// Get actual item count and total size from content tables
-	err = c.db.QueryRowContext(ctx,
+	_ = c.db.QueryRowContext(ctx,
 		`SELECT COUNT(*) FROM request_cache`,
 	).Scan(&stats.TotalItems)
-	if err != nil {
+	_ = c.db.QueryRowContext(ctx,
-		c.log.Warn("failed to count cache items for stats", "error", err)
-	}
-
-	err = c.db.QueryRowContext(ctx,
 		`SELECT COALESCE(SUM(size_bytes), 0) FROM output_content`,
 	).Scan(&stats.TotalSizeBytes)
-	if err != nil {
-		c.log.Warn("failed to sum cache size for stats", "error", err)
-	}
 
 	// Compute hit rate as a ratio
 	if stats.HitCount+stats.MissCount > 0 {
@@ -444,17 +453,15 @@ func (c *Cache) Stats(ctx context.Context) (*CacheStats, error) {
 
 // IncrementStats increments cache statistics.
 func (c *Cache) IncrementStats(ctx context.Context, hit bool, fetchBytes int64) {
-	var err error
-
 	if hit {
-		_, err = c.db.ExecContext(ctx, `
+		_, _ = c.db.ExecContext(ctx, `
 			UPDATE cache_stats
 			SET hit_count = hit_count + 1,
 			    last_updated_at = CURRENT_TIMESTAMP
 			WHERE id = 1
 		`)
 	} else {
-		_, err = c.db.ExecContext(ctx, `
+		_, _ = c.db.ExecContext(ctx, `
 			UPDATE cache_stats
 			SET miss_count = miss_count + 1,
 			    last_updated_at = CURRENT_TIMESTAMP
@@ -462,52 +469,14 @@ func (c *Cache) IncrementStats(ctx context.Context, hit bool, fetchBytes int64)
 		`)
 	}
 
-	if err != nil {
-		c.log.Warn("failed to count cache hit or miss", "hit", hit, "error", err)
-	}
-
 	if fetchBytes > 0 {
-		_, err = c.db.ExecContext(ctx, `
+		_, _ = c.db.ExecContext(ctx, `
 			UPDATE cache_stats
 			SET upstream_fetch_count = upstream_fetch_count + 1,
 			    upstream_fetch_bytes = upstream_fetch_bytes + ?,
 			    last_updated_at = CURRENT_TIMESTAMP
 			WHERE id = 1
 		`, fetchBytes)
-		if err != nil {
-			c.log.Warn("failed to count upstream fetch",
-				"fetch_bytes", fetchBytes, "error", err)
-		}
-	}
-}
-
-// writeMetadataSidecar writes the JSON metadata sidecar of a stored source.
-// A failure is logged and is otherwise non-fatal; the metadata is in the
-// database.
-func (c *Cache) writeMetadataSidecar(
-	req *ImageRequest,
-	pathHash PathHash,
-	contentHash ContentHash,
-	result *httpfetcher.FetchResult,
-) {
-	meta := &SourceMetadata{
-		Host:            req.SourceHost,
-		Path:            req.SourcePath,
-		Query:           req.SourceQuery,
-		ContentHash:     string(contentHash),
-		StatusCode:      result.StatusCode,
-		ContentType:     result.ContentType,
-		ContentLength:   result.ContentLength,
-		ResponseHeaders: result.Headers,
-		FetchedAt:       time.Now().UTC().Unix(),
-		FetchDurationMs: result.FetchDurationMs,
-		RemoteAddr:      result.RemoteAddr,
-	}
-
-	err := c.srcMetadata.Store(req.SourceHost, pathHash, meta)
-	if err != nil {
-		c.log.Warn("failed to write metadata sidecar",
-			"host", req.SourceHost, "path_hash", pathHash, "error", err)
 	}
 }
 
@@ -559,14 +528,10 @@ func (c *Cache) checkNegativeCache(
 	// Check if expired
 	if time.Now().After(expiresAt) {
 		// Clean up expired entry
-		_, err = c.db.ExecContext(ctx, `
+		_, _ = c.db.ExecContext(ctx, `
 			DELETE FROM negative_cache
 			WHERE source_host = ? AND source_path = ? AND source_query = ?
 		`, req.SourceHost, req.SourcePath, req.SourceQuery)
-		if err != nil {
-			c.log.Warn("failed to delete expired negative cache entry",
-				"host", req.SourceHost, "path", req.SourcePath, "error", err)
-		}
 
 		return false, nil
 	}

internal/imgcache/service.go

@@ -322,12 +322,7 @@ func (s *Service) fetchAndProcess(
 		// Store negative cache for certain errors
 		if isNegativeCacheable(err) {
 			statusCode := extractStatusCode(err)
-
+			_ = s.cache.StoreNegative(ctx, req, statusCode, err.Error())
-			storeErr := s.cache.StoreNegative(ctx, req, statusCode, err.Error())
-			if storeErr != nil {
-				s.log.Warn("failed to store negative cache entry",
-					"host", req.SourceHost, "path", req.SourcePath, "error", storeErr)
-			}
 		}
 
 		return nil, fmt.Errorf("upstream fetch failed: %w", err)

internal/imgcache/stats_internal_test.go

@@ -1,12 +1,9 @@
 package imgcache
 
 import (
-	"bytes"
 	"context"
 	"database/sql"
-	"log/slog"
 	"math"
-	"strings"
 	"testing"
 	"time"
 
@@ -104,82 +101,3 @@ func TestStats_ZeroCounts(t *testing.T) {
 		t.Errorf("HitRate = %f, want 0.0 for zero counts", stats.HitRate)
 	}
 }
-
-// TestStats_LogsFailedCountQueries verifies that a failed item count query
-// and a failed size query are each logged at warn and Stats still succeeds.
-func TestStats_LogsFailedCountQueries(t *testing.T) {
-	t.Parallel()
-
-	db := setupStatsTestDB(t)
-
-	var logBuf bytes.Buffer
-
-	cache, err := NewCache(db, CacheConfig{
-		StateDir:    t.TempDir(),
-		CacheTTL:    time.Hour,
-		NegativeTTL: 5 * time.Minute,
-		Logger:      slog.New(slog.NewJSONHandler(&logBuf, nil)),
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	_, err = db.ExecContext(t.Context(),
-		`DROP TABLE request_cache; DROP TABLE output_content`)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	_, err = cache.Stats(t.Context())
-	if err != nil {
-		t.Fatalf("Stats() error = %v, want nil", err)
-	}
-
-	for _, msg := range []string{
-		"failed to count cache items for stats",
-		"failed to sum cache size for stats",
-	} {
-		want := `"level":"WARN","msg":"` + msg + `"`
-		if !strings.Contains(logBuf.String(), want) {
-			t.Errorf("log missing %s; got %q", want, logBuf.String())
-		}
-	}
-}
-
-// TestIncrementStats_LogsFailedUpdates verifies that a failed hit or miss
-// count update and a failed upstream fetch count update are each logged at
-// warn.
-func TestIncrementStats_LogsFailedUpdates(t *testing.T) {
-	t.Parallel()
-
-	db := setupStatsTestDB(t)
-
-	var logBuf bytes.Buffer
-
-	cache, err := NewCache(db, CacheConfig{
-		StateDir:    t.TempDir(),
-		CacheTTL:    time.Hour,
-		NegativeTTL: 5 * time.Minute,
-		Logger:      slog.New(slog.NewJSONHandler(&logBuf, nil)),
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	_, err = db.ExecContext(t.Context(), `DROP TABLE cache_stats`)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	cache.IncrementStats(t.Context(), false, 1024)
-
-	for _, msg := range []string{
-		"failed to count cache hit or miss",
-		"failed to count upstream fetch",
-	} {
-		want := `"level":"WARN","msg":"` + msg + `"`
-		if !strings.Contains(logBuf.String(), want) {
-			t.Errorf("log missing %s; got %q", want, logBuf.String())
-		}
-	}
-}

internal/imgcache/storage.go

@@ -7,7 +7,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"log/slog"
 	"os"
 	"path/filepath"
 	"time"
@@ -393,7 +392,6 @@ func CacheKey(req *ImageRequest) VariantKey {
 // Unlike ContentStorage, the key is provided by the caller (not computed from content).
 type VariantStorage struct {
 	baseDir string
-	log     *slog.Logger
 }
 
 // VariantMeta contains metadata about a cached variant.
@@ -406,14 +404,13 @@ type VariantMeta struct {
 }
 
 // NewVariantStorage creates a new variant storage at the given base directory.
-// A failed .meta write is logged to log.
+func NewVariantStorage(baseDir string) (*VariantStorage, error) {
-func NewVariantStorage(baseDir string, log *slog.Logger) (*VariantStorage, error) {
 	err := os.MkdirAll(baseDir, StorageDirPerm)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create variant storage directory: %w", err)
 	}
 
-	return &VariantStorage{baseDir: baseDir, log: log}, nil
+	return &VariantStorage{baseDir: baseDir}, nil
 }
 
 // Store writes content and metadata to storage at the given key.
@@ -481,11 +478,7 @@ func (s *VariantStorage) Store(
 	}
 
 	// Metadata write failure is non-fatal; content is already stored.
-	err = os.WriteFile(metaPath, metaData, StorageFilePerm)
+	_ = os.WriteFile(metaPath, metaData, StorageFilePerm)
-	if err != nil {
-		s.log.Warn("failed to write variant metadata sidecar",
-			"path", metaPath, "error", err)
-	}
 
 	return size, nil
 }

internal/imgcache/storage_internal_test.go

@@ -4,10 +4,8 @@ import (
 	"bytes"
 	"errors"
 	"io"
-	"log/slog"
 	"os"
 	"path/filepath"
-	"strings"
 	"testing"
 )
 
@@ -406,35 +404,3 @@ func TestCacheKey(t *testing.T) {
 		t.Error("CacheKey() produced same key for different quality")
 	}
 }
-
-// TestVariantStorage_StoreLogsFailedMetaWrite verifies that a .meta write
-// that fails is logged at warn and the store still succeeds.
-func TestVariantStorage_StoreLogsFailedMetaWrite(t *testing.T) {
-	t.Parallel()
-
-	var logBuf bytes.Buffer
-
-	storage, err := NewVariantStorage(
-		t.TempDir(), slog.New(slog.NewJSONHandler(&logBuf, nil)))
-	if err != nil {
-		t.Fatalf("NewVariantStorage() error = %v", err)
-	}
-
-	key := CacheKey(&ImageRequest{SourceHost: testHostCDN, SourcePath: testPathCat})
-
-	// A directory where the .meta file goes makes the .meta write fail.
-	err = os.MkdirAll(storage.keyToPath(key)+".meta", StorageDirPerm)
-	if err != nil {
-		t.Fatalf("failed to create directory: %v", err)
-	}
-
-	_, err = storage.Store(key, bytes.NewReader([]byte("variant data")), "image/webp")
-	if err != nil {
-		t.Fatalf("Store() error = %v, want nil", err)
-	}
-
-	want := `"level":"WARN","msg":"failed to write variant metadata sidecar"`
-	if !strings.Contains(logBuf.String(), want) {
-		t.Errorf("log missing %s; got %q", want, logBuf.String())
-	}
-}

internal/middleware/middleware.go

@@ -4,13 +4,11 @@ package middleware
 import (
 	"log/slog"
 	"net/http"
-	"net/netip"
 	"time"
 
 	basicauth "github.com/99designs/basicauth-go"
 	"github.com/go-chi/chi/v5/middleware"
 	"github.com/go-chi/cors"
-	"github.com/go-chi/httprate"
 	metrics "github.com/slok/go-http-metrics/metrics/prometheus"
 	ghmm "github.com/slok/go-http-metrics/middleware"
 	"github.com/slok/go-http-metrics/middleware/std"
@@ -90,31 +88,6 @@ func (s *Middleware) ClientIP() func(http.Handler) http.Handler {
 	}
 }
 
-// RateLimit returns a middleware that limits each client to requestLimit
-// requests per window and refuses a request over the limit with 429 Too Many
-// Requests and a Retry-After header. Clients are told apart by the address
-// the ClientIP middleware stored in the request context, so ClientIP must
-// run first. An IPv6 client is counted by its /64, which one client usually
-// holds whole; an IPv4-mapped address (::ffff:a.b.c.d) is counted as the
-// IPv4 address it carries, since every such address falls in the same /64.
-// Counts are kept only for the current and the previous window, so memory
-// stays bounded.
-func (s *Middleware) RateLimit(
-	requestLimit int, window time.Duration,
-) func(http.Handler) http.Handler {
-	return httprate.LimitBy(requestLimit, window,
-		func(r *http.Request) (string, error) {
-			ip := clientip.FromContext(r.Context())
-
-			addr, err := netip.ParseAddr(ip)
-			if err == nil {
-				ip = addr.Unmap().String()
-			}
-
-			return httprate.CanonicalizeIP(ip), nil
-		})
-}
-
 type loggingResponseWriter struct {
 	http.ResponseWriter
 

internal/server/login_rate_limit_internal_test.go

@@ -1,280 +0,0 @@
-package server
-
-import (
-	"io"
-	"net/http"
-	"net/http/httptest"
-	"net/netip"
-	"net/url"
-	"path/filepath"
-	"regexp"
-	"strconv"
-	"strings"
-	"testing"
-
-	"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/logger"
-	"sneak.berlin/go/pixa/internal/middleware"
-)
-
-// testSigningKey is a throwaway signing key; submitting it logs in.
-const testSigningKey = "test-signing-key-0123456789abcdef"
-
-// wrongKey is submitted for a failed login.
-const wrongKey = "not-the-signing-key"
-
-// Addresses for the login rate limit tests. The test server trusts
-// 10.0.0.0/8 as its proxies, so the X-Forwarded-For sent by proxyPeer is
-// believed and the one sent by firstClient or secondClient is ignored.
-const (
-	firstClient     = "198.51.100.1:40000"
-	secondClient    = "198.51.100.2:40000"
-	proxyPeer       = "10.0.0.1:40000"
-	firstForwarded  = "203.0.113.1"
-	secondForwarded = "203.0.113.2"
-)
-
-// csrfFieldPattern extracts the CSRF token rendered into the login form.
-var csrfFieldPattern = regexp.MustCompile(
-	`name="gorilla\.csrf\.Token" value="([^"]+)"`)
-
-// newTestServer builds the server's real routes from the constructors
-// cmd/pixad uses, with a throwaway state directory. Debug marks requests
-// as plain HTTP, so the CSRF check runs without an https Referer.
-func newTestServer(t *testing.T) *Server {
-	t.Helper()
-
-	stateDir := t.TempDir()
-	cfg := &config.Config{
-		Debug:          true,
-		SigningKey:     testSigningKey,
-		StateDir:       stateDir,
-		DBURL:          "file:" + filepath.Join(stateDir, "state.sqlite3"),
-		TrustedProxies: []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")},
-	}
-
-	lc := fxtest.NewLifecycle(t)
-
-	log, err := logger.New(lc, logger.Params{Globals: &globals.Globals{}})
-	if err != nil {
-		t.Fatalf("logger.New() error = %v", err)
-	}
-
-	db, err := database.New(lc, database.Params{Logger: log, Config: cfg})
-	if err != nil {
-		t.Fatalf("database.New() error = %v", err)
-	}
-
-	h, err := handlers.New(lc, handlers.Params{
-		Logger: log, Database: db, Config: cfg,
-	})
-	if err != nil {
-		t.Fatalf("handlers.New() error = %v", err)
-	}
-
-	mw, err := middleware.New(lc, middleware.Params{Logger: log, Config: cfg})
-	if err != nil {
-		t.Fatalf("middleware.New() error = %v", err)
-	}
-
-	lc.RequireStart()
-	t.Cleanup(lc.RequireStop)
-
-	s := &Server{config: cfg, mw: mw, h: h}
-	s.SetupRoutes()
-
-	return s
-}
-
-// clientRequest builds a request for / arriving from remoteAddr, carrying
-// forwardedFor as its X-Forwarded-For header when that is not empty.
-func clientRequest(
-	t *testing.T, method string, body io.Reader, remoteAddr, forwardedFor string,
-) *http.Request {
-	t.Helper()
-
-	req := httptest.NewRequestWithContext(t.Context(), method, "/", body)
-	req.RemoteAddr = remoteAddr
-
-	if forwardedFor != "" {
-		req.Header.Set("X-Forwarded-For", forwardedFor)
-	}
-
-	return req
-}
-
-// postLogin loads the login form with GET / and submits key in it with
-// POST /, as a browser does, both from the same client. GET / is not rate
-// limited, so the form must load even for a client over the limit.
-func postLogin(
-	t *testing.T, s *Server, remoteAddr, forwardedFor, key string,
-) *httptest.ResponseRecorder {
-	t.Helper()
-
-	page := httptest.NewRecorder()
-	s.ServeHTTP(page,
-		clientRequest(t, http.MethodGet, nil, remoteAddr, forwardedFor))
-
-	if page.Code != http.StatusOK {
-		t.Fatalf("GET / status = %d, want %d", page.Code, http.StatusOK)
-	}
-
-	match := csrfFieldPattern.FindStringSubmatch(page.Body.String())
-	if match == nil {
-		t.Fatalf("no CSRF token field found in the login form")
-	}
-
-	form := url.Values{"key": {key}, "gorilla.csrf.Token": {match[1]}}
-	req := clientRequest(t, http.MethodPost,
-		strings.NewReader(form.Encode()), remoteAddr, forwardedFor)
-	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
-
-	for _, c := range page.Result().Cookies() {
-		req.AddCookie(c)
-	}
-
-	rec := httptest.NewRecorder()
-	s.ServeHTTP(rec, req)
-
-	return rec
-}
-
-// tripLoginRateLimit makes LoginAttemptsPerMinute failed logins from one
-// client, each answered with the login form again, then one more, which
-// must be refused with 429. It returns the response to that last attempt.
-func tripLoginRateLimit(
-	t *testing.T, s *Server, remoteAddr, forwardedFor string,
-) *httptest.ResponseRecorder {
-	t.Helper()
-
-	for attempt := range LoginAttemptsPerMinute {
-		rec := postLogin(t, s, remoteAddr, forwardedFor, wrongKey)
-		if rec.Code != http.StatusOK {
-			t.Fatalf("failed login %d status = %d, want %d",
-				attempt+1, rec.Code, http.StatusOK)
-		}
-	}
-
-	rec := postLogin(t, s, remoteAddr, forwardedFor, wrongKey)
-	if rec.Code != http.StatusTooManyRequests {
-		t.Fatalf("login over the limit status = %d, want %d",
-			rec.Code, http.StatusTooManyRequests)
-	}
-
-	return rec
-}
-
-// TestLoginRateLimitRefusesAttemptOverLimit verifies the login attempt
-// after LoginAttemptsPerMinute failed ones from one client is refused with
-// 429 and a Retry-After header, and that the client cannot get around the
-// limit by sending X-Forwarded-For: from a peer that is not a trusted
-// proxy, the header is ignored.
-func TestLoginRateLimitRefusesAttemptOverLimit(t *testing.T) {
-	t.Parallel()
-
-	s := newTestServer(t)
-
-	rec := tripLoginRateLimit(t, s, firstClient, "")
-
-	retryAfter := rec.Header().Get("Retry-After")
-
-	seconds, err := strconv.Atoi(retryAfter)
-	if err != nil || seconds <= 0 {
-		t.Errorf("Retry-After = %q, want a positive number of seconds",
-			retryAfter)
-	}
-
-	rec = postLogin(t, s, firstClient, secondForwarded, wrongKey)
-	if rec.Code != http.StatusTooManyRequests {
-		t.Errorf("login with X-Forwarded-For from an untrusted peer "+
-			"status = %d, want %d", rec.Code, http.StatusTooManyRequests)
-	}
-}
-
-// TestLoginRateLimitLeavesOtherClientsAlone verifies one client going over
-// the limit does not limit another: a failed login from a different
-// address is answered with the login form, and the signing key still logs
-// it in.
-func TestLoginRateLimitLeavesOtherClientsAlone(t *testing.T) {
-	t.Parallel()
-
-	s := newTestServer(t)
-
-	tripLoginRateLimit(t, s, firstClient, "")
-
-	rec := postLogin(t, s, secondClient, "", wrongKey)
-	if rec.Code != http.StatusOK {
-		t.Errorf("failed login from another client status = %d, want %d",
-			rec.Code, http.StatusOK)
-	}
-
-	rec = postLogin(t, s, secondClient, "", testSigningKey)
-	if rec.Code != http.StatusSeeOther {
-		t.Errorf("login with the signing key from another client "+
-			"status = %d, want %d", rec.Code, http.StatusSeeOther)
-	}
-}
-
-// TestLoginRateLimitCountsClientsBehindProxySeparately verifies the limit
-// counts the client address resolved from X-Forwarded-For, not the address
-// of the trusted proxy the requests arrive from, so two clients behind the
-// same proxy are counted separately.
-func TestLoginRateLimitCountsClientsBehindProxySeparately(t *testing.T) {
-	t.Parallel()
-
-	s := newTestServer(t)
-
-	tripLoginRateLimit(t, s, proxyPeer, firstForwarded)
-
-	rec := postLogin(t, s, proxyPeer, secondForwarded, wrongKey)
-	if rec.Code != http.StatusOK {
-		t.Errorf("failed login from a second client behind the proxy "+
-			"status = %d, want %d", rec.Code, http.StatusOK)
-	}
-}
-
-// TestLoginRateLimitCountsIPv6ClientsByPrefix verifies an IPv6 client is
-// counted by its /64: another address in the same /64 is refused too,
-// while an address in a different /64 is not.
-func TestLoginRateLimitCountsIPv6ClientsByPrefix(t *testing.T) {
-	t.Parallel()
-
-	s := newTestServer(t)
-
-	tripLoginRateLimit(t, s, proxyPeer, "2001:db8::1")
-
-	rec := postLogin(t, s, proxyPeer, "2001:db8::2", wrongKey)
-	if rec.Code != http.StatusTooManyRequests {
-		t.Errorf("login from the same /64 status = %d, want %d",
-			rec.Code, http.StatusTooManyRequests)
-	}
-
-	rec = postLogin(t, s, proxyPeer, "2001:db8:0:1::1", wrongKey)
-	if rec.Code != http.StatusOK {
-		t.Errorf("login from another /64 status = %d, want %d",
-			rec.Code, http.StatusOK)
-	}
-}
-
-// TestLoginRateLimitCountsIPv4MappedClientsSeparately verifies an IPv4
-// client that the proxy forwards in IPv4-mapped IPv6 form (::ffff:a.b.c.d)
-// is counted by its IPv4 address, not by the /64 that every such address
-// shares, so two of them behind the proxy are counted separately.
-func TestLoginRateLimitCountsIPv4MappedClientsSeparately(t *testing.T) {
-	t.Parallel()
-
-	s := newTestServer(t)
-
-	tripLoginRateLimit(t, s, proxyPeer, "::ffff:"+firstForwarded)
-
-	rec := postLogin(t, s, proxyPeer, "::ffff:"+secondForwarded, wrongKey)
-	if rec.Code != http.StatusOK {
-		t.Errorf("failed login from a second IPv4-mapped client "+
-			"status = %d, want %d", rec.Code, http.StatusOK)
-	}
-}

internal/server/routes.go

@@ -2,7 +2,6 @@ package server
 
 import (
 	"net/http"
-	"time"
 
 	sentryhttp "github.com/getsentry/sentry-go/http"
 	"github.com/go-chi/chi/v5"
@@ -13,10 +12,6 @@ import (
 	"sneak.berlin/go/pixa/internal/static"
 )
 
-// LoginAttemptsPerMinute is how many login attempts (POST /) one client may
-// make per minute; the next is refused with 429 Too Many Requests.
-const LoginAttemptsPerMinute = 5
-
 // SetupRoutes configures all HTTP routes.
 func (s *Server) SetupRoutes() {
 	s.router = chi.NewRouter()
@@ -55,14 +50,11 @@ func (s *Server) SetupRoutes() {
 	// token cookie is independent of the session cookie, so it also
 	// covers the login POST, where no session exists yet. LimitBody caps
 	// the POST body ahead of CSRF, which reads its token from that body.
-	// The login POST is rate limited per client after both, so every
-	// attempt that reaches the signing key comparison is counted.
 	s.router.Group(func(r chi.Router) {
 		r.Use(s.h.LimitBody(handlers.MaxFormBytes))
 		r.Use(s.h.CSRF())
 		r.Get("/", s.h.HandleRoot())
-		r.With(s.mw.RateLimit(LoginAttemptsPerMinute, time.Minute)).
+		r.Post("/", s.h.HandleRoot())
-			Post("/", s.h.HandleRoot())
 		r.Post("/generate", s.h.HandleGenerateURL())
 	})