Compare commits
2
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
2790125cb2 | ||
|
|
c23585dd6d |
@@ -200,9 +200,10 @@ path under `/v1/` answers 200, in maintenance mode too.
|
|||||||
`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), or with an image
|
`Content-Type` that is not one of the source formats (below), or with an image
|
||||||
whose first bytes libvips reads as another format; 502 when the upstream
|
whose first bytes libvips reads as another format; 502 when the upstream
|
||||||
answered with an error status, and for 5 minutes after that for the same
|
answered with an error status, and for 5 minutes after that for an image of
|
||||||
source URL; 503 when pixa is busy or in maintenance mode; 500 for any other
|
the same source URL that pixa can neither serve from its cache nor make from
|
||||||
failure.
|
the source image still in its cache; 503 when pixa is busy or in maintenance
|
||||||
|
mode; 500 for any other 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
|
||||||
@@ -418,7 +419,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
|
||||||
@@ -473,6 +476,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,6 +30,27 @@ P2: security: per-origin rate limiting
|
|||||||
|
|
||||||
# Completed Steps
|
# Completed Steps
|
||||||
|
|
||||||
|
- 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
|
||||||
@@ -38,17 +59,6 @@ P2: security: per-origin rate limiting
|
|||||||
and the query now have each `%` written as `%25` and each `:` as `%3A`; a path
|
and the query now have each `%` written as `%25` and each `:` as `%3A`; a path
|
||||||
and query holding neither are signed and keyed as before. Signature
|
and query holding neither are signed and keyed as before. Signature
|
||||||
Specification in `README.md` says so.
|
Specification in `README.md` says so.
|
||||||
- 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 the example config allowlists no host (closes #249): it allowlisted
|
- 2026-10-15 the example config allowlists no host (closes #249): it allowlisted
|
||||||
`github.com` and `user-images.githubusercontent.com`, where anyone can put an
|
`github.com` and `user-images.githubusercontent.com`, where anyone can put an
|
||||||
image, so a deployment set up as Getting Started says served those images to
|
image, so a deployment set up as Getting Started says served those images to
|
||||||
|
|||||||
@@ -124,8 +124,7 @@ 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, and the removal of expired negative cache
|
// write-pressure passes. No-op when the disk cache is disabled.
|
||||||
// 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
|
||||||
|
|||||||
+11
-52
@@ -32,12 +32,6 @@ 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
|
||||||
@@ -56,9 +50,8 @@ 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 the eviction goroutine only removes the expired
|
// no-ops, and no eviction machinery runs. The config layer sets
|
||||||
// negative cache rows. The config layer sets this when the
|
// this when the operator configures cache_max_bytes: 0.
|
||||||
// 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
|
||||||
@@ -489,27 +482,17 @@ func (c *Cache) GetSourceContent(
|
|||||||
return c.srcContent.LoadWithSize(contentHash)
|
return c.srcContent.LoadWithSize(contentHash)
|
||||||
}
|
}
|
||||||
|
|
||||||
// CleanExpired removes the negative cache rows past their expiry, one
|
// CleanExpired removes expired entries from the cache.
|
||||||
// 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 {
|
||||||
for {
|
// Clean expired negative cache entries
|
||||||
if ctx.Err() != nil {
|
_, err := c.db.ExecContext(ctx, `
|
||||||
return ctx.Err()
|
DELETE FROM negative_cache WHERE expires_at < CURRENT_TIMESTAMP
|
||||||
}
|
`)
|
||||||
|
if err != nil {
|
||||||
deleted, err := c.cleanExpiredBatch(ctx)
|
return fmt.Errorf("failed to clean negative cache: %w", err)
|
||||||
if err != nil {
|
|
||||||
return err
|
|
||||||
}
|
|
||||||
|
|
||||||
if deleted < cleanExpiredBatchSize {
|
|
||||||
return nil
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// Stats returns cache statistics.
|
// Stats returns cache statistics.
|
||||||
@@ -773,27 +756,3 @@ 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
|
|
||||||
}
|
|
||||||
|
|||||||
@@ -437,16 +437,14 @@ 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 and
|
// reconciles the database accounting with the cache directories at
|
||||||
// removes the expired negative cache rows at startup and again on every
|
// startup and again on every periodic tick thereafter, and evicts to
|
||||||
// periodic tick thereafter, and evicts to the configured limit on the
|
// the configured limit on the given periodic interval and on
|
||||||
// given periodic interval and on write-pressure notifications. On a
|
// write-pressure notifications. It is a no-op on a disabled cache or
|
||||||
// disabled cache it only removes the expired negative cache rows, as
|
// when already started. The goroutine outlives the caller, so it runs
|
||||||
// the negative cache is used either way. It is a no-op when already
|
// with its own context, which StopEviction cancels.
|
||||||
// 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.evictionCancel != nil {
|
if c.disabled || c.evictionCancel != nil {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -482,7 +480,6 @@ 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)
|
||||||
@@ -503,7 +500,6 @@ 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:
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -539,20 +535,6 @@ 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,7 +3,6 @@ package imgcache
|
|||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"errors"
|
"errors"
|
||||||
"strconv"
|
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
)
|
)
|
||||||
@@ -95,240 +94,69 @@ func TestNegativeCache_Expired(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// TestEvictionLoopRemovesExpiredNegativeCacheRows stores a failed fetch
|
// TestService_Get_ServesCachedVariantOfNegativeCachedURL caches a variant of
|
||||||
// that expires a second later and starts the eviction loop with a short
|
// the test photo, then keeps a failed fetch of the photo's URL in the negative
|
||||||
// interval: a later tick must remove its row although its URL is never
|
// cache, as a failed fetch for another variant does once the source has been
|
||||||
// requested again, and must keep a row that has not expired. It runs
|
// evicted. The cached variant is still served, and a variant not in the cache
|
||||||
// with the disk cache enabled and disabled, as the negative cache is
|
// is made from the cached source. Once the source is evicted, a variant not in
|
||||||
// used either way.
|
// the cache needs a fetch and is refused.
|
||||||
func TestEvictionLoopRemovesExpiredNegativeCacheRows(t *testing.T) {
|
func TestService_Get_ServesCachedVariantOfNegativeCachedURL(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
tests := []struct {
|
svc, fixtures := SetupTestService(t)
|
||||||
name string
|
|
||||||
disableDiskCache bool
|
|
||||||
}{
|
|
||||||
{name: "disk cache enabled", disableDiskCache: false},
|
|
||||||
{name: "disk cache disabled", disableDiskCache: true},
|
|
||||||
}
|
|
||||||
|
|
||||||
for _, tt := range tests {
|
resp, err := svc.Get(t.Context(), photoVariant(fixtures, 85, FitCover))
|
||||||
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 {
|
if err != nil {
|
||||||
tb.Fatalf("failed to count negative_cache rows: %v", err)
|
t.Fatalf("Get() first request error = %v", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
return count
|
_ = resp.Content.Close()
|
||||||
}
|
|
||||||
|
|
||||||
// TestCleanExpiredDeletesInBatches stores one expired negative cache row
|
err = svc.cache.StoreNegative(t.Context(), photoVariant(fixtures, 85, FitCover),
|
||||||
// more than one batch holds, and one row that has not expired. One
|
httpStatusBadGateway, "upstream answered 503")
|
||||||
// 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 {
|
if err != nil {
|
||||||
t.Fatalf("failed to create cache: %v", err)
|
t.Fatalf("StoreNegative() error = %v", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
ctx := t.Context()
|
resp, err = svc.Get(t.Context(), photoVariant(fixtures, 85, FitCover))
|
||||||
|
|
||||||
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 {
|
if err != nil {
|
||||||
t.Fatalf("failed to insert a row that has not expired: %v", err)
|
t.Fatalf("Get() of the cached variant error = %v", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
deleted, err := cache.cleanExpiredBatch(ctx)
|
_ = 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 {
|
if err != nil {
|
||||||
t.Fatalf("cleanExpiredBatch failed: %v", err)
|
t.Fatalf("Get() of a variant made from the cached source error = %v", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
t.Logf("one statement deleted %d of %d expired rows",
|
_ = resp.Content.Close()
|
||||||
deleted, cleanExpiredBatchSize+1)
|
|
||||||
|
|
||||||
if deleted != cleanExpiredBatchSize {
|
contentHash, _, err := svc.cache.LookupSource(t.Context(),
|
||||||
t.Errorf("one statement deleted %d rows, want %d",
|
photoVariant(fixtures, 85, FitCover))
|
||||||
deleted, cleanExpiredBatchSize)
|
if err != nil || contentHash == "" {
|
||||||
|
t.Fatalf("LookupSource() = %q, %v, want the cached source", contentHash, err)
|
||||||
}
|
}
|
||||||
|
|
||||||
err = cache.CleanExpired(ctx)
|
err = svc.cache.evictSourceBlob(t.Context(), contentHash)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("CleanExpired failed: %v", err)
|
t.Fatalf("evictSourceBlob() error = %v", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
if count := negativeCacheTableRowCount(t, cache); count != 1 {
|
_, err = svc.Get(t.Context(), photoVariant(fixtures, 60, FitCover))
|
||||||
t.Errorf("negative_cache holds %d rows after CleanExpired, want 1", count)
|
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)
|
||||||
}
|
}
|
||||||
|
|
||||||
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) {
|
||||||
|
|||||||
@@ -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"
|
||||||
@@ -129,25 +130,13 @@ func NewService(cfg *ServiceConfig) (*Service, error) {
|
|||||||
var ErrNegativeCached = errors.New("request is in negative cache (recently failed)")
|
var ErrNegativeCached = errors.New("request is in negative cache (recently failed)")
|
||||||
|
|
||||||
// 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 +207,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 +421,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()
|
||||||
|
|
||||||
|
|||||||
@@ -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