Compare commits
2
Commits
main
..
09c627bf8b
| Author | SHA1 | Date | |
|---|---|---|---|
 sneak
|
09c627bf8b | ||
|
sneak
|
3bcb2cd6e5 |
@@ -94,38 +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.
|
||||
|
||||
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`
|
||||
- `<size>`: `orig` or `<width>x<height>` (e.g. `800x600`)
|
||||
|
||||
The login form (`POST /`) is limited to 5 attempts per minute per client
|
||||
address, counting an IPv6 client by its /64; an attempt over the limit is
|
||||
refused with 429 and a `Retry-After` header. Behind a reverse proxy the client
|
||||
address comes from `X-Forwarded-For` only when the proxy's address is in
|
||||
`trusted_proxies`; otherwise all users behind the proxy are counted as one
|
||||
client. With the default `trusted_proxies` (the RFC 1918 ranges), a client
|
||||
with a private address can choose the address it is counted by through its own
|
||||
`X-Forwarded-For`, whether it connects directly or through the proxy, because
|
||||
its own address is trusted too. Setting `trusted_proxies` to the proxy's own
|
||||
address closes this.
|
||||
|
||||
### Image Metadata
|
||||
|
||||
pixa decodes and re-encodes every image it serves, and removes all metadata from
|
||||
the output: EXIF (GPS position, camera make, model and serial number, capture
|
||||
time, embedded thumbnail), XMP, IPTC and the ICC colour profile. This cannot be
|
||||
turned off.
|
||||
|
||||
- The `orig` format means the source's own format, not the source's bytes: an
|
||||
`orig` image is re-encoded and stripped like any other.
|
||||
- An image with an EXIF orientation is turned upright first, so it displays the
|
||||
same without the tag; a requested size applies to the upright image.
|
||||
- An image with an ICC profile is converted to sRGB first, since clients show an
|
||||
image with no profile as sRGB. Colours outside sRGB, such as the most
|
||||
saturated ones in a Display P3 photo, are clipped.
|
||||
|
||||
### Source Hosts
|
||||
|
||||
Source hosts may be allowlisted in the configuration. Non-allowlisted
|
||||
@@ -153,15 +124,11 @@ Where:
|
||||
- `width` — requested width in pixels, `0` for original
|
||||
- `height` — requested height in pixels, `0` for original
|
||||
- `format` — output format (jpeg, png, webp, avif, gif, orig)
|
||||
- `expiration` — the URL's `exp` query parameter, the Unix timestamp when
|
||||
the signature expires; a request whose `exp` is not a whole number, an
|
||||
empty `exp=` included, is refused with 400
|
||||
- `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
|
||||
- `expiration` — Unix timestamp when signature expires
|
||||
- `quality` — the URL's `q` query parameter (1-100), or `85` when the URL
|
||||
has no `q`
|
||||
- `fit` — the URL's `fit` query parameter (cover, contain, fill, inside,
|
||||
outside), or `cover` when the URL has no `fit`; a request whose `fit` is
|
||||
anything else, an empty `fit=` included, is refused with 400
|
||||
outside), or `cover` when the URL has no `fit`
|
||||
|
||||
**Example:** resize `https://cdn.example.com/photos/cat.jpg` to 800x600
|
||||
WebP with expiration 1704067200, default quality and fit:
|
||||
@@ -231,8 +198,7 @@ Key settings in more detail:
|
||||
inside one of these ranges; the logged and login-recorded client
|
||||
address is then the rightmost forwarded entry that is not itself a
|
||||
trusted proxy. Otherwise the direct peer address is used and the header
|
||||
is ignored, so a client connecting directly from an address outside
|
||||
these ranges cannot spoof its address.
|
||||
is ignored, so a client connecting directly cannot spoof its address.
|
||||
An omitted key defaults to the RFC 1918 private ranges (`10.0.0.0/8`,
|
||||
`172.16.0.0/12`, `192.168.0.0/16`), since pixa is deployed behind a
|
||||
proxy on a private network; an explicitly empty list (`[]`) trusts no
|
||||
|
||||
@@ -30,46 +30,6 @@ exhaustion
|
||||
|
||||
# Completed Steps
|
||||
|
||||
- 2026-09-28 strip metadata from processed images (closes #82): every output is
|
||||
exported with govips' `StripMetadata`, so it carries no EXIF, XMP, IPTC or ICC
|
||||
profile; the image is first turned upright with `AutoRotate` (before sizes are
|
||||
worked out) and, when it has an ICC profile, converted to sRGB; the `orig`
|
||||
format is re-encoded and stripped like any other, as pixa never serves the
|
||||
source bytes; there is no setting to keep metadata; documented in `README.md`.
|
||||
- 2026-09-28 rate limit the login form (closes #66): `POST /` is limited to 5
|
||||
attempts per minute per client address, and an attempt over the limit is
|
||||
refused with 429 and a `Retry-After` header; the address is the one
|
||||
`internal/clientip` resolves through `trusted_proxies`, an IPv6 client is
|
||||
counted by its /64, and an IPv4-mapped address as the IPv4 address it
|
||||
carries; the limit is a `RateLimit` middleware in `internal/middleware` on
|
||||
`github.com/go-chi/httprate`, which the image routes can reuse; the library
|
||||
keeps counts for the current and the previous minute only; documented in
|
||||
`README.md`.
|
||||
- 2026-09-28 refuse an unparseable `exp` on `/v1/image/` and log swallowed
|
||||
cache errors (closes #72): an `exp` in the URL that is not a whole
|
||||
number, an empty `exp=` included, is a 400 naming `exp` and the value,
|
||||
instead of being ignored and answered with 401 as if the URL had no
|
||||
`exp`; only an `exp` missing from the URL is unchanged; `README.md` says
|
||||
so where it documents `exp`. A failed variant `.meta` write, source
|
||||
metadata JSON write, `Stats` count query, stats counter update, negative
|
||||
cache write or expired negative cache delete is now logged at `warn`
|
||||
with the path or key and the error, and stays non-fatal.
|
||||
- 2026-09-28 refuse an empty `fit` on `/v1/image/` (closes #139): a
|
||||
`fit` in the URL with an empty value (`fit=`) is a 400 naming `fit`,
|
||||
instead of being served as `cover` and verified against a signature
|
||||
made for `cover`; only a `fit` missing from the URL is still `cover`;
|
||||
any other value still goes through the existing fit-mode check;
|
||||
`README.md` says so where it documents `fit`.
|
||||
- 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
|
||||
#133): a variable whose name starts with `PIXA_` but is neither a
|
||||
setting's variable nor `PIXA_CONFIG_PATH` aborts startup naming it, as
|
||||
@@ -257,9 +217,10 @@ exhaustion
|
||||
|
||||
# Future Steps
|
||||
|
||||
- P1: strip EXIF and other metadata from processed images (privacy)
|
||||
- P2: security
|
||||
- referer blacklist
|
||||
- per-IP rate limiting on the image routes
|
||||
- per-IP rate limiting
|
||||
- per-origin rate limiting
|
||||
- P2: HTTP response handling
|
||||
- Last-Modified headers
|
||||
|
||||
@@ -11,7 +11,6 @@ require (
|
||||
github.com/getsentry/sentry-go v0.40.0
|
||||
github.com/go-chi/chi/v5 v5.2.3
|
||||
github.com/go-chi/cors v1.2.2
|
||||
github.com/go-chi/httprate v0.16.0
|
||||
github.com/gorilla/csrf v1.7.3
|
||||
github.com/gorilla/securecookie v1.1.2
|
||||
github.com/prometheus/client_golang v1.23.2
|
||||
@@ -92,7 +91,6 @@ require (
|
||||
github.com/inconshreveable/mousetrap v1.1.0 // indirect
|
||||
github.com/josharian/intern v1.0.0 // indirect
|
||||
github.com/json-iterator/go v1.1.12 // indirect
|
||||
github.com/klauspost/cpuid/v2 v2.2.10 // indirect
|
||||
github.com/kylelemons/godebug v1.1.0 // indirect
|
||||
github.com/mailru/easyjson v0.7.7 // indirect
|
||||
github.com/mattn/go-colorable v0.1.13 // indirect
|
||||
@@ -115,7 +113,6 @@ require (
|
||||
github.com/tidwall/match v1.1.1 // indirect
|
||||
github.com/tidwall/pretty v1.2.0 // indirect
|
||||
github.com/x448/float16 v0.8.4 // indirect
|
||||
github.com/zeebo/xxh3 v1.0.2 // indirect
|
||||
go.etcd.io/etcd/api/v3 v3.6.2 // indirect
|
||||
go.etcd.io/etcd/client/pkg/v3 v3.6.2 // indirect
|
||||
go.etcd.io/etcd/client/v3 v3.6.2 // indirect
|
||||
|
||||
@@ -114,8 +114,6 @@ github.com/go-chi/chi/v5 v5.2.3 h1:WQIt9uxdsAbgIYgid+BpYc+liqQZGMHRaUwp0JUcvdE=
|
||||
github.com/go-chi/chi/v5 v5.2.3/go.mod h1:L2yAIGWB3H+phAw1NxKwWM+7eUH/lU8pOMm5hHcoops=
|
||||
github.com/go-chi/cors v1.2.2 h1:Jmey33TE+b+rB7fT8MUy1u0I4L+NARQlK6LhzKPSyQE=
|
||||
github.com/go-chi/cors v1.2.2/go.mod h1:sSbTewc+6wYHBBCW7ytsFSn836hqM7JxpglAy2Vzc58=
|
||||
github.com/go-chi/httprate v0.16.0 h1:8V5DH9j6pSK6UQoBsTpvMyFxycqaKEIToyPKzHJjUa8=
|
||||
github.com/go-chi/httprate v0.16.0/go.mod h1:A8lo+qRhk+s9LiuP5saS7XCGDXRXMcrueq0NfIuCa/I=
|
||||
github.com/go-errors/errors v1.4.2 h1:J6MZopCL4uSllY1OfXM374weqZFFItUbrImctkmUxIA=
|
||||
github.com/go-errors/errors v1.4.2/go.mod h1:sIVyrIiJhuEF+Pj9Ebtd6P/rEYROXFi3BopGUQ5a5Og=
|
||||
github.com/go-jose/go-jose/v4 v4.0.5 h1:M6T8+mKZl/+fNNuFHvGIzDz7BTLQPIounk/b9dw3AaE=
|
||||
@@ -253,8 +251,6 @@ github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI
|
||||
github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
|
||||
github.com/klauspost/compress v1.18.0 h1:c/Cqfb0r+Yi+JtIEq73FWXVkRonBlf0CRNYc8Zttxdo=
|
||||
github.com/klauspost/compress v1.18.0/go.mod h1:2Pp+KzxcywXVXMr50+X0Q/Lsb43OQHYWRCY2AiWywWQ=
|
||||
github.com/klauspost/cpuid/v2 v2.2.10 h1:tBs3QSyvjDyFTq3uoc/9xFpCuOsJQFNPiAhYdw2skhE=
|
||||
github.com/klauspost/cpuid/v2 v2.2.10/go.mod h1:hqwkgyIinND0mEev00jJYCxPNVRVXFQeu1XKlok6oO0=
|
||||
github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ=
|
||||
github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFBFZlji/RkVcI2GknAs/DXo4wKdlNEc=
|
||||
github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
|
||||
@@ -400,10 +396,6 @@ github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcY
|
||||
github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
|
||||
github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
|
||||
github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
|
||||
github.com/zeebo/assert v1.3.0 h1:g7C04CbJuIDKNPFHmsk4hwZDO5O+kntRxzaUoNXj+IQ=
|
||||
github.com/zeebo/assert v1.3.0/go.mod h1:Pq9JiuJQpG8JLJdtkwrJESF0Foym2/D9XMU5ciN/wJ0=
|
||||
github.com/zeebo/xxh3 v1.0.2 h1:xZmwmqxHZA8AI603jOQ0tMqmBr9lPeFwGg6d+xy9DC0=
|
||||
github.com/zeebo/xxh3 v1.0.2/go.mod h1:5NWz9Sef7zIDm2JHfFlcQvNekmcEl9ekUZQQKCYaDcA=
|
||||
go.etcd.io/etcd/api/v3 v3.6.2 h1:25aCkIMjUmiiOtnBIp6PhNj4KdcURuBak0hU2P1fgRc=
|
||||
go.etcd.io/etcd/api/v3 v3.6.2/go.mod h1:eFhhvfR8Px1P6SEuLT600v+vrhdDTdcfMzmnxVXXSbk=
|
||||
go.etcd.io/etcd/client/pkg/v3 v3.6.2 h1:zw+HRghi/G8fKpgKdOcEKpnBTE4OO39T6MegA0RopVU=
|
||||
|
||||
@@ -18,14 +18,13 @@ import (
|
||||
"sneak.berlin/go/pixa/internal/templates"
|
||||
)
|
||||
|
||||
// errInvalidFormField reports a generator form field, or the q or exp
|
||||
// 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.
|
||||
// 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.
|
||||
var errInvalidFormField = errors.New("invalid")
|
||||
|
||||
// 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.
|
||||
// Bounds for the generator's quality and ttl fields. maxTTL is in seconds:
|
||||
// the expiry calculation time.Duration(ttl) * time.Second overflows above it.
|
||||
const (
|
||||
minQuality = 1
|
||||
maxQuality = 100
|
||||
@@ -249,9 +248,9 @@ func parseFormDimension(form url.Values, field string) (int, error) {
|
||||
return value, nil
|
||||
}
|
||||
|
||||
// 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.
|
||||
// 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.
|
||||
func parseFormInt(
|
||||
form url.Values, field string, def, minValue, maxValue int,
|
||||
) (int, error) {
|
||||
|
||||
@@ -4,7 +4,6 @@ import (
|
||||
"bytes"
|
||||
"context"
|
||||
"database/sql"
|
||||
"encoding/json"
|
||||
"image"
|
||||
"image/color"
|
||||
"image/jpeg"
|
||||
@@ -292,105 +291,3 @@ func TestHandleImage_InvalidFitMode_Returns400(t *testing.T) {
|
||||
t.Fatalf("status = %d, want %d", status, http.StatusBadRequest)
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleImage_InvalidQuery_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, a parameter given more than once
|
||||
// with 400 naming it, and a query string that cannot be decoded with 400
|
||||
// showing it, instead of serving the image at the default quality 85 or at
|
||||
// the first value given.
|
||||
func TestHandleImage_InvalidQuery_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=80&q=500", `invalid q: given more than once`},
|
||||
{"q=80&q=", `invalid q: given more than once`},
|
||||
{"fit=cover&fit=contain", `invalid fit: given more than once`},
|
||||
{"q=80%", `invalid query string "q=80%": invalid URL escape "%"`},
|
||||
{
|
||||
"q=50;fit=contain",
|
||||
`invalid query string "q=50;fit=contain": ` +
|
||||
`invalid semicolon separator in query`,
|
||||
},
|
||||
}
|
||||
|
||||
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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleImage_EmptyFit_Returns400 verifies that the plain image route
|
||||
// answers a fit that is in the URL but empty with 400 naming fit, instead of
|
||||
// serving the image as cover. Only a fit missing from the URL means cover.
|
||||
func TestHandleImage_EmptyFit_Returns400(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?fit=", 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)
|
||||
}
|
||||
|
||||
wantError := `invalid fit: not a fit mode, got ""`
|
||||
if body.Error != wantError {
|
||||
t.Errorf("error = %q, want %q", body.Error, wantError)
|
||||
}
|
||||
}
|
||||
|
||||
+18
-75
@@ -2,15 +2,12 @@ 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"
|
||||
)
|
||||
@@ -92,66 +89,32 @@ func (s *Handlers) parseImageRequest(
|
||||
// Convert to ImageRequest
|
||||
req := parsed.ToImageRequest()
|
||||
|
||||
// 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
|
||||
}
|
||||
}
|
||||
|
||||
// Parse signature params from query string
|
||||
query := r.URL.Query()
|
||||
req.Signature = query.Get("sig")
|
||||
|
||||
req.Expires, err = parseExpires(query)
|
||||
if err != nil {
|
||||
s.respondError(w, err.Error(), http.StatusBadRequest)
|
||||
|
||||
return nil, false
|
||||
if expStr := query.Get("exp"); expStr != "" {
|
||||
exp, parseErr := strconv.ParseInt(expStr, 10, 64)
|
||||
if parseErr == nil {
|
||||
req.Expires = time.Unix(exp, 0)
|
||||
}
|
||||
}
|
||||
|
||||
// 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
|
||||
// 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
|
||||
}
|
||||
}
|
||||
|
||||
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)
|
||||
}
|
||||
|
||||
// Only a fit missing from the URL is cover. A fit in the URL that is not a
|
||||
// fit mode is refused by the fit-mode check below; that check would take an
|
||||
// empty fit as missing, so an empty one is refused here.
|
||||
req.FitMode = imgcache.FitMode(query.Get("fit"))
|
||||
if query.Has("fit") && req.FitMode == "" {
|
||||
s.respondError(w, `invalid fit: not a fit mode, got ""`, http.StatusBadRequest)
|
||||
|
||||
return nil, false
|
||||
// Default quality if not set
|
||||
if req.Quality == 0 {
|
||||
req.Quality = 85
|
||||
}
|
||||
|
||||
// Default fit mode if not set
|
||||
@@ -174,26 +137,6 @@ func (s *Handlers) parseImageRequest(
|
||||
return req, true
|
||||
}
|
||||
|
||||
// parseExpires reads the exp query parameter, a Unix time in seconds. An exp
|
||||
// missing from the URL gives the zero time, which the signature check takes
|
||||
// as no expiration. An exp in the URL that is not a whole number, an empty
|
||||
// one included, is an error naming exp and the value.
|
||||
func parseExpires(query url.Values) (time.Time, error) {
|
||||
if !query.Has("exp") {
|
||||
return time.Time{}, nil
|
||||
}
|
||||
|
||||
expStr := query.Get("exp")
|
||||
|
||||
exp, err := strconv.ParseInt(expStr, 10, 64)
|
||||
if err != nil {
|
||||
return time.Time{}, fmt.Errorf("%w exp: not a number, got %q",
|
||||
errInvalidFormField, expStr)
|
||||
}
|
||||
|
||||
return time.Unix(exp, 0), nil
|
||||
}
|
||||
|
||||
// respondImageError maps image retrieval errors to HTTP responses.
|
||||
func (s *Handlers) respondImageError(
|
||||
w http.ResponseWriter, req *imgcache.ImageRequest, err error,
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
@@ -119,53 +118,3 @@ func TestHandleImage_GeneratedSignedURLVerifies(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleImage_InvalidExp_Returns400 sends a signed-host URL whose exp is
|
||||
// not a whole number, and one whose exp is empty. Each is refused with 400
|
||||
// naming exp and the value, not with the 401 a URL without exp still gets.
|
||||
func TestHandleImage_InvalidExp_Returns400(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tests := []struct {
|
||||
query string
|
||||
wantStatus int
|
||||
wantError string
|
||||
}{
|
||||
{"sig=x&exp=banana", http.StatusBadRequest,
|
||||
`invalid exp: not a number, got "banana"`},
|
||||
{"sig=x&exp=", http.StatusBadRequest, `invalid exp: not a number, got ""`},
|
||||
{"sig=x", http.StatusUnauthorized, "unauthorized"},
|
||||
}
|
||||
|
||||
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/"+signedHost+"/images/photo.jpg/50x50.jpeg?"+tt.query, nil)
|
||||
rec := httptest.NewRecorder()
|
||||
|
||||
r.ServeHTTP(rec, req)
|
||||
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 rec.Code != tt.wantStatus || body.Error != tt.wantError {
|
||||
t.Errorf("got %d %q, want %d %q",
|
||||
rec.Code, body.Error, tt.wantStatus, tt.wantError)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -161,13 +161,6 @@ func (p *ImageProcessor) Process(
|
||||
}
|
||||
defer img.Close()
|
||||
|
||||
// Turn the image upright now: encode strips the EXIF orientation tag,
|
||||
// and sizes below must be worked out on the upright image.
|
||||
err = img.AutoRotate()
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to auto-rotate: %w", err)
|
||||
}
|
||||
|
||||
// Get original dimensions
|
||||
origWidth := img.Width()
|
||||
origHeight := img.Height()
|
||||
@@ -411,21 +404,6 @@ func (p *ImageProcessor) encode(
|
||||
return nil, fmt.Errorf("%w: %s", ErrUnsupportedOutputFormat, format)
|
||||
}
|
||||
|
||||
// Stripping drops the ICC profile as well, and clients show an image
|
||||
// with no profile as sRGB, so convert to sRGB first. "srgb" names
|
||||
// libvips' built-in profile; govips' own sRGB path variable is set on
|
||||
// first use but read without a lock, so concurrent requests race on it.
|
||||
if img.HasICCProfile() {
|
||||
err := img.TransformICCProfileWithFallback("srgb", "srgb")
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to convert to sRGB: %w", err)
|
||||
}
|
||||
}
|
||||
|
||||
// Drop EXIF, XMP, IPTC and the ICC profile. govips ignores this for
|
||||
// GIF, which carries none of them.
|
||||
params.StripMetadata = true
|
||||
|
||||
output, _, err := img.Export(¶ms)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
|
||||
@@ -9,9 +9,7 @@ import (
|
||||
"image/jpeg"
|
||||
"image/png"
|
||||
"io"
|
||||
"math"
|
||||
"os"
|
||||
"slices"
|
||||
"testing"
|
||||
|
||||
"github.com/davidbyttow/govips/v2/vips"
|
||||
@@ -563,152 +561,3 @@ func TestImageProcessor_EncodeAVIF(t *testing.T) {
|
||||
|
||||
encodeAndCheck(t, FormatAVIF, 85, mimeAVIF)
|
||||
}
|
||||
|
||||
// processAndDecode runs input through Process and decodes the output with
|
||||
// vips, so a test can inspect the image a client would receive.
|
||||
func processAndDecode(t *testing.T, input []byte, req *Request) *vips.ImageRef {
|
||||
t.Helper()
|
||||
|
||||
result, err := New(Params{}).Process(
|
||||
context.Background(), bytes.NewReader(input), req,
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatalf("Process() error = %v", err)
|
||||
}
|
||||
|
||||
defer func() { _ = result.Content.Close() }()
|
||||
|
||||
data, err := io.ReadAll(result.Content)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to read result: %v", err)
|
||||
}
|
||||
|
||||
output, err := vips.NewImageFromBuffer(data)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to decode output: %v", err)
|
||||
}
|
||||
|
||||
t.Cleanup(output.Close)
|
||||
|
||||
return output
|
||||
}
|
||||
|
||||
func TestImageProcessor_StripsEXIF(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// gps-exif.jpg carries GPS coordinates, a camera make, model and serial
|
||||
// number, and a capture time.
|
||||
input, err := os.ReadFile("testdata/gps-exif.jpg")
|
||||
if err != nil {
|
||||
t.Fatalf("failed to read test JPEG: %v", err)
|
||||
}
|
||||
|
||||
fixture, err := vips.NewImageFromBuffer(input)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to decode test JPEG: %v", err)
|
||||
}
|
||||
|
||||
t.Cleanup(fixture.Close)
|
||||
|
||||
if !slices.Contains(fixture.GetFields(), "exif-ifd3-GPSLatitude") {
|
||||
t.Fatal("testdata/gps-exif.jpg has no GPS latitude")
|
||||
}
|
||||
|
||||
formats := []Format{
|
||||
FormatJPEG, FormatPNG, FormatWebP, FormatAVIF, FormatGIF, FormatOriginal,
|
||||
}
|
||||
|
||||
for _, format := range formats {
|
||||
t.Run(string(format), func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
output := processAndDecode(t, input, &Request{Format: format})
|
||||
|
||||
if output.HasExif() {
|
||||
t.Errorf("output has EXIF: %v", output.GetExif())
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestImageProcessor_AppliesEXIFOrientation(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// orientation-6.jpg is stored 16x8, red on the left and blue on the
|
||||
// right, with EXIF orientation 6 (turn 90 degrees clockwise to view).
|
||||
// Upright it is 8x16, red on top and blue below.
|
||||
input, err := os.ReadFile("testdata/orientation-6.jpg")
|
||||
if err != nil {
|
||||
t.Fatalf("failed to read test JPEG: %v", err)
|
||||
}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
size Size
|
||||
wantW int
|
||||
wantH int
|
||||
}{
|
||||
{name: "original size", size: Size{}, wantW: 8, wantH: 16},
|
||||
{name: "width only", size: Size{Width: 4}, wantW: 4, wantH: 8},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
output := processAndDecode(t, input, &Request{
|
||||
Size: tt.size,
|
||||
Format: FormatPNG,
|
||||
})
|
||||
|
||||
if output.Width() != tt.wantW || output.Height() != tt.wantH {
|
||||
t.Fatalf("output is %dx%d, want %dx%d",
|
||||
output.Width(), output.Height(), tt.wantW, tt.wantH)
|
||||
}
|
||||
|
||||
top, err := output.GetPoint(tt.wantW/2, 0)
|
||||
if err != nil {
|
||||
t.Fatalf("GetPoint() error = %v", err)
|
||||
}
|
||||
|
||||
bottom, err := output.GetPoint(tt.wantW/2, tt.wantH-1)
|
||||
if err != nil {
|
||||
t.Fatalf("GetPoint() error = %v", err)
|
||||
}
|
||||
|
||||
if top[0] <= top[2] || bottom[2] <= bottom[0] {
|
||||
t.Errorf("top pixel = %v, bottom pixel = %v, want red above blue",
|
||||
top, bottom)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestImageProcessor_ConvertsWideGamutToSRGB(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// display-p3.jpg is a flat 8x8 image with the Display P3 profile
|
||||
// embedded, filled with Display P3 (234, 51, 35), which is sRGB red.
|
||||
input, err := os.ReadFile("testdata/display-p3.jpg")
|
||||
if err != nil {
|
||||
t.Fatalf("failed to read test JPEG: %v", err)
|
||||
}
|
||||
|
||||
output := processAndDecode(t, input, &Request{Format: FormatPNG})
|
||||
|
||||
if output.HasICCProfile() {
|
||||
t.Error("output has an ICC profile")
|
||||
}
|
||||
|
||||
pixel, err := output.GetPoint(4, 4)
|
||||
if err != nil {
|
||||
t.Fatalf("GetPoint() error = %v", err)
|
||||
}
|
||||
|
||||
want := []float64{255, 0, 0}
|
||||
for i := range want {
|
||||
if math.Abs(pixel[i]-want[i]) > 5 {
|
||||
t.Fatalf("pixel = %v, want within 5 of %v", pixel, want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
BIN
Binary file not shown.
|
Before Width: | Height: | Size: 1.3 KiB |
BIN
Binary file not shown.
|
Before Width: | Height: | Size: 1.0 KiB |
Binary file not shown.
|
Before Width: | Height: | Size: 811 B |
+24
-59
@@ -125,7 +125,7 @@ func NewCache(db *sql.DB, config CacheConfig) (*Cache, error) {
|
||||
}
|
||||
|
||||
variants, err := NewVariantStorage(
|
||||
filepath.Join(config.StateDir, "cache", "variants"), log,
|
||||
filepath.Join(config.StateDir, "cache", "variants"),
|
||||
)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to create variant storage: %w", err)
|
||||
@@ -263,7 +263,23 @@ func (c *Cache) StoreSource(
|
||||
return "", fmt.Errorf("failed to insert source metadata: %w", err)
|
||||
}
|
||||
|
||||
c.writeMetadataSidecar(req, pathHash, contentHash, result)
|
||||
// Store metadata JSON file
|
||||
meta := &SourceMetadata{
|
||||
Host: req.SourceHost,
|
||||
Path: req.SourcePath,
|
||||
Query: req.SourceQuery,
|
||||
ContentHash: string(contentHash),
|
||||
StatusCode: result.StatusCode,
|
||||
ContentType: result.ContentType,
|
||||
ContentLength: result.ContentLength,
|
||||
ResponseHeaders: result.Headers,
|
||||
FetchedAt: time.Now().UTC().Unix(),
|
||||
FetchDurationMs: result.FetchDurationMs,
|
||||
RemoteAddr: result.RemoteAddr,
|
||||
}
|
||||
|
||||
// A failure here is non-fatal; the metadata is in the database.
|
||||
_ = c.srcMetadata.Store(req.SourceHost, pathHash, meta)
|
||||
|
||||
c.notifyWritePressure()
|
||||
|
||||
@@ -420,19 +436,12 @@ func (c *Cache) Stats(ctx context.Context) (*CacheStats, error) {
|
||||
}
|
||||
|
||||
// Get actual item count and total size from content tables
|
||||
err = c.db.QueryRowContext(ctx,
|
||||
_ = c.db.QueryRowContext(ctx,
|
||||
`SELECT COUNT(*) FROM request_cache`,
|
||||
).Scan(&stats.TotalItems)
|
||||
if err != nil {
|
||||
c.log.Warn("failed to count cache items for stats", "error", err)
|
||||
}
|
||||
|
||||
err = c.db.QueryRowContext(ctx,
|
||||
_ = c.db.QueryRowContext(ctx,
|
||||
`SELECT COALESCE(SUM(size_bytes), 0) FROM output_content`,
|
||||
).Scan(&stats.TotalSizeBytes)
|
||||
if err != nil {
|
||||
c.log.Warn("failed to sum cache size for stats", "error", err)
|
||||
}
|
||||
|
||||
// Compute hit rate as a ratio
|
||||
if stats.HitCount+stats.MissCount > 0 {
|
||||
@@ -444,17 +453,15 @@ func (c *Cache) Stats(ctx context.Context) (*CacheStats, error) {
|
||||
|
||||
// IncrementStats increments cache statistics.
|
||||
func (c *Cache) IncrementStats(ctx context.Context, hit bool, fetchBytes int64) {
|
||||
var err error
|
||||
|
||||
if hit {
|
||||
_, err = c.db.ExecContext(ctx, `
|
||||
_, _ = c.db.ExecContext(ctx, `
|
||||
UPDATE cache_stats
|
||||
SET hit_count = hit_count + 1,
|
||||
last_updated_at = CURRENT_TIMESTAMP
|
||||
WHERE id = 1
|
||||
`)
|
||||
} else {
|
||||
_, err = c.db.ExecContext(ctx, `
|
||||
_, _ = c.db.ExecContext(ctx, `
|
||||
UPDATE cache_stats
|
||||
SET miss_count = miss_count + 1,
|
||||
last_updated_at = CURRENT_TIMESTAMP
|
||||
@@ -462,52 +469,14 @@ func (c *Cache) IncrementStats(ctx context.Context, hit bool, fetchBytes int64)
|
||||
`)
|
||||
}
|
||||
|
||||
if err != nil {
|
||||
c.log.Warn("failed to count cache hit or miss", "hit", hit, "error", err)
|
||||
}
|
||||
|
||||
if fetchBytes > 0 {
|
||||
_, err = c.db.ExecContext(ctx, `
|
||||
_, _ = c.db.ExecContext(ctx, `
|
||||
UPDATE cache_stats
|
||||
SET upstream_fetch_count = upstream_fetch_count + 1,
|
||||
upstream_fetch_bytes = upstream_fetch_bytes + ?,
|
||||
last_updated_at = CURRENT_TIMESTAMP
|
||||
WHERE id = 1
|
||||
`, fetchBytes)
|
||||
if err != nil {
|
||||
c.log.Warn("failed to count upstream fetch",
|
||||
"fetch_bytes", fetchBytes, "error", err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// writeMetadataSidecar writes the JSON metadata sidecar of a stored source.
|
||||
// A failure is logged and is otherwise non-fatal; the metadata is in the
|
||||
// database.
|
||||
func (c *Cache) writeMetadataSidecar(
|
||||
req *ImageRequest,
|
||||
pathHash PathHash,
|
||||
contentHash ContentHash,
|
||||
result *httpfetcher.FetchResult,
|
||||
) {
|
||||
meta := &SourceMetadata{
|
||||
Host: req.SourceHost,
|
||||
Path: req.SourcePath,
|
||||
Query: req.SourceQuery,
|
||||
ContentHash: string(contentHash),
|
||||
StatusCode: result.StatusCode,
|
||||
ContentType: result.ContentType,
|
||||
ContentLength: result.ContentLength,
|
||||
ResponseHeaders: result.Headers,
|
||||
FetchedAt: time.Now().UTC().Unix(),
|
||||
FetchDurationMs: result.FetchDurationMs,
|
||||
RemoteAddr: result.RemoteAddr,
|
||||
}
|
||||
|
||||
err := c.srcMetadata.Store(req.SourceHost, pathHash, meta)
|
||||
if err != nil {
|
||||
c.log.Warn("failed to write metadata sidecar",
|
||||
"host", req.SourceHost, "path_hash", pathHash, "error", err)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -559,14 +528,10 @@ func (c *Cache) checkNegativeCache(
|
||||
// Check if expired
|
||||
if time.Now().After(expiresAt) {
|
||||
// Clean up expired entry
|
||||
_, err = c.db.ExecContext(ctx, `
|
||||
_, _ = c.db.ExecContext(ctx, `
|
||||
DELETE FROM negative_cache
|
||||
WHERE source_host = ? AND source_path = ? AND source_query = ?
|
||||
`, req.SourceHost, req.SourcePath, req.SourceQuery)
|
||||
if err != nil {
|
||||
c.log.Warn("failed to delete expired negative cache entry",
|
||||
"host", req.SourceHost, "path", req.SourcePath, "error", err)
|
||||
}
|
||||
|
||||
return false, nil
|
||||
}
|
||||
|
||||
@@ -322,12 +322,7 @@ func (s *Service) fetchAndProcess(
|
||||
// Store negative cache for certain errors
|
||||
if isNegativeCacheable(err) {
|
||||
statusCode := extractStatusCode(err)
|
||||
|
||||
storeErr := s.cache.StoreNegative(ctx, req, statusCode, err.Error())
|
||||
if storeErr != nil {
|
||||
s.log.Warn("failed to store negative cache entry",
|
||||
"host", req.SourceHost, "path", req.SourcePath, "error", storeErr)
|
||||
}
|
||||
_ = s.cache.StoreNegative(ctx, req, statusCode, err.Error())
|
||||
}
|
||||
|
||||
return nil, fmt.Errorf("upstream fetch failed: %w", err)
|
||||
|
||||
@@ -1,12 +1,9 @@
|
||||
package imgcache
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"database/sql"
|
||||
"log/slog"
|
||||
"math"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
@@ -104,82 +101,3 @@ func TestStats_ZeroCounts(t *testing.T) {
|
||||
t.Errorf("HitRate = %f, want 0.0 for zero counts", stats.HitRate)
|
||||
}
|
||||
}
|
||||
|
||||
// TestStats_LogsFailedCountQueries verifies that a failed item count query
|
||||
// and a failed size query are each logged at warn and Stats still succeeds.
|
||||
func TestStats_LogsFailedCountQueries(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db := setupStatsTestDB(t)
|
||||
|
||||
var logBuf bytes.Buffer
|
||||
|
||||
cache, err := NewCache(db, CacheConfig{
|
||||
StateDir: t.TempDir(),
|
||||
CacheTTL: time.Hour,
|
||||
NegativeTTL: 5 * time.Minute,
|
||||
Logger: slog.New(slog.NewJSONHandler(&logBuf, nil)),
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
_, err = db.ExecContext(t.Context(),
|
||||
`DROP TABLE request_cache; DROP TABLE output_content`)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
_, err = cache.Stats(t.Context())
|
||||
if err != nil {
|
||||
t.Fatalf("Stats() error = %v, want nil", err)
|
||||
}
|
||||
|
||||
for _, msg := range []string{
|
||||
"failed to count cache items for stats",
|
||||
"failed to sum cache size for stats",
|
||||
} {
|
||||
want := `"level":"WARN","msg":"` + msg + `"`
|
||||
if !strings.Contains(logBuf.String(), want) {
|
||||
t.Errorf("log missing %s; got %q", want, logBuf.String())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestIncrementStats_LogsFailedUpdates verifies that a failed hit or miss
|
||||
// count update and a failed upstream fetch count update are each logged at
|
||||
// warn.
|
||||
func TestIncrementStats_LogsFailedUpdates(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db := setupStatsTestDB(t)
|
||||
|
||||
var logBuf bytes.Buffer
|
||||
|
||||
cache, err := NewCache(db, CacheConfig{
|
||||
StateDir: t.TempDir(),
|
||||
CacheTTL: time.Hour,
|
||||
NegativeTTL: 5 * time.Minute,
|
||||
Logger: slog.New(slog.NewJSONHandler(&logBuf, nil)),
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
_, err = db.ExecContext(t.Context(), `DROP TABLE cache_stats`)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
cache.IncrementStats(t.Context(), false, 1024)
|
||||
|
||||
for _, msg := range []string{
|
||||
"failed to count cache hit or miss",
|
||||
"failed to count upstream fetch",
|
||||
} {
|
||||
want := `"level":"WARN","msg":"` + msg + `"`
|
||||
if !strings.Contains(logBuf.String(), want) {
|
||||
t.Errorf("log missing %s; got %q", want, logBuf.String())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -7,7 +7,6 @@ import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"log/slog"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"time"
|
||||
@@ -393,7 +392,6 @@ func CacheKey(req *ImageRequest) VariantKey {
|
||||
// Unlike ContentStorage, the key is provided by the caller (not computed from content).
|
||||
type VariantStorage struct {
|
||||
baseDir string
|
||||
log *slog.Logger
|
||||
}
|
||||
|
||||
// VariantMeta contains metadata about a cached variant.
|
||||
@@ -406,14 +404,13 @@ type VariantMeta struct {
|
||||
}
|
||||
|
||||
// NewVariantStorage creates a new variant storage at the given base directory.
|
||||
// A failed .meta write is logged to log.
|
||||
func NewVariantStorage(baseDir string, log *slog.Logger) (*VariantStorage, error) {
|
||||
func NewVariantStorage(baseDir string) (*VariantStorage, error) {
|
||||
err := os.MkdirAll(baseDir, StorageDirPerm)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to create variant storage directory: %w", err)
|
||||
}
|
||||
|
||||
return &VariantStorage{baseDir: baseDir, log: log}, nil
|
||||
return &VariantStorage{baseDir: baseDir}, nil
|
||||
}
|
||||
|
||||
// Store writes content and metadata to storage at the given key.
|
||||
@@ -481,11 +478,7 @@ func (s *VariantStorage) Store(
|
||||
}
|
||||
|
||||
// Metadata write failure is non-fatal; content is already stored.
|
||||
err = os.WriteFile(metaPath, metaData, StorageFilePerm)
|
||||
if err != nil {
|
||||
s.log.Warn("failed to write variant metadata sidecar",
|
||||
"path", metaPath, "error", err)
|
||||
}
|
||||
_ = os.WriteFile(metaPath, metaData, StorageFilePerm)
|
||||
|
||||
return size, nil
|
||||
}
|
||||
|
||||
@@ -4,10 +4,8 @@ import (
|
||||
"bytes"
|
||||
"errors"
|
||||
"io"
|
||||
"log/slog"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
@@ -406,35 +404,3 @@ func TestCacheKey(t *testing.T) {
|
||||
t.Error("CacheKey() produced same key for different quality")
|
||||
}
|
||||
}
|
||||
|
||||
// TestVariantStorage_StoreLogsFailedMetaWrite verifies that a .meta write
|
||||
// that fails is logged at warn and the store still succeeds.
|
||||
func TestVariantStorage_StoreLogsFailedMetaWrite(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var logBuf bytes.Buffer
|
||||
|
||||
storage, err := NewVariantStorage(
|
||||
t.TempDir(), slog.New(slog.NewJSONHandler(&logBuf, nil)))
|
||||
if err != nil {
|
||||
t.Fatalf("NewVariantStorage() error = %v", err)
|
||||
}
|
||||
|
||||
key := CacheKey(&ImageRequest{SourceHost: testHostCDN, SourcePath: testPathCat})
|
||||
|
||||
// A directory where the .meta file goes makes the .meta write fail.
|
||||
err = os.MkdirAll(storage.keyToPath(key)+".meta", StorageDirPerm)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to create directory: %v", err)
|
||||
}
|
||||
|
||||
_, err = storage.Store(key, bytes.NewReader([]byte("variant data")), "image/webp")
|
||||
if err != nil {
|
||||
t.Fatalf("Store() error = %v, want nil", err)
|
||||
}
|
||||
|
||||
want := `"level":"WARN","msg":"failed to write variant metadata sidecar"`
|
||||
if !strings.Contains(logBuf.String(), want) {
|
||||
t.Errorf("log missing %s; got %q", want, logBuf.String())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -4,13 +4,11 @@ package middleware
|
||||
import (
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"net/netip"
|
||||
"time"
|
||||
|
||||
basicauth "github.com/99designs/basicauth-go"
|
||||
"github.com/go-chi/chi/v5/middleware"
|
||||
"github.com/go-chi/cors"
|
||||
"github.com/go-chi/httprate"
|
||||
metrics "github.com/slok/go-http-metrics/metrics/prometheus"
|
||||
ghmm "github.com/slok/go-http-metrics/middleware"
|
||||
"github.com/slok/go-http-metrics/middleware/std"
|
||||
@@ -90,31 +88,6 @@ func (s *Middleware) ClientIP() func(http.Handler) http.Handler {
|
||||
}
|
||||
}
|
||||
|
||||
// RateLimit returns a middleware that limits each client to requestLimit
|
||||
// requests per window and refuses a request over the limit with 429 Too Many
|
||||
// Requests and a Retry-After header. Clients are told apart by the address
|
||||
// the ClientIP middleware stored in the request context, so ClientIP must
|
||||
// run first. An IPv6 client is counted by its /64, which one client usually
|
||||
// holds whole; an IPv4-mapped address (::ffff:a.b.c.d) is counted as the
|
||||
// IPv4 address it carries, since every such address falls in the same /64.
|
||||
// Counts are kept only for the current and the previous window, so memory
|
||||
// stays bounded.
|
||||
func (s *Middleware) RateLimit(
|
||||
requestLimit int, window time.Duration,
|
||||
) func(http.Handler) http.Handler {
|
||||
return httprate.LimitBy(requestLimit, window,
|
||||
func(r *http.Request) (string, error) {
|
||||
ip := clientip.FromContext(r.Context())
|
||||
|
||||
addr, err := netip.ParseAddr(ip)
|
||||
if err == nil {
|
||||
ip = addr.Unmap().String()
|
||||
}
|
||||
|
||||
return httprate.CanonicalizeIP(ip), nil
|
||||
})
|
||||
}
|
||||
|
||||
type loggingResponseWriter struct {
|
||||
http.ResponseWriter
|
||||
|
||||
|
||||
@@ -1,280 +0,0 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/netip"
|
||||
"net/url"
|
||||
"path/filepath"
|
||||
"regexp"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"go.uber.org/fx/fxtest"
|
||||
|
||||
"sneak.berlin/go/pixa/internal/config"
|
||||
"sneak.berlin/go/pixa/internal/database"
|
||||
"sneak.berlin/go/pixa/internal/globals"
|
||||
"sneak.berlin/go/pixa/internal/handlers"
|
||||
"sneak.berlin/go/pixa/internal/logger"
|
||||
"sneak.berlin/go/pixa/internal/middleware"
|
||||
)
|
||||
|
||||
// testSigningKey is a throwaway signing key; submitting it logs in.
|
||||
const testSigningKey = "test-signing-key-0123456789abcdef"
|
||||
|
||||
// wrongKey is submitted for a failed login.
|
||||
const wrongKey = "not-the-signing-key"
|
||||
|
||||
// Addresses for the login rate limit tests. The test server trusts
|
||||
// 10.0.0.0/8 as its proxies, so the X-Forwarded-For sent by proxyPeer is
|
||||
// believed and the one sent by firstClient or secondClient is ignored.
|
||||
const (
|
||||
firstClient = "198.51.100.1:40000"
|
||||
secondClient = "198.51.100.2:40000"
|
||||
proxyPeer = "10.0.0.1:40000"
|
||||
firstForwarded = "203.0.113.1"
|
||||
secondForwarded = "203.0.113.2"
|
||||
)
|
||||
|
||||
// csrfFieldPattern extracts the CSRF token rendered into the login form.
|
||||
var csrfFieldPattern = regexp.MustCompile(
|
||||
`name="gorilla\.csrf\.Token" value="([^"]+)"`)
|
||||
|
||||
// newTestServer builds the server's real routes from the constructors
|
||||
// cmd/pixad uses, with a throwaway state directory. Debug marks requests
|
||||
// as plain HTTP, so the CSRF check runs without an https Referer.
|
||||
func newTestServer(t *testing.T) *Server {
|
||||
t.Helper()
|
||||
|
||||
stateDir := t.TempDir()
|
||||
cfg := &config.Config{
|
||||
Debug: true,
|
||||
SigningKey: testSigningKey,
|
||||
StateDir: stateDir,
|
||||
DBURL: "file:" + filepath.Join(stateDir, "state.sqlite3"),
|
||||
TrustedProxies: []netip.Prefix{netip.MustParsePrefix("10.0.0.0/8")},
|
||||
}
|
||||
|
||||
lc := fxtest.NewLifecycle(t)
|
||||
|
||||
log, err := logger.New(lc, logger.Params{Globals: &globals.Globals{}})
|
||||
if err != nil {
|
||||
t.Fatalf("logger.New() error = %v", err)
|
||||
}
|
||||
|
||||
db, err := database.New(lc, database.Params{Logger: log, Config: cfg})
|
||||
if err != nil {
|
||||
t.Fatalf("database.New() error = %v", err)
|
||||
}
|
||||
|
||||
h, err := handlers.New(lc, handlers.Params{
|
||||
Logger: log, Database: db, Config: cfg,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("handlers.New() error = %v", err)
|
||||
}
|
||||
|
||||
mw, err := middleware.New(lc, middleware.Params{Logger: log, Config: cfg})
|
||||
if err != nil {
|
||||
t.Fatalf("middleware.New() error = %v", err)
|
||||
}
|
||||
|
||||
lc.RequireStart()
|
||||
t.Cleanup(lc.RequireStop)
|
||||
|
||||
s := &Server{config: cfg, mw: mw, h: h}
|
||||
s.SetupRoutes()
|
||||
|
||||
return s
|
||||
}
|
||||
|
||||
// clientRequest builds a request for / arriving from remoteAddr, carrying
|
||||
// forwardedFor as its X-Forwarded-For header when that is not empty.
|
||||
func clientRequest(
|
||||
t *testing.T, method string, body io.Reader, remoteAddr, forwardedFor string,
|
||||
) *http.Request {
|
||||
t.Helper()
|
||||
|
||||
req := httptest.NewRequestWithContext(t.Context(), method, "/", body)
|
||||
req.RemoteAddr = remoteAddr
|
||||
|
||||
if forwardedFor != "" {
|
||||
req.Header.Set("X-Forwarded-For", forwardedFor)
|
||||
}
|
||||
|
||||
return req
|
||||
}
|
||||
|
||||
// postLogin loads the login form with GET / and submits key in it with
|
||||
// POST /, as a browser does, both from the same client. GET / is not rate
|
||||
// limited, so the form must load even for a client over the limit.
|
||||
func postLogin(
|
||||
t *testing.T, s *Server, remoteAddr, forwardedFor, key string,
|
||||
) *httptest.ResponseRecorder {
|
||||
t.Helper()
|
||||
|
||||
page := httptest.NewRecorder()
|
||||
s.ServeHTTP(page,
|
||||
clientRequest(t, http.MethodGet, nil, remoteAddr, forwardedFor))
|
||||
|
||||
if page.Code != http.StatusOK {
|
||||
t.Fatalf("GET / status = %d, want %d", page.Code, http.StatusOK)
|
||||
}
|
||||
|
||||
match := csrfFieldPattern.FindStringSubmatch(page.Body.String())
|
||||
if match == nil {
|
||||
t.Fatalf("no CSRF token field found in the login form")
|
||||
}
|
||||
|
||||
form := url.Values{"key": {key}, "gorilla.csrf.Token": {match[1]}}
|
||||
req := clientRequest(t, http.MethodPost,
|
||||
strings.NewReader(form.Encode()), remoteAddr, forwardedFor)
|
||||
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
|
||||
|
||||
for _, c := range page.Result().Cookies() {
|
||||
req.AddCookie(c)
|
||||
}
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
s.ServeHTTP(rec, req)
|
||||
|
||||
return rec
|
||||
}
|
||||
|
||||
// tripLoginRateLimit makes LoginAttemptsPerMinute failed logins from one
|
||||
// client, each answered with the login form again, then one more, which
|
||||
// must be refused with 429. It returns the response to that last attempt.
|
||||
func tripLoginRateLimit(
|
||||
t *testing.T, s *Server, remoteAddr, forwardedFor string,
|
||||
) *httptest.ResponseRecorder {
|
||||
t.Helper()
|
||||
|
||||
for attempt := range LoginAttemptsPerMinute {
|
||||
rec := postLogin(t, s, remoteAddr, forwardedFor, wrongKey)
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("failed login %d status = %d, want %d",
|
||||
attempt+1, rec.Code, http.StatusOK)
|
||||
}
|
||||
}
|
||||
|
||||
rec := postLogin(t, s, remoteAddr, forwardedFor, wrongKey)
|
||||
if rec.Code != http.StatusTooManyRequests {
|
||||
t.Fatalf("login over the limit status = %d, want %d",
|
||||
rec.Code, http.StatusTooManyRequests)
|
||||
}
|
||||
|
||||
return rec
|
||||
}
|
||||
|
||||
// TestLoginRateLimitRefusesAttemptOverLimit verifies the login attempt
|
||||
// after LoginAttemptsPerMinute failed ones from one client is refused with
|
||||
// 429 and a Retry-After header, and that the client cannot get around the
|
||||
// limit by sending X-Forwarded-For: from a peer that is not a trusted
|
||||
// proxy, the header is ignored.
|
||||
func TestLoginRateLimitRefusesAttemptOverLimit(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
s := newTestServer(t)
|
||||
|
||||
rec := tripLoginRateLimit(t, s, firstClient, "")
|
||||
|
||||
retryAfter := rec.Header().Get("Retry-After")
|
||||
|
||||
seconds, err := strconv.Atoi(retryAfter)
|
||||
if err != nil || seconds <= 0 {
|
||||
t.Errorf("Retry-After = %q, want a positive number of seconds",
|
||||
retryAfter)
|
||||
}
|
||||
|
||||
rec = postLogin(t, s, firstClient, secondForwarded, wrongKey)
|
||||
if rec.Code != http.StatusTooManyRequests {
|
||||
t.Errorf("login with X-Forwarded-For from an untrusted peer "+
|
||||
"status = %d, want %d", rec.Code, http.StatusTooManyRequests)
|
||||
}
|
||||
}
|
||||
|
||||
// TestLoginRateLimitLeavesOtherClientsAlone verifies one client going over
|
||||
// the limit does not limit another: a failed login from a different
|
||||
// address is answered with the login form, and the signing key still logs
|
||||
// it in.
|
||||
func TestLoginRateLimitLeavesOtherClientsAlone(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
s := newTestServer(t)
|
||||
|
||||
tripLoginRateLimit(t, s, firstClient, "")
|
||||
|
||||
rec := postLogin(t, s, secondClient, "", wrongKey)
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Errorf("failed login from another client status = %d, want %d",
|
||||
rec.Code, http.StatusOK)
|
||||
}
|
||||
|
||||
rec = postLogin(t, s, secondClient, "", testSigningKey)
|
||||
if rec.Code != http.StatusSeeOther {
|
||||
t.Errorf("login with the signing key from another client "+
|
||||
"status = %d, want %d", rec.Code, http.StatusSeeOther)
|
||||
}
|
||||
}
|
||||
|
||||
// TestLoginRateLimitCountsClientsBehindProxySeparately verifies the limit
|
||||
// counts the client address resolved from X-Forwarded-For, not the address
|
||||
// of the trusted proxy the requests arrive from, so two clients behind the
|
||||
// same proxy are counted separately.
|
||||
func TestLoginRateLimitCountsClientsBehindProxySeparately(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
s := newTestServer(t)
|
||||
|
||||
tripLoginRateLimit(t, s, proxyPeer, firstForwarded)
|
||||
|
||||
rec := postLogin(t, s, proxyPeer, secondForwarded, wrongKey)
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Errorf("failed login from a second client behind the proxy "+
|
||||
"status = %d, want %d", rec.Code, http.StatusOK)
|
||||
}
|
||||
}
|
||||
|
||||
// TestLoginRateLimitCountsIPv6ClientsByPrefix verifies an IPv6 client is
|
||||
// counted by its /64: another address in the same /64 is refused too,
|
||||
// while an address in a different /64 is not.
|
||||
func TestLoginRateLimitCountsIPv6ClientsByPrefix(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
s := newTestServer(t)
|
||||
|
||||
tripLoginRateLimit(t, s, proxyPeer, "2001:db8::1")
|
||||
|
||||
rec := postLogin(t, s, proxyPeer, "2001:db8::2", wrongKey)
|
||||
if rec.Code != http.StatusTooManyRequests {
|
||||
t.Errorf("login from the same /64 status = %d, want %d",
|
||||
rec.Code, http.StatusTooManyRequests)
|
||||
}
|
||||
|
||||
rec = postLogin(t, s, proxyPeer, "2001:db8:0:1::1", wrongKey)
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Errorf("login from another /64 status = %d, want %d",
|
||||
rec.Code, http.StatusOK)
|
||||
}
|
||||
}
|
||||
|
||||
// TestLoginRateLimitCountsIPv4MappedClientsSeparately verifies an IPv4
|
||||
// client that the proxy forwards in IPv4-mapped IPv6 form (::ffff:a.b.c.d)
|
||||
// is counted by its IPv4 address, not by the /64 that every such address
|
||||
// shares, so two of them behind the proxy are counted separately.
|
||||
func TestLoginRateLimitCountsIPv4MappedClientsSeparately(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
s := newTestServer(t)
|
||||
|
||||
tripLoginRateLimit(t, s, proxyPeer, "::ffff:"+firstForwarded)
|
||||
|
||||
rec := postLogin(t, s, proxyPeer, "::ffff:"+secondForwarded, wrongKey)
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Errorf("failed login from a second IPv4-mapped client "+
|
||||
"status = %d, want %d", rec.Code, http.StatusOK)
|
||||
}
|
||||
}
|
||||
@@ -2,7 +2,6 @@ package server
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"time"
|
||||
|
||||
sentryhttp "github.com/getsentry/sentry-go/http"
|
||||
"github.com/go-chi/chi/v5"
|
||||
@@ -13,10 +12,6 @@ import (
|
||||
"sneak.berlin/go/pixa/internal/static"
|
||||
)
|
||||
|
||||
// LoginAttemptsPerMinute is how many login attempts (POST /) one client may
|
||||
// make per minute; the next is refused with 429 Too Many Requests.
|
||||
const LoginAttemptsPerMinute = 5
|
||||
|
||||
// SetupRoutes configures all HTTP routes.
|
||||
func (s *Server) SetupRoutes() {
|
||||
s.router = chi.NewRouter()
|
||||
@@ -55,14 +50,11 @@ func (s *Server) SetupRoutes() {
|
||||
// token cookie is independent of the session cookie, so it also
|
||||
// covers the login POST, where no session exists yet. LimitBody caps
|
||||
// the POST body ahead of CSRF, which reads its token from that body.
|
||||
// The login POST is rate limited per client after both, so every
|
||||
// attempt that reaches the signing key comparison is counted.
|
||||
s.router.Group(func(r chi.Router) {
|
||||
r.Use(s.h.LimitBody(handlers.MaxFormBytes))
|
||||
r.Use(s.h.CSRF())
|
||||
r.Get("/", s.h.HandleRoot())
|
||||
r.With(s.mw.RateLimit(LoginAttemptsPerMinute, time.Minute)).
|
||||
Post("/", s.h.HandleRoot())
|
||||
r.Post("/", s.h.HandleRoot())
|
||||
r.Post("/generate", s.h.HandleGenerateURL())
|
||||
})
|
||||
|
||||
|
||||
Reference in New Issue
Block a user