Refuse a q outside 1-100 on /v1/image/ with 400 (closes #134)
check / check (push) Failing after 58s

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
This commit is contained in:
2026-09-28 15:19:12 +00:00
parent 2dff1b5515
commit 35c576e677
4 changed files with 69 additions and 23 deletions
+9 -8
View File
@@ -18,13 +18,14 @@ import (
"sneak.berlin/go/pixa/internal/templates"
)
// errInvalidFormField reports a generator form field whose value is
// non-numeric or out of range. The offending field name is wrapped in so the
// response can name it.
// errInvalidFormField reports a generator form field, or the q parameter of
// /v1/image/, whose value is non-numeric or out of range. The offending field
// 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:
// 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) {
+44 -13
View File
@@ -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
query := r.URL.Query()
// Parse signature params from query string. r.URL.Query() would silently
// 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
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. 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),
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