Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
a621c089e5 | ||
|
|
e320d46a2a | ||
|
|
5650a0067b | ||
|
|
a8f18a4d98 | ||
|
|
1cf47162b2 | ||
|
|
292f41f439 | ||
|
|
69f65d4302 | ||
|
|
2790125cb2 | ||
|
|
c23585dd6d |
@@ -198,13 +198,12 @@ path under `/v1/` answers 200, in maintenance mode too.
|
|||||||
redirects to, is `localhost`, ends in `.localhost` or `.local`, or has an
|
redirects to, is `localhost`, ends in `.localhost` or `.local`, or has an
|
||||||
address in a blocked network (see `blocked_networks`); 429 over
|
address in a blocked network (see `blocked_networks`); 429 over
|
||||||
`image_requests_per_minute` (below); 415 when the upstream answers with a
|
`image_requests_per_minute` (below); 415 when the upstream answers with a
|
||||||
`Content-Type` that is not one of the source formats (below), with a body
|
`Content-Type` that is not one of the source formats (below), or with an image
|
||||||
whose first bytes are not those of the format its `Content-Type` names, or
|
whose first bytes libvips reads as another format; 502 when the upstream
|
||||||
with an image whose first bytes libvips reads as another format; 502 when the
|
answered with an error status, and for 5 minutes after that for an image of
|
||||||
upstream answered with an error status, and for 5 minutes after that, or after
|
the same source URL that pixa can neither serve from its cache nor make from
|
||||||
a body whose first bytes are not those of its `Content-Type`, for the same
|
the source image still in its cache; 503 when pixa is busy or in maintenance
|
||||||
source URL; 503 when pixa is busy or in maintenance mode; 500 for any other
|
mode; 500 for any other failure.
|
||||||
failure.
|
|
||||||
- `GET` or `HEAD` `/v1/e/<token>/<name>` — an image through an encrypted URL
|
- `GET` or `HEAD` `/v1/e/<token>/<name>` — an image through an encrypted URL
|
||||||
(see Encrypted URLs). Needs: nothing but the URL. Answers: 200; 304 when
|
(see Encrypted URLs). Needs: nothing but the URL. Answers: 200; 304 when
|
||||||
`If-None-Match` matches the image's `ETag`; 400 for a token that does not
|
`If-None-Match` matches the image's `ETag`; 400 for a token that does not
|
||||||
@@ -281,12 +280,8 @@ above 8192.
|
|||||||
The source image may be JPEG, PNG, GIF, WebP, AVIF or JPEG XL. The upstream's
|
The source image may be JPEG, PNG, GIF, WebP, AVIF or JPEG XL. The upstream's
|
||||||
`Content-Type` must name its format, and the image's first bytes must match it.
|
`Content-Type` must name its format, and the image's first bytes must match it.
|
||||||
An upstream answering with any other `Content-Type`, SVG included, is refused
|
An upstream answering with any other `Content-Type`, SVG included, is refused
|
||||||
with 415. So is a body whose first bytes are not those of the format its
|
with 415, and so is an image whose first bytes libvips reads as another format,
|
||||||
`Content-Type` names, such as a JPEG sent as `image/png`, text sent as an image,
|
such as an SVG whose first bytes pass the check for AVIF.
|
||||||
or an empty body. pixa then does not fetch that source URL again for 5 minutes,
|
|
||||||
and answers 502 for it meanwhile, as after an upstream error status. An image
|
|
||||||
whose first bytes libvips reads as another format, such as an SVG whose first
|
|
||||||
bytes pass the check for AVIF, is refused with 415 too.
|
|
||||||
|
|
||||||
With the format `auto`, pixa chooses the format for each request from its
|
With the format `auto`, pixa chooses the format for each request from its
|
||||||
`Accept` header, in this order:
|
`Accept` header, in this order:
|
||||||
@@ -319,9 +314,26 @@ cache at once, they share one upstream fetch (or one read of the cached source)
|
|||||||
and one transcode: the first request does the work, and the others wait for its
|
and one transcode: the first request does the work, and the others wait for its
|
||||||
image or its error, holding no upstream connection or processing slot of their
|
image or its error, holding no upstream connection or processing slot of their
|
||||||
own. A waiting request stops waiting when its own client goes away. The work
|
own. A waiting request stops waiting when its own client goes away. The work
|
||||||
goes on for the others even if the first request's client goes away, until that
|
goes on for the others even if the first request's client goes away. The shared
|
||||||
request's `downstream_timeout` ends. The shared fetch sends the first request's
|
fetch sends the first request's ID upstream, and the lines logged for the fetch
|
||||||
ID upstream, and the lines logged for the fetch and the transcode carry that ID.
|
and the transcode carry that ID.
|
||||||
|
|
||||||
|
A transcode whose request's `downstream_timeout` has ended (for a shared one,
|
||||||
|
the first request's) stops waiting for a processing slot, or, once it has one,
|
||||||
|
gives it back without beginning to encode. A request waiting for a shared
|
||||||
|
transcode that stops this way is answered 503 with the error
|
||||||
|
`server busy, try again later`, though its own `downstream_timeout` may not have
|
||||||
|
ended. govips gives pixa no way to stop libvips once an encode has begun, so an
|
||||||
|
encode keeps its slot until it finishes, which can be after
|
||||||
|
`downstream_timeout`. The limit of 8192 on each side of the source and of the
|
||||||
|
output bounds how long that takes. With one libvips thread, as pixad runs it,
|
||||||
|
encoding 8192x8192 random pixels takes up to about 1 second as JPEG, 15 as PNG,
|
||||||
|
16 as GIF, 63 as WebP, 110 as AVIF and 96 as JPEG XL, the slowest of 8 or 16
|
||||||
|
bits per sample, with or without an alpha channel. WebP, AVIF and JPEG XL are
|
||||||
|
that slow only with an alpha channel: 8-bit RGB takes about 44, 53 and 28
|
||||||
|
seconds. Decoding such a source adds up to about 11 seconds. A WebP, AVIF or
|
||||||
|
JPEG XL encode can therefore hold its slot past the default `downstream_timeout`
|
||||||
|
of 60 seconds.
|
||||||
|
|
||||||
The login form (`POST /`) is limited to 5 attempts per minute per client
|
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
|
address, counting an IPv6 client by its /64; an attempt over the limit is
|
||||||
@@ -424,7 +436,9 @@ HMAC-SHA256(secret, "host:path:query:width:height:format:expiration:quality:fit"
|
|||||||
|
|
||||||
Where:
|
Where:
|
||||||
|
|
||||||
- `host` — source origin hostname (e.g. `cdn.example.com`)
|
- `host` — source host as the URL writes it up to the first `/`, in lower case,
|
||||||
|
keeping an IPv6 address's brackets and any `:` and port after it (e.g.
|
||||||
|
`cdn.example.com`, `cdn.example.com:8443`, `[2001:db8::1]:8443`)
|
||||||
- `path` — source path (e.g. `/photos/cat.jpg`), with each `%` written as `%25`
|
- `path` — source path (e.g. `/photos/cat.jpg`), with each `%` written as `%25`
|
||||||
and each `:` as `%3A`, so `/photos/a:b.jpg` is signed as `/photos/a%3Ab.jpg`
|
and each `:` as `%3A`, so `/photos/a:b.jpg` is signed as `/photos/a%3Ab.jpg`
|
||||||
- `query` — source query string, empty string if none, with `%` and `:` written
|
- `query` — source query string, empty string if none, with `%` and `:` written
|
||||||
@@ -479,6 +493,12 @@ entry that is neither a host name (letters, digits, hyphens, underscores and
|
|||||||
dots, with at most one leading dot) nor an IP address, such as one with a port
|
dots, with at most one leading dot) nor an IP address, such as one with a port
|
||||||
or a `*.` wildcard, aborts startup.
|
or a `*.` wildcard, aborts startup.
|
||||||
|
|
||||||
|
pixa lower-cases the host of a `/v1/image/` URL before anything else uses it, so
|
||||||
|
`CDN.Example.com` is matched, fetched, cached and counted against
|
||||||
|
`upstream_connections_per_host` as `cdn.example.com`. Such a URL whose host has
|
||||||
|
a port, as in `cdn.example.com:8443`, or a `:` with no port, needs a signature
|
||||||
|
even when the host is allowlisted. A host holding `@` is refused with 400.
|
||||||
|
|
||||||
### Configuration
|
### Configuration
|
||||||
|
|
||||||
Every setting can be given as an environment variable, in a YAML config file
|
Every setting can be given as an environment variable, in a YAML config file
|
||||||
|
|||||||
@@ -30,15 +30,56 @@ P2: security: per-origin rate limiting
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
- 2026-10-15 a source whose first bytes do not match its `Content-Type` answers
|
- 2026-10-15 a transcode stops once its request's `downstream_timeout` has
|
||||||
415 (closes #253): a body whose first bytes are not those of the format its
|
ended, unless it has begun encoding (closes #243): the image processor looked
|
||||||
`Content-Type` names, such as a JPEG sent as `image/png`, text, or a body
|
at its context only while it waited for a processing slot, so an image was
|
||||||
under 12 bytes, answered 500 on both image routes and was logged as an error
|
decoded and encoded, holding the slot, however long after its request's
|
||||||
of pixa's, and each request for it fetched the whole body again. Both routes
|
deadline that took. `Process` now returns the context's error, and gives the
|
||||||
now answer 415, and the image service logs it at warning level as a refused
|
slot back, when the context has ended by the time it has the slot or before it
|
||||||
source. It also gets a negative cache entry, as an upstream error status does,
|
encodes. A request that waited for a shared transcode stopped at the first
|
||||||
so the source URL is not fetched again for 5 minutes and answers 502
|
request's deadline is answered 503 `server busy, try again later`, not 500,
|
||||||
meanwhile.
|
though its own `downstream_timeout` may not have ended. govips gives pixa no
|
||||||
|
way to stop libvips once an encode has begun, so that encode keeps its slot
|
||||||
|
until it finishes; the limit of 8192 on each side of the source and of the
|
||||||
|
output bounds how long that takes. With one libvips thread, 8192x8192 random
|
||||||
|
pixels take up to about 1 second to encode as JPEG, 15 as PNG, 16 as GIF, 63
|
||||||
|
as WebP, 110 as AVIF and 96 as JPEG XL, the last three with an alpha channel,
|
||||||
|
plus up to about 11 seconds to decode the source; `README.md` gives these
|
||||||
|
times and no longer says the work stops when `downstream_timeout` ends. The
|
||||||
|
settings of the WebP, AVIF and JPEG XL encoders, which can hold a slot past
|
||||||
|
the default `downstream_timeout` of 60 seconds, are unchanged.
|
||||||
|
- 2026-10-15 expired negative cache rows are removed on a schedule (closes
|
||||||
|
#244): the row of a failed fetch, which makes pixa answer 502 for that source
|
||||||
|
URL for 5 minutes, was deleted only when the same URL was requested again
|
||||||
|
after that, so a client asking for a new missing path each time added rows
|
||||||
|
that stayed for good. The eviction goroutine now calls `CleanExpired` at
|
||||||
|
startup and on each periodic tick, every 5 minutes in pixad, so a row stays at
|
||||||
|
most about 10 minutes. `CleanExpired` deletes at most 1000 rows per statement,
|
||||||
|
so a request waits behind 1000 deletions at most, however many rows have
|
||||||
|
expired. The goroutine runs with the disk cache disabled
|
||||||
|
(`cache_max_bytes: 0`) too, where it does nothing else, as the negative cache
|
||||||
|
is used either way.
|
||||||
|
- 2026-10-15 a cached variant is served while its source URL is in the negative
|
||||||
|
cache (closes #245): `Get` checked the negative cache before the variant
|
||||||
|
cache, so once a fetch of the source failed, as one does when another variant
|
||||||
|
misses after the source was evicted and the upstream is down, every variant of
|
||||||
|
that source answered 502 for 5 minutes, also one on disk or one that could be
|
||||||
|
made from the cached source. The negative cache is now checked only just
|
||||||
|
before a fetch from the upstream, so a cached variant is served and a variant
|
||||||
|
not in the cache is made from the cached source when there is one. `README.md`
|
||||||
|
says so under `/v1/image/`. A refusal from the negative cache now counts as a
|
||||||
|
miss, as any other request that misses the variant cache does.
|
||||||
|
- 2026-10-15 the host of a `/v1/image/` URL is lower-cased, and one with a port
|
||||||
|
needs a signature (closes #246): with `cdn.example.com` allowlisted,
|
||||||
|
`CDN.Example.com` and `cdn.example.com:8443` passed the allowlist too, and
|
||||||
|
each spelling was fetched and cached on its own and had connections of its own
|
||||||
|
under `upstream_connections_per_host`. The image URL parser now lower-cases
|
||||||
|
the host and refuses one holding `@`, which a URL reads as a user name and
|
||||||
|
password, with 400. A host with a port, or with a `:` and no port, needs a
|
||||||
|
signature even when it is allowlisted. That is checked in the image service,
|
||||||
|
not in `allowlist.IsAllowed`, which also matches `referer_blocklist`, where a
|
||||||
|
`Referer` is blocked whatever its port. A URL is signed over its host as it
|
||||||
|
writes it, in lower case, with any IPv6 brackets, `:` and port kept.
|
||||||
- 2026-10-15 a `:` in the source path or query no longer moves where the path
|
- 2026-10-15 a `:` in the source path or query no longer moves where the path
|
||||||
ends and the query begins (closes #247): the signed data and the cache key
|
ends and the query begins (closes #247): the signed data and the cache key
|
||||||
joined host, path and query with `:` and no escaping, so a URL signed for
|
joined host, path and query with `:` and no escaping, so a URL signed for
|
||||||
@@ -510,11 +551,11 @@ P2: security: per-origin rate limiting
|
|||||||
cancellation but keeps its deadline, and the others wait for its image or
|
cancellation but keeps its deadline, and the others wait for its image or
|
||||||
error holding no upstream connection or processing slot, and stop waiting when
|
error holding no upstream connection or processing slot, and stop waiting when
|
||||||
their own context ends; the request doing the processing waits for it even
|
their own context ends; the request doing the processing waits for it even
|
||||||
then, up to its deadline; a request whose context has already ended starts
|
then, until it ends, which can be after its deadline; a request whose context
|
||||||
nothing; each request counts one miss, and the processing counts its fetch and
|
has already ended starts nothing; each request counts one miss, and the
|
||||||
transcode once; a panic while processing is reported to Sentry when
|
processing counts its fetch and transcode once; a panic while processing is
|
||||||
`sentry_dsn` is set and becomes an error for every waiting request instead of
|
reported to Sentry when `sentry_dsn` is set and becomes an error for every
|
||||||
stopping pixad; documented in `README.md`.
|
waiting request instead of stopping pixad; documented in `README.md`.
|
||||||
- 2026-09-29 only the image routes send CORS headers (closes #98): the CORS
|
- 2026-09-29 only the image routes send CORS headers (closes #98): the CORS
|
||||||
middleware, with the `access_control_allow_origin` origin, moved from the
|
middleware, with the `access_control_allow_origin` origin, moved from the
|
||||||
router root onto a `/v1` subrouter holding `/v1/image/` and `/v1/e/`, where it
|
router root onto a `/v1` subrouter holding `/v1/image/` and `/v1/e/`, where it
|
||||||
|
|||||||
@@ -1,180 +0,0 @@
|
|||||||
package handlers
|
|
||||||
|
|
||||||
import (
|
|
||||||
"bytes"
|
|
||||||
"encoding/json"
|
|
||||||
"image/color"
|
|
||||||
"log/slog"
|
|
||||||
"net/http"
|
|
||||||
"slices"
|
|
||||||
"testing"
|
|
||||||
"testing/fstest"
|
|
||||||
"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"
|
|
||||||
)
|
|
||||||
|
|
||||||
// mismatchedSourcePath is the path of the source that
|
|
||||||
// TestFirstBytesNotMatchingContentType serves on allowlistedHost. The mock
|
|
||||||
// fetcher names a file's Content-Type from its extension, so the source is
|
|
||||||
// sent as image/png.
|
|
||||||
const mismatchedSourcePath = "/images/source.png"
|
|
||||||
|
|
||||||
// newRoutesFetchingThrough returns both image routes, and the Handlers behind
|
|
||||||
// them, whose image service fetches through fetcher and has allowlistedHost on
|
|
||||||
// its allowlist. The handlers and the image service log to log.
|
|
||||||
func newRoutesFetchingThrough(
|
|
||||||
t *testing.T, fetcher httpfetcher.Fetcher, log *slog.Logger,
|
|
||||||
) (*Handlers, http.Handler) {
|
|
||||||
t.Helper()
|
|
||||||
|
|
||||||
cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{
|
|
||||||
StateDir: t.TempDir(),
|
|
||||||
NegativeTTL: 5 * time.Minute,
|
|
||||||
})
|
|
||||||
if err != nil {
|
|
||||||
t.Fatalf("imgcache.NewCache() error = %v", err)
|
|
||||||
}
|
|
||||||
|
|
||||||
svc, err := imgcache.NewService(&imgcache.ServiceConfig{
|
|
||||||
Cache: cache,
|
|
||||||
Fetcher: fetcher,
|
|
||||||
SigningKey: testSigningKey,
|
|
||||||
Allowlist: []string{allowlistedHost},
|
|
||||||
Logger: log,
|
|
||||||
})
|
|
||||||
if err != nil {
|
|
||||||
t.Fatalf("imgcache.NewService() error = %v", err)
|
|
||||||
}
|
|
||||||
|
|
||||||
encGen, err := encurl.NewGenerator(testSigningKey)
|
|
||||||
if err != nil {
|
|
||||||
t.Fatalf("encurl.NewGenerator() error = %v", err)
|
|
||||||
}
|
|
||||||
|
|
||||||
h := &Handlers{log: log, imgSvc: svc, encGen: encGen}
|
|
||||||
|
|
||||||
r := chi.NewRouter()
|
|
||||||
r.Get("/v1/image/*", h.HandleImage())
|
|
||||||
r.Get("/v1/e/{token}/*", h.HandleImageEnc())
|
|
||||||
|
|
||||||
return h, r
|
|
||||||
}
|
|
||||||
|
|
||||||
// mismatchedSourcePlainURL returns the /v1/image/ URL of mismatchedSourcePath
|
|
||||||
// on allowlistedHost at 50x50 as a JPEG.
|
|
||||||
func mismatchedSourcePlainURL(*testing.T, *Handlers) string {
|
|
||||||
return "/v1/image/" + allowlistedHost + mismatchedSourcePath + "/50x50.jpeg"
|
|
||||||
}
|
|
||||||
|
|
||||||
// mismatchedSourceEncURL returns an encrypted /v1/e/ URL, made with h's
|
|
||||||
// generator, of mismatchedSourcePath on allowlistedHost at 50x50 as a JPEG.
|
|
||||||
func mismatchedSourceEncURL(t *testing.T, h *Handlers) string {
|
|
||||||
t.Helper()
|
|
||||||
|
|
||||||
token, err := h.encGen.Generate(&encurl.Payload{
|
|
||||||
SourceHost: allowlistedHost,
|
|
||||||
SourcePath: mismatchedSourcePath,
|
|
||||||
Width: 50,
|
|
||||||
Height: 50,
|
|
||||||
Format: imgcache.FormatJPEG,
|
|
||||||
})
|
|
||||||
if err != nil {
|
|
||||||
t.Fatalf("Generate() error = %v", err)
|
|
||||||
}
|
|
||||||
|
|
||||||
return "/v1/e/" + token + "/img.jpg"
|
|
||||||
}
|
|
||||||
|
|
||||||
// loggedLevels reads the JSON log lines in logs and returns the messages
|
|
||||||
// logged at each level.
|
|
||||||
func loggedLevels(t *testing.T, logs *bytes.Buffer) map[string][]string {
|
|
||||||
t.Helper()
|
|
||||||
|
|
||||||
t.Logf("logged:\n%s", logs.String())
|
|
||||||
|
|
||||||
messages := make(map[string][]string)
|
|
||||||
|
|
||||||
dec := json.NewDecoder(logs)
|
|
||||||
for dec.More() {
|
|
||||||
var line struct {
|
|
||||||
Level string `json:"level"`
|
|
||||||
Msg string `json:"msg"`
|
|
||||||
}
|
|
||||||
|
|
||||||
err := dec.Decode(&line)
|
|
||||||
if err != nil {
|
|
||||||
t.Fatalf("decoding log line: %v", err)
|
|
||||||
}
|
|
||||||
|
|
||||||
messages[line.Level] = append(messages[line.Level], line.Msg)
|
|
||||||
}
|
|
||||||
|
|
||||||
return messages
|
|
||||||
}
|
|
||||||
|
|
||||||
// TestFirstBytesNotMatchingContentType checks that both image routes answer
|
|
||||||
// 415 for an upstream body whose first bytes are not those of the format its
|
|
||||||
// Content-Type names, and log it as a refused source, not as an error of
|
|
||||||
// pixa's. The next request for that source must answer 502 without fetching
|
|
||||||
// it again, as after an upstream error status.
|
|
||||||
func TestFirstBytesNotMatchingContentType(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
bodies := map[string][]byte{
|
|
||||||
"JPEG sent as image/png": generateTestJPEG(t, 10, 10, color.Black),
|
|
||||||
"text sent as image/png": []byte("This is just text, not a PNG"),
|
|
||||||
"body too short to name a format": []byte("short"),
|
|
||||||
}
|
|
||||||
|
|
||||||
// The source's URL on each image route.
|
|
||||||
sourceURLs := map[string]func(*testing.T, *Handlers) string{
|
|
||||||
"plain URL": mismatchedSourcePlainURL,
|
|
||||||
"encrypted URL": mismatchedSourceEncURL,
|
|
||||||
}
|
|
||||||
|
|
||||||
for bodyName, body := range bodies {
|
|
||||||
for urlName, sourceURL := range sourceURLs {
|
|
||||||
t.Run(urlName+", "+bodyName, func(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
fetcher := &countingFetcher{
|
|
||||||
Fetcher: httpfetcher.NewMock(fstest.MapFS{
|
|
||||||
allowlistedHost + mismatchedSourcePath: &fstest.MapFile{
|
|
||||||
Data: body,
|
|
||||||
},
|
|
||||||
}),
|
|
||||||
}
|
|
||||||
|
|
||||||
var logs bytes.Buffer
|
|
||||||
|
|
||||||
h, routes := newRoutesFetchingThrough(t, fetcher,
|
|
||||||
slog.New(slog.NewJSONHandler(&logs, nil)))
|
|
||||||
target := sourceURL(t, h)
|
|
||||||
|
|
||||||
checkErrorBody(t, sendGet(t, routes, target),
|
|
||||||
http.StatusUnsupportedMediaType, "unsupported source format")
|
|
||||||
|
|
||||||
levels := loggedLevels(t, &logs)
|
|
||||||
if len(levels[slog.LevelError.String()]) != 0 {
|
|
||||||
t.Errorf("logged at error level: %q",
|
|
||||||
levels[slog.LevelError.String()])
|
|
||||||
}
|
|
||||||
|
|
||||||
if !slices.Contains(levels[slog.LevelWarn.String()], "source refused") {
|
|
||||||
t.Error(`no "source refused" line logged at warning level`)
|
|
||||||
}
|
|
||||||
|
|
||||||
checkErrorBody(t, sendGet(t, routes, target),
|
|
||||||
http.StatusBadGateway, "upstream error")
|
|
||||||
|
|
||||||
if n := fetcher.fetches.Load(); n != 1 {
|
|
||||||
t.Errorf("upstream fetched %d times, want 1", n)
|
|
||||||
}
|
|
||||||
})
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
@@ -124,7 +124,8 @@ func (s *Handlers) initImageService() error {
|
|||||||
s.imgCache = cache
|
s.imgCache = cache
|
||||||
|
|
||||||
// Background eviction: startup reconciliation, then periodic and
|
// Background eviction: startup reconciliation, then periodic and
|
||||||
// write-pressure passes. No-op when the disk cache is disabled.
|
// write-pressure passes, and the removal of expired negative cache
|
||||||
|
// rows, which is all it does when the disk cache is disabled.
|
||||||
cache.StartEviction(imgcache.DefaultEvictionInterval)
|
cache.StartEviction(imgcache.DefaultEvictionInterval)
|
||||||
|
|
||||||
// Writes the counts requests could not write by their deadline
|
// Writes the counts requests could not write by their deadline
|
||||||
|
|||||||
@@ -15,7 +15,6 @@ import (
|
|||||||
"sneak.berlin/go/pixa/internal/httpfetcher"
|
"sneak.berlin/go/pixa/internal/httpfetcher"
|
||||||
"sneak.berlin/go/pixa/internal/imageprocessor"
|
"sneak.berlin/go/pixa/internal/imageprocessor"
|
||||||
"sneak.berlin/go/pixa/internal/imgcache"
|
"sneak.berlin/go/pixa/internal/imgcache"
|
||||||
"sneak.berlin/go/pixa/internal/magic"
|
|
||||||
)
|
)
|
||||||
|
|
||||||
// HandleImage handles the main image proxy route:
|
// HandleImage handles the main image proxy route:
|
||||||
@@ -221,18 +220,6 @@ func (s *Handlers) HandleTooManyRequests() http.HandlerFunc {
|
|||||||
func (s *Handlers) respondImageError(
|
func (s *Handlers) respondImageError(
|
||||||
w http.ResponseWriter, req *imgcache.ImageRequest, err error,
|
w http.ResponseWriter, req *imgcache.ImageRequest, err error,
|
||||||
) {
|
) {
|
||||||
// An upstream body whose first bytes are not those of the format its
|
|
||||||
// Content-Type names is the upstream's error, not pixa's: the image
|
|
||||||
// service has logged it as a refused source.
|
|
||||||
if errors.Is(err, magic.ErrMagicByteMismatch) ||
|
|
||||||
errors.Is(err, magic.ErrUnknownFormat) ||
|
|
||||||
errors.Is(err, magic.ErrNotEnoughData) {
|
|
||||||
s.respondError(w, "unsupported source format",
|
|
||||||
http.StatusUnsupportedMediaType)
|
|
||||||
|
|
||||||
return
|
|
||||||
}
|
|
||||||
|
|
||||||
s.log.Error("failed to get image",
|
s.log.Error("failed to get image",
|
||||||
"host", req.SourceHost,
|
"host", req.SourceHost,
|
||||||
"path", req.SourcePath,
|
"path", req.SourcePath,
|
||||||
@@ -253,7 +240,8 @@ func (s *Handlers) respondImageError(
|
|||||||
}
|
}
|
||||||
|
|
||||||
if errors.Is(err, httpfetcher.ErrTooManyConnections) ||
|
if errors.Is(err, httpfetcher.ErrTooManyConnections) ||
|
||||||
errors.Is(err, imageprocessor.ErrTooManyImages) {
|
errors.Is(err, imageprocessor.ErrTooManyImages) ||
|
||||||
|
errors.Is(err, imgcache.ErrProcessingTimedOut) {
|
||||||
s.respondError(w, "server busy, try again later",
|
s.respondError(w, "server busy, try again later",
|
||||||
http.StatusServiceUnavailable)
|
http.StatusServiceUnavailable)
|
||||||
|
|
||||||
|
|||||||
@@ -15,7 +15,6 @@ import (
|
|||||||
"sneak.berlin/go/pixa/internal/httpfetcher"
|
"sneak.berlin/go/pixa/internal/httpfetcher"
|
||||||
"sneak.berlin/go/pixa/internal/imageprocessor"
|
"sneak.berlin/go/pixa/internal/imageprocessor"
|
||||||
"sneak.berlin/go/pixa/internal/imgcache"
|
"sneak.berlin/go/pixa/internal/imgcache"
|
||||||
"sneak.berlin/go/pixa/internal/magic"
|
|
||||||
)
|
)
|
||||||
|
|
||||||
// HandleImageEnc handles requests to /v1/e/{token}/* for encrypted
|
// HandleImageEnc handles requests to /v1/e/{token}/* for encrypted
|
||||||
@@ -159,14 +158,12 @@ func (s *Handlers) handleImageError(w http.ResponseWriter, err error) {
|
|||||||
case errors.Is(err, httpfetcher.ErrUpstreamTimeout):
|
case errors.Is(err, httpfetcher.ErrUpstreamTimeout):
|
||||||
s.respondError(w, "upstream timeout", http.StatusGatewayTimeout)
|
s.respondError(w, "upstream timeout", http.StatusGatewayTimeout)
|
||||||
case errors.Is(err, httpfetcher.ErrTooManyConnections),
|
case errors.Is(err, httpfetcher.ErrTooManyConnections),
|
||||||
errors.Is(err, imageprocessor.ErrTooManyImages):
|
errors.Is(err, imageprocessor.ErrTooManyImages),
|
||||||
|
errors.Is(err, imgcache.ErrProcessingTimedOut):
|
||||||
s.respondError(w, "server busy, try again later",
|
s.respondError(w, "server busy, try again later",
|
||||||
http.StatusServiceUnavailable)
|
http.StatusServiceUnavailable)
|
||||||
case errors.Is(err, httpfetcher.ErrInvalidContentType),
|
case errors.Is(err, httpfetcher.ErrInvalidContentType),
|
||||||
errors.Is(err, imageprocessor.ErrUnsupportedInputFormat),
|
errors.Is(err, imageprocessor.ErrUnsupportedInputFormat):
|
||||||
errors.Is(err, magic.ErrMagicByteMismatch),
|
|
||||||
errors.Is(err, magic.ErrUnknownFormat),
|
|
||||||
errors.Is(err, magic.ErrNotEnoughData):
|
|
||||||
s.respondError(w, "unsupported source format",
|
s.respondError(w, "unsupported source format",
|
||||||
http.StatusUnsupportedMediaType)
|
http.StatusUnsupportedMediaType)
|
||||||
case errors.Is(err, imageprocessor.ErrOutputTooLarge):
|
case errors.Is(err, imageprocessor.ErrOutputTooLarge):
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
package handlers
|
package handlers
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"context"
|
||||||
"fmt"
|
"fmt"
|
||||||
"log/slog"
|
"log/slog"
|
||||||
"net/http"
|
"net/http"
|
||||||
@@ -47,3 +48,40 @@ func TestServerBusyAnswers503(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestProcessingTimedOutAnswers503 checks that respondImageError and
|
||||||
|
// handleImageError, which answer the errors of the two image routes, answer
|
||||||
|
// 503 with the server busy error when the image service reports that
|
||||||
|
// processing stopped at the deadline of the request doing it, wrapped as the
|
||||||
|
// service wraps it. A request that waited for that processing gets this error
|
||||||
|
// while its own deadline may have time left. The request's source only goes
|
||||||
|
// into a log line, so it is left empty.
|
||||||
|
func TestProcessingTimedOutAnswers503(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
h := &Handlers{log: slog.New(slog.DiscardHandler)}
|
||||||
|
err := fmt.Errorf("%w: failed to encode: %w",
|
||||||
|
imgcache.ErrProcessingTimedOut, context.DeadlineExceeded)
|
||||||
|
|
||||||
|
plain := httptest.NewRecorder()
|
||||||
|
h.respondImageError(plain, &imgcache.ImageRequest{}, err)
|
||||||
|
|
||||||
|
encrypted := httptest.NewRecorder()
|
||||||
|
h.handleImageError(encrypted, err)
|
||||||
|
|
||||||
|
for name, rec := range map[string]*httptest.ResponseRecorder{
|
||||||
|
"respondImageError": plain, "handleImageError": encrypted,
|
||||||
|
} {
|
||||||
|
t.Logf("%s: status = %d, body = %q", name, rec.Code, rec.Body.String())
|
||||||
|
|
||||||
|
if rec.Code != http.StatusServiceUnavailable {
|
||||||
|
t.Errorf("%s: status = %d, want %d",
|
||||||
|
name, rec.Code, http.StatusServiceUnavailable)
|
||||||
|
}
|
||||||
|
|
||||||
|
if !strings.Contains(rec.Body.String(), "server busy, try again later") {
|
||||||
|
t.Errorf("%s: body = %q, want the server busy error",
|
||||||
|
name, rec.Body.String())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -0,0 +1,106 @@
|
|||||||
|
package imageprocessor
|
||||||
|
|
||||||
|
import (
|
||||||
|
"bytes"
|
||||||
|
"context"
|
||||||
|
"errors"
|
||||||
|
"io"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
)
|
||||||
|
|
||||||
|
// readerAfterDeadline is a Process input whose first Read waits until done
|
||||||
|
// is closed, so the deadline passes after Process has taken its slot and
|
||||||
|
// before it encodes.
|
||||||
|
type readerAfterDeadline struct {
|
||||||
|
data *bytes.Reader
|
||||||
|
done <-chan struct{}
|
||||||
|
waited bool
|
||||||
|
}
|
||||||
|
|
||||||
|
func (r *readerAfterDeadline) Read(p []byte) (int, error) {
|
||||||
|
if !r.waited {
|
||||||
|
r.waited = true
|
||||||
|
|
||||||
|
<-r.done
|
||||||
|
}
|
||||||
|
|
||||||
|
return r.data.Read(p)
|
||||||
|
}
|
||||||
|
|
||||||
|
// inputNotToBeRead is a Process input that fails the test when it is read.
|
||||||
|
type inputNotToBeRead struct {
|
||||||
|
t *testing.T
|
||||||
|
}
|
||||||
|
|
||||||
|
func (r inputNotToBeRead) Read([]byte) (int, error) {
|
||||||
|
r.t.Error("Process read its input although its deadline had passed")
|
||||||
|
|
||||||
|
return 0, io.EOF
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestProcessStopsWhenDeadlinePassed checks that Process returns
|
||||||
|
// context.DeadlineExceeded, and holds no slot afterwards, when its context's
|
||||||
|
// deadline has passed before it starts, then without reading its input, or
|
||||||
|
// while it reads its input. govips gives pixa no way to stop libvips once an
|
||||||
|
// encode has begun, so Process must not begin one for a request that has
|
||||||
|
// ended.
|
||||||
|
func TestProcessStopsWhenDeadlinePassed(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
input := createTestJPEG(t, 50, 50)
|
||||||
|
|
||||||
|
t.Run("before Process starts", func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
ctx, cancel := context.WithDeadline(t.Context(),
|
||||||
|
time.Now().Add(-time.Second))
|
||||||
|
defer cancel()
|
||||||
|
|
||||||
|
// With a slot free, the select in acquireSlot takes the slot or sees
|
||||||
|
// the ended context at random, so Process is called many times
|
||||||
|
for range 100 {
|
||||||
|
checkProcessStopsAtDeadline(ctx, t, inputNotToBeRead{t: t})
|
||||||
|
}
|
||||||
|
})
|
||||||
|
|
||||||
|
t.Run("while Process reads its input", func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
ctx, cancel := context.WithTimeout(t.Context(), 100*time.Millisecond)
|
||||||
|
defer cancel()
|
||||||
|
|
||||||
|
checkProcessStopsAtDeadline(ctx, t, &readerAfterDeadline{
|
||||||
|
data: bytes.NewReader(input), done: ctx.Done(),
|
||||||
|
})
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
// checkProcessStopsAtDeadline runs Process on input with ctx, whose deadline
|
||||||
|
// has passed by the time Process could encode, and fails the test unless
|
||||||
|
// Process returns context.DeadlineExceeded and gives its slot back.
|
||||||
|
func checkProcessStopsAtDeadline(
|
||||||
|
ctx context.Context, t *testing.T, input io.Reader,
|
||||||
|
) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
proc := New(Params{MaxConcurrentProcessing: 1})
|
||||||
|
|
||||||
|
result, err := proc.Process(ctx, input, smallJPEGRequest())
|
||||||
|
if result != nil {
|
||||||
|
t.Logf("Process() made a %dx%d image after its deadline passed",
|
||||||
|
result.Width, result.Height)
|
||||||
|
|
||||||
|
_ = result.Content.Close()
|
||||||
|
}
|
||||||
|
|
||||||
|
t.Logf("Process() error = %v", err)
|
||||||
|
|
||||||
|
if !errors.Is(err, context.DeadlineExceeded) {
|
||||||
|
t.Errorf("Process() error = %v, want context.DeadlineExceeded", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if held := len(proc.processingSemaphore); held != 0 {
|
||||||
|
t.Errorf("%d slots still held after Process() returned", held)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -208,7 +208,10 @@ func New(params Params) *ImageProcessor {
|
|||||||
// Process transforms an image according to the request. When
|
// Process transforms an image according to the request. When
|
||||||
// MaxConcurrentProcessing images are already being processed, it waits up
|
// MaxConcurrentProcessing images are already being processed, it waits up
|
||||||
// to ProcessingWaitTimeout for one to finish, then fails with
|
// to ProcessingWaitTimeout for one to finish, then fails with
|
||||||
// ErrTooManyImages.
|
// ErrTooManyImages. When ctx has ended once Process has a slot, or before it
|
||||||
|
// encodes, it returns ctx's error and gives the slot back. govips gives pixa
|
||||||
|
// no way to stop libvips once an encode has begun, so an encode keeps its
|
||||||
|
// slot until it finishes.
|
||||||
func (p *ImageProcessor) Process(
|
func (p *ImageProcessor) Process(
|
||||||
ctx context.Context,
|
ctx context.Context,
|
||||||
input io.Reader,
|
input io.Reader,
|
||||||
@@ -290,7 +293,7 @@ func (p *ImageProcessor) Process(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Encode to target format
|
// Encode to target format
|
||||||
output, err := p.encode(img, outputFormat, req.Quality)
|
output, err := p.encode(ctx, img, outputFormat, req.Quality)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("failed to encode: %w", err)
|
return nil, fmt.Errorf("failed to encode: %w", err)
|
||||||
}
|
}
|
||||||
@@ -417,25 +420,28 @@ func (p *ImageProcessor) WaitForProcessing(ctx context.Context) int {
|
|||||||
|
|
||||||
// acquireSlot takes a slot in processingSemaphore, waiting at most
|
// acquireSlot takes a slot in processingSemaphore, waiting at most
|
||||||
// processingWaitTimeout for one to free up, and returns the func that gives
|
// processingWaitTimeout for one to free up, and returns the func that gives
|
||||||
// it back. A free slot is taken even when ctx has ended; only the wait for
|
// it back. When ctx has ended by the time it has a slot, it gives the slot
|
||||||
// one stops when ctx ends, as the rest of Process does not check ctx.
|
// back and returns ctx's error.
|
||||||
func (p *ImageProcessor) acquireSlot(ctx context.Context) (func(), error) {
|
func (p *ImageProcessor) acquireSlot(ctx context.Context) (func(), error) {
|
||||||
release := func() { <-p.processingSemaphore }
|
|
||||||
|
|
||||||
select {
|
select {
|
||||||
case p.processingSemaphore <- struct{}{}:
|
case p.processingSemaphore <- struct{}{}:
|
||||||
return release, nil
|
|
||||||
default:
|
|
||||||
}
|
|
||||||
|
|
||||||
select {
|
|
||||||
case p.processingSemaphore <- struct{}{}:
|
|
||||||
return release, nil
|
|
||||||
case <-time.After(p.processingWaitTimeout):
|
case <-time.After(p.processingWaitTimeout):
|
||||||
return nil, ErrTooManyImages
|
return nil, ErrTooManyImages
|
||||||
case <-ctx.Done():
|
case <-ctx.Done():
|
||||||
return nil, ctx.Err()
|
return nil, ctx.Err()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
release := func() { <-p.processingSemaphore }
|
||||||
|
|
||||||
|
// select takes the slot even when ctx has ended at the same moment
|
||||||
|
err := ctx.Err()
|
||||||
|
if err != nil {
|
||||||
|
release()
|
||||||
|
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
|
||||||
|
return release, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// detectFormat returns the format string for a govips image type.
|
// detectFormat returns the format string for a govips image type.
|
||||||
@@ -527,10 +533,17 @@ func (p *ImageProcessor) resize(
|
|||||||
|
|
||||||
const defaultQuality = 85
|
const defaultQuality = 85
|
||||||
|
|
||||||
// encode encodes an image to the specified format.
|
// encode encodes an image to the specified format. govips gives pixa no way
|
||||||
|
// to stop libvips once an encode has begun, so when ctx has ended encode
|
||||||
|
// returns ctx's error without beginning one.
|
||||||
func (p *ImageProcessor) encode(
|
func (p *ImageProcessor) encode(
|
||||||
img *vips.ImageRef, format Format, quality int,
|
ctx context.Context, img *vips.ImageRef, format Format, quality int,
|
||||||
) ([]byte, error) {
|
) ([]byte, error) {
|
||||||
|
err := ctx.Err()
|
||||||
|
if err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
|
||||||
if quality <= 0 {
|
if quality <= 0 {
|
||||||
quality = defaultQuality
|
quality = defaultQuality
|
||||||
}
|
}
|
||||||
@@ -540,7 +553,7 @@ func (p *ImageProcessor) encode(
|
|||||||
// libvips' built-in profile; govips' own sRGB path variable is set on
|
// 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.
|
// first use but read without a lock, so concurrent requests race on it.
|
||||||
if img.HasICCProfile() {
|
if img.HasICCProfile() {
|
||||||
err := img.TransformICCProfileWithFallback("srgb", "srgb")
|
err = img.TransformICCProfileWithFallback("srgb", "srgb")
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("failed to convert to sRGB: %w", err)
|
return nil, fmt.Errorf("failed to convert to sRGB: %w", err)
|
||||||
}
|
}
|
||||||
@@ -632,10 +645,11 @@ func exportWebP(img *vips.ImageRef, quality int) ([]byte, error) {
|
|||||||
|
|
||||||
// avifEffort is the AVIF effort, from 0 (fastest) to 9; 1 is the lowest
|
// avifEffort is the AVIF effort, from 0 (fastest) to 9; 1 is the lowest
|
||||||
// govips can set. With one thread, as pixad runs libvips, 1 takes about 51
|
// govips can set. With one thread, as pixad runs libvips, 1 takes about 51
|
||||||
// seconds to save an 8192x8192 image of random pixels, the worst case,
|
// seconds to save an 8192x8192 RGB image of random pixels, and about 110
|
||||||
// against the default downstream_timeout of 60 seconds. On an image of
|
// with an alpha channel, the worst case, against the default
|
||||||
// milder noise, which 1 saves in about 12 seconds, 2 takes nearly a minute
|
// downstream_timeout of 60 seconds. On an image of milder noise, which 1
|
||||||
// and libvips' default, 4, takes minutes.
|
// saves in about 12 seconds, 2 takes nearly a minute and libvips' default,
|
||||||
|
// 4, takes minutes.
|
||||||
const avifEffort = 1
|
const avifEffort = 1
|
||||||
|
|
||||||
// avifBitdepth is the AVIF bit depth, 8 bits per sample for every image.
|
// avifBitdepth is the AVIF bit depth, 8 bits per sample for every image.
|
||||||
|
|||||||
+52
-11
@@ -32,6 +32,12 @@ const httpStatusOK = 200
|
|||||||
// variant not among them is served as before, reading its .meta file.
|
// variant not among them is served as before, reading its .meta file.
|
||||||
const metaCacheSize = 10000
|
const metaCacheSize = 10000
|
||||||
|
|
||||||
|
// cleanExpiredBatchSize is the most expired negative cache rows one
|
||||||
|
// statement of CleanExpired deletes. Each batch is a statement of its
|
||||||
|
// own, so a request waits behind one batch at most, however many rows
|
||||||
|
// have expired.
|
||||||
|
const cleanExpiredBatchSize = 1000
|
||||||
|
|
||||||
// CacheConfig holds cache configuration.
|
// CacheConfig holds cache configuration.
|
||||||
type CacheConfig struct {
|
type CacheConfig struct {
|
||||||
StateDir string
|
StateDir string
|
||||||
@@ -50,8 +56,9 @@ type CacheConfig struct {
|
|||||||
|
|
||||||
// DisableDiskCache turns the disk cache off entirely: no cache
|
// DisableDiskCache turns the disk cache off entirely: no cache
|
||||||
// directories are created, lookups always miss, stores are
|
// directories are created, lookups always miss, stores are
|
||||||
// no-ops, and no eviction machinery runs. The config layer sets
|
// no-ops, and the eviction goroutine only removes the expired
|
||||||
// this when the operator configures cache_max_bytes: 0.
|
// negative cache rows. The config layer sets this when the
|
||||||
|
// operator configures cache_max_bytes: 0.
|
||||||
DisableDiskCache bool
|
DisableDiskCache bool
|
||||||
|
|
||||||
// Logger receives accounting and eviction log output. A nil
|
// Logger receives accounting and eviction log output. A nil
|
||||||
@@ -482,17 +489,27 @@ func (c *Cache) GetSourceContent(
|
|||||||
return c.srcContent.LoadWithSize(contentHash)
|
return c.srcContent.LoadWithSize(contentHash)
|
||||||
}
|
}
|
||||||
|
|
||||||
// CleanExpired removes expired entries from the cache.
|
// CleanExpired removes the negative cache rows past their expiry, one
|
||||||
|
// batch per statement, until none is left. Once ctx ends it starts no
|
||||||
|
// further batch and returns ctx's error. The eviction goroutine calls it
|
||||||
|
// at startup and on every periodic tick, so a row stays about
|
||||||
|
// NegativeTTL plus one eviction interval, whether or not its URL is
|
||||||
|
// requested again.
|
||||||
func (c *Cache) CleanExpired(ctx context.Context) error {
|
func (c *Cache) CleanExpired(ctx context.Context) error {
|
||||||
// Clean expired negative cache entries
|
for {
|
||||||
_, err := c.db.ExecContext(ctx, `
|
if ctx.Err() != nil {
|
||||||
DELETE FROM negative_cache WHERE expires_at < CURRENT_TIMESTAMP
|
return ctx.Err()
|
||||||
`)
|
}
|
||||||
if err != nil {
|
|
||||||
return fmt.Errorf("failed to clean negative cache: %w", err)
|
|
||||||
}
|
|
||||||
|
|
||||||
return nil
|
deleted, err := c.cleanExpiredBatch(ctx)
|
||||||
|
if err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
|
||||||
|
if deleted < cleanExpiredBatchSize {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Stats returns cache statistics.
|
// Stats returns cache statistics.
|
||||||
@@ -756,3 +773,27 @@ func (c *Cache) checkNegativeCache(
|
|||||||
|
|
||||||
return true, nil
|
return true, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// cleanExpiredBatch deletes up to cleanExpiredBatchSize negative cache
|
||||||
|
// rows past their expiry in one statement, and returns how many it
|
||||||
|
// deleted. DELETE ... LIMIT needs a SQLite build option, so the subquery
|
||||||
|
// picks the rows.
|
||||||
|
func (c *Cache) cleanExpiredBatch(ctx context.Context) (int64, error) {
|
||||||
|
result, err := c.db.ExecContext(ctx, `
|
||||||
|
DELETE FROM negative_cache WHERE id IN (
|
||||||
|
SELECT id FROM negative_cache
|
||||||
|
WHERE expires_at < CURRENT_TIMESTAMP
|
||||||
|
LIMIT ?
|
||||||
|
)
|
||||||
|
`, cleanExpiredBatchSize)
|
||||||
|
if err != nil {
|
||||||
|
return 0, fmt.Errorf("failed to clean negative cache: %w", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
deleted, err := result.RowsAffected()
|
||||||
|
if err != nil {
|
||||||
|
return 0, fmt.Errorf("failed to count cleaned negative cache rows: %w", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
return deleted, nil
|
||||||
|
}
|
||||||
|
|||||||
@@ -15,6 +15,7 @@ import (
|
|||||||
|
|
||||||
"github.com/getsentry/sentry-go"
|
"github.com/getsentry/sentry-go"
|
||||||
"sneak.berlin/go/pixa/internal/httpfetcher"
|
"sneak.berlin/go/pixa/internal/httpfetcher"
|
||||||
|
"sneak.berlin/go/pixa/internal/imageprocessor"
|
||||||
"sneak.berlin/go/pixa/internal/magic"
|
"sneak.berlin/go/pixa/internal/magic"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -435,6 +436,47 @@ func TestService_Get_ReturnsByItsDeadline(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestService_Get_WaiterGetsProcessingTimedOut has a request with no deadline
|
||||||
|
// join the held fetch of a first request with a short one. With the only
|
||||||
|
// processing slot held, the first request's transcode waits for it until the
|
||||||
|
// first request's deadline ends, and stops then. The waiting request, whose
|
||||||
|
// own context has not ended, must get ErrProcessingTimedOut, which the image
|
||||||
|
// routes answer 503 as when pixa is busy.
|
||||||
|
func TestService_Get_WaiterGetsProcessingTimedOut(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
svc, fixtures, fetcher := setupHeldFetchService(t)
|
||||||
|
svc.processor = imageprocessor.New(
|
||||||
|
imageprocessor.Params{MaxConcurrentProcessing: 1},
|
||||||
|
)
|
||||||
|
|
||||||
|
holdProcessingSlot(t, svc.processor)
|
||||||
|
|
||||||
|
const firstTimeout = 500 * time.Millisecond
|
||||||
|
|
||||||
|
firstCtx, cancelFirst := context.WithTimeout(t.Context(), firstTimeout)
|
||||||
|
defer cancelFirst()
|
||||||
|
|
||||||
|
first := startGet(firstCtx, svc, photoVariant(fixtures, 85, FitCover))
|
||||||
|
|
||||||
|
<-fetcher.started
|
||||||
|
|
||||||
|
waiter := startGet(t.Context(), svc, photoVariant(fixtures, 85, FitCover))
|
||||||
|
|
||||||
|
time.Sleep(arrivalWait)
|
||||||
|
fetcher.releaseFetches()
|
||||||
|
|
||||||
|
t.Logf("first request: Get() error = %v", (<-first).err)
|
||||||
|
|
||||||
|
got := <-waiter
|
||||||
|
t.Logf("waiting request: Get() error = %v", got.err)
|
||||||
|
|
||||||
|
if !errors.Is(got.err, ErrProcessingTimedOut) {
|
||||||
|
t.Errorf("waiting request: Get() error = %v, want %v",
|
||||||
|
got.err, ErrProcessingTimedOut)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// panickingFetcher panics on every fetch.
|
// panickingFetcher panics on every fetch.
|
||||||
type panickingFetcher struct{}
|
type panickingFetcher struct{}
|
||||||
|
|
||||||
|
|||||||
@@ -437,14 +437,16 @@ func (c *Cache) notifyWritePressure() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// StartEviction launches the background eviction goroutine, which
|
// StartEviction launches the background eviction goroutine, which
|
||||||
// reconciles the database accounting with the cache directories at
|
// reconciles the database accounting with the cache directories and
|
||||||
// startup and again on every periodic tick thereafter, and evicts to
|
// removes the expired negative cache rows at startup and again on every
|
||||||
// the configured limit on the given periodic interval and on
|
// periodic tick thereafter, and evicts to the configured limit on the
|
||||||
// write-pressure notifications. It is a no-op on a disabled cache or
|
// given periodic interval and on write-pressure notifications. On a
|
||||||
// when already started. The goroutine outlives the caller, so it runs
|
// disabled cache it only removes the expired negative cache rows, as
|
||||||
// with its own context, which StopEviction cancels.
|
// the negative cache is used either way. It is a no-op when already
|
||||||
|
// started. The goroutine outlives the caller, so it runs with its own
|
||||||
|
// context, which StopEviction cancels.
|
||||||
func (c *Cache) StartEviction(interval time.Duration) {
|
func (c *Cache) StartEviction(interval time.Duration) {
|
||||||
if c.disabled || c.evictionCancel != nil {
|
if c.evictionCancel != nil {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -480,6 +482,7 @@ func (c *Cache) evictionLoop(ctx context.Context, interval time.Duration) {
|
|||||||
defer close(c.evictionDone)
|
defer close(c.evictionDone)
|
||||||
|
|
||||||
c.runReconciliationPass(ctx)
|
c.runReconciliationPass(ctx)
|
||||||
|
c.runCleanExpired(ctx)
|
||||||
c.runEvictionPass(ctx)
|
c.runEvictionPass(ctx)
|
||||||
|
|
||||||
ticker := time.NewTicker(interval)
|
ticker := time.NewTicker(interval)
|
||||||
@@ -500,6 +503,7 @@ func (c *Cache) evictionLoop(ctx context.Context, interval time.Duration) {
|
|||||||
// unaccounted for to one interval, on a process that is
|
// unaccounted for to one interval, on a process that is
|
||||||
// already running this loop regardless.
|
// already running this loop regardless.
|
||||||
c.runReconciliationPass(ctx)
|
c.runReconciliationPass(ctx)
|
||||||
|
c.runCleanExpired(ctx)
|
||||||
case <-c.evictionPressure:
|
case <-c.evictionPressure:
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -535,6 +539,20 @@ func (c *Cache) runReconciliationPass(ctx context.Context) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// runCleanExpired runs CleanExpired, logging a failure instead of
|
||||||
|
// propagating it (the loop must keep running). It does nothing once ctx
|
||||||
|
// is cancelled.
|
||||||
|
func (c *Cache) runCleanExpired(ctx context.Context) {
|
||||||
|
if ctx.Err() != nil {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
err := c.CleanExpired(ctx)
|
||||||
|
if err != nil {
|
||||||
|
c.log.Warn("negative cache cleanup failed", "error", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// reconcileAccounting synchronizes the database size accounting with
|
// reconcileAccounting synchronizes the database size accounting with
|
||||||
// the actual contents of the cache directories. It runs at startup and
|
// the actual contents of the cache directories. It runs at startup and
|
||||||
// again on every periodic eviction tick thereafter, off the request
|
// again on every periodic eviction tick thereafter, off the request
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ package imgcache
|
|||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"errors"
|
"errors"
|
||||||
|
"strconv"
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
)
|
)
|
||||||
@@ -94,6 +95,307 @@ func TestNegativeCache_Expired(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestService_Get_ServesCachedVariantOfNegativeCachedURL caches a variant of
|
||||||
|
// the test photo, then keeps a failed fetch of the photo's URL in the negative
|
||||||
|
// cache, as a failed fetch for another variant does once the source has been
|
||||||
|
// evicted. The cached variant is still served, and a variant not in the cache
|
||||||
|
// is made from the cached source. Once the source is evicted, a variant not in
|
||||||
|
// the cache needs a fetch and is refused.
|
||||||
|
func TestService_Get_ServesCachedVariantOfNegativeCachedURL(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
svc, fixtures := SetupTestService(t)
|
||||||
|
|
||||||
|
resp, err := svc.Get(t.Context(), photoVariant(fixtures, 85, FitCover))
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Get() first request error = %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
_ = resp.Content.Close()
|
||||||
|
|
||||||
|
err = svc.cache.StoreNegative(t.Context(), photoVariant(fixtures, 85, FitCover),
|
||||||
|
httpStatusBadGateway, "upstream answered 503")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("StoreNegative() error = %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
resp, err = svc.Get(t.Context(), photoVariant(fixtures, 85, FitCover))
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Get() of the cached variant error = %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
_ = resp.Content.Close()
|
||||||
|
|
||||||
|
if resp.CacheStatus != CacheHit {
|
||||||
|
t.Errorf("cached variant CacheStatus = %q, want %q",
|
||||||
|
resp.CacheStatus, CacheHit)
|
||||||
|
}
|
||||||
|
|
||||||
|
resp, err = svc.Get(t.Context(), photoVariant(fixtures, 40, FitCover))
|
||||||
|
t.Logf("Get() of a variant made from the cached source: error = %v", err)
|
||||||
|
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Get() of a variant made from the cached source error = %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
_ = resp.Content.Close()
|
||||||
|
|
||||||
|
contentHash, _, err := svc.cache.LookupSource(t.Context(),
|
||||||
|
photoVariant(fixtures, 85, FitCover))
|
||||||
|
if err != nil || contentHash == "" {
|
||||||
|
t.Fatalf("LookupSource() = %q, %v, want the cached source", contentHash, err)
|
||||||
|
}
|
||||||
|
|
||||||
|
err = svc.cache.evictSourceBlob(t.Context(), contentHash)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("evictSourceBlob() error = %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
_, err = svc.Get(t.Context(), photoVariant(fixtures, 60, FitCover))
|
||||||
|
t.Logf("Get() of a variant whose source was evicted: error = %v", err)
|
||||||
|
|
||||||
|
if !errors.Is(err, ErrNegativeCached) {
|
||||||
|
t.Errorf("Get() of a variant whose source was evicted error = %v, want %v",
|
||||||
|
err, ErrNegativeCached)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestEvictionLoopRemovesExpiredNegativeCacheRows stores a failed fetch
|
||||||
|
// that expires a second later and starts the eviction loop with a short
|
||||||
|
// interval: a later tick must remove its row although its URL is never
|
||||||
|
// requested again, and must keep a row that has not expired. It runs
|
||||||
|
// with the disk cache enabled and disabled, as the negative cache is
|
||||||
|
// used either way.
|
||||||
|
func TestEvictionLoopRemovesExpiredNegativeCacheRows(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
disableDiskCache bool
|
||||||
|
}{
|
||||||
|
{name: "disk cache enabled", disableDiskCache: false},
|
||||||
|
{name: "disk cache disabled", disableDiskCache: true},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
cache, err := NewCache(evictionTestDB(t), CacheConfig{
|
||||||
|
StateDir: t.TempDir(),
|
||||||
|
NegativeTTL: time.Second,
|
||||||
|
DisableDiskCache: tt.disableDiskCache,
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("failed to create cache: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
expired := &ImageRequest{
|
||||||
|
SourceHost: testHostExample, SourcePath: "/gone.jpg",
|
||||||
|
}
|
||||||
|
|
||||||
|
err = cache.StoreNegative(t.Context(), expired, 404, "not found")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("StoreNegative failed: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
const notExpiredPath = "/later.jpg"
|
||||||
|
|
||||||
|
_, err = cache.db.ExecContext(t.Context(), `
|
||||||
|
INSERT INTO negative_cache
|
||||||
|
(source_host, source_path, status_code, expires_at)
|
||||||
|
VALUES (?, ?, 404, datetime('now', '+1 hour'))
|
||||||
|
`, testHostExample, notExpiredPath)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("failed to insert a row that has not expired: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
started := time.Now()
|
||||||
|
|
||||||
|
cache.StartEviction(100 * time.Millisecond)
|
||||||
|
defer func() { _ = cache.StopEviction(t.Context()) }()
|
||||||
|
|
||||||
|
for negativeCacheRowCount(t, cache, expired.SourcePath) > 0 {
|
||||||
|
if time.Since(started) > 10*time.Second {
|
||||||
|
t.Fatal("the expired row is still in negative_cache")
|
||||||
|
}
|
||||||
|
|
||||||
|
time.Sleep(50 * time.Millisecond)
|
||||||
|
}
|
||||||
|
|
||||||
|
t.Logf("the expired row was removed %s after the loop started",
|
||||||
|
time.Since(started))
|
||||||
|
|
||||||
|
if negativeCacheRowCount(t, cache, notExpiredPath) != 1 {
|
||||||
|
t.Error("the row that has not expired was removed")
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// negativeCacheRowCount returns how many negative_cache rows hold
|
||||||
|
// sourcePath.
|
||||||
|
func negativeCacheRowCount(tb testing.TB, cache *Cache, sourcePath string) int {
|
||||||
|
tb.Helper()
|
||||||
|
|
||||||
|
var count int
|
||||||
|
|
||||||
|
err := cache.db.QueryRowContext(tb.Context(),
|
||||||
|
`SELECT COUNT(*) FROM negative_cache WHERE source_path = ?`,
|
||||||
|
sourcePath,
|
||||||
|
).Scan(&count)
|
||||||
|
if err != nil {
|
||||||
|
tb.Fatalf("failed to count negative_cache rows: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
return count
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestCleanExpiredDeletesInBatches stores one expired negative cache row
|
||||||
|
// more than one batch holds, and one row that has not expired. One
|
||||||
|
// statement must delete one batch and no more, so a request waits behind
|
||||||
|
// one batch at most; CleanExpired must then remove the expired row left
|
||||||
|
// over and keep the row that has not expired.
|
||||||
|
func TestCleanExpiredDeletesInBatches(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
cache, err := NewCache(evictionTestDB(t), CacheConfig{StateDir: t.TempDir()})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("failed to create cache: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
ctx := t.Context()
|
||||||
|
|
||||||
|
insertExpiredNegativeCacheRows(t, cache, cleanExpiredBatchSize+1)
|
||||||
|
|
||||||
|
const notExpiredPath = "/later.jpg"
|
||||||
|
|
||||||
|
_, err = cache.db.ExecContext(ctx, `
|
||||||
|
INSERT INTO negative_cache
|
||||||
|
(source_host, source_path, status_code, expires_at)
|
||||||
|
VALUES (?, ?, 404, datetime('now', '+1 hour'))
|
||||||
|
`, testHostExample, notExpiredPath)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("failed to insert a row that has not expired: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
deleted, err := cache.cleanExpiredBatch(ctx)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("cleanExpiredBatch failed: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
t.Logf("one statement deleted %d of %d expired rows",
|
||||||
|
deleted, cleanExpiredBatchSize+1)
|
||||||
|
|
||||||
|
if deleted != cleanExpiredBatchSize {
|
||||||
|
t.Errorf("one statement deleted %d rows, want %d",
|
||||||
|
deleted, cleanExpiredBatchSize)
|
||||||
|
}
|
||||||
|
|
||||||
|
err = cache.CleanExpired(ctx)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("CleanExpired failed: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if count := negativeCacheTableRowCount(t, cache); count != 1 {
|
||||||
|
t.Errorf("negative_cache holds %d rows after CleanExpired, want 1", count)
|
||||||
|
}
|
||||||
|
|
||||||
|
if negativeCacheRowCount(t, cache, notExpiredPath) != 1 {
|
||||||
|
t.Error("the row that has not expired was removed")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestCleanExpiredRemovesMoreThanOneBatch stores one expired negative
|
||||||
|
// cache row more than one batch holds and calls CleanExpired once: it
|
||||||
|
// must remove them all, so it fails if CleanExpired stops after its
|
||||||
|
// first batch.
|
||||||
|
func TestCleanExpiredRemovesMoreThanOneBatch(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
cache, err := NewCache(evictionTestDB(t), CacheConfig{StateDir: t.TempDir()})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("failed to create cache: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
insertExpiredNegativeCacheRows(t, cache, cleanExpiredBatchSize+1)
|
||||||
|
|
||||||
|
before := negativeCacheTableRowCount(t, cache)
|
||||||
|
|
||||||
|
err = cache.CleanExpired(t.Context())
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("CleanExpired failed: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
after := negativeCacheTableRowCount(t, cache)
|
||||||
|
|
||||||
|
t.Logf("CleanExpired removed %d of %d expired rows, %d per batch",
|
||||||
|
before-after, before, cleanExpiredBatchSize)
|
||||||
|
|
||||||
|
if after != 0 {
|
||||||
|
t.Errorf("negative_cache holds %d rows after CleanExpired, want 0", after)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestCleanExpiredStopsOnceContextEnds runs CleanExpired with a context
|
||||||
|
// that has already ended: it must return the context's error and delete
|
||||||
|
// no row.
|
||||||
|
func TestCleanExpiredStopsOnceContextEnds(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
cache, err := NewCache(evictionTestDB(t), CacheConfig{StateDir: t.TempDir()})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("failed to create cache: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
insertExpiredNegativeCacheRows(t, cache, 1)
|
||||||
|
|
||||||
|
ctx, cancel := context.WithCancel(t.Context())
|
||||||
|
cancel()
|
||||||
|
|
||||||
|
err = cache.CleanExpired(ctx)
|
||||||
|
if !errors.Is(err, context.Canceled) {
|
||||||
|
t.Errorf("CleanExpired() error = %v, want context.Canceled", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if count := negativeCacheTableRowCount(t, cache); count != 1 {
|
||||||
|
t.Errorf("negative_cache holds %d rows, want the expired row kept", count)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// insertExpiredNegativeCacheRows inserts count negative_cache rows that
|
||||||
|
// expired an hour ago, each for a different source path.
|
||||||
|
func insertExpiredNegativeCacheRows(t *testing.T, cache *Cache, count int) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
for i := range count {
|
||||||
|
_, err := cache.db.ExecContext(t.Context(), `
|
||||||
|
INSERT INTO negative_cache
|
||||||
|
(source_host, source_path, status_code, expires_at)
|
||||||
|
VALUES (?, ?, 404, datetime('now', '-1 hour'))
|
||||||
|
`, testHostExample, "/expired/"+strconv.Itoa(i)+".jpg")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("failed to insert an expired row: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// negativeCacheTableRowCount returns how many rows negative_cache holds.
|
||||||
|
func negativeCacheTableRowCount(tb testing.TB, cache *Cache) int {
|
||||||
|
tb.Helper()
|
||||||
|
|
||||||
|
var count int
|
||||||
|
|
||||||
|
err := cache.db.QueryRowContext(tb.Context(),
|
||||||
|
`SELECT COUNT(*) FROM negative_cache`,
|
||||||
|
).Scan(&count)
|
||||||
|
if err != nil {
|
||||||
|
tb.Fatalf("failed to count negative_cache rows: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
return count
|
||||||
|
}
|
||||||
|
|
||||||
func TestService_Get_ReturnsErrorForNegativeCachedURL(t *testing.T) {
|
func TestService_Get_ReturnsErrorForNegativeCachedURL(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
|
|||||||
@@ -9,6 +9,7 @@ import (
|
|||||||
"log/slog"
|
"log/slog"
|
||||||
"net/url"
|
"net/url"
|
||||||
"runtime/debug"
|
"runtime/debug"
|
||||||
|
"strings"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
"github.com/dustin/go-humanize"
|
"github.com/dustin/go-humanize"
|
||||||
@@ -128,26 +129,20 @@ func NewService(cfg *ServiceConfig) (*Service, error) {
|
|||||||
// ErrNegativeCached is returned when a URL is in the negative cache (recently failed).
|
// ErrNegativeCached is returned when a URL is in the negative cache (recently failed).
|
||||||
var ErrNegativeCached = errors.New("request is in negative cache (recently failed)")
|
var ErrNegativeCached = errors.New("request is in negative cache (recently failed)")
|
||||||
|
|
||||||
|
// ErrProcessingTimedOut is returned when processing an image stopped because
|
||||||
|
// the downstream_timeout of the request doing it ended, to that request and to
|
||||||
|
// every request that waited for its image.
|
||||||
|
var ErrProcessingTimedOut = errors.New(
|
||||||
|
"image processing stopped at the deadline of the request doing it")
|
||||||
|
|
||||||
// Get retrieves a processed image, fetching and processing if necessary.
|
// Get retrieves a processed image, fetching and processing if necessary.
|
||||||
|
// While its source URL is in the negative cache, a cached variant is still
|
||||||
|
// served and a variant not in the cache is still made from a cached source;
|
||||||
|
// only a request that needs the source fetched is refused.
|
||||||
func (s *Service) Get(ctx context.Context, req *ImageRequest) (*ImageResponse, error) {
|
func (s *Service) Get(ctx context.Context, req *ImageRequest) (*ImageResponse, error) {
|
||||||
// Propagate AllowHTTP setting to the request
|
// Propagate AllowHTTP setting to the request
|
||||||
req.AllowHTTP = s.allowHTTP
|
req.AllowHTTP = s.allowHTTP
|
||||||
|
|
||||||
// Check negative cache first - skip fetching for recently-failed URLs
|
|
||||||
negHit, err := s.cache.checkNegativeCache(ctx, req)
|
|
||||||
if err != nil {
|
|
||||||
s.log.Warn("negative cache check failed", "error", err)
|
|
||||||
}
|
|
||||||
|
|
||||||
if negHit {
|
|
||||||
s.log.Debug("negative cache hit",
|
|
||||||
"host", req.SourceHost,
|
|
||||||
"path", req.SourcePath,
|
|
||||||
)
|
|
||||||
|
|
||||||
return nil, fmt.Errorf("%w: %w", httpfetcher.ErrUpstreamError, ErrNegativeCached)
|
|
||||||
}
|
|
||||||
|
|
||||||
// Check variant cache first (disk only, no DB)
|
// Check variant cache first (disk only, no DB)
|
||||||
result, err := s.cache.Lookup(ctx, req)
|
result, err := s.cache.Lookup(ctx, req)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -218,7 +213,16 @@ func (s *Service) ValidateRequest(req *ImageRequest) error {
|
|||||||
return fmt.Errorf("invalid source URL: %w", err)
|
return fmt.Errorf("invalid source URL: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
if s.allowlist.IsAllowed(parsedURL) {
|
// An allowlist entry names no port, so a host followed by ":", with or
|
||||||
|
// without a port, needs a signature. The port is read from the parsed
|
||||||
|
// URL, as the fetcher reads it: in an IPv6 address written without
|
||||||
|
// brackets, such as 2001:db8::1:8443, it is what follows the last ":".
|
||||||
|
// This is checked here and not in allowlist.IsAllowed, which also matches
|
||||||
|
// referer_blocklist, where a Referer must be blocked whatever its port.
|
||||||
|
hostHasPort := parsedURL.Port() != "" ||
|
||||||
|
strings.HasSuffix(parsedURL.Host, ":")
|
||||||
|
|
||||||
|
if !hostHasPort && s.allowlist.IsAllowed(parsedURL) {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -423,12 +427,28 @@ func (s *Service) processFromSourceOrFetch(
|
|||||||
|
|
||||||
// fetchAndProcess fetches from upstream, processes, and caches the result.
|
// fetchAndProcess fetches from upstream, processes, and caches the result.
|
||||||
// It counts the fetch with the bytes read from upstream, including when
|
// It counts the fetch with the bytes read from upstream, including when
|
||||||
// reading the response or a later step fails.
|
// reading the response or a later step fails. It fetches nothing while the
|
||||||
|
// source URL is in the negative cache.
|
||||||
func (s *Service) fetchAndProcess(
|
func (s *Service) fetchAndProcess(
|
||||||
ctx context.Context,
|
ctx context.Context,
|
||||||
req *ImageRequest,
|
req *ImageRequest,
|
||||||
cacheKey VariantKey,
|
cacheKey VariantKey,
|
||||||
) (*processedVariant, error) {
|
) (*processedVariant, error) {
|
||||||
|
// Check negative cache - skip fetching for recently-failed URLs
|
||||||
|
negHit, err := s.cache.checkNegativeCache(ctx, req)
|
||||||
|
if err != nil {
|
||||||
|
s.log.Warn("negative cache check failed", "error", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if negHit {
|
||||||
|
s.log.Debug("negative cache hit",
|
||||||
|
"host", req.SourceHost,
|
||||||
|
"path", req.SourcePath,
|
||||||
|
)
|
||||||
|
|
||||||
|
return nil, fmt.Errorf("%w: %w", httpfetcher.ErrUpstreamError, ErrNegativeCached)
|
||||||
|
}
|
||||||
|
|
||||||
// Fetch from upstream
|
// Fetch from upstream
|
||||||
sourceURL := req.SourceURL()
|
sourceURL := req.SourceURL()
|
||||||
|
|
||||||
@@ -438,7 +458,13 @@ func (s *Service) fetchAndProcess(
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
// Store negative cache for certain errors
|
// Store negative cache for certain errors
|
||||||
if isNegativeCacheable(err) {
|
if isNegativeCacheable(err) {
|
||||||
s.storeNegative(ctx, req, extractStatusCode(err), 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)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return nil, fmt.Errorf("upstream fetch failed: %w", err)
|
return nil, fmt.Errorf("upstream fetch failed: %w", err)
|
||||||
@@ -484,16 +510,9 @@ func (s *Service) fetchAndProcess(
|
|||||||
"cipher", fetchResult.TLSCipherSuite,
|
"cipher", fetchResult.TLSCipherSuite,
|
||||||
)
|
)
|
||||||
|
|
||||||
// Validate magic bytes match content type. A body that does not is the
|
// Validate magic bytes match content type
|
||||||
// upstream's error, so it gets a negative cache entry, as an upstream
|
|
||||||
// error status does.
|
|
||||||
err = magic.ValidateMagicBytes(sourceData, fetchResult.ContentType)
|
err = magic.ValidateMagicBytes(sourceData, fetchResult.ContentType)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
s.log.Warn("source refused", "request_id", middleware.GetReqID(ctx),
|
|
||||||
"host", req.SourceHost, "path", req.SourcePath,
|
|
||||||
"content_type", fetchResult.ContentType, "error", err)
|
|
||||||
s.storeNegative(ctx, req, httpStatusUnsupportedMediaType, err)
|
|
||||||
|
|
||||||
return nil, fmt.Errorf("content validation failed: %w", err)
|
return nil, fmt.Errorf("content validation failed: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -509,19 +528,6 @@ func (s *Service) fetchAndProcess(
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
// storeNegative stores a negative cache entry for req's source, whose fetch
|
|
||||||
// failed with err, so the source is not fetched again until the entry
|
|
||||||
// expires. A failure to store the entry is only logged.
|
|
||||||
func (s *Service) storeNegative(
|
|
||||||
ctx context.Context, req *ImageRequest, statusCode int, err error,
|
|
||||||
) {
|
|
||||||
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)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// processedVariant is a variant as processAndStore made it. Each request that
|
// processedVariant is a variant as processAndStore made it. Each request that
|
||||||
// shared its processing serves it through a reader of its own.
|
// shared its processing serves it through a reader of its own.
|
||||||
type processedVariant struct {
|
type processedVariant struct {
|
||||||
@@ -549,7 +555,13 @@ func (s *Service) processAndStore(
|
|||||||
FitMode: imageprocessor.FitMode(req.FitMode),
|
FitMode: imageprocessor.FitMode(req.FitMode),
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ctx has the deadline of the request doing the processing, which can
|
||||||
|
// end while a request waiting for its image has time left
|
||||||
processResult, err := s.processor.Process(ctx, source, processReq)
|
processResult, err := s.processor.Process(ctx, source, processReq)
|
||||||
|
if errors.Is(err, context.DeadlineExceeded) {
|
||||||
|
return nil, fmt.Errorf("%w: %w", ErrProcessingTimedOut, err)
|
||||||
|
}
|
||||||
|
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("image processing failed: %w", err)
|
return nil, fmt.Errorf("image processing failed: %w", err)
|
||||||
}
|
}
|
||||||
@@ -630,9 +642,8 @@ func signatureRequest(req *ImageRequest) *signature.Request {
|
|||||||
|
|
||||||
// HTTP status codes for error responses.
|
// HTTP status codes for error responses.
|
||||||
const (
|
const (
|
||||||
httpStatusBadGateway = 502
|
httpStatusBadGateway = 502
|
||||||
httpStatusInternalError = 500
|
httpStatusInternalError = 500
|
||||||
httpStatusUnsupportedMediaType = 415
|
|
||||||
)
|
)
|
||||||
|
|
||||||
// isNegativeCacheable returns true if the error should be cached.
|
// isNegativeCacheable returns true if the error should be cached.
|
||||||
|
|||||||
@@ -250,6 +250,97 @@ func TestService_ValidateRequest_SignatureExactHostMatch(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestService_ValidateRequest_HostWithPortNeedsSignature verifies that a host
|
||||||
|
// followed by a port, or by a colon with no port, matches no allowlist entry,
|
||||||
|
// so that a request for it without a signature is refused, while the same
|
||||||
|
// host with no port is allowlisted.
|
||||||
|
func TestService_ValidateRequest_HostWithPortNeedsSignature(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
svc, _ := SetupTestService(t,
|
||||||
|
WithSigningKey(testSigningKey),
|
||||||
|
WithAllowlist(testHostCDN, "203.0.113.5", "2001:db8::1"),
|
||||||
|
)
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
host string
|
||||||
|
allowlisted bool
|
||||||
|
}{
|
||||||
|
{testHostCDN, true},
|
||||||
|
{"cdn.example.com:8443", false},
|
||||||
|
{"cdn.example.com:443", false},
|
||||||
|
{"cdn.example.com:", false},
|
||||||
|
{"203.0.113.5", true},
|
||||||
|
{"203.0.113.5:8080", false},
|
||||||
|
{"[2001:db8::1]", true},
|
||||||
|
{"[2001:db8::1]:8443", false},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.host, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
req := &ImageRequest{
|
||||||
|
SourceHost: tt.host,
|
||||||
|
SourcePath: testPathCat,
|
||||||
|
Size: Size{Width: 50, Height: 50},
|
||||||
|
Format: FormatJPEG,
|
||||||
|
Quality: 85,
|
||||||
|
FitMode: FitCover,
|
||||||
|
}
|
||||||
|
|
||||||
|
err := svc.ValidateRequest(req)
|
||||||
|
t.Logf("ValidateRequest() for host %q with no signature: %v",
|
||||||
|
tt.host, err)
|
||||||
|
|
||||||
|
if allowlisted := err == nil; allowlisted != tt.allowlisted {
|
||||||
|
t.Errorf("host %q served without a signature = %v, want %v",
|
||||||
|
tt.host, allowlisted, tt.allowlisted)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestService_ValidateRequest_UnbracketedIPv6PortNeedsSignature verifies that
|
||||||
|
// an allowlisted IPv6 address written without brackets and followed by a
|
||||||
|
// port, or by a colon with no port, needs a signature. Go's URL parser, which
|
||||||
|
// the fetcher uses, reads 2001:db8::1:8443 as the host 2001:db8::1 with the
|
||||||
|
// port 8443.
|
||||||
|
func TestService_ValidateRequest_UnbracketedIPv6PortNeedsSignature(
|
||||||
|
t *testing.T,
|
||||||
|
) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
svc, _ := SetupTestService(t,
|
||||||
|
WithSigningKey(testSigningKey),
|
||||||
|
WithAllowlist("2001:db8::1"),
|
||||||
|
)
|
||||||
|
|
||||||
|
for _, host := range []string{"2001:db8::1:8443", "2001:db8::1:"} {
|
||||||
|
t.Run(host, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
req := &ImageRequest{
|
||||||
|
SourceHost: host,
|
||||||
|
SourcePath: testPathCat,
|
||||||
|
Size: Size{Width: 50, Height: 50},
|
||||||
|
Format: FormatJPEG,
|
||||||
|
Quality: 85,
|
||||||
|
FitMode: FitCover,
|
||||||
|
}
|
||||||
|
|
||||||
|
err := svc.ValidateRequest(req)
|
||||||
|
t.Logf("ValidateRequest() for host %q with no signature: %v",
|
||||||
|
host, err)
|
||||||
|
|
||||||
|
if err == nil {
|
||||||
|
t.Errorf("host %q served without a signature, want refused",
|
||||||
|
host)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestService_Get_InvalidFile(t *testing.T) {
|
func TestService_Get_InvalidFile(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
|
|||||||
@@ -15,6 +15,7 @@ import (
|
|||||||
var (
|
var (
|
||||||
ErrInvalidPath = errors.New("invalid image path")
|
ErrInvalidPath = errors.New("invalid image path")
|
||||||
ErrMissingHost = errors.New("missing source host")
|
ErrMissingHost = errors.New("missing source host")
|
||||||
|
ErrInvalidHost = errors.New("invalid source host")
|
||||||
ErrMissingSize = errors.New("missing size specification")
|
ErrMissingSize = errors.New("missing size specification")
|
||||||
ErrInvalidSize = errors.New("invalid size format")
|
ErrInvalidSize = errors.New("invalid size format")
|
||||||
ErrInvalidFormat = errors.New("invalid or unsupported format")
|
ErrInvalidFormat = errors.New("invalid or unsupported format")
|
||||||
@@ -143,6 +144,17 @@ func parseImageComponents(remainder string) (*ParsedURL, error) {
|
|||||||
return nil, ErrMissingHost
|
return nil, ErrMissingHost
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A URL reads what comes before "@" as a user name and password, and the
|
||||||
|
// rest as the host
|
||||||
|
if strings.Contains(host, "@") {
|
||||||
|
return nil, fmt.Errorf("%w: %q holds @", ErrInvalidHost, host)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Host names do not depend on letter case, so the allowlist, the upstream
|
||||||
|
// connection limits, the cache and the signature see one spelling of each
|
||||||
|
// host
|
||||||
|
host = strings.ToLower(host)
|
||||||
|
|
||||||
// Extract query string if present in path
|
// Extract query string if present in path
|
||||||
if qIndex := strings.Index(path, "?"); qIndex != -1 {
|
if qIndex := strings.Index(path, "?"); qIndex != -1 {
|
||||||
query = path[qIndex+1:]
|
query = path[qIndex+1:]
|
||||||
|
|||||||
@@ -234,6 +234,49 @@ func TestParseImagePath(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestParseImagePath_LowerCasesHost verifies that the host is lower-cased, so
|
||||||
|
// that every spelling of its letter case is the same host to the allowlist,
|
||||||
|
// the upstream connection limits and the cache, and that the path keeps its
|
||||||
|
// case.
|
||||||
|
func TestParseImagePath_LowerCasesHost(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
got, err := ParseImagePath("CDN.Example.COM/Photos/Cat.jpg/800x600.webp")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("ParseImagePath() error = %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
t.Logf("parsed host %q, path %q", got.Host, got.Path)
|
||||||
|
|
||||||
|
if got.Host != testHostCDN {
|
||||||
|
t.Errorf("Host = %q, want %q", got.Host, testHostCDN)
|
||||||
|
}
|
||||||
|
|
||||||
|
if got.Path != "/Photos/Cat.jpg" {
|
||||||
|
t.Errorf("Path = %q, want %q", got.Path, "/Photos/Cat.jpg")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestParseImagePath_RefusesUserinfo verifies that a host holding "@" is
|
||||||
|
// refused: a URL reads what comes before the "@" as a user name and password
|
||||||
|
// and the rest as the host.
|
||||||
|
func TestParseImagePath_RefusesUserinfo(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
for _, input := range []string{
|
||||||
|
"user@cdn.example.com/photos/cat.jpg/800x600.webp",
|
||||||
|
"user:password@cdn.example.com/photos/cat.jpg/800x600.webp",
|
||||||
|
} {
|
||||||
|
got, err := ParseImagePath(input)
|
||||||
|
t.Logf("ParseImagePath(%q) = %+v, error = %v", input, got, err)
|
||||||
|
|
||||||
|
if !errors.Is(err, ErrInvalidHost) {
|
||||||
|
t.Errorf("ParseImagePath(%q) error = %v, want %v",
|
||||||
|
input, err, ErrInvalidHost)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestParsedURL_ToImageRequest(t *testing.T) {
|
func TestParsedURL_ToImageRequest(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user