Refuse a q outside 1-100 on /v1/image/ with 400 (closes #134)
check / check (push) Successful in 3m44s
check / check (push) Successful in 3m44s
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:
@@ -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.
|
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`
|
- `<format>`: one of `orig`, `png`, `jpeg`, `webp`
|
||||||
- `<size>`: `orig` or `<width>x<height>` (e.g. `800x600`)
|
- `<size>`: `orig` or `<width>x<height>` (e.g. `800x600`)
|
||||||
|
|
||||||
@@ -125,8 +128,9 @@ Where:
|
|||||||
- `height` — requested height in pixels, `0` for original
|
- `height` — requested height in pixels, `0` for original
|
||||||
- `format` — output format (jpeg, png, webp, avif, gif, orig)
|
- `format` — output format (jpeg, png, webp, avif, gif, orig)
|
||||||
- `expiration` — Unix timestamp when signature expires
|
- `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,
|
- `fit` — the URL's `fit` query parameter (cover, contain, fill, inside,
|
||||||
outside), or `cover` when the URL has no `fit`
|
outside), or `cover` when the URL has no `fit`
|
||||||
|
|
||||||
|
|||||||
@@ -30,6 +30,16 @@ exhaustion
|
|||||||
|
|
||||||
# Completed Steps
|
# 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
|
- 2026-09-28 unknown `PIXA_` environment variables abort startup (closes
|
||||||
#133): a variable whose name starts with `PIXA_` but is neither a
|
#133): a variable whose name starts with `PIXA_` but is neither a
|
||||||
setting's variable nor `PIXA_CONFIG_PATH` aborts startup naming it, as
|
setting's variable nor `PIXA_CONFIG_PATH` aborts startup naming it, as
|
||||||
|
|||||||
@@ -18,13 +18,14 @@ import (
|
|||||||
"sneak.berlin/go/pixa/internal/templates"
|
"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")
|
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 (
|
const (
|
||||||
minQuality = 1
|
minQuality = 1
|
||||||
maxQuality = 100
|
maxQuality = 100
|
||||||
@@ -248,9 +249,9 @@ func parseFormDimension(form url.Values, field string) (int, error) {
|
|||||||
return value, nil
|
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(
|
func parseFormInt(
|
||||||
form url.Values, field string, def, minValue, maxValue int,
|
form url.Values, field string, def, minValue, maxValue int,
|
||||||
) (int, error) {
|
) (int, error) {
|
||||||
|
|||||||
+43
-12
@@ -2,12 +2,15 @@ package handlers
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"errors"
|
"errors"
|
||||||
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
"net/http"
|
"net/http"
|
||||||
|
"net/url"
|
||||||
"strconv"
|
"strconv"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
"github.com/go-chi/chi/v5"
|
"github.com/go-chi/chi/v5"
|
||||||
|
"sneak.berlin/go/pixa/internal/encurl"
|
||||||
"sneak.berlin/go/pixa/internal/httpfetcher"
|
"sneak.berlin/go/pixa/internal/httpfetcher"
|
||||||
"sneak.berlin/go/pixa/internal/imgcache"
|
"sneak.berlin/go/pixa/internal/imgcache"
|
||||||
)
|
)
|
||||||
@@ -89,8 +92,28 @@ func (s *Handlers) parseImageRequest(
|
|||||||
// Convert to ImageRequest
|
// Convert to ImageRequest
|
||||||
req := parsed.ToImageRequest()
|
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")
|
req.Signature = query.Get("sig")
|
||||||
|
|
||||||
if expStr := query.Get("exp"); expStr != "" {
|
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 != "" {
|
if fit := query.Get("fit"); fit != "" {
|
||||||
req.FitMode = imgcache.FitMode(fit)
|
req.FitMode = imgcache.FitMode(fit)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Default quality if not set
|
|
||||||
if req.Quality == 0 {
|
|
||||||
req.Quality = 85
|
|
||||||
}
|
|
||||||
|
|
||||||
// Default fit mode if not set
|
// Default fit mode if not set
|
||||||
if req.FitMode == "" {
|
if req.FitMode == "" {
|
||||||
req.FitMode = imgcache.FitCover
|
req.FitMode = imgcache.FitCover
|
||||||
|
|||||||
Reference in New Issue
Block a user