Compare commits
5
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
58da57ca49 | ||
|
|
98bce3bb1a | ||
|
|
01434ecf3b | ||
|
|
88fe5c6fed | ||
|
|
fe33fc16c7 |
@@ -418,9 +418,7 @@ HMAC-SHA256(secret, "host:path:query:width:height:format:expiration:quality:fit"
|
||||
|
||||
Where:
|
||||
|
||||
- `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`)
|
||||
- `host` — source origin hostname (e.g. `cdn.example.com`)
|
||||
- `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
|
||||
@@ -475,12 +473,6 @@ 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,17 +30,6 @@ P2: security: per-origin rate limiting
|
||||
|
||||
# Completed Steps
|
||||
|
||||
- 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
|
||||
@@ -49,6 +38,17 @@ P2: security: per-origin rate limiting
|
||||
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 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
|
||||
`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
|
||||
|
||||
@@ -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
|
||||
|
||||
+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,242 @@ func TestNegativeCache_Expired(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// 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,7 +9,6 @@ import (
|
||||
"log/slog"
|
||||
"net/url"
|
||||
"runtime/debug"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/dustin/go-humanize"
|
||||
@@ -219,16 +218,7 @@ func (s *Service) ValidateRequest(req *ImageRequest) error {
|
||||
return fmt.Errorf("invalid source URL: %w", err)
|
||||
}
|
||||
|
||||
// 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) {
|
||||
if s.allowlist.IsAllowed(parsedURL) {
|
||||
return nil
|
||||
}
|
||||
|
||||
|
||||
@@ -250,97 +250,6 @@ 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()
|
||||
|
||||
|
||||
@@ -15,7 +15,6 @@ 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")
|
||||
@@ -144,17 +143,6 @@ 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,49 +234,6 @@ 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()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user