5 Commits
Author SHA1 Message Date
clawbot 6d6c76937b Bound concurrent image processing and upstream fetches (closes #64)
check / check (push) Successful in 4m3s
max_concurrent_processing (default: the number of CPUs Go uses) bounds
the images processed at once, and upstream_connections (default 64) the
fetches from all upstream hosts together, beside the per-host limit. A
request that finds either full waits up to 10 seconds, then gets 503
"server busy, try again later". The processor holds its slot from before
it reads the input until it returns, and takes a free slot even after the
request context has ended; a fetch holds its connection until the
response body is closed, after its image is processed. libvips now starts
with one worker thread per image and no operation cache. Both settings
have PIXA_ variables and are in README.md and config.example.yml.

Model: opus-5-5
2026-09-29 03:31:44 +00:00
clawbot 23ff05e293 Test the processing and upstream connection limits (closes #64)
Failing tests for two limits that do not exist yet. Config:
max_concurrent_processing and upstream_connections, their defaults, and
valid and invalid values from the file and the environment. Image
processor: never more images at once than its limit, waiting and then
failing with ErrTooManyImages when no slot frees, and freeing its slot on
every error. Fetcher: connections to all hosts counted together, apart
from the per-host limit, and freed on errors. Both image routes answer
503 when either wait gives up. TestEnvironmentSetsEveryKey sets the two
new variables, as it compares the whole config. The tests do not compile
until the limits exist.

Model: opus-5-5
2026-09-29 03:26:28 +00:00
clawbot e98b998cb6 Make the cache stats count what is cached, fetched and transcoded (closes #56)
check / check (push) Successful in 18s
Cache.Stats read request_cache and output_content, which nothing
writes, so TotalItems and TotalSizeBytes were always 0. They now count
source_content plus variant_content and use UsageBytes; a disabled disk
cache reports 0 for both. The upstream fetch count and bytes and the
transform count never moved: Get now passes the bytes it fetched
(including those read before a failed body read) and counts each
successful transcode. Hits, misses and these counters are written with
context.WithoutCancel, so a client disconnect or the request timeout no
longer loses them. The unused tables stay; metaCache is #70.

Model: opus-5-5
2026-09-29 05:25:10 +02:00
clawbot ed3f8770e6 Give pixad a fixed uid and gid 65532 (closes #151)
check / check (push) Successful in 20s
adduser took the first free uid, 1000, and the entrypoint gives a
bind-mounted /var/lib/pixa to pixad, so on the host a person's login
account ended up owning pixa's database and cache. The image now creates
the pixad group with gid 65532 and the pixad user with uid 65532, which
host login and system accounts do not use. The first-run step of
"Running under upaas" in README.md names the uid and gid.

Model: opus-5-5
2026-09-29 04:44:49 +02:00
clawbot 2afe61e301 Keep max-age within an expiring image URL's lifetime (closes #63)
check / check (push) Successful in 12s
Both image routes sent Cache-Control: public, max-age=31536000,
immutable unconditionally, so a browser or proxy could keep serving an
image for a year after its signed or encrypted URL had expired. max-age
is now the whole seconds left until the URL expires, never negative and
at most one year; a URL with no expiry keeps one year. The 304 answer
uses the same value. An encrypted URL's expiry now reaches
ImageRequest.Expires through ToImageRequest. immutable stays: freshness
now ends no later than the URL's expiry. README.md documents the header.

Model: opus-5-5
2026-09-29 03:51:58 +02:00
22 changed files with 1563 additions and 102 deletions
+6 -2
View File
@@ -68,8 +68,12 @@ RUN apk add --no-cache \
COPY --from=builder /pixad /usr/local/bin/pixad COPY --from=builder /pixad /usr/local/bin/pixad
COPY deploy/docker-entrypoint.sh /usr/local/bin/docker-entrypoint.sh COPY deploy/docker-entrypoint.sh /usr/local/bin/docker-entrypoint.sh
# Create non-root user, config directory, and data directory # Create non-root user, config directory, and data directory. pixad
RUN adduser -D -H -s /sbin/nologin pixad && \ # gets uid and gid 65532, which host login and system accounts do not
# use: a bind-mounted /var/lib/pixa is given to pixad, and on the host
# it must not belong to a person's account.
RUN addgroup -g 65532 pixad && \
adduser -D -H -s /sbin/nologin -u 65532 -G pixad pixad && \
mkdir -p /var/lib/pixa /etc/pixa && \ mkdir -p /var/lib/pixa /etc/pixa && \
chown pixad:pixad /var/lib/pixa chown pixad:pixad /var/lib/pixa
+23 -2
View File
@@ -58,8 +58,10 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for pixa needs:
`healthy`. The probe uses the port from `PORT` (default `8080`), so a `healthy`. The probe uses the port from `PORT` (default `8080`), so a
port changed only in a mounted config file is not seen by it: change port changed only in a mounted config file is not seen by it: change
the port with `PORT`. the port with `PORT`.
- **First run:** create the host directory. It may be owned by root: the - **First run:** create the host directory, owned by root or by uid
container gives it to its `pixad` user when it starts. `65532` and gid `65532`. The server runs as the container's `pixad`
user, which has that uid and gid, and the container gives the
directory to `pixad` when it starts.
## Rationale ## Rationale
@@ -100,6 +102,13 @@ than once, is refused with 400.
- `<format>`: one of `orig`, `png`, `jpeg`, `webp` - `<format>`: one of `orig`, `png`, `jpeg`, `webp`
- `<size>`: `orig` or `<width>x<height>` (e.g. `800x600`) - `<size>`: `orig` or `<width>x<height>` (e.g. `800x600`)
An image is served with `Cache-Control: public, max-age=<seconds>, immutable`.
When the URL has an expiry (an `exp`, or the TTL of an encrypted URL),
`max-age` is the whole seconds left until then, at most one year, so no browser
or proxy cache keeps the image after pixa would refuse the URL. A URL with no
expiry gets one year. `immutable` only stops a client revalidating while its
copy is fresh.
The login form (`POST /`) is limited to 5 attempts per minute per client The login form (`POST /`) is limited to 5 attempts per minute per client
address, counting an IPv6 client by its /64; an attempt over the limit is address, counting an IPv6 client by its /64; an attempt over the limit is
refused with 429 and a `Retry-After` header. Behind a reverse proxy the client refused with 429 and a `Retry-After` header. Behind a reverse proxy the client
@@ -212,6 +221,8 @@ variables set by the file's `env:` section are checked the same way.
| `PIXA_TRUSTED_PROXIES` | `trusted_proxies` | CIDR ranges of proxies whose `X-Forwarded-For` is believed; default RFC 1918 | | `PIXA_TRUSTED_PROXIES` | `trusted_proxies` | CIDR ranges of proxies whose `X-Forwarded-For` is believed; default RFC 1918 |
| `PIXA_ALLOW_HTTP` | `allow_http` | Allow plain-HTTP upstreams, for testing only; default `false` | | `PIXA_ALLOW_HTTP` | `allow_http` | Allow plain-HTTP upstreams, for testing only; default `false` |
| `PIXA_UPSTREAM_CONNECTIONS_PER_HOST` | `upstream_connections_per_host` | Concurrent connections per upstream host; default `20` | | `PIXA_UPSTREAM_CONNECTIONS_PER_HOST` | `upstream_connections_per_host` | Concurrent connections per upstream host; default `20` |
| `PIXA_UPSTREAM_CONNECTIONS` | `upstream_connections` | Concurrent connections to all upstream hosts together; default `64` |
| `PIXA_MAX_CONCURRENT_PROCESSING` | `max_concurrent_processing` | Images processed at once; default the number of CPUs |
| `PIXA_METRICS_USERNAME` | `metrics.username` | Username for `/metrics`, which is served only when both are set | | `PIXA_METRICS_USERNAME` | `metrics.username` | Username for `/metrics`, which is served only when both are set |
| `PIXA_METRICS_PASSWORD` | `metrics.password` | Password for `/metrics`; set together with the username | | `PIXA_METRICS_PASSWORD` | `metrics.password` | Password for `/metrics`; set together with the username |
| `PIXA_SENTRY_DSN` | `sentry_dsn` | Sentry DSN for error reporting; empty disables it | | `PIXA_SENTRY_DSN` | `sentry_dsn` | Sentry DSN for error reporting; empty disables it |
@@ -246,6 +257,16 @@ Key settings in more detail:
- `cache_max_bytes` — disk cache size limit in bytes; `0` disables the - `cache_max_bytes` — disk cache size limit in bytes; `0` disables the
disk cache entirely; omitted defaults to 75% of the free space on disk cache entirely; omitted defaults to 75% of the free space on
the filesystem containing `<state_dir>/cache/` (minimum 500 MiB) the filesystem containing `<state_dir>/cache/` (minimum 500 MiB)
- `upstream_connections` — the most connections to upstream hosts at once, all
hosts together, on top of `upstream_connections_per_host`; default `64`. A
fetch holds its connection until its image has been processed. A fetch that
finds all of them in use waits up to 10 seconds for one to free up; if none
does, the request is answered 503 with the error
`server busy, try again later`
- `max_concurrent_processing` — the most images decoded and encoded at once;
default the number of CPUs pixa can use (`GOMAXPROCS`), which follows a
container's CPU limit. A request that finds all of them in use waits up to 10
seconds for one to free up; if none does, it is answered 503 the same way
See `config.example.yml` for all options with defaults. See `config.example.yml` for all options with defaults.
+34 -3
View File
@@ -25,11 +25,43 @@ The disk cache is now size-bounded with LRU eviction
# Next Step # Next Step
P1: rate limit global concurrent upstream fetches to prevent resource P2: security: referer blacklist
exhaustion
# Completed Steps # Completed Steps
- 2026-09-29 bound concurrent image processing and upstream fetches (closes
#64): `max_concurrent_processing` (default the number of CPUs pixa can use)
limits the images decoded and encoded at once, and `upstream_connections`
(default 64) the connections to all upstream hosts together, on top of
`upstream_connections_per_host`; a fetch holds its connection until its image
has been processed; a request that finds either limit reached waits up to 10
seconds for a free one, then gets 503 `server busy, try again later`; libvips
runs one worker thread per image with its operation cache off; documented in
`README.md` and `config.example.yml`.
- 2026-09-29 fixed uid and gid for `pixad` (closes #151): the image creates the
`pixad` group with gid 65532 and the `pixad` user with uid 65532, instead of
the first free uid 1000, so a bind-mounted `/var/lib/pixa` given to `pixad`
is not owned on the host by a person's login account; the first-run step of
"Running under upaas" in `README.md` names the uid and gid.
- 2026-09-29 `max-age` never outlives an expiring URL (closes #63): both image
routes build `Cache-Control` from the request's `Expires`, which an encrypted
URL's expiry now fills too; `max-age` is one year, or the whole seconds left
until the `exp` of a `/v1/image/` URL or the expiry of an encrypted URL when
that is sooner, never negative; an allowlisted host's URL that has an `exp`
follows it too; `immutable` stays, as freshness now ends at the expiry;
documented in `README.md`.
- 2026-09-28 cache stats report real numbers (closes #56): `Cache.Stats`
counts the cached source images and processed variants (`source_content`
plus `variant_content`) and takes their size from `Cache.UsageBytes`,
instead of reading `request_cache` and `output_content`, which nothing
writes; those two tables are left in the schema; a disabled disk cache
reports no items and no size. A hit is counted even when the request
context has ended. A miss is counted after it is served or fails, also
when the request context has ended by then, with the bytes it read from
upstream, so `upstream_fetch_count` and `upstream_fetch_bytes` move,
including for an upstream body that fails partway or a fetched source
that then fails the magic byte check; `transform_count` counts each image
the image processor transcodes.
- 2026-09-28 strip metadata from processed images (closes #82): every output is - 2026-09-28 strip metadata from processed images (closes #82): every output is
exported with govips' `StripMetadata`, so it carries no EXIF, XMP, IPTC or ICC exported with govips' `StripMetadata`, so it carries no EXIF, XMP, IPTC or ICC
profile; the image is first turned upright with `AutoRotate` (before sizes are profile; the image is first turned upright with `AutoRotate` (before sizes are
@@ -258,7 +290,6 @@ exhaustion
# Future Steps # Future Steps
- P2: security - P2: security
- referer blacklist
- per-IP rate limiting on the image routes - per-IP rate limiting on the image routes
- per-origin rate limiting - per-origin rate limiting
- P2: HTTP response handling - P2: HTTP response handling
+12
View File
@@ -61,6 +61,18 @@ allow_http: false
# Maximum concurrent connections per upstream host (default: 20) # Maximum concurrent connections per upstream host (default: 20)
upstream_connections_per_host: 20 upstream_connections_per_host: 20
# Maximum concurrent connections to all upstream hosts together, on top of
# the per-host limit (default: 64). A fetch holds its connection until its
# image has been processed. A fetch that finds none free waits up to 10
# seconds for one, and if none frees up the request is answered 503.
upstream_connections: 64
# Maximum number of images decoded and encoded at once (default: the
# number of CPUs pixa can use, which follows a container's CPU limit). A
# request that finds none free waits up to 10 seconds for one, and if none
# frees up it is answered 503.
# max_concurrent_processing: 4
# Maximum disk cache size in bytes. Explicit values are used exactly as # Maximum disk cache size in bytes. Explicit values are used exactly as
# given; 0 disables the disk cache entirely (every request fetches and # given; 0 disables the disk cache entirely (every request fetches and
# processes uncached). When omitted, the default is 75% of the free # processes uncached). When omitted, the default is 75% of the free
@@ -0,0 +1,162 @@
package config
import (
"runtime"
"testing"
)
// The variables that set the two concurrency limits.
const (
testMaxConcurrentProcessingVar = "PIXA_MAX_CONCURRENT_PROCESSING"
testUpstreamConnectionsVar = "PIXA_UPSTREAM_CONNECTIONS"
)
// TestOmittedConcurrencyLimitsUseDefaults checks that an omitted
// max_concurrent_processing is the number of CPUs Go uses and an omitted
// upstream_connections is 64.
func TestOmittedConcurrencyLimitsUseDefaults(t *testing.T) {
t.Parallel()
c, err := configFromYAML(t, signingKeyLine)
if err != nil {
t.Fatalf("minimal config should be valid, got error: %v", err)
}
if c.MaxConcurrentProcessing != runtime.GOMAXPROCS(0) {
t.Errorf("MaxConcurrentProcessing = %d, want %d, one per CPU",
c.MaxConcurrentProcessing, runtime.GOMAXPROCS(0))
}
if c.UpstreamConnections != 64 {
t.Errorf("UpstreamConnections = %d, want 64", c.UpstreamConnections)
}
}
// TestExplicitConcurrencyLimitsAreUsed checks that valid values for the
// two limits are used as given.
func TestExplicitConcurrencyLimitsAreUsed(t *testing.T) {
t.Parallel()
c, err := configFromYAML(t, signingKeyLine+
"max_concurrent_processing: 3\nupstream_connections: 10\n")
if err != nil {
t.Fatalf("valid config should load, got error: %v", err)
}
if c.MaxConcurrentProcessing != 3 {
t.Errorf("MaxConcurrentProcessing = %d, want 3", c.MaxConcurrentProcessing)
}
if c.UpstreamConnections != 10 {
t.Errorf("UpstreamConnections = %d, want 10", c.UpstreamConnections)
}
}
// TestInvalidConcurrencyLimitAbortsStartup checks that a limit that is
// not a whole number of at least 1, or is null, aborts startup naming the
// key and the value, and the variable too where the value could have come
// from it.
func TestInvalidConcurrencyLimitAbortsStartup(t *testing.T) {
t.Parallel()
processing := keyMaxConcurrentProcessing
connections := keyUpstreamConnections
runAbortCases(t, []abortCase{
{
name: "max_concurrent_processing zero",
yaml: signingKeyLine + processing + ": 0\n",
wantErrSubstrings: []string{
processing, testMaxConcurrentProcessingVar, "value 0",
},
},
{
name: "max_concurrent_processing negative",
yaml: signingKeyLine + processing + ": -2\n",
wantErrSubstrings: []string{
processing, testMaxConcurrentProcessingVar, "value -2",
},
},
{
name: "max_concurrent_processing not a number",
yaml: signingKeyLine + processing + ": lots\n",
wantErrSubstrings: []string{
processing, testMaxConcurrentProcessingVar, "lots",
},
},
{
name: "max_concurrent_processing fractional",
yaml: signingKeyLine + processing + ": 1.5\n",
wantErrSubstrings: []string{processing, "1.5"},
},
{
name: "max_concurrent_processing null",
yaml: signingKeyLine + processing + ": null\n",
wantErrSubstrings: []string{processing, nullValueText},
},
{
name: "upstream_connections zero",
yaml: signingKeyLine + connections + ": 0\n",
wantErrSubstrings: []string{
connections, testUpstreamConnectionsVar, "value 0",
},
},
{
name: "upstream_connections negative",
yaml: signingKeyLine + connections + ": -5\n",
wantErrSubstrings: []string{
connections, testUpstreamConnectionsVar, "value -5",
},
},
{
name: "upstream_connections not a number",
yaml: signingKeyLine + connections + ": many\n",
wantErrSubstrings: []string{
connections, testUpstreamConnectionsVar, "many",
},
},
{
name: "upstream_connections null",
yaml: signingKeyLine + connections + ": null\n",
wantErrSubstrings: []string{connections, nullValueText},
},
})
}
// TestConcurrencyLimitsFromEnvironment checks that the two variables set
// the limits over the config file, and that an invalid value in either
// aborts startup naming the variable and the value.
func TestConcurrencyLimitsFromEnvironment(t *testing.T) {
t.Setenv(testMaxConcurrentProcessingVar, "3")
t.Setenv(testUpstreamConnectionsVar, "10")
c, err := configFromYAML(t, signingKeyLine+
"max_concurrent_processing: 5\nupstream_connections: 50\n")
if err != nil {
t.Fatalf("limits from the environment should load: %v", err)
}
if c.MaxConcurrentProcessing != 3 || c.UpstreamConnections != 10 {
t.Errorf("limits = %d and %d, want 3 and 10 from the environment",
c.MaxConcurrentProcessing, c.UpstreamConnections)
}
cases := []struct {
variable string
value string
}{
{testMaxConcurrentProcessingVar, "lots"},
{testMaxConcurrentProcessingVar, "0"},
{testUpstreamConnectionsVar, "-1"},
{testUpstreamConnectionsVar, "ten"},
}
for _, tc := range cases {
t.Run(tc.variable+"="+tc.value, func(t *testing.T) {
t.Setenv(tc.variable, tc.value)
_, err := configFromYAML(t, signingKeyLine)
wantStartupError(t, err, tc.variable, tc.value)
})
}
}
+48 -6
View File
@@ -10,6 +10,7 @@ import (
"net/url" "net/url"
"os" "os"
"path/filepath" "path/filepath"
"runtime"
"sort" "sort"
"strconv" "strconv"
"strings" "strings"
@@ -24,6 +25,7 @@ const (
DefaultPort = 8080 DefaultPort = 8080
DefaultStateDir = "/var/lib/pixa" DefaultStateDir = "/var/lib/pixa"
DefaultUpstreamConnectionsPerHost = 20 DefaultUpstreamConnectionsPerHost = 20
DefaultUpstreamConnections = 64
) )
// Configuration key names. // Configuration key names.
@@ -41,6 +43,8 @@ const (
keyAllowlistHosts = "allowlist_hosts" keyAllowlistHosts = "allowlist_hosts"
keyAllowHTTP = "allow_http" keyAllowHTTP = "allow_http"
keyUpstreamConnectionsPerHost = "upstream_connections_per_host" keyUpstreamConnectionsPerHost = "upstream_connections_per_host"
keyUpstreamConnections = "upstream_connections"
keyMaxConcurrentProcessing = "max_concurrent_processing"
keyCacheMaxBytes = "cache_max_bytes" keyCacheMaxBytes = "cache_max_bytes"
keyBlockedNetworks = "blocked_networks" keyBlockedNetworks = "blocked_networks"
keyTrustedProxies = "trusted_proxies" keyTrustedProxies = "trusted_proxies"
@@ -69,7 +73,7 @@ var (
errEmptyEntry = errors.New("contains an empty entry") errEmptyEntry = errors.New("contains an empty entry")
errNotAValidURL = errors.New("not a valid URL") errNotAValidURL = errors.New("not a valid URL")
errPortOutOfRange = errors.New("outside the valid port range") errPortOutOfRange = errors.New("outside the valid port range")
errTooFewConnections = errors.New("must be at least 1") errMustBeAtLeastOne = errors.New("must be at least 1")
errValueTooShort = errors.New("value too short") errValueTooShort = errors.New("value too short")
errPlaceholderKey = errors.New( errPlaceholderKey = errors.New(
"is the placeholder from config.example.yml; " + "is the placeholder from config.example.yml; " +
@@ -112,6 +116,12 @@ type Config struct {
AllowHTTP bool // Allow non-TLS upstream (testing only) AllowHTTP bool // Allow non-TLS upstream (testing only)
UpstreamConnectionsPerHost int // Max concurrent connections per upstream host UpstreamConnectionsPerHost int // Max concurrent connections per upstream host
// UpstreamConnections is the most concurrent connections to all
// upstream hosts together, on top of the per-host limit.
// MaxConcurrentProcessing is the most images processed at once.
UpstreamConnections int
MaxConcurrentProcessing int
// BlockedNetworks are operator-supplied CIDR ranges to refuse in // BlockedNetworks are operator-supplied CIDR ranges to refuse in
// addition to the built-in SSRF blocklist. Enforced by the upstream // addition to the built-in SSRF blocklist. Enforced by the upstream
// fetcher's dialer; the built-in ranges always apply. // fetcher's dialer; the built-in ranges always apply.
@@ -240,6 +250,12 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
AllowHTTP: loader.boolVal(keyAllowHTTP, false), AllowHTTP: loader.boolVal(keyAllowHTTP, false),
UpstreamConnectionsPerHost: loader.intVal( UpstreamConnectionsPerHost: loader.intVal(
keyUpstreamConnectionsPerHost, DefaultUpstreamConnectionsPerHost), keyUpstreamConnectionsPerHost, DefaultUpstreamConnectionsPerHost),
UpstreamConnections: loader.intVal(
keyUpstreamConnections, DefaultUpstreamConnections),
// Decoding and encoding are CPU-bound, so the default is one image
// per CPU Go uses, which follows a container's CPU limit.
MaxConcurrentProcessing: loader.intVal(
keyMaxConcurrentProcessing, runtime.GOMAXPROCS(0)),
CacheMaxBytes: loader.int64Val(keyCacheMaxBytes, 0), CacheMaxBytes: loader.int64Val(keyCacheMaxBytes, 0),
BlockedNetworks: blockedNetworks, BlockedNetworks: blockedNetworks,
TrustedProxies: trustedProxies, TrustedProxies: trustedProxies,
@@ -353,7 +369,8 @@ func isKnownConfigKey(key string) bool {
switch key { switch key {
case keyDebug, keyMaintenanceMode, keyPort, keyStateDir, keySentryDSN, case keyDebug, keyMaintenanceMode, keyPort, keyStateDir, keySentryDSN,
keyDBURL, keyMetrics, keySigningKey, keyAllowlistHosts, keyAllowHTTP, keyDBURL, keyMetrics, keySigningKey, keyAllowlistHosts, keyAllowHTTP,
keyUpstreamConnectionsPerHost, keyCacheMaxBytes, keyBlockedNetworks, keyUpstreamConnectionsPerHost, keyUpstreamConnections,
keyMaxConcurrentProcessing, keyCacheMaxBytes, keyBlockedNetworks,
keyTrustedProxies, "env": keyTrustedProxies, "env":
return true return true
} }
@@ -379,6 +396,8 @@ func envVarNames() map[string]string {
keyAllowlistHosts: "PIXA_ALLOWLIST_HOSTS", keyAllowlistHosts: "PIXA_ALLOWLIST_HOSTS",
keyAllowHTTP: "PIXA_ALLOW_HTTP", keyAllowHTTP: "PIXA_ALLOW_HTTP",
keyUpstreamConnectionsPerHost: "PIXA_UPSTREAM_CONNECTIONS_PER_HOST", keyUpstreamConnectionsPerHost: "PIXA_UPSTREAM_CONNECTIONS_PER_HOST",
keyUpstreamConnections: "PIXA_UPSTREAM_CONNECTIONS",
keyMaxConcurrentProcessing: "PIXA_MAX_CONCURRENT_PROCESSING",
keyCacheMaxBytes: "PIXA_CACHE_MAX_BYTES", keyCacheMaxBytes: "PIXA_CACHE_MAX_BYTES",
keyBlockedNetworks: "PIXA_BLOCKED_NETWORKS", keyBlockedNetworks: "PIXA_BLOCKED_NETWORKS",
keyTrustedProxies: "PIXA_TRUSTED_PROXIES", keyTrustedProxies: "PIXA_TRUSTED_PROXIES",
@@ -518,10 +537,9 @@ func (c *Config) validate() error {
settingName(keyPort), c.Port, errPortOutOfRange, maxPort) settingName(keyPort), c.Port, errPortOutOfRange, maxPort)
} }
if c.UpstreamConnectionsPerHost < 1 { err = c.validateConcurrencyLimits()
return fmt.Errorf("%s: value %d %w", if err != nil {
settingName(keyUpstreamConnectionsPerHost), return err
c.UpstreamConnectionsPerHost, errTooFewConnections)
} }
if c.StateDir == "" { if c.StateDir == "" {
@@ -559,6 +577,30 @@ func (c *Config) validate() error {
return nil return nil
} }
// validateConcurrencyLimits checks that the two upstream connection limits
// and the image processing limit are at least 1.
func (c *Config) validateConcurrencyLimits() error {
if c.UpstreamConnectionsPerHost < 1 {
return fmt.Errorf("%s: value %d %w",
settingName(keyUpstreamConnectionsPerHost),
c.UpstreamConnectionsPerHost, errMustBeAtLeastOne)
}
if c.UpstreamConnections < 1 {
return fmt.Errorf("%s: value %d %w",
settingName(keyUpstreamConnections),
c.UpstreamConnections, errMustBeAtLeastOne)
}
if c.MaxConcurrentProcessing < 1 {
return fmt.Errorf("%s: value %d %w",
settingName(keyMaxConcurrentProcessing),
c.MaxConcurrentProcessing, errMustBeAtLeastOne)
}
return nil
}
// validateAllowlistHost checks that an allowlist_hosts entry is a bare // validateAllowlistHost checks that an allowlist_hosts entry is a bare
// hostname, optionally with a leading dot for suffix matching. URLs, // hostname, optionally with a leading dot for suffix matching. URLs,
// paths, and whitespace indicate a misconfigured entry. An entry with // paths, and whitespace indicate a misconfigured entry. An entry with
+4
View File
@@ -66,6 +66,8 @@ func TestEnvironmentSetsEveryKey(t *testing.T) {
t.Setenv("PIXA_ALLOWLIST_HOSTS", "s3.sneak.cloud,.example.com") t.Setenv("PIXA_ALLOWLIST_HOSTS", "s3.sneak.cloud,.example.com")
t.Setenv("PIXA_ALLOW_HTTP", "true") t.Setenv("PIXA_ALLOW_HTTP", "true")
t.Setenv("PIXA_UPSTREAM_CONNECTIONS_PER_HOST", "5") t.Setenv("PIXA_UPSTREAM_CONNECTIONS_PER_HOST", "5")
t.Setenv("PIXA_UPSTREAM_CONNECTIONS", "10")
t.Setenv("PIXA_MAX_CONCURRENT_PROCESSING", "3")
t.Setenv("PIXA_CACHE_MAX_BYTES", "1024") t.Setenv("PIXA_CACHE_MAX_BYTES", "1024")
t.Setenv("PIXA_BLOCKED_NETWORKS", "203.0.113.0/24") t.Setenv("PIXA_BLOCKED_NETWORKS", "203.0.113.0/24")
t.Setenv("PIXA_TRUSTED_PROXIES", "192.0.2.0/24") t.Setenv("PIXA_TRUSTED_PROXIES", "192.0.2.0/24")
@@ -88,6 +90,8 @@ func TestEnvironmentSetsEveryKey(t *testing.T) {
AllowlistHosts: []string{testHostS3, ".example.com"}, AllowlistHosts: []string{testHostS3, ".example.com"},
AllowHTTP: true, AllowHTTP: true,
UpstreamConnectionsPerHost: 5, UpstreamConnectionsPerHost: 5,
UpstreamConnections: 10,
MaxConcurrentProcessing: 3,
CacheMaxBytes: 1024, CacheMaxBytes: 1024,
cacheMaxBytesExplicit: true, cacheMaxBytesExplicit: true,
BlockedNetworks: []netip.Prefix{netip.MustParsePrefix("203.0.113.0/24")}, BlockedNetworks: []netip.Prefix{netip.MustParsePrefix("203.0.113.0/24")},
+8 -1
View File
@@ -103,7 +103,8 @@ func (g *Generator) Parse(token string) (*Payload, error) {
} }
// ToImageRequest converts the payload to an ImageRequest. // ToImageRequest converts the payload to an ImageRequest.
// Applies default values for omitted optional fields. // Applies default values for omitted optional fields. An ExpiresAt of 0, a URL
// that never expires, gives the zero Expires.
func (p *Payload) ToImageRequest() *imgcache.ImageRequest { func (p *Payload) ToImageRequest() *imgcache.ImageRequest {
format := p.Format format := p.Format
if format == "" { if format == "" {
@@ -120,6 +121,11 @@ func (p *Payload) ToImageRequest() *imgcache.ImageRequest {
fitMode = DefaultFitMode fitMode = DefaultFitMode
} }
var expires time.Time
if p.ExpiresAt != 0 {
expires = time.Unix(p.ExpiresAt, 0)
}
return &imgcache.ImageRequest{ return &imgcache.ImageRequest{
SourceHost: p.SourceHost, SourceHost: p.SourceHost,
SourcePath: p.SourcePath, SourcePath: p.SourcePath,
@@ -131,6 +137,7 @@ func (p *Payload) ToImageRequest() *imgcache.ImageRequest {
Format: format, Format: format,
Quality: quality, Quality: quality,
FitMode: fitMode, FitMode: fitMode,
Expires: expires,
} }
} }
+7 -5
View File
@@ -111,15 +111,17 @@ func (s *Handlers) initImageService() error {
fetcherCfg.MaxConnectionsPerHost = s.config.UpstreamConnectionsPerHost fetcherCfg.MaxConnectionsPerHost = s.config.UpstreamConnectionsPerHost
} }
fetcherCfg.MaxConnections = s.config.UpstreamConnections
fetcherCfg.BlockedNetworks = s.config.BlockedNetworks fetcherCfg.BlockedNetworks = s.config.BlockedNetworks
// Create the service // Create the service
svc, err := imgcache.NewService(&imgcache.ServiceConfig{ svc, err := imgcache.NewService(&imgcache.ServiceConfig{
Cache: cache, Cache: cache,
FetcherConfig: fetcherCfg, FetcherConfig: fetcherCfg,
SigningKey: s.config.SigningKey, SigningKey: s.config.SigningKey,
Allowlist: s.config.AllowlistHosts, Allowlist: s.config.AllowlistHosts,
Logger: s.log, MaxConcurrentProcessing: s.config.MaxConcurrentProcessing,
Logger: s.log,
}) })
if err != nil { if err != nil {
return err return err
+28 -1
View File
@@ -12,6 +12,7 @@ import (
"github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5"
"sneak.berlin/go/pixa/internal/encurl" "sneak.berlin/go/pixa/internal/encurl"
"sneak.berlin/go/pixa/internal/httpfetcher" "sneak.berlin/go/pixa/internal/httpfetcher"
"sneak.berlin/go/pixa/internal/imageprocessor"
"sneak.berlin/go/pixa/internal/imgcache" "sneak.berlin/go/pixa/internal/imgcache"
) )
@@ -217,9 +218,35 @@ func (s *Handlers) respondImageError(
return return
} }
if errors.Is(err, httpfetcher.ErrTooManyConnections) ||
errors.Is(err, imageprocessor.ErrTooManyImages) {
s.respondError(w, "server busy, try again later",
http.StatusServiceUnavailable)
return
}
s.respondError(w, "internal error", http.StatusInternalServerError) s.respondError(w, "internal error", http.StatusInternalServerError)
} }
// cacheControl returns the Cache-Control header for an image served through a
// URL that expires at expires, or never when expires is the zero time. A cache
// may keep the image for a year, but not past the URL's expiry, after which
// pixa refuses the URL. The seconds left are rounded down and never negative.
// immutable only stops revalidation while the image is fresh, so it also ends
// at the expiry.
func cacheControl(expires time.Time) string {
const oneYear = 365 * 24 * time.Hour
maxAge := oneYear
if !expires.IsZero() {
maxAge = min(maxAge, max(time.Until(expires), 0))
}
return fmt.Sprintf("public, max-age=%d, immutable", int64(maxAge/time.Second))
}
// writeImageResponse writes headers and streams the image content, // writeImageResponse writes headers and streams the image content,
// handling conditional and HEAD requests. // handling conditional and HEAD requests.
func (s *Handlers) writeImageResponse( func (s *Handlers) writeImageResponse(
@@ -235,7 +262,7 @@ func (s *Handlers) writeImageResponse(
} }
// Cache control headers // Cache control headers
w.Header().Set("Cache-Control", "public, max-age=31536000, immutable") w.Header().Set("Cache-Control", cacheControl(req.Expires))
w.Header().Set("X-Pixa-Cache", string(resp.CacheStatus)) w.Header().Set("X-Pixa-Cache", string(resp.CacheStatus))
if resp.ETag != "" { if resp.ETag != "" {
@@ -0,0 +1,203 @@
package handlers
import (
"image/color"
"log/slog"
"net/http"
"net/http/httptest"
"strconv"
"strings"
"testing"
"testing/fstest"
"time"
"github.com/go-chi/chi/v5"
"sneak.berlin/go/pixa/internal/encurl"
"sneak.berlin/go/pixa/internal/imgcache"
)
// photoPath is the path of the JPEG that newSignedHostServer serves.
const photoPath = "/images/photo.jpg"
// newSignedHostServer returns a router for both image routes, and the Handlers
// behind it, whose fetcher serves a JPEG at photoPath on signedHost. signedHost
// is not on the allowlist, so a /v1/image/ URL for it is served only with a
// valid signature.
func newSignedHostServer(t *testing.T) (*Handlers, http.Handler) {
t.Helper()
cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{
StateDir: t.TempDir(),
CacheTTL: time.Hour,
NegativeTTL: 5 * time.Minute,
})
if err != nil {
t.Fatalf("imgcache.NewCache() error = %v", err)
}
jpegData := generateTestJPEG(t, 100, 100, color.RGBA{255, 0, 0, 255})
svc, err := imgcache.NewService(&imgcache.ServiceConfig{
Cache: cache,
Fetcher: newMockFetcher(fstest.MapFS{
signedHost + photoPath: &fstest.MapFile{Data: jpegData},
}),
SigningKey: testSigningKey,
})
if err != nil {
t.Fatalf("imgcache.NewService() error = %v", err)
}
encGen, err := encurl.NewGenerator(testSigningKey)
if err != nil {
t.Fatalf("encurl.NewGenerator() error = %v", err)
}
h := &Handlers{
log: slog.New(slog.DiscardHandler),
imgSvc: svc,
encGen: encGen,
}
r := chi.NewRouter()
r.Get("/v1/image/*", h.HandleImage())
r.Get("/v1/e/{token}/*", h.HandleImageEnc())
return h, r
}
// getMaxAge sends a GET for target to srv, requires a 200, and returns the
// max-age of the response's Cache-Control header, which must read
// "public, max-age=<seconds>, immutable".
func getMaxAge(t *testing.T, srv http.Handler, target string) int {
t.Helper()
req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, target, nil)
rec := httptest.NewRecorder()
srv.ServeHTTP(rec, req)
header := rec.Header().Get("Cache-Control")
t.Logf("GET %s: %d, Cache-Control: %s", target, rec.Code, header)
if rec.Code != http.StatusOK {
t.Fatalf("status = %d, want %d", rec.Code, http.StatusOK)
}
value, hasPrefix := strings.CutPrefix(header, "public, max-age=")
value, hasSuffix := strings.CutSuffix(value, ", immutable")
maxAge, err := strconv.Atoi(value)
if !hasPrefix || !hasSuffix || err != nil {
t.Fatalf("Cache-Control = %q, want public, max-age=<seconds>, immutable",
header)
}
return maxAge
}
// TestHandleImage_SignedURL_MaxAgeEndsAtExp verifies that an image served
// through a signed URL expiring in 60 seconds may be cached for at most those
// 60 seconds. The lower bound of 50 shows the max-age is the time left, not 0.
func TestHandleImage_SignedURL_MaxAgeEndsAtExp(t *testing.T) {
t.Parallel()
h, srv := newSignedHostServer(t)
signedURL, err := h.imgSvc.GenerateSignedURL("", &imgcache.ImageRequest{
SourceHost: signedHost,
SourcePath: photoPath,
Size: imgcache.Size{Width: 50, Height: 50},
Format: imgcache.FormatJPEG,
}, time.Minute)
if err != nil {
t.Fatalf("GenerateSignedURL() error = %v", err)
}
maxAge := getMaxAge(t, srv, signedURL)
if maxAge < 50 || maxAge > 60 {
t.Errorf("max-age = %d, want 50 to 60", maxAge)
}
}
// TestHandleImage_AllowlistedHost_MaxAge verifies the max-age of an image from
// an allowlisted host, which is served without checking sig or exp. A URL with
// no exp may be cached for a year. A URL whose exp has passed is the one request
// that reaches the header after its expiry, and must get 0, never less.
func TestHandleImage_AllowlistedHost_MaxAge(t *testing.T) {
t.Parallel()
pastExp := strconv.FormatInt(time.Now().Add(-time.Hour).Unix(), 10)
tests := []struct {
name string
query string
wantMaxAge int
}{
{"no exp", "", 31536000},
{"exp already past", "?exp=" + pastExp, 0},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
fix := setupTestHandler(t)
r := chi.NewRouter()
r.Get("/v1/image/*", fix.handler.HandleImage())
maxAge := getMaxAge(t, r,
"/v1/image/"+fix.goodHost+"/images/photo.jpg/50x50.jpeg"+tt.query)
if maxAge != tt.wantMaxAge {
t.Errorf("max-age = %d, want %d", maxAge, tt.wantMaxAge)
}
})
}
}
// TestHandleImageEnc_MaxAge verifies that an image served through an encrypted
// URL with a 60 second TTL may be cached for at most those 60 seconds, that one
// with a two-year TTL may be cached for a year, and that one made without a
// TTL, which never expires, may be cached for a year.
func TestHandleImageEnc_MaxAge(t *testing.T) {
t.Parallel()
tests := []struct {
name string
expiresAt int64
wantAtLeast int
wantAtMost int
}{
{"60 second TTL", time.Now().Add(time.Minute).Unix(), 50, 60},
{"two-year TTL", time.Now().Add(2 * 365 * 24 * time.Hour).Unix(), 31536000, 31536000},
{"no TTL", 0, 31536000, 31536000},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
h, srv := newSignedHostServer(t)
token, err := h.encGen.Generate(&encurl.Payload{
SourceHost: signedHost,
SourcePath: photoPath,
Width: 50,
Height: 50,
Format: imgcache.FormatJPEG,
ExpiresAt: tt.expiresAt,
})
if err != nil {
t.Fatalf("Generate() error = %v", err)
}
maxAge := getMaxAge(t, srv, "/v1/e/"+token+"/img.jpg")
if maxAge < tt.wantAtLeast || maxAge > tt.wantAtMost {
t.Errorf("max-age = %d, want %d to %d",
maxAge, tt.wantAtLeast, tt.wantAtMost)
}
})
}
}
@@ -14,9 +14,9 @@ import (
) )
// signedHost is not on the allowlist setupTestHandler builds, so a request // signedHost is not on the allowlist setupTestHandler builds, so a request
// for it needs a valid signature. No image is served for it: a request that // for it needs a valid signature. setupTestHandler serves no image for it: a
// passes the signature check gets 502 from the failed fetch, and one that // request that passes the signature check gets 502 from the failed fetch, and
// fails the check gets 401. // one that fails the check gets 401.
const signedHost = "signed.example.com" const signedHost = "signed.example.com"
// getImage sends a GET for target to the image route of fix and returns the // getImage sends a GET for target to the image route of fix and returns the
+7 -2
View File
@@ -12,6 +12,7 @@ import (
"sneak.berlin/go/pixa/internal/encurl" "sneak.berlin/go/pixa/internal/encurl"
"sneak.berlin/go/pixa/internal/httpfetcher" "sneak.berlin/go/pixa/internal/httpfetcher"
"sneak.berlin/go/pixa/internal/imageprocessor"
"sneak.berlin/go/pixa/internal/imgcache" "sneak.berlin/go/pixa/internal/imgcache"
) )
@@ -89,8 +90,8 @@ func (s *Handlers) HandleImageEnc() http.HandlerFunc {
w.Header().Set("Content-Length", strconv.FormatInt(resp.ContentLength, 10)) w.Header().Set("Content-Length", strconv.FormatInt(resp.ContentLength, 10))
} }
// Cache headers - encrypted URLs can be cached since they're immutable // Cache headers: max-age ends at the URL's expiry
w.Header().Set("Cache-Control", "public, max-age=31536000, immutable") w.Header().Set("Cache-Control", cacheControl(req.Expires))
w.Header().Set("X-Pixa-Cache", string(resp.CacheStatus)) w.Header().Set("X-Pixa-Cache", string(resp.CacheStatus))
// Stream the response // Stream the response
@@ -124,6 +125,10 @@ func (s *Handlers) handleImageError(w http.ResponseWriter, err error) {
s.respondError(w, "upstream error", http.StatusBadGateway) s.respondError(w, "upstream error", http.StatusBadGateway)
case errors.Is(err, httpfetcher.ErrUpstreamTimeout): case errors.Is(err, httpfetcher.ErrUpstreamTimeout):
s.respondError(w, "upstream timeout", http.StatusGatewayTimeout) s.respondError(w, "upstream timeout", http.StatusGatewayTimeout)
case errors.Is(err, httpfetcher.ErrTooManyConnections),
errors.Is(err, imageprocessor.ErrTooManyImages):
s.respondError(w, "server busy, try again later",
http.StatusServiceUnavailable)
default: default:
s.log.Error("image request failed", "error", err) s.log.Error("image request failed", "error", err)
s.respondError(w, "internal error", http.StatusInternalServerError) s.respondError(w, "internal error", http.StatusInternalServerError)
@@ -0,0 +1,49 @@
package handlers
import (
"fmt"
"log/slog"
"net/http"
"net/http/httptest"
"strings"
"testing"
"sneak.berlin/go/pixa/internal/httpfetcher"
"sneak.berlin/go/pixa/internal/imageprocessor"
"sneak.berlin/go/pixa/internal/imgcache"
)
// TestServerBusyAnswers503 checks that both image routes answer 503 with
// a clear error when the image service gives up waiting for a free upstream
// connection or processing slot, wrapped as the service wraps them.
func TestServerBusyAnswers503(t *testing.T) {
t.Parallel()
h := &Handlers{log: slog.New(slog.DiscardHandler)}
req := &imgcache.ImageRequest{SourceHost: "img.example.com", SourcePath: "/a.jpg"}
for _, err := range []error{
fmt.Errorf("upstream fetch failed: %w", httpfetcher.ErrTooManyConnections),
fmt.Errorf("image processing failed: %w", imageprocessor.ErrTooManyImages),
} {
plain := httptest.NewRecorder()
h.respondImageError(plain, req, err)
encrypted := httptest.NewRecorder()
h.handleImageError(encrypted, err)
for route, rec := range map[string]*httptest.ResponseRecorder{
"/v1/image/": plain, "/v1/e/": encrypted,
} {
if rec.Code != http.StatusServiceUnavailable {
t.Errorf("%s for %v: status = %d, want %d",
route, err, rec.Code, http.StatusServiceUnavailable)
}
if !strings.Contains(rec.Body.String(), "server busy, try again later") {
t.Errorf("%s for %v: body = %q, want the server busy error",
route, err, rec.Body.String())
}
}
}
}
+78 -27
View File
@@ -1,5 +1,6 @@
// Package httpfetcher fetches content from upstream HTTP origins with SSRF // Package httpfetcher fetches content from upstream HTTP origins with SSRF
// protection, per-host connection limits, and content-type validation. // protection, connection limits per host and for all hosts together, and
// content-type validation.
package httpfetcher package httpfetcher
import ( import (
@@ -28,8 +29,13 @@ const (
DefaultIdleConnTimeout = 90 * time.Second DefaultIdleConnTimeout = 90 * time.Second
DefaultMaxRedirects = 10 DefaultMaxRedirects = 10
DefaultMaxConnectionsPerHost = 20 DefaultMaxConnectionsPerHost = 20
DefaultMaxConnections = 64
) )
// ConnectionWaitTimeout is how long Fetch waits for a free connection when
// MaxConnections fetches are already in progress.
const ConnectionWaitTimeout = 10 * time.Second
// MIME content types. // MIME content types.
const ( const (
contentTypeJPEG = "image/jpeg" contentTypeJPEG = "image/jpeg"
@@ -70,6 +76,7 @@ var (
ErrInvalidContentType = errors.New("invalid or unsupported content type") ErrInvalidContentType = errors.New("invalid or unsupported content type")
ErrUpstreamError = errors.New("upstream server error") ErrUpstreamError = errors.New("upstream server error")
ErrUpstreamTimeout = errors.New("upstream request timeout") ErrUpstreamTimeout = errors.New("upstream request timeout")
ErrTooManyConnections = errors.New("too many concurrent upstream connections")
) )
// Internal fetcher errors. // Internal fetcher errors.
@@ -122,6 +129,9 @@ type Config struct {
AllowHTTP bool AllowHTTP bool
// MaxConnectionsPerHost limits concurrent connections to each upstream host. // MaxConnectionsPerHost limits concurrent connections to each upstream host.
MaxConnectionsPerHost int MaxConnectionsPerHost int
// MaxConnections limits concurrent connections to all upstream hosts
// together.
MaxConnections int
// BlockedNetworks are operator-supplied CIDR ranges refused by the // BlockedNetworks are operator-supplied CIDR ranges refused by the
// dialer, in addition to the always-enforced built-in ranges. // dialer, in addition to the always-enforced built-in ranges.
BlockedNetworks []netip.Prefix BlockedNetworks []netip.Prefix
@@ -143,15 +153,22 @@ func DefaultConfig() *Config {
}, },
AllowHTTP: false, AllowHTTP: false,
MaxConnectionsPerHost: DefaultMaxConnectionsPerHost, MaxConnectionsPerHost: DefaultMaxConnectionsPerHost,
MaxConnections: DefaultMaxConnections,
} }
} }
// HTTPFetcher implements Fetcher with SSRF protection and per-host connection limits. // HTTPFetcher implements Fetcher with SSRF protection and connection limits
// per host and for all hosts together.
type HTTPFetcher struct { type HTTPFetcher struct {
client *http.Client client *http.Client
config *Config config *Config
hostSems map[string]chan struct{} // per-host semaphores hostSems map[string]chan struct{} // per-host semaphores
hostSemMu sync.Mutex // protects hostSems map hostSemMu sync.Mutex // protects hostSems map
// allHostsSemaphore has one slot per connection allowed to all hosts
// together (config.MaxConnections).
allHostsSemaphore chan struct{}
// connectionWaitTimeout is ConnectionWaitTimeout; tests shorten it.
connectionWaitTimeout time.Duration
} }
// New creates a new HTTPFetcher with SSRF protection. // New creates a new HTTPFetcher with SSRF protection.
@@ -192,13 +209,18 @@ func New(config *Config) *HTTPFetcher {
} }
return &HTTPFetcher{ return &HTTPFetcher{
client: client, client: client,
config: config, config: config,
hostSems: make(map[string]chan struct{}), hostSems: make(map[string]chan struct{}),
allHostsSemaphore: make(chan struct{}, config.MaxConnections),
connectionWaitTimeout: ConnectionWaitTimeout,
} }
} }
// Fetch retrieves content from the given URL with SSRF protection. // Fetch retrieves content from the given URL with SSRF protection. When
// MaxConnections fetches are already in progress, it waits up to
// ConnectionWaitTimeout for one to finish, then fails with
// ErrTooManyConnections.
func (f *HTTPFetcher) Fetch(ctx context.Context, url string) (*FetchResult, error) { func (f *HTTPFetcher) Fetch(ctx context.Context, url string) (*FetchResult, error) {
// Validate URL before making request // Validate URL before making request
err := validateURL(ctx, url, f.config.AllowHTTP) err := validateURL(ctx, url, f.config.AllowHTTP)
@@ -206,24 +228,17 @@ func (f *HTTPFetcher) Fetch(ctx context.Context, url string) (*FetchResult, erro
return nil, err return nil, err
} }
// Extract host for rate limiting release, err := f.acquireConnection(ctx, extractHost(url))
host := extractHost(url) if err != nil {
return nil, err
// Acquire semaphore slot for this host
sem := f.getHostSemaphore(host)
select {
case sem <- struct{}{}:
// Acquired slot
case <-ctx.Done():
return nil, ctx.Err()
} }
// If we fail before returning a result, release the slot // If we fail before returning a result, release the connection
success := false success := false
defer func() { defer func() {
if !success { if !success {
<-sem release()
} }
}() }()
@@ -267,17 +282,52 @@ func (f *HTTPFetcher) Fetch(ctx context.Context, url string) (*FetchResult, erro
return nil, fmt.Errorf("upstream request failed: %w", err) return nil, fmt.Errorf("upstream request failed: %w", err)
} }
result, err := f.buildResult(resp, remoteAddr, fetchDuration, sem) result, err := f.buildResult(resp, remoteAddr, fetchDuration, release)
if err != nil { if err != nil {
return nil, err return nil, err
} }
// Mark success so defer doesn't release the semaphore // Mark success so defer doesn't release the connection; closing the
// result's Content does
success = true success = true
return result, nil return result, nil
} }
// acquireConnection takes a slot for host, then one of the slots shared by
// all hosts, and returns the func that gives both back. The host's slot
// comes first, so fetches queued for one busy host hold no shared slot.
// Only the wait for a shared slot is bounded: after connectionWaitTimeout
// it fails with ErrTooManyConnections.
func (f *HTTPFetcher) acquireConnection(
ctx context.Context, host string,
) (func(), error) {
hostSem := f.getHostSemaphore(host)
select {
case hostSem <- struct{}{}:
case <-ctx.Done():
return nil, ctx.Err()
}
select {
case f.allHostsSemaphore <- struct{}{}:
case <-time.After(f.connectionWaitTimeout):
<-hostSem
return nil, ErrTooManyConnections
case <-ctx.Done():
<-hostSem
return nil, ctx.Err()
}
return func() {
<-hostSem
<-f.allHostsSemaphore
}, nil
}
// getHostSemaphore returns the semaphore for a host, creating it if necessary. // getHostSemaphore returns the semaphore for a host, creating it if necessary.
func (f *HTTPFetcher) getHostSemaphore(host string) chan struct{} { func (f *HTTPFetcher) getHostSemaphore(host string) chan struct{} {
f.hostSemMu.Lock() f.hostSemMu.Lock()
@@ -293,12 +343,12 @@ func (f *HTTPFetcher) getHostSemaphore(host string) chan struct{} {
} }
// buildResult validates the upstream response and assembles a FetchResult // buildResult validates the upstream response and assembles a FetchResult
// whose Content releases the host semaphore slot when closed. // whose Content calls release when closed.
func (f *HTTPFetcher) buildResult( func (f *HTTPFetcher) buildResult(
resp *http.Response, resp *http.Response,
remoteAddr string, remoteAddr string,
fetchDuration time.Duration, fetchDuration time.Duration,
sem chan struct{}, release func(),
) (*FetchResult, error) { ) (*FetchResult, error) {
// Extract HTTP version (strip "HTTP/" prefix) // Extract HTTP version (strip "HTTP/" prefix)
httpVersion := strings.TrimPrefix(resp.Proto, "HTTP/") httpVersion := strings.TrimPrefix(resp.Proto, "HTTP/")
@@ -333,7 +383,7 @@ func (f *HTTPFetcher) buildResult(
} }
return &FetchResult{ return &FetchResult{
Content: &semaphoreReleasingReadCloser{limitedBody, resp.Body, sem}, Content: &semaphoreReleasingReadCloser{limitedBody, resp.Body, release},
ContentLength: resp.ContentLength, ContentLength: resp.ContentLength,
ContentType: contentType, ContentType: contentType,
Headers: resp.Header, Headers: resp.Header,
@@ -574,17 +624,18 @@ func (r *limitedReader) Read(p []byte) (int, error) {
return n, err return n, err
} }
// semaphoreReleasingReadCloser releases a semaphore slot when closed. // semaphoreReleasingReadCloser releases the fetch's connection slots when
// closed.
type semaphoreReleasingReadCloser struct { type semaphoreReleasingReadCloser struct {
*limitedReader *limitedReader
closer io.Closer closer io.Closer
sem chan struct{} release func()
} }
func (r *semaphoreReleasingReadCloser) Close() error { func (r *semaphoreReleasingReadCloser) Close() error {
err := r.closer.Close() err := r.closer.Close()
<-r.sem // Release semaphore slot r.release()
return err return err
} }
@@ -0,0 +1,128 @@
package httpfetcher
import (
"errors"
"net"
"strconv"
"testing"
"time"
)
// imageURLOnPort is the fake upstream's image route on testPublicHost at
// port. Each port is a different host to the per-host limit, while the test
// dialer sends every port to the one test server.
func imageURLOnPort(port int) string {
return "http://" + net.JoinHostPort(testPublicHost, strconv.Itoa(port)) +
"/image"
}
func TestDefaultConfigMaxConnections(t *testing.T) {
t.Parallel()
if got := DefaultConfig().MaxConnections; got != DefaultMaxConnections {
t.Errorf("MaxConnections = %d, want %d", got, DefaultMaxConnections)
}
}
// TestFetchLimitsConnectionsToAllHostsTogether checks that MaxConnections
// counts the fetches to every host together, apart from the per-host
// limit: with MaxConnections at 2 and two responses open from two hosts, a
// fetch from a third host, which has nothing open, waits the whole wait
// timeout and fails with ErrTooManyConnections. Closing one response lets
// it through.
func TestFetchLimitsConnectionsToAllHostsTogether(t *testing.T) {
t.Parallel()
srv := startUpstream(t)
cfg := DefaultConfig()
cfg.MaxConnections = 2
f, _ := newServerFetcher(t, srv, cfg)
f.connectionWaitTimeout = 100 * time.Millisecond
first, err := f.Fetch(testContext(t), imageURLOnPort(81))
if err != nil {
t.Fatalf("first Fetch() error = %v", err)
}
second, err := f.Fetch(testContext(t), imageURLOnPort(82))
if err != nil {
t.Fatalf("second Fetch() error = %v", err)
}
defer func() { _ = second.Content.Close() }()
start := time.Now()
_, err = f.Fetch(testContext(t), imageURLOnPort(83))
if !errors.Is(err, ErrTooManyConnections) {
t.Fatalf("third Fetch() error = %v, want ErrTooManyConnections", err)
}
if waited := time.Since(start); waited < f.connectionWaitTimeout {
t.Errorf("third Fetch() failed after %v, before waiting %v",
waited, f.connectionWaitTimeout)
}
if held := semLen(f, testPublicHost+":83"); held != 0 {
t.Errorf("the refused fetch kept its host's slot: %d held", held)
}
err = first.Content.Close()
if err != nil {
t.Fatalf("close first body: %v", err)
}
third, err := f.Fetch(testContext(t), imageURLOnPort(83))
if err != nil {
t.Fatalf("Fetch() after a response was closed: error = %v", err)
}
_ = third.Content.Close()
}
// TestFetchReleasesConnectionOnError checks that a fetch that fails after
// taking its connection gives it back: with MaxConnections at 1, the slot
// must be free after the failure and the next fetch must succeed.
func TestFetchReleasesConnectionOnError(t *testing.T) {
t.Parallel()
cases := []struct {
name string
url string
want error
}{
{"upstream answers 500", upstreamURL("/status/500"), ErrUpstreamError},
{"upstream sends HTML", upstreamURL("/html"), ErrInvalidContentType},
// 198.51.100.7 (TEST-NET-2) passes the SSRF checks, and the test
// dialer refuses every host but testPublicHost.
{"connecting fails", "http://198.51.100.7/image", errUnexpectedDial},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
srv := startUpstream(t)
cfg := DefaultConfig()
cfg.MaxConnections = 1
f, _ := newServerFetcher(t, srv, cfg)
f.connectionWaitTimeout = 100 * time.Millisecond
_, err := f.Fetch(testContext(t), tc.url)
if !errors.Is(err, tc.want) {
t.Fatalf("Fetch() error = %v, want %v", err, tc.want)
}
if held := len(f.allHostsSemaphore); held != 0 {
t.Fatalf("connection still held after the error: %d held", held)
}
res := fetchImage(t, f, "/image")
_ = res.Content.Close()
})
}
}
+74 -5
View File
@@ -7,7 +7,9 @@ import (
"errors" "errors"
"fmt" "fmt"
"io" "io"
"runtime"
"sync" "sync"
"time"
"github.com/davidbyttow/govips/v2/vips" "github.com/davidbyttow/govips/v2/vips"
) )
@@ -17,11 +19,21 @@ import (
//nolint:gochecknoglobals // package-level sync.Once for one-time vips init //nolint:gochecknoglobals // package-level sync.Once for one-time vips init
var vipsOnce sync.Once var vipsOnce sync.Once
// initVips initializes libvips with quiet logging. // initVips initializes libvips with quiet logging, one worker thread per
// image and no operation cache. Process already works on one image per CPU
// by default, so more threads per image would only compete for the CPUs.
// Each request decodes different source bytes, so the operation cache
// would rarely be hit and would hold memory outside MaxConcurrentProcessing;
// repeated requests are served from pixa's disk cache instead.
func initVips() { func initVips() {
vipsOnce.Do(func() { vipsOnce.Do(func() {
vips.LoggingSettings(nil, vips.LogLevelError) vips.LoggingSettings(nil, vips.LogLevelError)
vips.Startup(nil) vips.Startup(&vips.Config{
ConcurrencyLevel: 1,
MaxCacheSize: 0,
MaxCacheMem: 0,
MaxCacheFiles: 0,
})
}) })
} }
@@ -106,9 +118,23 @@ var ErrInputDataTooLarge = errors.New("input data exceeds maximum allowed size")
// not supported. // not supported.
var ErrUnsupportedOutputFormat = errors.New("unsupported output format") var ErrUnsupportedOutputFormat = errors.New("unsupported output format")
// ErrTooManyImages is returned when MaxConcurrentProcessing images are being
// processed and none finishes within ProcessingWaitTimeout.
var ErrTooManyImages = errors.New("too many images being processed at once")
// ProcessingWaitTimeout is how long Process waits for a free slot when
// MaxConcurrentProcessing images are already being processed.
const ProcessingWaitTimeout = 10 * time.Second
// ImageProcessor implements image transformation using libvips via govips. // ImageProcessor implements image transformation using libvips via govips.
type ImageProcessor struct { type ImageProcessor struct {
maxInputBytes int64 maxInputBytes int64
// processingSemaphore has one slot per image that may be processed at
// once. Process holds a slot from before it reads its input until it
// returns, so the input, the decoded image and the output all count.
processingSemaphore chan struct{}
// processingWaitTimeout is ProcessingWaitTimeout; tests shorten it.
processingWaitTimeout time.Duration
} }
// Params holds configuration for creating an ImageProcessor. // Params holds configuration for creating an ImageProcessor.
@@ -117,6 +143,9 @@ type Params struct {
// MaxInputBytes is the maximum allowed input size in bytes. // MaxInputBytes is the maximum allowed input size in bytes.
// If <= 0, DefaultMaxInputBytes is used. // If <= 0, DefaultMaxInputBytes is used.
MaxInputBytes int64 MaxInputBytes int64
// MaxConcurrentProcessing is the most images processed at once.
// If <= 0, the number of CPUs Go uses (runtime.GOMAXPROCS(0)) is used.
MaxConcurrentProcessing int
} }
// New creates a new image processor with the given parameters. // New creates a new image processor with the given parameters.
@@ -129,17 +158,34 @@ func New(params Params) *ImageProcessor {
maxInputBytes = DefaultMaxInputBytes maxInputBytes = DefaultMaxInputBytes
} }
maxConcurrentProcessing := params.MaxConcurrentProcessing
if maxConcurrentProcessing <= 0 {
maxConcurrentProcessing = runtime.GOMAXPROCS(0)
}
return &ImageProcessor{ return &ImageProcessor{
maxInputBytes: maxInputBytes, maxInputBytes: maxInputBytes,
processingSemaphore: make(chan struct{}, maxConcurrentProcessing),
processingWaitTimeout: ProcessingWaitTimeout,
} }
} }
// Process transforms an image according to the request. // Process transforms an image according to the request. When
// MaxConcurrentProcessing images are already being processed, it waits up
// to ProcessingWaitTimeout for one to finish, then fails with
// ErrTooManyImages.
func (p *ImageProcessor) Process( func (p *ImageProcessor) Process(
_ context.Context, ctx context.Context,
input io.Reader, input io.Reader,
req *Request, req *Request,
) (*Result, error) { ) (*Result, error) {
release, err := p.acquireSlot(ctx)
if err != nil {
return nil, err
}
defer release()
// Read input with a size limit to prevent unbounded memory consumption. // Read input with a size limit to prevent unbounded memory consumption.
// We read at most maxInputBytes+1 so we can detect if the input exceeds // We read at most maxInputBytes+1 so we can detect if the input exceeds
// the limit without consuming additional memory. // the limit without consuming additional memory.
@@ -285,6 +331,29 @@ func FormatToMIME(format Format) string {
} }
} }
// acquireSlot takes a slot in processingSemaphore, waiting at most
// processingWaitTimeout for one to free up, and returns the func that gives
// it back. A free slot is taken even when ctx has ended; only the wait for
// one stops when ctx ends, as the rest of Process does not check ctx.
func (p *ImageProcessor) acquireSlot(ctx context.Context) (func(), error) {
release := func() { <-p.processingSemaphore }
select {
case p.processingSemaphore <- struct{}{}:
return release, nil
default:
}
select {
case p.processingSemaphore <- struct{}{}:
return release, nil
case <-time.After(p.processingWaitTimeout):
return nil, ErrTooManyImages
case <-ctx.Done():
return nil, ctx.Err()
}
}
// detectFormat returns the format string from a vips image. // detectFormat returns the format string from a vips image.
func (p *ImageProcessor) detectFormat(img *vips.ImageRef) string { func (p *ImageProcessor) detectFormat(img *vips.ImageRef) string {
format := img.Format() format := img.Format()
@@ -0,0 +1,302 @@
package imageprocessor
import (
"bytes"
"context"
"errors"
"io"
"runtime"
"strings"
"sync"
"testing"
"testing/iotest"
"time"
)
// errTestReadFailed is the error the unreadable test input returns.
var errTestReadFailed = errors.New("test input cannot be read")
// readingCounter counts the Process calls reading their input at the same
// time and remembers the most there ever were.
type readingCounter struct {
mu sync.Mutex
reading int
most int
}
func (c *readingCounter) start() {
c.mu.Lock()
defer c.mu.Unlock()
c.reading++
c.most = max(c.most, c.reading)
}
func (c *readingCounter) stop() {
c.mu.Lock()
defer c.mu.Unlock()
c.reading--
}
func (c *readingCounter) mostReading() int {
c.mu.Lock()
defer c.mu.Unlock()
return c.most
}
// gatedReader is a Process input. Its first Read counts the call in,
// reports it on entered and blocks until gate is closed; it counts the call
// out when it returns io.EOF. Process reads its input only while it holds a
// processing slot, so the count never goes above MaxConcurrentProcessing.
type gatedReader struct {
data *bytes.Reader
gate <-chan struct{}
entered chan<- struct{}
counter *readingCounter
started bool
}
func (r *gatedReader) Read(p []byte) (int, error) {
if !r.started {
r.started = true
r.counter.start()
r.entered <- struct{}{}
<-r.gate
}
n, err := r.data.Read(p)
if errors.Is(err, io.EOF) {
r.counter.stop()
}
return n, err
}
// smallJPEGRequest asks for a 5x5 JPEG.
func smallJPEGRequest() *Request {
return &Request{
Size: Size{Width: 5, Height: 5},
Format: FormatJPEG,
Quality: 85,
FitMode: FitCover,
}
}
// processInBackground runs Process on reader in a new goroutine and sends
// its error on results.
func processInBackground(
proc *ImageProcessor, reader *gatedReader, results chan<- error,
) {
go func() {
result, err := proc.Process(context.Background(), reader, smallJPEGRequest())
if err == nil {
_ = result.Content.Close()
}
results <- err
}()
}
// waitForEntries fails the test unless count Process calls report on
// entered within a few seconds.
func waitForEntries(t *testing.T, entered <-chan struct{}, count int) {
t.Helper()
for range count {
select {
case <-entered:
case <-time.After(5 * time.Second):
t.Fatal("Process calls did not start reading their input")
}
}
}
func TestNewDefaultsMaxConcurrentProcessingToCPUs(t *testing.T) {
t.Parallel()
for _, limit := range []int{0, -1} {
proc := New(Params{MaxConcurrentProcessing: limit})
if got := cap(proc.processingSemaphore); got != runtime.GOMAXPROCS(0) {
t.Errorf("MaxConcurrentProcessing %d: %d slots, want %d, one per CPU",
limit, got, runtime.GOMAXPROCS(0))
}
}
proc := New(Params{MaxConcurrentProcessing: 3})
if got := cap(proc.processingSemaphore); got != 3 {
t.Errorf("MaxConcurrentProcessing 3: %d slots, want 3", got)
}
}
// TestProcessNeverExceedsMaxConcurrentProcessing starts more Process calls
// than MaxConcurrentProcessing allows and holds the first ones inside
// Process until the test lets them go. No more than the limit may be
// working at once, and the calls held back must wait for a slot and then
// succeed.
func TestProcessNeverExceedsMaxConcurrentProcessing(t *testing.T) {
t.Parallel()
const (
limit = 2
calls = 6
)
proc := New(Params{MaxConcurrentProcessing: limit})
input := createTestJPEG(t, 50, 50)
counter := &readingCounter{}
gate := make(chan struct{})
entered := make(chan struct{}, calls)
results := make(chan error, calls)
openGate := sync.OnceFunc(func() { close(gate) })
t.Cleanup(openGate)
for range calls {
processInBackground(proc, &gatedReader{
data: bytes.NewReader(input), gate: gate, entered: entered,
counter: counter,
}, results)
}
waitForEntries(t, entered, limit)
// A call beyond the limit would start reading its input now.
select {
case <-entered:
t.Fatalf("a Process call started while %d were already working", limit)
case <-time.After(100 * time.Millisecond):
}
openGate()
for range calls {
err := <-results
if err != nil {
t.Errorf("Process() error = %v, want nil once a slot is free", err)
}
}
if most := counter.mostReading(); most > limit {
t.Errorf("%d Process calls worked at once, want at most %d", most, limit)
}
}
// TestProcessWaitsThenFailsWhenNoSlotFrees holds the only slot and checks
// that another call waits the whole wait timeout, then fails with
// ErrTooManyImages instead of processing anyway.
func TestProcessWaitsThenFailsWhenNoSlotFrees(t *testing.T) {
t.Parallel()
proc := New(Params{MaxConcurrentProcessing: 1})
proc.processingWaitTimeout = 100 * time.Millisecond
input := createTestJPEG(t, 10, 10)
gate := make(chan struct{})
entered := make(chan struct{}, 1)
held := make(chan error, 1)
openGate := sync.OnceFunc(func() { close(gate) })
t.Cleanup(openGate)
processInBackground(proc, &gatedReader{
data: bytes.NewReader(input), gate: gate, entered: entered,
counter: &readingCounter{},
}, held)
waitForEntries(t, entered, 1)
start := time.Now()
_, err := proc.Process(context.Background(), bytes.NewReader(input),
smallJPEGRequest())
if !errors.Is(err, ErrTooManyImages) {
t.Fatalf("Process() error = %v, want ErrTooManyImages", err)
}
if waited := time.Since(start); waited < proc.processingWaitTimeout {
t.Errorf("Process() failed after %v, before waiting %v",
waited, proc.processingWaitTimeout)
}
openGate()
err = <-held
if err != nil {
t.Errorf("Process() holding the slot: error = %v, want nil", err)
}
}
// TestProcessReleasesSlotOnError checks that Process gives its slot back
// when it fails, whether it fails early or late: with one slot, the slot
// must be free after the failure and the next call must succeed.
func TestProcessReleasesSlotOnError(t *testing.T) {
t.Parallel()
valid := createTestJPEG(t, 10, 10)
unsupported := smallJPEGRequest()
unsupported.Format = "bmp"
cases := []struct {
name string
input io.Reader
req *Request
// want is the error Process must return; nil means any error.
want error
}{
{
name: "input cannot be read",
input: iotest.ErrReader(errTestReadFailed),
req: smallJPEGRequest(),
want: errTestReadFailed,
},
{
name: "input over the byte limit",
input: bytes.NewReader(createTestJPEG(t, 800, 600)),
req: smallJPEGRequest(),
want: ErrInputDataTooLarge,
},
{
name: "input not an image",
input: strings.NewReader("not an image"),
req: smallJPEGRequest(),
},
{
name: "output format not supported",
input: bytes.NewReader(valid),
req: unsupported,
want: ErrUnsupportedOutputFormat,
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
proc := New(Params{MaxInputBytes: 4096, MaxConcurrentProcessing: 1})
proc.processingWaitTimeout = 100 * time.Millisecond
_, err := proc.Process(context.Background(), tc.input, tc.req)
if err == nil || (tc.want != nil && !errors.Is(err, tc.want)) {
t.Fatalf("Process() error = %v, want %v", err, tc.want)
}
if held := len(proc.processingSemaphore); held != 0 {
t.Fatalf("slot still held after the error: %d held", held)
}
result, err := proc.Process(context.Background(), bytes.NewReader(valid),
smallJPEGRequest())
if err != nil {
t.Fatalf("Process() after the error = %v, want nil", err)
}
_ = result.Content.Close()
})
}
}
+27 -12
View File
@@ -419,19 +419,21 @@ func (c *Cache) Stats(ctx context.Context) (*CacheStats, error) {
return nil, fmt.Errorf("failed to get cache stats: %w", err) return nil, fmt.Errorf("failed to get cache stats: %w", err)
} }
// Get actual item count and total size from content tables // Count and size the cached source images and processed variants. A
err = c.db.QueryRowContext(ctx, // disabled cache holds none, whatever rows an earlier run left.
`SELECT COUNT(*) FROM request_cache`, if !c.disabled {
).Scan(&stats.TotalItems) err = c.db.QueryRowContext(ctx, `
if err != nil { SELECT (SELECT COUNT(*) FROM source_content)
c.log.Warn("failed to count cache items for stats", "error", err) + (SELECT COUNT(*) FROM variant_content)
} `).Scan(&stats.TotalItems)
if err != nil {
c.log.Warn("failed to count cache items for stats", "error", err)
}
err = c.db.QueryRowContext(ctx, stats.TotalSizeBytes, err = c.UsageBytes(ctx)
`SELECT COALESCE(SUM(size_bytes), 0) FROM output_content`, if err != nil {
).Scan(&stats.TotalSizeBytes) c.log.Warn("failed to sum cache size for stats", "error", err)
if err != nil { }
c.log.Warn("failed to sum cache size for stats", "error", err)
} }
// Compute hit rate as a ratio // Compute hit rate as a ratio
@@ -481,6 +483,19 @@ func (c *Cache) IncrementStats(ctx context.Context, hit bool, fetchBytes int64)
} }
} }
// IncrementTransformCount counts one image transcoded by the image processor.
func (c *Cache) IncrementTransformCount(ctx context.Context) {
_, err := c.db.ExecContext(ctx, `
UPDATE cache_stats
SET transform_count = transform_count + 1,
last_updated_at = CURRENT_TIMESTAMP
WHERE id = 1
`)
if err != nil {
c.log.Warn("failed to count transform", "error", err)
}
}
// writeMetadataSidecar writes the JSON metadata sidecar of a stored source. // writeMetadataSidecar writes the JSON metadata sidecar of a stored source.
// A failure is logged and is otherwise non-fatal; the metadata is in the // A failure is logged and is otherwise non-fatal; the metadata is in the
// database. // database.
+6 -3
View File
@@ -95,7 +95,8 @@ type ImageRequest struct {
FitMode FitMode FitMode FitMode
// Signature is the HMAC signature for non-allowlisted hosts // Signature is the HMAC signature for non-allowlisted hosts
Signature string Signature string
// Expires is the signature expiration timestamp // Expires is when the URL expires: the exp of a signed URL, or the expiry
// of an encrypted URL; the zero time if it has none
Expires time.Time Expires time.Time
// AllowHTTP indicates whether HTTP (non-TLS) is allowed for this request // AllowHTTP indicates whether HTTP (non-TLS) is allowed for this request
AllowHTTP bool AllowHTTP bool
@@ -162,9 +163,11 @@ type ImageCache interface {
// CacheStats contains cache statistics // CacheStats contains cache statistics
type CacheStats struct { type CacheStats struct {
// TotalItems is the number of cached items // TotalItems is the number of cached source images plus processed
// variants
TotalItems int64 TotalItems int64
// TotalSizeBytes is the total size of cached content // TotalSizeBytes is the total size of cached source images and
// processed variants
TotalSizeBytes int64 TotalSizeBytes int64
// HitCount is the number of cache hits // HitCount is the number of cache hits
HitCount int64 HitCount int64
+39 -29
View File
@@ -43,6 +43,9 @@ type ServiceConfig struct {
SigningKey string SigningKey string
// Allowlist is the list of hosts that don't require signatures // Allowlist is the list of hosts that don't require signatures
Allowlist []string Allowlist []string
// MaxConcurrentProcessing is the most images processed at once; zero
// uses the image processor's default, one per CPU
MaxConcurrentProcessing int
// Logger for logging // Logger for logging
Logger *slog.Logger Logger *slog.Logger
} }
@@ -91,9 +94,10 @@ func NewService(cfg *ServiceConfig) (*Service, error) {
} }
maxResponseSize := fetcherCfg.MaxResponseSize maxResponseSize := fetcherCfg.MaxResponseSize
processor := imageprocessor.New( processor := imageprocessor.New(imageprocessor.Params{
imageprocessor.Params{MaxInputBytes: maxResponseSize}, MaxInputBytes: maxResponseSize,
) MaxConcurrentProcessing: cfg.MaxConcurrentProcessing,
})
return &Service{ return &Service{
cache: cfg.Cache, cache: cfg.Cache,
@@ -143,7 +147,8 @@ func (s *Service) Get(ctx context.Context, req *ImageRequest) (*ImageResponse, e
s.log.Error("failed to get cached variant", "key", result.CacheKey, "error", err) s.log.Error("failed to get cached variant", "key", result.CacheKey, "error", err)
// Fall through to re-process // Fall through to re-process
} else { } else {
s.cache.IncrementStats(ctx, true, 0) // Counted also when the request context has ended meanwhile
s.cache.IncrementStats(context.WithoutCancel(ctx), true, 0)
return &ImageResponse{ return &ImageResponse{
Content: reader, Content: reader,
@@ -155,12 +160,15 @@ func (s *Service) Get(ctx context.Context, req *ImageRequest) (*ImageResponse, e
} }
} }
// Cache miss - check if we have source content cached // Cache miss - process the cached source or fetch it, then count the
// miss with the bytes it fetched from upstream, also when it failed or
// the request context has ended meanwhile
cacheKey := CacheKey(req) cacheKey := CacheKey(req)
s.cache.IncrementStats(ctx, false, 0) response, fetchedBytes, err := s.processFromSourceOrFetch(ctx, req, cacheKey)
s.cache.IncrementStats(context.WithoutCancel(ctx), false, fetchedBytes)
response, err := s.processFromSourceOrFetch(ctx, req, cacheKey)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@@ -268,22 +276,20 @@ func (s *Service) loadCachedSource(contentHash ContentHash) []byte {
} }
// processFromSourceOrFetch processes an image, using cached source content // processFromSourceOrFetch processes an image, using cached source content
// if available. // if available. It also returns the number of bytes fetched from upstream,
// as fetchAndProcess does, or 0 when the cached source was used.
func (s *Service) processFromSourceOrFetch( func (s *Service) processFromSourceOrFetch(
ctx context.Context, ctx context.Context,
req *ImageRequest, req *ImageRequest,
cacheKey VariantKey, cacheKey VariantKey,
) (*ImageResponse, error) { ) (*ImageResponse, int64, error) {
// Check if we have cached source content // Check if we have cached source content
contentHash, _, err := s.cache.LookupSource(ctx, req) contentHash, _, err := s.cache.LookupSource(ctx, req)
if err != nil { if err != nil {
s.log.Warn("source lookup failed", "error", err) s.log.Warn("source lookup failed", "error", err)
} }
var ( var sourceData []byte
sourceData []byte
fetchBytes int64
)
if contentHash != "" { if contentHash != "" {
s.log.Debug("using cached source", "hash", contentHash) s.log.Debug("using cached source", "hash", contentHash)
@@ -292,26 +298,25 @@ func (s *Service) processFromSourceOrFetch(
// Fetch from upstream if we don't have source data or it's empty // Fetch from upstream if we don't have source data or it's empty
if len(sourceData) == 0 { if len(sourceData) == 0 {
resp, err := s.fetchAndProcess(ctx, req, cacheKey) return s.fetchAndProcess(ctx, req, cacheKey)
if err != nil {
return nil, err
}
return resp, nil
} }
// Process using cached source // Process using cached source; nothing was fetched from upstream
fetchBytes = int64(len(sourceData)) resp, err := s.processAndStore(
ctx, req, cacheKey, sourceData, int64(len(sourceData)),
)
return s.processAndStore(ctx, req, cacheKey, sourceData, fetchBytes) return resp, 0, err
} }
// fetchAndProcess fetches from upstream, processes, and caches the result. // fetchAndProcess fetches from upstream, processes, and caches the result.
// It also returns the number of bytes read from upstream, including when
// reading the response or a later step fails.
func (s *Service) fetchAndProcess( func (s *Service) fetchAndProcess(
ctx context.Context, ctx context.Context,
req *ImageRequest, req *ImageRequest,
cacheKey VariantKey, cacheKey VariantKey,
) (*ImageResponse, error) { ) (*ImageResponse, int64, error) {
// Fetch from upstream // Fetch from upstream
sourceURL := req.SourceURL() sourceURL := req.SourceURL()
@@ -330,20 +335,20 @@ func (s *Service) fetchAndProcess(
} }
} }
return nil, fmt.Errorf("upstream fetch failed: %w", err) return nil, 0, fmt.Errorf("upstream fetch failed: %w", err)
} }
defer func() { _ = fetchResult.Content.Close() }() defer func() { _ = fetchResult.Content.Close() }()
// Read and validate the source content // Read and validate the source content
sourceData, err := io.ReadAll(fetchResult.Content) sourceData, err := io.ReadAll(fetchResult.Content)
fetchBytes := int64(len(sourceData))
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to read upstream response: %w", err) return nil, fetchBytes, fmt.Errorf("failed to read upstream response: %w", err)
} }
// Calculate download bitrate // Calculate download bitrate
fetchBytes := int64(len(sourceData))
var downloadRate string var downloadRate string
if fetchResult.FetchDurationMs > 0 { if fetchResult.FetchDurationMs > 0 {
@@ -368,7 +373,7 @@ func (s *Service) fetchAndProcess(
// Validate magic bytes match content type // Validate magic bytes match content type
err = magic.ValidateMagicBytes(sourceData, fetchResult.ContentType) err = magic.ValidateMagicBytes(sourceData, fetchResult.ContentType)
if err != nil { if err != nil {
return nil, fmt.Errorf("content validation failed: %w", err) return nil, fetchBytes, fmt.Errorf("content validation failed: %w", err)
} }
// Store source content // Store source content
@@ -378,7 +383,9 @@ func (s *Service) fetchAndProcess(
// Continue even if caching fails // Continue even if caching fails
} }
return s.processAndStore(ctx, req, cacheKey, sourceData, fetchBytes) resp, err := s.processAndStore(ctx, req, cacheKey, sourceData, fetchBytes)
return resp, fetchBytes, err
} }
// processAndStore processes an image and stores the result. // processAndStore processes an image and stores the result.
@@ -406,6 +413,9 @@ func (s *Service) processAndStore(
processDuration := time.Since(processStart) processDuration := time.Since(processStart)
// Counted also when the request context has ended meanwhile
s.cache.IncrementTransformCount(context.WithoutCancel(ctx))
// Read processed content // Read processed content
processedData, err := io.ReadAll(processResult.Content) processedData, err := io.ReadAll(processResult.Content)
_ = processResult.Content.Close() _ = processResult.Content.Close()
+315 -1
View File
@@ -4,6 +4,9 @@ import (
"bytes" "bytes"
"context" "context"
"database/sql" "database/sql"
"image/color"
"io"
"io/fs"
"log/slog" "log/slog"
"math" "math"
"strings" "strings"
@@ -11,6 +14,7 @@ import (
"time" "time"
"sneak.berlin/go/pixa/internal/database" "sneak.berlin/go/pixa/internal/database"
"sneak.berlin/go/pixa/internal/httpfetcher"
) )
func setupStatsTestDB(t *testing.T) *sql.DB { func setupStatsTestDB(t *testing.T) *sql.DB {
@@ -125,7 +129,7 @@ func TestStats_LogsFailedCountQueries(t *testing.T) {
} }
_, err = db.ExecContext(t.Context(), _, err = db.ExecContext(t.Context(),
`DROP TABLE request_cache; DROP TABLE output_content`) `DROP TABLE source_content; DROP TABLE variant_content`)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@@ -183,3 +187,313 @@ func TestIncrementStats_LogsFailedUpdates(t *testing.T) {
} }
} }
} }
// TestStats_TotalsCountSourcesAndVariants verifies that TotalItems and
// TotalSizeBytes cover the stored source images and processed variants.
func TestStats_TotalsCountSourcesAndVariants(t *testing.T) {
t.Parallel()
cache, _ := newEvictionTestCache(t, 1<<30)
storeEvictionTestSource(t, cache, testHostCDN, testPathCat,
bytes.Repeat([]byte{0xAA}, 1000))
storeEvictionTestVariant(t, cache, testVariantKeyOne,
bytes.Repeat([]byte{0xAB}, 500))
storeEvictionTestVariant(t, cache, testVariantKeyTwo,
bytes.Repeat([]byte{0xAC}, 250))
stats, err := cache.Stats(t.Context())
if err != nil {
t.Fatalf("Stats() error = %v", err)
}
if stats.TotalItems != 3 {
t.Errorf("TotalItems = %d, want 3 (1 source, 2 variants)", stats.TotalItems)
}
if stats.TotalSizeBytes != 1750 {
t.Errorf("TotalSizeBytes = %d, want 1750 (1000+500+250)",
stats.TotalSizeBytes)
}
}
// TestStats_DisabledCacheReportsNoItems verifies that a disabled disk cache
// reports no items and no size, even when its database still holds the
// rows of an earlier run with the disk cache enabled.
func TestStats_DisabledCacheReportsNoItems(t *testing.T) {
t.Parallel()
enabled, _ := newEvictionTestCache(t, 1<<30)
storeEvictionTestSource(t, enabled, testHostCDN, testPathCat,
bytes.Repeat([]byte{0xAA}, 1000))
storeEvictionTestVariant(t, enabled, testVariantKeyOne,
bytes.Repeat([]byte{0xAB}, 500))
disabled, err := NewCache(enabled.db, CacheConfig{
StateDir: t.TempDir(),
CacheTTL: time.Hour,
NegativeTTL: 5 * time.Minute,
DisableDiskCache: true,
})
if err != nil {
t.Fatal(err)
}
stats, err := disabled.Stats(t.Context())
if err != nil {
t.Fatalf("Stats() error = %v", err)
}
if stats.TotalItems != 0 || stats.TotalSizeBytes != 0 {
t.Errorf("TotalItems = %d, TotalSizeBytes = %d, want 0 and 0",
stats.TotalItems, stats.TotalSizeBytes)
}
}
// cacheStatsCounters holds the counters of the cache_stats row, in column
// order.
type cacheStatsCounters struct {
hitCount int64
missCount int64
upstreamFetchCount int64
upstreamFetchBytes int64
transformCount int64
}
// readCacheStatsCounters reads the counters of the cache_stats row.
func readCacheStatsCounters(t *testing.T, cache *Cache) cacheStatsCounters {
t.Helper()
var got cacheStatsCounters
err := cache.db.QueryRowContext(t.Context(), `
SELECT hit_count, miss_count, upstream_fetch_count,
upstream_fetch_bytes, transform_count
FROM cache_stats WHERE id = 1
`).Scan(&got.hitCount, &got.missCount, &got.upstreamFetchCount,
&got.upstreamFetchBytes, &got.transformCount)
if err != nil {
t.Fatalf("failed to read cache_stats: %v", err)
}
return got
}
// TestService_Get_CountsStats walks Get through a miss that fetches the
// source, a hit, a miss that reuses the cached source, and two misses whose
// source cannot be used, checking every cache_stats counter after each.
func TestService_Get_CountsStats(t *testing.T) {
t.Parallel()
svc, fixtures := SetupTestService(t)
// NewTestFS builds the same files the test service's fetcher serves.
testFS, _ := NewTestFS(t)
photo, err := fs.ReadFile(testFS, fixtures.GoodHostJPEG)
if err != nil {
t.Fatal(err)
}
fake, err := fs.ReadFile(testFS, fixtures.InvalidFile)
if err != nil {
t.Fatal(err)
}
photoBytes, fakeBytes := int64(len(photo)), int64(len(fake))
// want is hits, misses, upstream fetches, upstream bytes, transforms.
steps := []struct {
name string
path string
size int
wantErr bool
want cacheStatsCounters
}{
{"miss that fetches the source", testPathPhoto, 50, false,
cacheStatsCounters{0, 1, 1, photoBytes, 1}},
{"hit", testPathPhoto, 50, false,
cacheStatsCounters{1, 1, 1, photoBytes, 1}},
{"miss that reuses the cached source", testPathPhoto, 25, false,
cacheStatsCounters{1, 2, 1, photoBytes, 2}},
{"miss whose source fails the magic byte check", "/images/fake.jpg", 50, true,
cacheStatsCounters{1, 3, 2, photoBytes + fakeBytes, 2}},
{"miss whose source is not found", "/images/nonexistent.jpg", 50, true,
cacheStatsCounters{1, 4, 2, photoBytes + fakeBytes, 2}},
}
for _, step := range steps {
resp, err := svc.Get(t.Context(), &ImageRequest{
SourceHost: fixtures.GoodHost,
SourcePath: step.path,
Size: Size{Width: step.size, Height: step.size},
Format: FormatJPEG,
Quality: 85,
FitMode: FitCover,
})
if (err != nil) != step.wantErr {
t.Fatalf("%s: Get() error = %v, want error %t", step.name, err, step.wantErr)
}
if err == nil {
_ = resp.Content.Close()
}
got := readCacheStatsCounters(t, svc.cache)
if got != step.want {
t.Fatalf("after the %s: counters = %+v, want %+v", step.name, got, step.want)
}
}
}
// fakeUpstream answers every fetch with itself as a JPEG body. The body
// serves data, then calls cancel, when set, and returns err; io.EOF ends
// the body normally.
type fakeUpstream struct {
data *bytes.Reader
cancel context.CancelFunc
err error
}
func (u *fakeUpstream) Fetch(
context.Context, string,
) (*httpfetcher.FetchResult, error) {
return &httpfetcher.FetchResult{
Content: io.NopCloser(u),
ContentLength: -1,
ContentType: testContentTypeJPEG,
}, nil
}
func (u *fakeUpstream) Read(p []byte) (int, error) {
if u.data.Len() > 0 {
return u.data.Read(p)
}
if u.cancel != nil {
u.cancel()
}
return 0, u.err
}
// TestService_Get_CountsInterruptedMisses checks every cache_stats counter
// after a miss whose request context ends during or after the upstream
// fetch, and after a miss whose upstream body is over the size limit.
func TestService_Get_CountsInterruptedMisses(t *testing.T) {
t.Parallel()
photo := generateTestJPEG(t, 100, 100, color.RGBA{255, 0, 0, 255})
half := len(photo) / 2
// want is hits, misses, upstream fetches, upstream bytes, transforms.
tests := []struct {
name string
served int // bytes of the photo the upstream body serves
cancel bool // whether the body then ends the request context
readErr error // what the body then returns
wantErr bool
want cacheStatsCounters
}{
{"request context ends during the fetch", half, true, context.Canceled, true,
cacheStatsCounters{0, 1, 1, int64(half), 0}},
{"request context ends after the fetch", len(photo), true, io.EOF, false,
cacheStatsCounters{0, 1, 1, int64(len(photo)), 1}},
{"upstream body over the size limit", half, false,
httpfetcher.ErrResponseTooLarge, true,
cacheStatsCounters{0, 1, 1, int64(half), 0}},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
svc, fixtures := SetupTestService(t)
ctx, cancel := context.WithCancel(t.Context())
defer cancel()
upstream := &fakeUpstream{
data: bytes.NewReader(photo[:tc.served]),
err: tc.readErr,
}
if tc.cancel {
upstream.cancel = cancel
}
svc.fetcher = upstream
resp, err := svc.Get(ctx, &ImageRequest{
SourceHost: fixtures.GoodHost,
SourcePath: testPathPhoto,
Size: Size{Width: 50, Height: 50},
Format: FormatJPEG,
Quality: 85,
FitMode: FitCover,
})
t.Logf("Get() error = %v", err)
if (err != nil) != tc.wantErr {
t.Fatalf("Get() error = %v, want error %t", err, tc.wantErr)
}
if err == nil {
_ = resp.Content.Close()
}
got := readCacheStatsCounters(t, svc.cache)
if got != tc.want {
t.Errorf("counters = %+v, want %+v", got, tc.want)
}
})
}
}
// TestService_Get_CountsHitAfterRequestEnds checks every cache_stats counter
// after a hit served with a request context that has already ended: only
// the hit count moves.
func TestService_Get_CountsHitAfterRequestEnds(t *testing.T) {
t.Parallel()
svc, fixtures := SetupTestService(t)
req := &ImageRequest{
SourceHost: fixtures.GoodHost,
SourcePath: testPathPhoto,
Size: Size{Width: 50, Height: 50},
Format: FormatJPEG,
Quality: 85,
FitMode: FitCover,
}
// A first request caches the variant.
resp, err := svc.Get(t.Context(), req)
if err != nil {
t.Fatalf("first Get() error = %v", err)
}
_ = resp.Content.Close()
want := readCacheStatsCounters(t, svc.cache)
want.hitCount++
ctx, cancel := context.WithCancel(t.Context())
cancel()
resp, err = svc.Get(ctx, req)
if err != nil {
t.Fatalf("Get() with an ended request context: error = %v", err)
}
_ = resp.Content.Close()
if resp.CacheStatus != CacheHit {
t.Fatalf("CacheStatus = %v, want %v", resp.CacheStatus, CacheHit)
}
got := readCacheStatsCounters(t, svc.cache)
if got != want {
t.Errorf("counters = %+v, want %+v", got, want)
}
}