diff --git a/README.md b/README.md index 681a624..cb8cefd 100644 --- a/README.md +++ b/README.md @@ -125,8 +125,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 - has no `q` +- `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` diff --git a/TODO.md b/TODO.md index 3e8c7b4..6281a0a 100644 --- a/TODO.md +++ b/TODO.md @@ -30,6 +30,12 @@ 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 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`); an absent or empty `q` is still 85; `README.md` + states the range. - 2026-09-28 start on a fresh upaas volume (closes #129): the image starts as root only to give `/var/lib/pixa` to `pixad` when `pixad` does not own it (`deploy/docker-entrypoint.sh`), then runs the server diff --git a/internal/handlers/auth.go b/internal/handlers/auth.go index 4edf22f..7ebbe1c 100644 --- a/internal/handlers/auth.go +++ b/internal/handlers/auth.go @@ -23,8 +23,9 @@ import ( // response can name it. var errInvalidFormField = errors.New("invalid") -// Bounds for the generator's quality and ttl fields. maxTTL is in seconds: -// the expiry calculation time.Duration(ttl) * time.Second overflows above it. +// Bounds for the generator's quality and ttl fields; the quality bounds also +// 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 -// field is empty and an error naming the field when the value is non-numeric -// or outside minValue to maxValue. +// parseFormInt reads an optional integer form field or URL query parameter, +// returning def when the field is empty and an error naming the field when the +// value is non-numeric or outside minValue to maxValue. func parseFormInt( form url.Values, field string, def, minValue, maxValue int, ) (int, error) { diff --git a/internal/handlers/image.go b/internal/handlers/image.go index 7411e56..586c0d3 100644 --- a/internal/handlers/image.go +++ b/internal/handlers/image.go @@ -2,12 +2,14 @@ package handlers import ( "errors" + "fmt" "io" "net/http" "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" ) @@ -100,23 +102,22 @@ func (s *Handlers) parseImageRequest( } } - // Parse optional quality and fit params - if qStr := query.Get("q"); qStr != "" { - q, parseErr := strconv.Atoi(qStr) - if parseErr == nil && q > 0 && q <= 100 { - req.Quality = q - } + // 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 + // checks its quality field. + req.Quality, err = parseFormInt(query, "q", + encurl.DefaultQuality, minQuality, maxQuality) + if err != nil { + s.respondError(w, fmt.Sprintf("%v, got %q", err, query.Get("q")), + 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