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

A q that was not a number or was outside 1-100 was dropped and 85 used,
so q=500 was served and verified against a signature made for 85. It is
now a 400 naming q and the value, read with the generator's quality
check; only a q missing from the URL is 85.

The route also refuses with 400 a query string that cannot be decoded
(r.URL.Query() drops such a pair, so q=80% arrived as no q) and any
parameter given more than once, which was read from its first value only
(q=80&q=500 was served at 80).

Model: opus-5-5

README.md

@@ -94,6 +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`)
 
@@ -125,8 +128,9 @@ Where:
 - `height` — requested height in pixels, `0` for original
 - `format` — output format (jpeg, png, webp, avif, gif, orig)
 - `expiration` — Unix timestamp when signature expires
-- `quality` — the URL's `q` query parameter (1-100), or `85` when the URL
+- `quality` — the URL's `q` query parameter, a whole number from 1 to 100,
-  has no `q`
+  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`
 

TODO.md

@@ -30,6 +30,16 @@ exhaustion
 
 # Completed Steps
 
+- 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
+  400 naming `q` and the value, instead of being served at the default
+  85; the route reads `q` with the generator's quality check
+  (`parseFormInt` with `minQuality` and `maxQuality`); only a `q` missing
+  from the URL is still 85; a query string that cannot be decoded, such
+  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

internal/handlers/auth.go

@@ -18,13 +18,14 @@ import (
 	"sneak.berlin/go/pixa/internal/templates"
 )
 
-// errInvalidFormField reports a generator form field whose value is
+// errInvalidFormField reports a generator form field, or the q parameter of
-// non-numeric or out of range. The offending field name is wrapped in so the
+// /v1/image/, whose value is non-numeric or out of range. The offending field
-// response can name it.
+// name is wrapped in so the response can name it.
 var errInvalidFormField = errors.New("invalid")
 
-// Bounds for the generator's quality and ttl fields. maxTTL is in seconds:
+// Bounds for the generator's quality and ttl fields; the quality bounds also
-// the expiry calculation time.Duration(ttl) * time.Second overflows above it.
+// apply to the q parameter of /v1/image/. maxTTL is in seconds: the expiry
+// calculation time.Duration(ttl) * time.Second overflows above it.
 const (
 	minQuality = 1
 	maxQuality = 100
@@ -248,9 +249,9 @@ func parseFormDimension(form url.Values, field string) (int, error) {
 	return value, nil
 }
 
-// parseFormInt reads an optional integer form field, returning def when the
+// parseFormInt reads an optional integer form field or URL query parameter,
-// field is empty and an error naming the field when the value is non-numeric
+// returning def when the field is empty and an error naming the field when the
-// or outside minValue to maxValue.
+// value is non-numeric or outside minValue to maxValue.
 func parseFormInt(
 	form url.Values, field string, def, minValue, maxValue int,
 ) (int, error) {

internal/handlers/handlers_internal_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"database/sql"
+	"encoding/json"
 	"image"
 	"image/color"
 	"image/jpeg"
@@ -291,3 +292,68 @@ func TestHandleImage_InvalidFitMode_Returns400(t *testing.T) {
 		t.Fatalf("status = %d, want %d", status, http.StatusBadRequest)
 	}
 }
+
+// TestHandleImage_InvalidQuery_Returns400 verifies that the plain image route
+// answers a q that is not a whole number from 1 to 100, an empty one
+// included, with 400 naming q and the value, a parameter given more than once
+// with 400 naming it, and a query string that cannot be decoded with 400
+// 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=banana", `invalid q: not a number, got "banana"`},
+		{"q=0", `invalid q: must be from 1 to 100, got "0"`},
+		{"q=101", `invalid q: must be from 1 to 100, got "101"`},
+		{"q=", `invalid q: not a number, got ""`},
+		{"q=&q=500", `invalid q: given more than once`},
+		{"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.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?"+tt.query, 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)
+			}
+
+			if body.Error != tt.wantError {
+				t.Errorf("error = %q, want %q", body.Error, tt.wantError)
+			}
+		})
+	}
+}

internal/handlers/image.go

@@ -2,12 +2,15 @@ package handlers
 
 import (
 	"errors"
+	"fmt"
 	"io"
 	"net/http"
+	"net/url"
 	"strconv"
 	"time"
 
 	"github.com/go-chi/chi/v5"
+	"sneak.berlin/go/pixa/internal/encurl"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
 	"sneak.berlin/go/pixa/internal/imgcache"
 )
@@ -89,8 +92,28 @@ func (s *Handlers) parseImageRequest(
 	// Convert to ImageRequest
 	req := parsed.ToImageRequest()
 
-	// Parse signature params from query string
+	// Parse signature params from query string. r.URL.Query() would silently
-	query := r.URL.Query()
+	// drop a pair it cannot decode, such as q=80%, so that q would be served
+	// at 85; a query string that cannot be decoded is refused instead. A
+	// 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
+	}
+
+	for name, values := range query {
+		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")
 
 	if expStr := query.Get("exp"); expStr != "" {
@@ -100,23 +123,31 @@ func (s *Handlers) parseImageRequest(
 		}
 	}
 
-	// Parse optional quality and fit params
+	// Parse optional quality and fit params. Only a q missing from the URL is
-	if qStr := query.Get("q"); qStr != "" {
+	// 85. A q in the URL that is not a whole number from 1 to 100, an empty
-		q, parseErr := strconv.Atoi(qStr)
+	// one included, is refused, checked as the generator checks its quality
-		if parseErr == nil && q > 0 && q <= 100 {
+	// field; that check alone would take an empty q as missing.
-			req.Quality = q
+	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),
+			http.StatusBadRequest)
+
+		return nil, false
 	}
 
 	if fit := query.Get("fit"); fit != "" {
 		req.FitMode = imgcache.FitMode(fit)
 	}
 
-	// Default quality if not set
-	if req.Quality == 0 {
-		req.Quality = 85
-	}
-
 	// Default fit mode if not set
 	if req.FitMode == "" {
 		req.FitMode = imgcache.FitCover