Author SHA1 Message Date
sneak 98d23ad32e Refuse a q outside 1-100 on /v1/image/ with 400 (closes #134)
check / check (push) Successful in 3m16s
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. Only a q missing from the URL is
85. README.md states the range.

Model: opus-5-5
2026-09-28 14:44:25 +00:00
sneak a3dd1aad4e test: /v1/image/ answers an invalid q with 400 (closes #134)
Route tests for q=banana, q=0, q=101, an empty q= and q=&q=500: each
must be a 400 whose error names q and the value. They fail on next,
where each is served at the default quality 85.

Model: opus-5-5
2026-09-28 14:44:25 +00:00
5 changed files with 94 additions and 21 deletions
+3 -2
View File
@@ -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`
+6
View File
@@ -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, 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; `README.md` states the range.
- 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
+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) {
@@ -4,6 +4,7 @@ import (
"bytes"
"context"
"database/sql"
"encoding/json"
"image"
"image/color"
"image/jpeg"
@@ -291,3 +292,57 @@ func TestHandleImage_InvalidFitMode_Returns400(t *testing.T) {
t.Fatalf("status = %d, want %d", status, http.StatusBadRequest)
}
}
// TestHandleImage_InvalidQuality_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, instead of serving the image at
// the default quality 85.
func TestHandleImage_InvalidQuality_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: not a number, got ""`},
}
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)
}
})
}
}
+21 -11
View File
@@ -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,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