Refuse a q outside 1-100 on /v1/image/ with 400 (closes #134)
check / check (push) Successful in 2m34s
check / check (push) Successful in 2m34s
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. That check takes an empty value as missing, so an empty q in the URL is refused before it. The query string is read with url.ParseQuery, since r.URL.Query() drops a pair it cannot decode, such as q=80%; one that cannot be decoded is a 400 showing it. Only a q missing from the URL is 85. README.md states the range. Model: opus-5-5
This commit is contained in:
@@ -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) {
|
||||
|
||||
+33
-13
@@ -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,17 @@ 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.
|
||||
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
|
||||
}
|
||||
|
||||
req.Signature = query.Get("sig")
|
||||
|
||||
if expStr := query.Get("exp"); expStr != "" {
|
||||
@@ -100,23 +112,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
|
||||
|
||||
Reference in New Issue
Block a user