Compare commits
5
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
69f65d4302 | ||
|
|
2790125cb2 | ||
|
|
c23585dd6d | ||
|
|
5d1e3ea871 | ||
|
|
761ade2e36 |
@@ -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
|
||||
`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
|
||||
answered with an error status, and for 5 minutes after that for the same
|
||||
source URL; 503 when pixa is busy or in maintenance mode; 500 for any other
|
||||
failure.
|
||||
answered with an error status, and for 5 minutes after that for an image of
|
||||
the same source URL that pixa can neither serve from its cache nor make from
|
||||
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
|
||||
(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
|
||||
@@ -418,9 +419,14 @@ HMAC-SHA256(secret, "host:path:query:width:height:format:expiration:quality:fit"
|
||||
|
||||
Where:
|
||||
|
||||
- `host` — source origin hostname (e.g. `cdn.example.com`)
|
||||
- `path` — source path (e.g. `/photos/cat.jpg`)
|
||||
- `query` — source query string, empty string if none
|
||||
- `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`
|
||||
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
|
||||
as in `path`; neither then holds a `:`, so path `/a:b` with no query and path
|
||||
`/a` with query `b:` are signed differently
|
||||
- `width` — requested width in pixels, `0` for original
|
||||
- `height` — requested height in pixels, `0` for original
|
||||
- `format` — output format, one of those listed under Routes, with `original`
|
||||
@@ -470,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
|
||||
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
|
||||
|
||||
Every setting can be given as an environment variable, in a YAML config file
|
||||
|
||||
@@ -30,6 +30,46 @@ P2: security: per-origin rate limiting
|
||||
|
||||
# Completed Steps
|
||||
|
||||
- 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
|
||||
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
|
||||
`https://h/a:b` verified for `https://h/a?b:`; the two also shared a cache
|
||||
key, and whichever was fetched first was served for both. In both, the 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
|
||||
Specification in `README.md` says so.
|
||||
- 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
|
||||
image, so a deployment set up as Getting Started says served those images to
|
||||
@@ -38,6 +78,15 @@ P2: security: per-origin rate limiting
|
||||
commented entry under `example.com`. Getting Started and "Running under upaas"
|
||||
in `README.md` say that every image on an allowlisted host is served without a
|
||||
signature, so an allowlisted host must be one the operator controls.
|
||||
- 2026-10-15 a chunked upstream body of exactly `upstream_max_response_size`
|
||||
bytes is accepted (closes #248): once the limit was used up, the fetcher's
|
||||
next read refused the body without looking for its end. Go's HTTP client
|
||||
returns the end of a body sent with a `Content-Length` together with its last
|
||||
bytes, but the end of a chunked body can come in a later read, so such a body
|
||||
was refused after it had been downloaded whole, and the image routes
|
||||
answered 500. Once the limit is used up, the fetcher now reads one more byte:
|
||||
if the body has ended it is accepted, and if there is more it is refused with
|
||||
`ErrResponseTooLarge`, as before.
|
||||
- 2026-10-13 `fit=outside` no longer makes an output above 8192 on a side
|
||||
(closes #257): it scales the image to cover the requested size, and the side
|
||||
scaled past that size was held to no maximum, so `1x8192?fit=outside` of a
|
||||
|
||||
@@ -124,7 +124,8 @@ func (s *Handlers) initImageService() error {
|
||||
s.imgCache = cache
|
||||
|
||||
// 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)
|
||||
|
||||
// Writes the counts requests could not write by their deadline
|
||||
|
||||
@@ -9,6 +9,7 @@ import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"slices"
|
||||
"strconv"
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
@@ -297,6 +298,139 @@ func TestFetchMaxResponseSizeEnforced(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// fetchWithMaxResponseSize serves handler as the fake upstream and fetches
|
||||
// its /image route with MaxResponseSize set to maxResponseSize.
|
||||
func fetchWithMaxResponseSize(
|
||||
t *testing.T,
|
||||
maxResponseSize int64,
|
||||
handler http.HandlerFunc,
|
||||
) *FetchResult {
|
||||
t.Helper()
|
||||
|
||||
srv := httptest.NewServer(handler)
|
||||
t.Cleanup(srv.Close)
|
||||
|
||||
cfg := DefaultConfig()
|
||||
cfg.MaxResponseSize = maxResponseSize
|
||||
|
||||
f, _ := newServerFetcher(t, srv, cfg)
|
||||
|
||||
res := fetchImage(t, f, "/image")
|
||||
t.Cleanup(func() { _ = res.Content.Close() })
|
||||
|
||||
return res
|
||||
}
|
||||
|
||||
// TestFetchContentLengthBodyAtMaxResponseSize checks that a body sent with a
|
||||
// Content-Length is read whole at exactly MaxResponseSize bytes and refused
|
||||
// at one byte more.
|
||||
func TestFetchContentLengthBodyAtMaxResponseSize(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
const maxResponseSize = 8
|
||||
|
||||
for _, tc := range []struct {
|
||||
size int
|
||||
wantErr error
|
||||
}{
|
||||
{maxResponseSize, nil},
|
||||
{maxResponseSize + 1, ErrResponseTooLarge},
|
||||
} {
|
||||
t.Run(strconv.Itoa(tc.size)+" bytes", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
body := strings.Repeat("x", tc.size)
|
||||
|
||||
res := fetchWithMaxResponseSize(t, maxResponseSize,
|
||||
func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.Header().Set("Content-Type", contentTypeJPEG)
|
||||
w.Header().Set("Content-Length", strconv.Itoa(tc.size))
|
||||
_, _ = io.WriteString(w, body)
|
||||
})
|
||||
|
||||
if res.ContentLength != int64(tc.size) {
|
||||
t.Fatalf("ContentLength = %d, want %d", res.ContentLength, tc.size)
|
||||
}
|
||||
|
||||
data, err := io.ReadAll(res.Content)
|
||||
t.Logf("body of %d bytes: read %d, error %v", tc.size, len(data), err)
|
||||
|
||||
if !errors.Is(err, tc.wantErr) {
|
||||
t.Fatalf("read error = %v, want %v", err, tc.wantErr)
|
||||
}
|
||||
|
||||
if err == nil && string(data) != body {
|
||||
t.Errorf("read %q, want %q", data, body)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestFetchChunkedBodyAtMaxResponseSize checks that a chunked body is read
|
||||
// whole at exactly MaxResponseSize bytes and refused at one byte more. The
|
||||
// upstream sends the end of the body only once the test has read
|
||||
// MaxResponseSize bytes, so the read after them comes before the end of the
|
||||
// body has arrived, as it can from a real upstream.
|
||||
func TestFetchChunkedBodyAtMaxResponseSize(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
const maxResponseSize = 8
|
||||
|
||||
for _, tc := range []struct {
|
||||
size int
|
||||
wantErr error
|
||||
}{
|
||||
{maxResponseSize, nil},
|
||||
{maxResponseSize + 1, ErrResponseTooLarge},
|
||||
} {
|
||||
t.Run(strconv.Itoa(tc.size)+" bytes", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
body := strings.Repeat("x", tc.size)
|
||||
endBody := make(chan struct{})
|
||||
|
||||
res := fetchWithMaxResponseSize(t, maxResponseSize,
|
||||
func(w http.ResponseWriter, r *http.Request) {
|
||||
w.Header().Set("Content-Type", contentTypeJPEG)
|
||||
_, _ = io.WriteString(w, body)
|
||||
// With no Content-Length, flushing sends the body
|
||||
// chunked; its end is sent when the handler returns.
|
||||
_ = http.NewResponseController(w).Flush()
|
||||
|
||||
select {
|
||||
case <-endBody:
|
||||
case <-r.Context().Done():
|
||||
}
|
||||
})
|
||||
|
||||
if res.ContentLength != -1 {
|
||||
t.Fatalf("ContentLength = %d, want -1 (chunked)", res.ContentLength)
|
||||
}
|
||||
|
||||
head := make([]byte, maxResponseSize)
|
||||
|
||||
_, err := io.ReadFull(res.Content, head)
|
||||
if err != nil {
|
||||
t.Fatalf("read the first %d bytes: %v", maxResponseSize, err)
|
||||
}
|
||||
|
||||
close(endBody)
|
||||
|
||||
rest, err := io.ReadAll(res.Content)
|
||||
data := string(head) + string(rest)
|
||||
t.Logf("body of %d bytes: read %d, error %v", tc.size, len(data), err)
|
||||
|
||||
if !errors.Is(err, tc.wantErr) {
|
||||
t.Fatalf("read error = %v, want %v", err, tc.wantErr)
|
||||
}
|
||||
|
||||
if err == nil && data != body {
|
||||
t.Errorf("read %q, want %q", data, body)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestFetchSemaphoreReleasedOnError(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
@@ -660,14 +660,33 @@ func dialSSRFSafe(
|
||||
return nil, fmt.Errorf("%w to %s", errConnectFailed, host)
|
||||
}
|
||||
|
||||
// limitedReader wraps a reader and limits the number of bytes read.
|
||||
// limitedReader wraps a reader and refuses a body longer than remaining
|
||||
// bytes with ErrResponseTooLarge. remaining goes below 0 once the body is
|
||||
// found to be longer.
|
||||
type limitedReader struct {
|
||||
reader io.Reader
|
||||
remaining int64
|
||||
}
|
||||
|
||||
// Read reads at most the remaining bytes. Once they are read, it reads one
|
||||
// more byte to tell the end of the body from more data: at the end it
|
||||
// returns the wrapped reader's error, io.EOF; with more data it returns
|
||||
// ErrResponseTooLarge, as does every later Read.
|
||||
func (r *limitedReader) Read(p []byte) (int, error) {
|
||||
if r.remaining <= 0 {
|
||||
if r.remaining < 0 {
|
||||
return 0, ErrResponseTooLarge
|
||||
}
|
||||
|
||||
if r.remaining == 0 {
|
||||
var extra [1]byte
|
||||
|
||||
n, err := r.reader.Read(extra[:])
|
||||
r.remaining -= int64(n)
|
||||
|
||||
if n == 0 {
|
||||
return 0, err
|
||||
}
|
||||
|
||||
return 0, ErrResponseTooLarge
|
||||
}
|
||||
|
||||
|
||||
+52
-11
@@ -32,6 +32,12 @@ const httpStatusOK = 200
|
||||
// variant not among them is served as before, reading its .meta file.
|
||||
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.
|
||||
type CacheConfig struct {
|
||||
StateDir string
|
||||
@@ -50,8 +56,9 @@ type CacheConfig struct {
|
||||
|
||||
// DisableDiskCache turns the disk cache off entirely: no cache
|
||||
// directories are created, lookups always miss, stores are
|
||||
// no-ops, and no eviction machinery runs. The config layer sets
|
||||
// this when the operator configures cache_max_bytes: 0.
|
||||
// no-ops, and the eviction goroutine only removes the expired
|
||||
// negative cache rows. The config layer sets this when the
|
||||
// operator configures cache_max_bytes: 0.
|
||||
DisableDiskCache bool
|
||||
|
||||
// Logger receives accounting and eviction log output. A nil
|
||||
@@ -482,17 +489,27 @@ func (c *Cache) GetSourceContent(
|
||||
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 {
|
||||
// Clean expired negative cache entries
|
||||
_, err := c.db.ExecContext(ctx, `
|
||||
DELETE FROM negative_cache WHERE expires_at < CURRENT_TIMESTAMP
|
||||
`)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to clean negative cache: %w", err)
|
||||
}
|
||||
for {
|
||||
if ctx.Err() != nil {
|
||||
return ctx.Err()
|
||||
}
|
||||
|
||||
return nil
|
||||
deleted, err := c.cleanExpiredBatch(ctx)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if deleted < cleanExpiredBatchSize {
|
||||
return nil
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Stats returns cache statistics.
|
||||
@@ -756,3 +773,27 @@ func (c *Cache) checkNegativeCache(
|
||||
|
||||
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,14 +437,16 @@ func (c *Cache) notifyWritePressure() {
|
||||
}
|
||||
|
||||
// StartEviction launches the background eviction goroutine, which
|
||||
// reconciles the database accounting with the cache directories at
|
||||
// startup and again on every periodic tick thereafter, and evicts to
|
||||
// the configured limit on the given periodic interval and on
|
||||
// write-pressure notifications. It is a no-op on a disabled cache or
|
||||
// when already started. The goroutine outlives the caller, so it runs
|
||||
// with its own context, which StopEviction cancels.
|
||||
// reconciles the database accounting with the cache directories and
|
||||
// removes the expired negative cache rows at startup and again on every
|
||||
// periodic tick thereafter, and evicts to the configured limit on the
|
||||
// given periodic interval and on write-pressure notifications. On a
|
||||
// disabled cache it only removes the expired negative cache rows, as
|
||||
// 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) {
|
||||
if c.disabled || c.evictionCancel != nil {
|
||||
if c.evictionCancel != nil {
|
||||
return
|
||||
}
|
||||
|
||||
@@ -480,6 +482,7 @@ func (c *Cache) evictionLoop(ctx context.Context, interval time.Duration) {
|
||||
defer close(c.evictionDone)
|
||||
|
||||
c.runReconciliationPass(ctx)
|
||||
c.runCleanExpired(ctx)
|
||||
c.runEvictionPass(ctx)
|
||||
|
||||
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
|
||||
// already running this loop regardless.
|
||||
c.runReconciliationPass(ctx)
|
||||
c.runCleanExpired(ctx)
|
||||
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
|
||||
// the actual contents of the cache directories. It runs at startup and
|
||||
// again on every periodic eviction tick thereafter, off the request
|
||||
|
||||
@@ -3,6 +3,7 @@ package imgcache
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"strconv"
|
||||
"testing"
|
||||
"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) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
@@ -9,6 +9,7 @@ import (
|
||||
"log/slog"
|
||||
"net/url"
|
||||
"runtime/debug"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"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)")
|
||||
|
||||
// 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) {
|
||||
// Propagate AllowHTTP setting to the request
|
||||
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)
|
||||
result, err := s.cache.Lookup(ctx, req)
|
||||
if err != nil {
|
||||
@@ -218,7 +207,16 @@ func (s *Service) ValidateRequest(req *ImageRequest) error {
|
||||
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
|
||||
}
|
||||
|
||||
@@ -423,12 +421,28 @@ func (s *Service) processFromSourceOrFetch(
|
||||
|
||||
// fetchAndProcess fetches from upstream, processes, and caches the result.
|
||||
// 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(
|
||||
ctx context.Context,
|
||||
req *ImageRequest,
|
||||
cacheKey VariantKey,
|
||||
) (*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
|
||||
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) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
@@ -10,6 +10,7 @@ import (
|
||||
"log/slog"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"time"
|
||||
)
|
||||
|
||||
@@ -371,11 +372,13 @@ func HashPath(path string) PathHash {
|
||||
|
||||
// CacheKey generates a unique key for a request variant.
|
||||
// Format: sha256(host:path:query:width:height:format:quality:fit_mode)
|
||||
// The path and query go through escapePercentAndColon, so that a ":" in
|
||||
// either cannot be taken for the one between them.
|
||||
func CacheKey(req *ImageRequest) VariantKey {
|
||||
data := fmt.Sprintf("%s:%s:%s:%d:%d:%s:%d:%s",
|
||||
req.SourceHost,
|
||||
req.SourcePath,
|
||||
req.SourceQuery,
|
||||
escapePercentAndColon(req.SourcePath),
|
||||
escapePercentAndColon(req.SourceQuery),
|
||||
req.Size.Width,
|
||||
req.Size.Height,
|
||||
req.Format,
|
||||
@@ -387,6 +390,14 @@ func CacheKey(req *ImageRequest) VariantKey {
|
||||
return VariantKey(hex.EncodeToString(h[:]))
|
||||
}
|
||||
|
||||
// escapePercentAndColon returns s with each "%" written as "%25" and each
|
||||
// ":" as "%3A", so that the result holds no ":".
|
||||
func escapePercentAndColon(s string) string {
|
||||
s = strings.ReplaceAll(s, "%", "%25")
|
||||
|
||||
return strings.ReplaceAll(s, ":", "%3A")
|
||||
}
|
||||
|
||||
// VariantStorage handles key-based file storage for processed image variants.
|
||||
// Files are stored at: <basedir>/<ab>/<cd>/<cache_key>
|
||||
// Metadata is stored at: <basedir>/<ab>/<cd>/<cache_key>.meta
|
||||
|
||||
@@ -407,6 +407,62 @@ func TestCacheKey(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestCacheKey_ColonOrPercentInPathOrQuery checks that two different source
|
||||
// URLs whose path or query holds ":" or "%" get different keys.
|
||||
func TestCacheKey_ColonOrPercentInPathOrQuery(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
path1, query1 string
|
||||
path2, query2 string
|
||||
}{
|
||||
{
|
||||
name: "colon moved from path to query",
|
||||
path1: "/a:b", query1: "",
|
||||
path2: "/a", query2: "b:",
|
||||
},
|
||||
{
|
||||
name: "colon in path and its percent-encoding",
|
||||
path1: "/a:b", query1: "",
|
||||
path2: "/a%3Ab", query2: "",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range cases {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
key1 := CacheKey(&ImageRequest{
|
||||
SourceHost: testHostCDN,
|
||||
SourcePath: tt.path1,
|
||||
SourceQuery: tt.query1,
|
||||
Size: Size{Width: 800, Height: 600},
|
||||
Format: FormatWebP,
|
||||
Quality: 85,
|
||||
FitMode: FitCover,
|
||||
})
|
||||
key2 := CacheKey(&ImageRequest{
|
||||
SourceHost: testHostCDN,
|
||||
SourcePath: tt.path2,
|
||||
SourceQuery: tt.query2,
|
||||
Size: Size{Width: 800, Height: 600},
|
||||
Format: FormatWebP,
|
||||
Quality: 85,
|
||||
FitMode: FitCover,
|
||||
})
|
||||
|
||||
t.Logf("CacheKey() of path %q query %q = %s", tt.path1, tt.query1, key1)
|
||||
t.Logf("CacheKey() of path %q query %q = %s", tt.path2, tt.query2, key2)
|
||||
|
||||
if key1 == key2 {
|
||||
t.Errorf("CacheKey() gave path %q query %q and path %q query %q "+
|
||||
"the same key", tt.path1, tt.query1, tt.path2, tt.query2)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestVariantStorage_StoreLogsFailedMetaWrite verifies that a .meta write
|
||||
// that fails is logged at warn and the store still succeeds.
|
||||
func TestVariantStorage_StoreLogsFailedMetaWrite(t *testing.T) {
|
||||
|
||||
@@ -15,6 +15,7 @@ import (
|
||||
var (
|
||||
ErrInvalidPath = errors.New("invalid image path")
|
||||
ErrMissingHost = errors.New("missing source host")
|
||||
ErrInvalidHost = errors.New("invalid source host")
|
||||
ErrMissingSize = errors.New("missing size specification")
|
||||
ErrInvalidSize = errors.New("invalid size format")
|
||||
ErrInvalidFormat = errors.New("invalid or unsupported format")
|
||||
@@ -143,6 +144,17 @@ func parseImageComponents(remainder string) (*ParsedURL, error) {
|
||||
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
|
||||
if qIndex := strings.Index(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) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
@@ -0,0 +1,113 @@
|
||||
package signature_test
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"sneak.berlin/go/pixa/internal/signature"
|
||||
)
|
||||
|
||||
// TestSigner_Verify_ColonCannotMovePathQueryBoundary checks that a URL
|
||||
// signed for path "/a:b" with no query does not verify for path "/a" with
|
||||
// query "b:", a different resource on the same host.
|
||||
func TestSigner_Verify_ColonCannotMovePathQueryBoundary(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
signer := signature.New(goldenSigningKey)
|
||||
|
||||
signed := &signature.Request{
|
||||
SourceHost: testHost,
|
||||
SourcePath: "/a:b",
|
||||
Width: 800,
|
||||
Height: 600,
|
||||
Format: testFormatWebP,
|
||||
Quality: 85,
|
||||
FitMode: testFitCover,
|
||||
Expires: time.Now().Add(1 * time.Hour),
|
||||
}
|
||||
signed.Signature = signer.Sign(signed)
|
||||
|
||||
replayed := *signed
|
||||
replayed.SourcePath = "/a"
|
||||
replayed.SourceQuery = "b:"
|
||||
|
||||
t.Logf("signature for path %q query %q: %s",
|
||||
signed.SourcePath, signed.SourceQuery, signed.Signature)
|
||||
t.Logf("signature for path %q query %q: %s",
|
||||
replayed.SourcePath, replayed.SourceQuery, signer.Sign(&replayed))
|
||||
|
||||
err := signer.Verify(&replayed)
|
||||
if !errors.Is(err, signature.ErrInvalid) {
|
||||
t.Errorf("Verify() of path %q query %q with the signature for "+
|
||||
"path %q = %v, want %v",
|
||||
replayed.SourcePath, replayed.SourceQuery, signed.SourcePath,
|
||||
err, signature.ErrInvalid)
|
||||
}
|
||||
}
|
||||
|
||||
// TestSigner_EscapedGoldenVectors pins the signature of requests whose path
|
||||
// or query holds ":" or "%", which are signed with "%" written as "%25" and
|
||||
// ":" as "%3A". The expected values were computed once, outside pixa, from
|
||||
// the signed data in each comment.
|
||||
func TestSigner_EscapedGoldenVectors(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
signer := signature.New(goldenSigningKey)
|
||||
|
||||
vectors := []struct {
|
||||
name string
|
||||
path string
|
||||
query string
|
||||
wantSignature string
|
||||
}{
|
||||
{
|
||||
name: "colon in path",
|
||||
path: "/photos/a:b.jpg",
|
||||
// Signed data:
|
||||
// "cdn.example.com:/photos/a%3Ab.jpg::800:600:webp:1704067200:85:cover"
|
||||
wantSignature: "WnEpZ7bQp1Zvw8WDatlj2vu-4wYKWE3BFBizV8m__6k=",
|
||||
},
|
||||
{
|
||||
name: "colon in query",
|
||||
path: testPath,
|
||||
query: "t=1:2",
|
||||
// Signed data:
|
||||
// "cdn.example.com:/photos/cat.jpg:t=1%3A2:800:600:webp:1704067200:85:cover"
|
||||
wantSignature: "85kRoN8hhr4cCUkEUjNvUsvVYKjYYfWdeffx5TjJjqA=",
|
||||
},
|
||||
{
|
||||
name: "percent in path and query",
|
||||
path: "/photos/a%20b.jpg",
|
||||
query: "q=50%",
|
||||
// Signed data:
|
||||
// "cdn.example.com:/photos/a%2520b.jpg:q=50%25:800:600:webp:1704067200:85:cover"
|
||||
wantSignature: "MjDiiRdgXMrsjqPMsENtD4aG1hp1e-a6zy67vKTX4CQ=",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range vectors {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
req := &signature.Request{
|
||||
SourceHost: testHost,
|
||||
SourcePath: tt.path,
|
||||
SourceQuery: tt.query,
|
||||
Width: 800,
|
||||
Height: 600,
|
||||
Format: testFormatWebP,
|
||||
Quality: 85,
|
||||
FitMode: testFitCover,
|
||||
Expires: time.Unix(goldenExpiresUnix, 0),
|
||||
}
|
||||
|
||||
gotSignature := signer.Sign(req)
|
||||
t.Logf("Sign() of path %q query %q = %s", tt.path, tt.query, gotSignature)
|
||||
|
||||
if gotSignature != tt.wantSignature {
|
||||
t.Errorf("Sign() = %q, want %q", gotSignature, tt.wantSignature)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -10,6 +10,7 @@ import (
|
||||
"fmt"
|
||||
"net/url"
|
||||
"strconv"
|
||||
"strings"
|
||||
"time"
|
||||
)
|
||||
|
||||
@@ -154,15 +155,17 @@ func (s *Signer) GenerateSignedURL(
|
||||
|
||||
// buildSignatureData creates the string to be signed.
|
||||
// Format: "host:path:query:width:height:format:expiration:quality:fit"
|
||||
// All components are used verbatim (exact match). No normalization,
|
||||
// suffix matching, or wildcard expansion is performed. Quality and fit
|
||||
// are the effective transform values, so replaying a signed URL with a
|
||||
// different quality or fit mode fails verification.
|
||||
// The path and query go through escapePercentAndColon, so that a ":" in
|
||||
// either cannot be taken for the one between them; every other component
|
||||
// is used verbatim. No normalization, suffix matching, or wildcard
|
||||
// expansion is performed. Quality and fit are the effective transform
|
||||
// values, so replaying a signed URL with a different quality or fit mode
|
||||
// fails verification.
|
||||
func (s *Signer) buildSignatureData(req *Request) string {
|
||||
return fmt.Sprintf("%s:%s:%s:%d:%d:%s:%d:%d:%s",
|
||||
req.SourceHost,
|
||||
req.SourcePath,
|
||||
req.SourceQuery,
|
||||
escapePercentAndColon(req.SourcePath),
|
||||
escapePercentAndColon(req.SourceQuery),
|
||||
req.Width,
|
||||
req.Height,
|
||||
req.Format,
|
||||
@@ -172,6 +175,14 @@ func (s *Signer) buildSignatureData(req *Request) string {
|
||||
)
|
||||
}
|
||||
|
||||
// escapePercentAndColon returns s with each "%" written as "%25" and each
|
||||
// ":" as "%3A", so that the result holds no ":".
|
||||
func escapePercentAndColon(s string) string {
|
||||
s = strings.ReplaceAll(s, "%", "%25")
|
||||
|
||||
return strings.ReplaceAll(s, ":", "%3A")
|
||||
}
|
||||
|
||||
// ParseParams extracts signature and expiration from query parameters.
|
||||
func ParseParams(sig, expStr string) (string, time.Time, error) {
|
||||
if expStr == "" {
|
||||
|
||||
Reference in New Issue
Block a user