Author SHA1 Message Date
clawbot fc87c2117d Test that a read during a store keeps the stored content type (closes #70)
check / check (push) Failing after 4m2s
A GetVariant that begins before StoreVariant finishes can find the
variant file but not yet its .meta file, and so reads
application/octet-stream. When it adds that to memory after the store
added the real type, the wrong type is served to every later hit. The
part of GetVariant that runs after its check of memory moves, unchanged,
into loadVariantWithMeta, so the test can run it after a store. The
test fails now.

Model: opus-5-5
2026-09-29 07:22:21 +00:00
clawbot 1a07e9e261 Keep variant content types in memory for cache hits (closes #70)
Cache.metaCache was declared and never used, so every hit read and
parsed the variant's .meta file. It is now an LRU of up to 10,000
content types (hashicorp/golang-lru/v2), filled by StoreVariant and by
GetVariant after it reads a .meta file. For a variant it holds, Lookup
skips the disk check and GetVariant skips the .meta read; the variant
file is still opened and its size taken from it. Eviction removes the
entry before deleting the files, and GetVariant removes it when the
file will not open, so a missing variant is never served. The cap is a
constant, not a setting. README.md describes it.

Model: opus-5-5
2026-09-29 07:15:11 +00:00
clawbot dd9c85d5ec Test that a cache hit takes the content type from memory (closes #70)
Tests for keeping each variant's content type in memory. A second hit
must still get the stored content type after the variant's .meta file
is deleted, whether the first came from storing the variant or from
reading it after a restart; this fails now. A variant removed by
EvictToLimit, or whose file was deleted from disk, must be a miss and
must not be served, and concurrent stores, reads and evictions run
under the race detector; these pass now and guard the change.

Model: opus-5-5
2026-09-29 07:14:16 +00:00
26 changed files with 136 additions and 1778 deletions
+10 -26
View File
@@ -40,8 +40,9 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for pixa needs:
- **Port:** pixa listens on container port `8080`. - **Port:** pixa listens on container port `8080`.
- **Volume:** container path `/var/lib/pixa`, where pixa keeps its - **Volume:** container path `/var/lib/pixa`, where pixa keeps its
database and cache. Creating the host directory when it is missing is database and cache. upaas bind-mounts the host path it is given and
upaas's job, tracked in https://git.eeqj.de/sneak/upaas/issues/235. does not create it, so the host directory must exist before the first
deploy.
- **Environment variables:** - **Environment variables:**
- `PIXA_SIGNING_KEY` (required): secret for signed and encrypted URLs - `PIXA_SIGNING_KEY` (required): secret for signed and encrypted URLs
and login, 32+ characters, for example from and login, 32+ characters, for example from
@@ -57,6 +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, owned by root or by uid
`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
@@ -233,8 +238,6 @@ 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_UPSTREAM_FETCH_TIMEOUT` | `upstream_fetch_timeout` | Time allowed for one fetch from an upstream host; default `30s` | | `PIXA_UPSTREAM_FETCH_TIMEOUT` | `upstream_fetch_timeout` | Time allowed for one fetch from an upstream host; default `30s` |
| `PIXA_UPSTREAM_MAX_RESPONSE_SIZE` | `upstream_max_response_size` | Largest upstream response accepted, in bytes; default 50 MiB | | `PIXA_UPSTREAM_MAX_RESPONSE_SIZE` | `upstream_max_response_size` | Largest upstream response accepted, in bytes; default 50 MiB |
| `PIXA_DOWNSTREAM_TIMEOUT` | `downstream_timeout` | Time allowed for answering one client request; default `60s` | | `PIXA_DOWNSTREAM_TIMEOUT` | `downstream_timeout` | Time allowed for answering one client request; default `60s` |
@@ -243,7 +246,7 @@ variables set by the file's `env:` section are checked the same way.
| `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 |
| `PIXA_DEBUG` | `debug` | Debug logging and plain-HTTP local development; default `false` | | `PIXA_DEBUG` | `debug` | Debug logging and plain-HTTP local development; default `false` |
| `PIXA_MAINTENANCE_MODE` | `maintenance_mode` | Answer image requests with 503; the health check stays 200; default `false` | | `PIXA_MAINTENANCE_MODE` | `maintenance_mode` | Maintenance flag reported by the health check; default `false` |
Key settings in more detail: Key settings in more detail:
@@ -286,31 +289,12 @@ Key settings in more detail:
bytes; default `52428800` (50 MiB). It also limits the image data pixa bytes; default `52428800` (50 MiB). It also limits the image data pixa
decodes decodes
- `downstream_timeout` — time allowed for answering one client request, as a - `downstream_timeout` — time allowed for answering one client request, as a
duration; default `60s`. The upstream fetch counts toward it, and so do the duration; default `60s`. The upstream fetch counts toward it, so keep it
waits for an upstream connection and for a processing slot (up to 10 seconds longer than `upstream_fetch_timeout`
each), so keep it longer than `upstream_fetch_timeout` plus 20 seconds
- `signing_key` — HMAC secret for URL signatures - `signing_key` — HMAC secret for URL signatures
- `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, and `downstream_timeout` has not ended first, 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, and `downstream_timeout` has not
ended first, it is answered 503 the same way
- `maintenance_mode` — while `true`, the image routes (`/v1/image/` and
`/v1/e/`) answer every request with 503, a `Retry-After` header and a JSON
error body. The health check (`/.well-known/healthcheck.json`) still answers
200 and reports `"maintenance_mode": true`. It stays 200 because the image's
Docker `HEALTHCHECK` requests it: a 503 there would make the container
unhealthy, and upaas marks a deploy failed when its container is unhealthy.
The login and URL generator pages and `/metrics` keep working
See `config.example.yml` for all options with defaults. See `config.example.yml` for all options with defaults.
+9 -34
View File
@@ -25,46 +25,20 @@ The disk cache is now size-bounded with LRU eviction
# Next Step # Next Step
P2: security: referer blacklist P1: rate limit global concurrent upstream fetches to prevent resource
exhaustion
# Completed Steps # Completed Steps
- 2026-09-29 the container makes `/var/lib/pixa` usable by itself (closes
#159): `deploy/docker-entrypoint.sh` creates the directory if it is missing,
gives the directory and everything in it to `pixad` when the directory or one
of its top-level entries belongs to another user or group, sets its mode to
`750`, then runs the server as `pixad`; data left by an earlier run under
another uid is taken over this way; "Running under upaas" in `README.md` no
longer tells the operator to create or chown the host directory.
- 2026-09-29 variant content types kept in memory (closes #70): - 2026-09-29 variant content types kept in memory (closes #70):
`Cache.metaCache` holds the content types of up to 10,000 variants in an LRU `Cache.metaCache` holds the content types of up to 10,000 variants in an LRU
(`github.com/hashicorp/golang-lru/v2`), filled by `StoreVariant` and by (`github.com/hashicorp/golang-lru/v2`), filled by `StoreVariant` and by
`GetVariant` after it reads a `.meta` file, where a type `StoreVariant` added `GetVariant` after it reads a `.meta` file; for a variant it holds, `Lookup`
meanwhile is kept over the one read, and never with the skips the check of the disk and `GetVariant` skips the `.meta` read, still
`application/octet-stream` served for a variant without one; for a variant it opening the variant file and taking the size from it; eviction removes the
holds, `GetVariant` skips the `.meta` read, still opening the variant file and entry before deleting the files, and `GetVariant` removes it when the file
taking the size from it; eviction removes the entry before deleting the files, will not open; the cap is a constant, not a setting; the unused `variantMeta`
and `GetVariant` removes it when the file will not open; the cap is a type is gone; `README.md` describes it.
constant, not a setting; the unused `variantMeta` type is gone; `README.md`
describes it.
- 2026-09-29 maintenance mode refuses image requests (closes #71): while
`maintenance_mode` is on, `/v1/image/` and `/v1/e/` answer 503 with a
`Retry-After` header and the JSON error body, from one middleware in
`internal/server/routes.go`; the health check stays 200 and reports
`maintenance_mode`, as the image's Docker `HEALTHCHECK` requests it and upaas
marks a deploy failed when its container is unhealthy; the login and URL
generator pages and `/metrics` keep working; documented in `README.md` and
`config.example.yml`.
- 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, and a request whose source is cached reads it only once it
has a processing slot; 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 Dockerfiles install through `script/bootstrap` (closes #95): the - 2026-09-29 Dockerfiles install through `script/bootstrap` (closes #95): the
`Dockerfile` lint and build stages and `Dockerfile.lint` copy `script/`, `Dockerfile` lint and build stages and `Dockerfile.lint` copy `script/`,
`go.mod` and `go.sum`, then run `script/bootstrap` in place of their own `go.mod` and `go.sum`, then run `script/bootstrap` in place of their own
@@ -351,6 +325,7 @@ P2: security: referer blacklist
# 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
-80
View File
@@ -1,80 +0,0 @@
package main
import (
"errors"
"testing"
"go.uber.org/fx"
)
// errTestHook is the error returned by the test hooks that fail.
var errTestHook = errors.New("test hook failed")
// TestRunAppExitCode checks the exit code runApp returns: the one a
// shutdown request carries, 0 for a request without one (as for SIGINT or
// SIGTERM), and 1 when the app fails to start or to stop.
func TestRunAppExitCode(t *testing.T) {
t.Parallel()
cases := []struct {
name string
hook func(shutdowner fx.Shutdowner) fx.Hook
want int
}{
{
name: "shutdown requested with exit code 1",
hook: func(shutdowner fx.Shutdowner) fx.Hook {
return fx.StartHook(func() error {
return shutdowner.Shutdown(fx.ExitCode(1))
})
},
want: 1,
},
{
name: "shutdown requested without an exit code",
hook: func(shutdowner fx.Shutdowner) fx.Hook {
return fx.StartHook(func() error {
return shutdowner.Shutdown()
})
},
want: 0,
},
{
name: "start fails",
hook: func(fx.Shutdowner) fx.Hook {
return fx.StartHook(func() error { return errTestHook })
},
want: 1,
},
{
name: "stop fails",
hook: func(shutdowner fx.Shutdowner) fx.Hook {
return fx.StartStopHook(
func() error { return shutdowner.Shutdown() },
func() error { return errTestHook },
)
},
want: 1,
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
app := fx.New(
fx.NopLogger,
fx.Invoke(func(lc fx.Lifecycle, shutdowner fx.Shutdowner) {
lc.Append(tc.hook(shutdowner))
}),
)
got := runApp(app)
t.Logf("runApp() = %d", got)
if got != tc.want {
t.Errorf("runApp() = %d, want %d", got, tc.want)
}
})
}
}
+2 -23
View File
@@ -16,12 +16,6 @@
# Server settings # Server settings
port: 8080 port: 8080
debug: false debug: false
# While true, the image routes (/v1/image/ and /v1/e/) answer every request
# with 503 and a Retry-After header. The health check keeps answering 200 and
# reports maintenance_mode as true. It stays 200 because the image's Docker
# HEALTHCHECK requests it: a 503 there would make the container unhealthy, and
# upaas marks a deploy failed when its container is unhealthy.
maintenance_mode: false maintenance_mode: false
# Data directory for SQLite database and cache files # Data directory for SQLite database and cache files
@@ -77,19 +71,6 @@ 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, unless
# downstream_timeout has ended first.
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, unless downstream_timeout has ended first.
# max_concurrent_processing: 4
# Time allowed for one fetch from an upstream host (default: 30s) # Time allowed for one fetch from an upstream host (default: 30s)
upstream_fetch_timeout: 30s upstream_fetch_timeout: 30s
@@ -97,10 +78,8 @@ upstream_fetch_timeout: 30s
# (1 GiB) (default: 52428800, 50 MiB) # (1 GiB) (default: 52428800, 50 MiB)
upstream_max_response_size: 52428800 upstream_max_response_size: 52428800
# Time allowed for answering one client request (default: 60s). The # Time allowed for answering one client request, the upstream fetch
# upstream fetch counts toward it, and so do the waits for an upstream # included, so keep it longer than upstream_fetch_timeout (default: 60s)
# connection and for a processing slot (up to 10 seconds each), so keep it
# longer than upstream_fetch_timeout plus 20 seconds.
downstream_timeout: 60s downstream_timeout: 60s
# The origin a browser lets read pixa's responses, sent as the CORS # The origin a browser lets read pixa's responses, sent as the CORS
+5 -13
View File
@@ -1,22 +1,14 @@
#!/bin/sh #!/bin/sh
# deploy/docker-entrypoint.sh: the Docker image's ENTRYPOINT. It runs as # deploy/docker-entrypoint.sh: the Docker image's ENTRYPOINT. It runs as
# root only to make /var/lib/pixa usable by pixad: a host directory # root only to give /var/lib/pixa to pixad: a host directory
# bind-mounted there keeps its host owner, often root, and data from an # bind-mounted there keeps its host owner, often root, and pixad could
# earlier run may belong to another uid. The server itself always runs # not write to it. The server itself always runs as pixad.
# as pixad.
set -eu set -eu
main() { main() {
mkdir -p /var/lib/pixa if [ "$(stat -c %U /var/lib/pixa)" != pixad ]; then
# Only the directory and its top-level entries are checked, so a chown pixad:pixad /var/lib/pixa
# normal start does not walk the cache. -depth gives each directory
# to pixad after its contents, so a start stopped part way leaves
# something at the top for the next start to find; -h changes a
# symlink itself, never the file it points to.
if [ -n "$(find /var/lib/pixa -maxdepth 1 \( ! -user pixad -o ! -group pixad \))" ]; then
find /var/lib/pixa -depth -exec chown -h pixad:pixad {} +
fi fi
chmod 750 /var/lib/pixa
exec su-exec pixad /usr/local/bin/pixad "$@" exec su-exec pixad /usr/local/bin/pixad "$@"
} }
@@ -1,162 +0,0 @@
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)
})
}
}
+13 -58
View File
@@ -10,7 +10,6 @@ import (
"net/url" "net/url"
"os" "os"
"path/filepath" "path/filepath"
"runtime"
"sort" "sort"
"strconv" "strconv"
"strings" "strings"
@@ -26,7 +25,6 @@ const (
DefaultPort = 8080 DefaultPort = 8080
DefaultStateDir = "/var/lib/pixa" DefaultStateDir = "/var/lib/pixa"
DefaultUpstreamConnectionsPerHost = 20 DefaultUpstreamConnectionsPerHost = 20
DefaultUpstreamConnections = 64
DefaultAccessControlAllowOrigin = "*" DefaultAccessControlAllowOrigin = "*"
DefaultUpstreamFetchTimeout = 30 * time.Second DefaultUpstreamFetchTimeout = 30 * time.Second
DefaultUpstreamMaxResponseSize = 50 << 20 // 50 MiB DefaultUpstreamMaxResponseSize = 50 << 20 // 50 MiB
@@ -48,8 +46,6 @@ 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"
@@ -83,7 +79,7 @@ var (
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")
errSizeOutOfRange = errors.New("outside the accepted range") errSizeOutOfRange = errors.New("outside the accepted range")
errMustBeAtLeastOne = errors.New("must be at least 1") errTooFewConnections = 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; " +
@@ -130,12 +126,6 @@ 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
// UpstreamFetchTimeout is the time allowed for one fetch from an // UpstreamFetchTimeout is the time allowed for one fetch from an
// upstream host. UpstreamMaxResponseSize is the largest upstream // upstream host. UpstreamMaxResponseSize is the largest upstream
// response accepted, in bytes, and also the image processor's input // response accepted, in bytes, and also the image processor's input
@@ -236,12 +226,14 @@ func New(_ fx.Lifecycle, params Params) (*Config, error) {
// unparseable or invalid is an error: defaults apply only to omitted // unparseable or invalid is an error: defaults apply only to omitted
// keys, never to invalid explicit values. // keys, never to invalid explicit values.
func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) { func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
err := validateKnownKeys(sc) if sc != nil {
if err != nil { err := validateKnownKeys(sc)
return nil, err if err != nil {
return nil, err
}
} }
err = validateAllowlistHostsValue(sc) err := validateAllowlistHostsValue(sc)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@@ -279,12 +271,6 @@ 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)),
UpstreamFetchTimeout: loader.durationVal( UpstreamFetchTimeout: loader.durationVal(
keyUpstreamFetchTimeout, DefaultUpstreamFetchTimeout), keyUpstreamFetchTimeout, DefaultUpstreamFetchTimeout),
UpstreamMaxResponseSize: loader.int64Val( UpstreamMaxResponseSize: loader.int64Val(
@@ -335,13 +321,8 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
// being silently ignored, and rejects keys that are explicitly set to // being silently ignored, and rejects keys that are explicitly set to
// null: a null is a SET value, never an omission, so it must not // null: a null is a SET value, never an omission, so it must not
// silently take the default. The env section is permitted because // silently take the default. The env section is permitted because
// smartconfig consumes it for environment variable injection. A nil sc // smartconfig consumes it for environment variable injection.
// means no config file, which has no keys to check.
func validateKnownKeys(sc *smartconfig.Config) error { func validateKnownKeys(sc *smartconfig.Config) error {
if sc == nil {
return nil
}
var unknown, nullKeys []string var unknown, nullKeys []string
for key, value := range sc.Data() { for key, value := range sc.Data() {
@@ -411,8 +392,7 @@ 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, keyUpstreamConnections, keyUpstreamConnectionsPerHost, keyCacheMaxBytes, keyBlockedNetworks,
keyMaxConcurrentProcessing, keyCacheMaxBytes, keyBlockedNetworks,
keyTrustedProxies, keyAccessControlAllowOrigin, keyUpstreamFetchTimeout, keyTrustedProxies, keyAccessControlAllowOrigin, keyUpstreamFetchTimeout,
keyUpstreamMaxResponseSize, keyDownstreamTimeout, "env": keyUpstreamMaxResponseSize, keyDownstreamTimeout, "env":
return true return true
@@ -439,8 +419,6 @@ 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",
@@ -584,9 +562,10 @@ func (c *Config) validate() error {
settingName(keyPort), c.Port, errPortOutOfRange, maxPort) settingName(keyPort), c.Port, errPortOutOfRange, maxPort)
} }
err = c.validateConcurrencyLimits() if c.UpstreamConnectionsPerHost < 1 {
if err != nil { return fmt.Errorf("%s: value %d %w",
return err settingName(keyUpstreamConnectionsPerHost),
c.UpstreamConnectionsPerHost, errTooFewConnections)
} }
if c.StateDir == "" { if c.StateDir == "" {
@@ -705,30 +684,6 @@ func (c *Config) validateAccessControlAllowOrigin() 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
@@ -67,8 +67,6 @@ 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")
@@ -95,8 +93,6 @@ 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")},
+5 -7
View File
@@ -113,17 +113,15 @@ 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,
MaxConcurrentProcessing: s.config.MaxConcurrentProcessing, Logger: s.log,
Logger: s.log,
}) })
if err != nil { if err != nil {
return err return err
-9
View File
@@ -12,7 +12,6 @@ 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"
) )
@@ -218,14 +217,6 @@ 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)
} }
-5
View File
@@ -12,7 +12,6 @@ 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"
) )
@@ -125,10 +124,6 @@ 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)
@@ -1,49 +0,0 @@
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())
}
}
}
}
+27 -78
View File
@@ -1,6 +1,5 @@
// Package httpfetcher fetches content from upstream HTTP origins with SSRF // Package httpfetcher fetches content from upstream HTTP origins with SSRF
// protection, connection limits per host and for all hosts together, and // protection, per-host connection limits, and content-type validation.
// content-type validation.
package httpfetcher package httpfetcher
import ( import (
@@ -29,13 +28,8 @@ 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"
@@ -76,7 +70,6 @@ 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.
@@ -129,9 +122,6 @@ 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
@@ -153,22 +143,15 @@ func DefaultConfig() *Config {
}, },
AllowHTTP: false, AllowHTTP: false,
MaxConnectionsPerHost: DefaultMaxConnectionsPerHost, MaxConnectionsPerHost: DefaultMaxConnectionsPerHost,
MaxConnections: DefaultMaxConnections,
} }
} }
// HTTPFetcher implements Fetcher with SSRF protection and connection limits // HTTPFetcher implements Fetcher with SSRF protection and per-host 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.
@@ -209,18 +192,13 @@ 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. When // Fetch retrieves content from the given URL with SSRF protection.
// 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)
@@ -228,17 +206,24 @@ func (f *HTTPFetcher) Fetch(ctx context.Context, url string) (*FetchResult, erro
return nil, err return nil, err
} }
release, err := f.acquireConnection(ctx, extractHost(url)) // Extract host for rate limiting
if err != nil { host := extractHost(url)
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 connection // If we fail before returning a result, release the slot
success := false success := false
defer func() { defer func() {
if !success { if !success {
release() <-sem
} }
}() }()
@@ -282,52 +267,17 @@ 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, release) result, err := f.buildResult(resp, remoteAddr, fetchDuration, sem)
if err != nil { if err != nil {
return nil, err return nil, err
} }
// Mark success so defer doesn't release the connection; closing the // Mark success so defer doesn't release the semaphore
// 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()
@@ -343,12 +293,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 calls release when closed. // whose Content releases the host semaphore slot 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,
release func(), sem chan struct{},
) (*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/")
@@ -383,7 +333,7 @@ func (f *HTTPFetcher) buildResult(
} }
return &FetchResult{ return &FetchResult{
Content: &semaphoreReleasingReadCloser{limitedBody, resp.Body, release}, Content: &semaphoreReleasingReadCloser{limitedBody, resp.Body, sem},
ContentLength: resp.ContentLength, ContentLength: resp.ContentLength,
ContentType: contentType, ContentType: contentType,
Headers: resp.Header, Headers: resp.Header,
@@ -624,18 +574,17 @@ func (r *limitedReader) Read(p []byte) (int, error) {
return n, err return n, err
} }
// semaphoreReleasingReadCloser releases the fetch's connection slots when // semaphoreReleasingReadCloser releases a semaphore slot when closed.
// closed.
type semaphoreReleasingReadCloser struct { type semaphoreReleasingReadCloser struct {
*limitedReader *limitedReader
closer io.Closer closer io.Closer
release func() sem chan struct{}
} }
func (r *semaphoreReleasingReadCloser) Close() error { func (r *semaphoreReleasingReadCloser) Close() error {
err := r.closer.Close() err := r.closer.Close()
r.release() <-r.sem // Release semaphore slot
return err return err
} }
@@ -1,165 +0,0 @@
package httpfetcher
import (
"context"
"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()
}
// TestFetchFreesHostSlotWhenContextEndsWaitingForConnection checks that a
// fetch whose request context ends while it waits for a connection shared
// by all hosts gives its host's slot back. With MaxConnections at 1 and one
// response open, a fetch from another host takes that host's slot and waits;
// its context ends long before the 10 second wait timeout.
func TestFetchFreesHostSlotWhenContextEndsWaitingForConnection(t *testing.T) {
t.Parallel()
srv := startUpstream(t)
cfg := DefaultConfig()
cfg.MaxConnections = 1
f, _ := newServerFetcher(t, srv, cfg)
first, err := f.Fetch(testContext(t), imageURLOnPort(81))
if err != nil {
t.Fatalf("first Fetch() error = %v", err)
}
defer func() { _ = first.Content.Close() }()
ctx, cancel := context.WithTimeout(t.Context(), 100*time.Millisecond)
defer cancel()
_, err = f.Fetch(ctx, imageURLOnPort(82))
if !errors.Is(err, context.DeadlineExceeded) {
t.Fatalf("second Fetch() error = %v, want context.DeadlineExceeded", err)
}
if held := semLen(f, testPublicHost+":82"); held != 0 {
t.Errorf("the fetch kept its host's slot after its context ended: "+
"%d held", held)
}
}
// 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()
})
}
}
+5 -74
View File
@@ -7,9 +7,7 @@ import (
"errors" "errors"
"fmt" "fmt"
"io" "io"
"runtime"
"sync" "sync"
"time"
"github.com/davidbyttow/govips/v2/vips" "github.com/davidbyttow/govips/v2/vips"
) )
@@ -19,21 +17,11 @@ 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, one worker thread per // initVips initializes libvips with quiet logging.
// 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(&vips.Config{ vips.Startup(nil)
ConcurrencyLevel: 1,
MaxCacheSize: 0,
MaxCacheMem: 0,
MaxCacheFiles: 0,
})
}) })
} }
@@ -118,23 +106,9 @@ 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.
@@ -143,9 +117,6 @@ 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.
@@ -158,34 +129,17 @@ 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. When // Process transforms an image according to the request.
// 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(
ctx context.Context, _ 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.
@@ -331,29 +285,6 @@ 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()
@@ -1,368 +0,0 @@
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()
})
}
}
// TestWaitForProcessing holds a processing slot with a Process call that
// cannot finish reading its input. WaitForProcessing must report that image
// when its context ends first, wait for it otherwise, return 0 once it has
// finished, and give back the slots it took while waiting.
func TestWaitForProcessing(t *testing.T) {
t.Parallel()
proc := New(Params{MaxConcurrentProcessing: 2})
gate := make(chan struct{})
entered := make(chan struct{}, 1)
results := make(chan error, 1)
openGate := sync.OnceFunc(func() { close(gate) })
t.Cleanup(openGate)
processInBackground(proc, &gatedReader{
data: bytes.NewReader(createTestJPEG(t, 10, 10)), gate: gate,
entered: entered, counter: &readingCounter{},
}, results)
waitForEntries(t, entered, 1)
ctx, cancel := context.WithTimeout(t.Context(), 100*time.Millisecond)
defer cancel()
stillProcessing := proc.WaitForProcessing(ctx)
t.Logf("WaitForProcessing() after its context ended: %d", stillProcessing)
if stillProcessing != 1 {
t.Errorf("WaitForProcessing() after its context ended = %d, want 1",
stillProcessing)
}
waited := make(chan int, 1)
go func() { waited <- proc.WaitForProcessing(t.Context()) }()
select {
case got := <-waited:
t.Fatalf("WaitForProcessing() = %d while an image was being processed",
got)
case <-time.After(100 * time.Millisecond):
}
openGate()
err := <-results
if err != nil {
t.Errorf("Process() error = %v, want nil", err)
}
select {
case got := <-waited:
if got != 0 {
t.Errorf("WaitForProcessing() once processing finished = %d, want 0",
got)
}
case <-time.After(5 * time.Second):
t.Fatal("WaitForProcessing() did not return once processing finished")
}
if held := len(proc.processingSemaphore); held != 0 {
t.Errorf("%d slots still held after WaitForProcessing() returned", held)
}
}
+13 -25
View File
@@ -159,13 +159,15 @@ type LookupResult struct {
CacheStatus CacheStatus CacheStatus CacheStatus
} }
// Lookup checks if a processed variant exists on disk. Hits touch the // Lookup checks if a processed variant exists on disk: a variant held
// variant's LRU timestamp; a disabled cache always misses. // in metaCache counts as present without a check of the disk. Hits
// touch the variant's LRU timestamp; a disabled cache always misses.
func (c *Cache) Lookup(ctx context.Context, req *ImageRequest) (*LookupResult, error) { func (c *Cache) Lookup(ctx context.Context, req *ImageRequest) (*LookupResult, error) {
cacheKey := CacheKey(req) cacheKey := CacheKey(req)
// Check variant storage directly - no DB needed for cache hits // Check memory, then variant storage - no DB needed for cache hits
if !c.disabled && c.variants.Exists(cacheKey) { if !c.disabled &&
(c.metaCache.Contains(cacheKey) || c.variants.Exists(cacheKey)) {
c.touchVariant(ctx, cacheKey) c.touchVariant(ctx, cacheKey)
return &LookupResult{ return &LookupResult{
@@ -184,9 +186,7 @@ func (c *Cache) Lookup(ctx context.Context, req *ImageRequest) (*LookupResult, e
// GetVariant returns a reader, size, and content type for a cached // GetVariant returns a reader, size, and content type for a cached
// variant. The content type comes from metaCache, or else from the // variant. The content type comes from metaCache, or else from the
// variant's .meta file and is then kept in metaCache. A variant with // variant's .meta file and is then kept in metaCache.
// no .meta file is served as application/octet-stream, which is not
// kept.
func (c *Cache) GetVariant(cacheKey VariantKey) (io.ReadCloser, int64, string, error) { func (c *Cache) GetVariant(cacheKey VariantKey) (io.ReadCloser, int64, string, error) {
if c.disabled { if c.disabled {
return nil, 0, "", ErrNotFound return nil, 0, "", ErrNotFound
@@ -407,16 +407,13 @@ func (c *Cache) GetSourceMetadataID(
return id, nil return id, nil
} }
// GetSourceContent returns a reader for cached source content by its hash, // GetSourceContent returns a reader for cached source content by its hash.
// and the content's size in bytes. func (c *Cache) GetSourceContent(contentHash ContentHash) (io.ReadCloser, error) {
func (c *Cache) GetSourceContent(
contentHash ContentHash,
) (io.ReadCloser, int64, error) {
if c.disabled { if c.disabled {
return nil, 0, ErrNotFound return nil, ErrNotFound
} }
return c.srcContent.LoadWithSize(contentHash) return c.srcContent.Load(contentHash)
} }
// CleanExpired removes expired entries from the cache. // CleanExpired removes expired entries from the cache.
@@ -525,12 +522,7 @@ func (c *Cache) IncrementTransformCount(ctx context.Context) {
// loadVariantWithMeta is GetVariant for a variant metaCache does not // loadVariantWithMeta is GetVariant for a variant metaCache does not
// hold: it reads the content type from the variant's .meta file and // hold: it reads the content type from the variant's .meta file and
// keeps it in metaCache, unless a StoreVariant has put one there // keeps it in metaCache.
// meanwhile, as the store's is newer. A read that finds no .meta file,
// as one can between a store's writing of the variant file and of its
// .meta file, serves application/octet-stream and keeps nothing, so
// metaCache only ever holds a type read from a .meta file or passed to
// StoreVariant.
func (c *Cache) loadVariantWithMeta( func (c *Cache) loadVariantWithMeta(
cacheKey VariantKey, cacheKey VariantKey,
) (io.ReadCloser, int64, string, error) { ) (io.ReadCloser, int64, string, error) {
@@ -539,11 +531,7 @@ func (c *Cache) loadVariantWithMeta(
return nil, 0, "", err return nil, 0, "", err
} }
if contentType == "" { c.metaCache.Add(cacheKey, contentType)
return reader, size, fallbackContentType, nil
}
c.metaCache.ContainsOrAdd(cacheKey, contentType)
return reader, size, contentType, nil return reader, size, contentType, nil
} }
+2 -2
View File
@@ -39,8 +39,8 @@ const tempFilePrefix = ".tmp-"
// to each variant file. // to each variant file.
const variantMetaSuffix = ".meta" const variantMetaSuffix = ".meta"
// fallbackContentType is the content type given to a variant file that // fallbackContentType is recorded when a reconciled variant file has
// has no readable .meta sidecar, when it is served or reconciled. // no readable .meta sidecar.
const fallbackContentType = "application/octet-stream" const fallbackContentType = "application/octet-stream"
// UsageBytes returns the total number of bytes of cache content // UsageBytes returns the total number of bytes of cache content
@@ -1,125 +0,0 @@
package imgcache
import (
"image/color"
"image/jpeg"
"io"
"os"
"testing"
"time"
"sneak.berlin/go/pixa/internal/imageprocessor"
)
// widthOnlyRequest asks for the test photo at width, its height scaled to
// keep the photo's aspect ratio.
func widthOnlyRequest(fixtures *TestFixtures, width int) *ImageRequest {
return &ImageRequest{
SourceHost: fixtures.GoodHost,
SourcePath: testPathPhoto,
Size: Size{Width: width},
Format: FormatJPEG,
Quality: 85,
FitMode: FitCover,
}
}
// holdProcessingSlot takes one of proc's processing slots and returns the
// func that gives it back. Process takes its slot before it reads its input,
// so once it has read a byte from the pipe it holds the slot, until the pipe
// is closed.
func holdProcessingSlot(
t *testing.T, proc *imageprocessor.ImageProcessor,
) func() {
t.Helper()
input, feed := io.Pipe()
go func() {
_, _ = proc.Process(t.Context(), input, &imageprocessor.Request{})
}()
_, err := feed.Write([]byte{0})
if err != nil {
t.Fatalf("Process call to hold the slot did not start: %v", err)
}
release := func() { _ = feed.Close() }
t.Cleanup(release)
return release
}
// TestService_Get_WaitsForSlotBeforeReadingCachedSource checks that a
// request whose source is cached holds none of it while it waits for a
// processing slot: it reads the cached file only once it has a slot. With
// the only slot held, a request for a new width of the cached 100x100 photo
// waits; the cached file is then rewritten as a 100x50 image before the slot
// is freed, so the request must answer with that image scaled to 40x20.
func TestService_Get_WaitsForSlotBeforeReadingCachedSource(t *testing.T) {
t.Parallel()
svc, fixtures := SetupTestService(t)
svc.processor = imageprocessor.New(
imageprocessor.Params{MaxConcurrentProcessing: 1},
)
// A first request caches the photo as a source.
resp, err := svc.Get(t.Context(), widthOnlyRequest(fixtures, 50))
if err != nil {
t.Fatalf("first Get() error = %v", err)
}
_ = resp.Content.Close()
contentHash, _, err := svc.cache.LookupSource(t.Context(),
widthOnlyRequest(fixtures, 50))
if err != nil || contentHash == "" {
t.Fatalf("LookupSource() = %q, %v; want the cached source",
contentHash, err)
}
release := holdProcessingSlot(t, svc.processor)
var (
waited *ImageResponse
waitedErr error
)
done := make(chan struct{})
go func() {
defer close(done)
waited, waitedErr = svc.Get(t.Context(), widthOnlyRequest(fixtures, 40))
}()
// Give the request time to reach the slot: had it read the cached source
// before waiting, it would have read it by now.
time.Sleep(100 * time.Millisecond)
err = os.WriteFile(svc.cache.srcContent.hashToPath(contentHash),
generateTestJPEG(t, 100, 50, color.RGBA{0, 0, 255, 255}), 0o600)
if err != nil {
t.Fatalf("failed to rewrite the cached source: %v", err)
}
release()
<-done
if waitedErr != nil {
t.Fatalf("Get() error = %v", waitedErr)
}
defer func() { _ = waited.Content.Close() }()
output, err := jpeg.DecodeConfig(waited.Content)
if err != nil {
t.Fatalf("failed to decode the response: %v", err)
}
if output.Width != 40 || output.Height != 20 {
t.Errorf("response is %dx%d, want 40x20: the request read the cached "+
"source before it had a processing slot", output.Width, output.Height)
}
}
@@ -2,7 +2,6 @@ package imgcache
import ( import (
"bytes" "bytes"
"encoding/json"
"errors" "errors"
"fmt" "fmt"
"io" "io"
@@ -84,16 +83,6 @@ func assertLookupMisses(t *testing.T, cache *Cache, request *ImageRequest) {
} }
} }
// renameFile renames the file at from to to.
func renameFile(t *testing.T, from, to string) {
t.Helper()
err := os.Rename(from, to)
if err != nil {
t.Fatalf("renaming %s: %v", from, err)
}
}
// TestSecondHitDoesNotReadMetaFile checks that once a variant has been // TestSecondHitDoesNotReadMetaFile checks that once a variant has been
// stored or read, a hit takes its content type from memory: with the // stored or read, a hit takes its content type from memory: with the
// .meta file deleted, GetVariant must still return the stored content // .meta file deleted, GetVariant must still return the stored content
@@ -156,91 +145,6 @@ func TestReadDuringStoreKeepsStoredContentType(t *testing.T) {
assertVariantServed(t, cache, testVariantKeyOne, content) assertVariantServed(t, cache, testVariantKeyOne, content)
} }
// TestReadOfOlderMetaFileKeepsStoredContentType checks that a read
// which got its content type from a .meta file that StoreVariant had
// not yet rewritten cannot replace the type the store kept in memory.
// The test writes such a .meta file, with a different content type,
// after the store, then runs the part of GetVariant that comes after
// its check of memory.
func TestReadOfOlderMetaFileKeepsStoredContentType(t *testing.T) {
t.Parallel()
cache, _ := newEvictionTestCache(t, 1<<20)
content := []byte("webp variant bytes")
storeEvictionTestVariant(t, cache, testVariantKeyOne, content)
olderMeta, err := json.Marshal(VariantMeta{
ContentType: testContentTypeJPEG,
Size: int64(len(content)),
})
if err != nil {
t.Fatalf("encoding .meta file: %v", err)
}
metaPath := cache.variants.keyToPath(testVariantKeyOne) + ".meta"
err = os.WriteFile(metaPath, olderMeta, StorageFilePerm)
if err != nil {
t.Fatalf("writing .meta file: %v", err)
}
reader, _, contentType, err := cache.loadVariantWithMeta(testVariantKeyOne)
if err != nil {
t.Fatalf("loadVariantWithMeta(%s) error = %v", testVariantKeyOne, err)
}
_ = reader.Close()
if contentType != testContentTypeJPEG {
t.Fatalf("loadVariantWithMeta(%s) content type = %q, want %q from the .meta file",
testVariantKeyOne, contentType, testContentTypeJPEG)
}
kept, _ := cache.metaCache.Get(testVariantKeyOne)
if kept != testContentTypeWebP {
t.Errorf("content type in memory = %q, want the stored %q",
kept, testContentTypeWebP)
}
}
// TestFailedReadDuringStoreKeepsStoredContentType checks that a read
// which found no .meta file cannot leave application/octet-stream in
// memory, even when another read has removed the content type
// StoreVariant kept there. In this order: the store; a read that finds
// the variant in memory but cannot open its file, and so removes it
// from memory; a read that opened the variant file before the store
// wrote its .meta file. Later hits must get the stored content type.
func TestFailedReadDuringStoreKeepsStoredContentType(t *testing.T) {
t.Parallel()
cache, _ := newEvictionTestCache(t, 1<<20)
content := []byte("webp variant bytes")
variantPath := cache.variants.keyToPath(testVariantKeyOne)
metaPath := variantPath + ".meta"
storeEvictionTestVariant(t, cache, testVariantKeyOne, content)
renameFile(t, variantPath, variantPath+".hidden")
assertVariantNotFound(t, cache, testVariantKeyOne)
renameFile(t, variantPath+".hidden", variantPath)
renameFile(t, metaPath, metaPath+".hidden")
reader, _, contentType, err := cache.GetVariant(testVariantKeyOne)
if err != nil {
t.Fatalf("GetVariant(%s) error = %v", testVariantKeyOne, err)
}
_ = reader.Close()
t.Logf("the read without a .meta file got content type %q", contentType)
renameFile(t, metaPath+".hidden", metaPath)
assertVariantServed(t, cache, testVariantKeyOne, content)
}
// TestEvictedVariantIsNotServed checks that a variant the evictor // TestEvictedVariantIsNotServed checks that a variant the evictor
// removed is a miss and cannot be read, although it had been stored // removed is a miss and cannot be read, although it had been stored
// and served before. // and served before.
+33 -48
View File
@@ -43,9 +43,6 @@ 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
} }
@@ -94,10 +91,9 @@ func NewService(cfg *ServiceConfig) (*Service, error) {
} }
maxResponseSize := fetcherCfg.MaxResponseSize maxResponseSize := fetcherCfg.MaxResponseSize
processor := imageprocessor.New(imageprocessor.Params{ processor := imageprocessor.New(
MaxInputBytes: maxResponseSize, imageprocessor.Params{MaxInputBytes: maxResponseSize},
MaxConcurrentProcessing: cfg.MaxConcurrentProcessing, )
})
return &Service{ return &Service{
cache: cfg.Cache, cache: cfg.Cache,
@@ -241,37 +237,38 @@ func (s *Service) GenerateSignedURL(
baseURL, path, sig, exp, req.Quality, req.FitMode), nil baseURL, path, sig, exp, req.Quality, req.FitMode), nil
} }
// loadCachedSource opens source content from cache, without reading it, and // loadCachedSource attempts to load source content from cache, returning nil
// returns it with its size; nil if the cached data is unavailable, empty or // if the cached data is unavailable or exceeds maxResponseSize.
// exceeds maxResponseSize. func (s *Service) loadCachedSource(contentHash ContentHash) []byte {
func (s *Service) loadCachedSource( reader, err := s.cache.GetSourceContent(contentHash)
contentHash ContentHash,
) (io.ReadCloser, int64) {
reader, size, err := s.cache.GetSourceContent(contentHash)
if err != nil { if err != nil {
s.log.Warn("failed to load cached source, fetching", "error", err) s.log.Warn("failed to load cached source, fetching", "error", err)
return nil, 0 return nil
} }
if size > s.maxResponseSize { // Bound the read to maxResponseSize to prevent unbounded memory use
_ = reader.Close() // from unexpectedly large cached files.
limited := io.LimitReader(reader, s.maxResponseSize+1)
data, err := io.ReadAll(limited)
_ = reader.Close()
if err != nil {
s.log.Warn("failed to read cached source, fetching", "error", err)
return nil
}
if int64(len(data)) > s.maxResponseSize {
s.log.Warn("cached source exceeds max response size, discarding", s.log.Warn("cached source exceeds max response size, discarding",
"hash", contentHash, "hash", contentHash,
"max_bytes", s.maxResponseSize, "max_bytes", s.maxResponseSize,
) )
return nil, 0 return nil
} }
if size == 0 { return data
_ = reader.Close()
return nil, 0
}
return reader, size
} }
// processFromSourceOrFetch processes an image, using cached source content // processFromSourceOrFetch processes an image, using cached source content
@@ -288,27 +285,22 @@ func (s *Service) processFromSourceOrFetch(
s.log.Warn("source lookup failed", "error", err) s.log.Warn("source lookup failed", "error", err)
} }
var ( var sourceData []byte
source io.ReadCloser
sourceSize int64
)
if contentHash != "" { if contentHash != "" {
s.log.Debug("using cached source", "hash", contentHash) s.log.Debug("using cached source", "hash", contentHash)
source, sourceSize = s.loadCachedSource(contentHash) sourceData = s.loadCachedSource(contentHash)
} }
// 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 source == nil { if len(sourceData) == 0 {
return s.fetchAndProcess(ctx, req, cacheKey) return s.fetchAndProcess(ctx, req, cacheKey)
} }
defer func() { _ = source.Close() }() // Process using cached source; nothing was fetched from upstream
resp, err := s.processAndStore(
// Process using cached source; nothing was fetched from upstream. The ctx, req, cacheKey, sourceData, int64(len(sourceData)),
// image processor reads the source only once it has a processing slot, )
// so a request waiting for one holds none of it in memory.
resp, err := s.processAndStore(ctx, req, cacheKey, source, sourceSize)
return resp, 0, err return resp, 0, err
} }
@@ -342,10 +334,6 @@ func (s *Service) fetchAndProcess(
return nil, 0, fmt.Errorf("upstream fetch failed: %w", err) return nil, 0, fmt.Errorf("upstream fetch failed: %w", err)
} }
// Closing the body frees the upstream connection. It is closed only
// after processing, so the fetcher's connection limit also bounds the
// fetched sources held in memory while their requests wait for a
// processing slot.
defer func() { _ = fetchResult.Content.Close() }() defer func() { _ = fetchResult.Content.Close() }()
// Read and validate the source content // Read and validate the source content
@@ -391,20 +379,17 @@ func (s *Service) fetchAndProcess(
// Continue even if caching fails // Continue even if caching fails
} }
resp, err := s.processAndStore( resp, err := s.processAndStore(ctx, req, cacheKey, sourceData, fetchBytes)
ctx, req, cacheKey, bytes.NewReader(sourceData), fetchBytes,
)
return resp, fetchBytes, err return resp, fetchBytes, err
} }
// processAndStore processes the image read from source and stores the // processAndStore processes an image and stores the result.
// result.
func (s *Service) processAndStore( func (s *Service) processAndStore(
ctx context.Context, ctx context.Context,
req *ImageRequest, req *ImageRequest,
cacheKey VariantKey, cacheKey VariantKey,
source io.Reader, sourceData []byte,
fetchBytes int64, fetchBytes int64,
) (*ImageResponse, error) { ) (*ImageResponse, error) {
// Process the image // Process the image
@@ -417,7 +402,7 @@ func (s *Service) processAndStore(
FitMode: imageprocessor.FitMode(req.FitMode), FitMode: imageprocessor.FitMode(req.FitMode),
} }
processResult, err := s.processor.Process(ctx, source, processReq) processResult, err := s.processor.Process(ctx, bytes.NewReader(sourceData), processReq)
if err != nil { if err != nil {
return nil, fmt.Errorf("image processing failed: %w", err) return nil, fmt.Errorf("image processing failed: %w", err)
} }
+3 -4
View File
@@ -531,8 +531,7 @@ func (s *VariantStorage) LoadWithSize(key VariantKey) (io.ReadCloser, int64, err
} }
// LoadWithMeta returns a reader, size, and content type for the content at // LoadWithMeta returns a reader, size, and content type for the content at
// the given key. The content type is read from the .meta file, and is // the given key.
// empty when that file is missing or unreadable.
func (s *VariantStorage) LoadWithMeta( func (s *VariantStorage) LoadWithMeta(
key VariantKey, key VariantKey,
) (io.ReadCloser, int64, string, error) { ) (io.ReadCloser, int64, string, error) {
@@ -541,8 +540,8 @@ func (s *VariantStorage) LoadWithMeta(
return nil, 0, "", err return nil, 0, "", err
} }
var contentType string // Load metadata for content type
contentType := "application/octet-stream" // fallback
metaPath := s.keyToPath(key) + ".meta" metaPath := s.keyToPath(key) + ".meta"
metaData, err := os.ReadFile(metaPath) //nolint:gosec // path derived from cache key metaData, err := os.ReadFile(metaPath) //nolint:gosec // path derived from cache key
@@ -18,7 +18,6 @@ import (
"sneak.berlin/go/pixa/internal/database" "sneak.berlin/go/pixa/internal/database"
"sneak.berlin/go/pixa/internal/globals" "sneak.berlin/go/pixa/internal/globals"
"sneak.berlin/go/pixa/internal/handlers" "sneak.berlin/go/pixa/internal/handlers"
"sneak.berlin/go/pixa/internal/healthcheck"
"sneak.berlin/go/pixa/internal/logger" "sneak.berlin/go/pixa/internal/logger"
"sneak.berlin/go/pixa/internal/middleware" "sneak.berlin/go/pixa/internal/middleware"
) )
@@ -72,15 +71,8 @@ func newTestServer(t *testing.T) *Server {
t.Fatalf("database.New() error = %v", err) t.Fatalf("database.New() error = %v", err)
} }
hc, err := healthcheck.New(lc, healthcheck.Params{
Globals: &globals.Globals{}, Config: cfg, Logger: log, Database: db,
})
if err != nil {
t.Fatalf("healthcheck.New() error = %v", err)
}
h, err := handlers.New(lc, handlers.Params{ h, err := handlers.New(lc, handlers.Params{
Logger: log, Healthcheck: hc, Database: db, Config: cfg, Logger: log, Database: db, Config: cfg,
}) })
if err != nil { if err != nil {
t.Fatalf("handlers.New() error = %v", err) t.Fatalf("handlers.New() error = %v", err)
@@ -1,169 +0,0 @@
package server
import (
"encoding/json"
"net/http"
"net/http/httptest"
"strconv"
"testing"
"sneak.berlin/go/pixa/internal/healthcheck"
)
// unsignedImagePath is an image URL that carries no signature.
const unsignedImagePath = "/v1/image/cdn.example.com/cat.jpg/100x100.jpeg"
// TestMaintenanceModeRefusesImageRequests verifies that while maintenance
// mode is on, both image routes answer 503 Service Unavailable with a
// Retry-After header and the JSON error body the image handlers send.
func TestMaintenanceModeRefusesImageRequests(t *testing.T) {
t.Parallel()
s := newTestServer(t)
s.config.MaintenanceMode = true
requests := []struct {
method string
path string
}{
{http.MethodGet, unsignedImagePath},
{http.MethodHead, unsignedImagePath},
{http.MethodGet, "/v1/e/token/cat.jpg"},
}
for _, tc := range requests {
t.Run(tc.method+" "+tc.path, func(t *testing.T) {
t.Parallel()
rec := httptest.NewRecorder()
s.ServeHTTP(rec, httptest.NewRequestWithContext(
t.Context(), tc.method, tc.path, nil))
t.Logf("status %d, body %s", rec.Code, rec.Body.String())
if rec.Code != http.StatusServiceUnavailable {
t.Fatalf("status = %d, want %d",
rec.Code, http.StatusServiceUnavailable)
}
retryAfter := rec.Header().Get("Retry-After")
seconds, err := strconv.Atoi(retryAfter)
if err != nil || seconds <= 0 {
t.Errorf("Retry-After = %q, want a positive number of seconds",
retryAfter)
}
// A HEAD response carries no body.
if tc.method == http.MethodHead {
return
}
var body struct {
Error string `json:"error"`
Status int `json:"status"`
Timestamp string `json:"timestamp"`
}
err = json.NewDecoder(rec.Body).Decode(&body)
if err != nil {
t.Fatalf("body is not JSON: %v", err)
}
if body.Error == "" || body.Status != http.StatusServiceUnavailable ||
body.Timestamp == "" {
t.Errorf("body = %+v, want an error, status %d and a timestamp",
body, http.StatusServiceUnavailable)
}
})
}
}
// TestImageRequestsServedWithoutMaintenanceMode verifies that while
// maintenance mode is off, image requests reach the image handlers instead
// of the 503. The handlers refuse an unsigned image URL with 401 and a token
// they cannot decrypt with 400, so either status shows a request got through.
func TestImageRequestsServedWithoutMaintenanceMode(t *testing.T) {
t.Parallel()
s := newTestServer(t)
s.config.MaintenanceMode = false
requests := []struct {
method string
path string
want int
}{
{http.MethodGet, unsignedImagePath, http.StatusUnauthorized},
{http.MethodHead, unsignedImagePath, http.StatusUnauthorized},
{http.MethodGet, "/v1/e/token/cat.jpg", http.StatusBadRequest},
}
for _, tc := range requests {
t.Run(tc.method+" "+tc.path, func(t *testing.T) {
t.Parallel()
rec := httptest.NewRecorder()
s.ServeHTTP(rec, httptest.NewRequestWithContext(
t.Context(), tc.method, tc.path, nil))
t.Logf("status %d, body %s", rec.Code, rec.Body.String())
if rec.Code != tc.want {
t.Errorf("status = %d, want %d from the image handler",
rec.Code, tc.want)
}
})
}
}
// TestMaintenanceModeKeepsOtherRoutes verifies that while maintenance mode
// is on, the health check still answers 200 and reports it, and the login
// page and /metrics still answer 200. The image's Docker HEALTHCHECK
// requests the health check: a 503 there would make the container
// unhealthy, and upaas marks a deploy failed when its container is
// unhealthy.
func TestMaintenanceModeKeepsOtherRoutes(t *testing.T) {
t.Parallel()
s := newTestServer(t)
s.config.MaintenanceMode = true
// /metrics is routed only when its username is set.
s.config.MetricsUsername = "metrics"
s.config.MetricsPassword = "metrics-password"
s.SetupRoutes()
rec := httptest.NewRecorder()
s.ServeHTTP(rec, httptest.NewRequestWithContext(t.Context(),
http.MethodGet, "/.well-known/healthcheck.json", nil))
t.Logf("health check status %d, body %s", rec.Code, rec.Body.String())
if rec.Code != http.StatusOK {
t.Fatalf("health check status = %d, want %d", rec.Code, http.StatusOK)
}
var health healthcheck.Response
err := json.NewDecoder(rec.Body).Decode(&health)
if err != nil || !health.Maintenance {
t.Errorf("health check maintenance_mode = %v (error %v), want true",
health.Maintenance, err)
}
rec = httptest.NewRecorder()
s.ServeHTTP(rec, clientRequest(t, http.MethodGet, nil, firstClient, ""))
if rec.Code != http.StatusOK {
t.Errorf("login page status = %d, want %d", rec.Code, http.StatusOK)
}
req := httptest.NewRequestWithContext(t.Context(),
http.MethodGet, "/metrics", nil)
req.SetBasicAuth(s.config.MetricsUsername, s.config.MetricsPassword)
rec = httptest.NewRecorder()
s.ServeHTTP(rec, req)
if rec.Code != http.StatusOK {
t.Errorf("/metrics status = %d, want %d", rec.Code, http.StatusOK)
}
}
+8 -49
View File
@@ -1,9 +1,7 @@
package server package server
import ( import (
"encoding/json"
"net/http" "net/http"
"strconv"
"time" "time"
sentryhttp "github.com/getsentry/sentry-go/http" sentryhttp "github.com/getsentry/sentry-go/http"
@@ -19,10 +17,6 @@ import (
// make per minute; the next is refused with 429 Too Many Requests. // make per minute; the next is refused with 429 Too Many Requests.
const LoginAttemptsPerMinute = 5 const LoginAttemptsPerMinute = 5
// MaintenanceRetryAfterSeconds is the Retry-After, in seconds, sent with
// the 503 that the image routes answer while maintenance mode is on.
const MaintenanceRetryAfterSeconds = 300
// SetupRoutes configures all HTTP routes. // SetupRoutes configures all HTTP routes.
func (s *Server) SetupRoutes() { func (s *Server) SetupRoutes() {
s.router = chi.NewRouter() s.router = chi.NewRouter()
@@ -74,23 +68,15 @@ func (s *Server) SetupRoutes() {
s.router.Get("/logout", s.h.HandleLogout()) s.router.Get("/logout", s.h.HandleLogout())
// Image routes, refused while maintenance mode is on. Only these: the // Main image proxy route
// image's Docker HEALTHCHECK requests the health check, a 503 there // /v1/image/<host>/<path>/<width>x<height>.<format>
// would make the container unhealthy, and upaas marks a deploy failed s.router.Get("/v1/image/*", s.h.HandleImage())
// when its container is unhealthy. s.router.Head("/v1/image/*", s.h.HandleImage())
s.router.Group(func(r chi.Router) {
r.Use(s.refuseDuringMaintenance)
// Main image proxy route // Encrypted image URL route
// /v1/image/<host>/<path>/<width>x<height>.<format> // The trailing filename (e.g., /img.jpg) is ignored but helps
r.Get("/v1/image/*", s.h.HandleImage()) // browsers with content type
r.Head("/v1/image/*", s.h.HandleImage()) s.router.Get("/v1/e/{token}/*", s.h.HandleImageEnc())
// Encrypted image URL route
// The trailing filename (e.g., /img.jpg) is ignored but helps
// browsers with content type
r.Get("/v1/e/{token}/*", s.h.HandleImageEnc())
})
// Metrics endpoint with auth // Metrics endpoint with auth
if s.config.MetricsUsername != "" { if s.config.MetricsUsername != "" {
@@ -100,30 +86,3 @@ func (s *Server) SetupRoutes() {
}) })
} }
} }
// refuseDuringMaintenance answers a request with 503 Service Unavailable,
// a Retry-After header and a JSON error body while maintenance mode is on,
// and passes it on otherwise. The body has the fields of the JSON errors
// the image handlers send.
func (s *Server) refuseDuringMaintenance(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if !s.MaintenanceMode() {
next.ServeHTTP(w, r)
return
}
w.Header().Set("Retry-After", strconv.Itoa(MaintenanceRetryAfterSeconds))
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusServiceUnavailable)
err := json.NewEncoder(w).Encode(map[string]any{
"error": "down for maintenance, try again later",
"status": http.StatusServiceUnavailable,
"timestamp": time.Now().UTC().Format(time.RFC3339),
})
if err != nil {
s.log.Error("json encode error", "error", err)
}
})
}
-96
View File
@@ -1,96 +0,0 @@
package server
import (
"log/slog"
"net"
"testing"
"time"
"go.uber.org/fx"
"go.uber.org/fx/fxtest"
"sneak.berlin/go/pixa/internal/config"
"sneak.berlin/go/pixa/internal/globals"
"sneak.berlin/go/pixa/internal/logger"
)
// shutdownRecorder is an fx.Shutdowner that sends the options of each
// shutdown request on requests.
type shutdownRecorder struct {
requests chan []fx.ShutdownOption
}
func (r shutdownRecorder) Shutdown(opts ...fx.ShutdownOption) error {
r.requests <- opts
return nil
}
// TestSentryInitFailureFailsStartup checks that a Sentry DSN that cannot be
// used makes the server's start hook fail, so fx stops what has already
// started, instead of the process exiting from a goroutine.
func TestSentryInitFailureFailsStartup(t *testing.T) {
t.Parallel()
lc := fxtest.NewLifecycle(t)
log, err := logger.New(lc, logger.Params{Globals: &globals.Globals{}})
if err != nil {
t.Fatalf("logger.New() error = %v", err)
}
_, err = New(lc, Params{
Logger: log,
Globals: &globals.Globals{Appname: "pixad"},
Config: &config.Config{SentryDSN: "not-a-dsn"},
})
if err != nil {
t.Fatalf("New() error = %v", err)
}
err = lc.Start(t.Context())
t.Logf("Start() error = %v", err)
if err == nil {
t.Fatal("Start() error = nil, want the Sentry initialization error")
}
}
// TestListenErrorRequestsShutdownWithExitCode1 occupies the server's port
// and checks that the listen error asks fx to shut down with exit code 1.
func TestListenErrorRequestsShutdownWithExitCode1(t *testing.T) {
t.Parallel()
busy, err := (&net.ListenConfig{}).Listen(t.Context(), "tcp", ":0")
if err != nil {
t.Fatalf("Listen() error = %v", err)
}
t.Cleanup(func() { _ = busy.Close() })
addr, ok := busy.Addr().(*net.TCPAddr)
if !ok {
t.Fatalf("listener address %v is not a TCP address", busy.Addr())
}
requests := make(chan []fx.ShutdownOption, 1)
s := &Server{
log: slog.New(slog.DiscardHandler),
config: &config.Config{Port: addr.Port},
shutdowner: shutdownRecorder{requests: requests},
}
s.httpServer = s.newHTTPServer()
go s.serveUntilShutdown()
select {
case opts := <-requests:
t.Logf("shutdown options = %v", opts)
if len(opts) != 1 || opts[0] != fx.ExitCode(1) {
t.Errorf("shutdown options = %v, want [fx.ExitCode(1)]", opts)
}
case <-time.After(5 * time.Second):
t.Fatal("no shutdown was requested after the listen error")
}
}