Author SHA1 Message Date
clawbot 25c6a959b5 Build the image cache's config from the config in newCacheConfig
check / check (push) Waiting to run
initImageService built it inline, so the choice between the default
limit, an explicit limit and the disk cache off had no test of its own.
It is now one small function, which the previous commit tests.

Model: opus-5-5
2026-10-04 16:27:12 +00:00
clawbot b77b0eaf22 Test the cache config the handlers build from cache_max_bytes
check / check (push) Waiting to run
With cache_max_bytes omitted the cache must work out the default
limit, with an explicit 0 the disk cache must be off, and an explicit
positive value must reach the cache unchanged. newCacheConfig does not
exist yet, so this commit does not compile; the next one adds it.

Model: opus-5-5
2026-10-04 16:26:38 +00:00
clawbot 602da7a45a Count what the cache holds in the default cache_max_bytes (closes #184)
The default limit was 75% of the space free at startup. The cache's
own files are not free space, so a fuller cache got a smaller limit
after a restart and eviction then deleted most of it. The default is
now 75% of the sum of the free space and what the cache already holds
by its own size accounting, at least 500 MiB. The cache works it out
when it opens, after the database is open, so the computation and its
tests moved from internal/config to internal/imgcache; the config only
records whether cache_max_bytes was set, and the handlers turn the
disk cache off only for an explicit 0.

Model: opus-5-5
2026-10-04 16:25:17 +00:00
clawbot 35e771e2b6 Test that the default cache_max_bytes counts what the cache holds
With a fake free-space probe, an empty cache with 4 GiB free gets a
3 GiB default, and the same cache once it holds those 3 GiB, with
1 GiB left free, must keep 3 GiB. The default is still 75% of the
free space alone, so the second check fails: this is the bug in
#184. computeDefaultMaxBytes
only calls the config's existing computation so the test compiles.

Model: opus-5-5
2026-10-04 16:25:17 +00:00
clawbot 48f21d4ecf Abort startup on a config file pixa cannot read (closes #176)
check / check (push) Waiting to run
Of the places pixa looks for its config file on its own, it passed over
any place where os.Stat failed, so a file in a directory pixa may not
enter was skipped without a word and pixa started on a later file or on
the environment and defaults. Now only a path that does not exist, or
that runs through a file (such as under a HOME of /dev/null), is passed
over; any other error aborts startup naming the file, as a file that
does not parse already did. README.md says so where it gives the search
order. A config.yml that links to itself tests this as root too.

Model: opus-5-5
2026-10-04 18:24:43 +02:00
clawbot f8c437b83f Merge TODO.md with git's union merge (closes #190)
check / check (push) Waiting to run
Every PR adds an entry at the top of Completed Steps in TODO.md, so each
merge to next left the other open PRs conflicting there. A root
.gitattributes, copied from sneak/prompts, marks TODO.md merge=union: two
branches that each add an entry at the same place merge without a conflict
and keep both. Git then never reports a conflict in TODO.md, so the
Workflow now says to read the merged entries after every merge or rebase.

Model: opus-5-5
2026-10-04 17:58:41 +02:00
clawbot 04093f53ad Stop TestEvictionRunsOnPeriodicSchedule racing the evictor (closes #183)
check / check (push) Waiting to run
The test wrote each variant file and then inserted its accounting row by
hand while the evictor was running. A reconciliation pass between the two
steps adopted the file first, and the hand insert failed on the unique key.

The test now writes the files only, while it holds the test database's
only connection, so the evictor's startup pass waits after walking the
still empty variant directory. A periodic reconciliation pass then adopts
the files and the eviction pass after it evicts them; no write-pressure
notification fires. The test waits until two of the three files are gone,
then checks that usage is within the limit and that no row points at a
missing file.

Model: opus-5-5
2026-10-04 16:58:34 +02:00
clawbot be6c715b36 Write the deployment guide and an example Caddy config (closes #89)
check / check (push) Waiting to run
README.md gains a "Deployment" section: what the reverse proxy in front
of pixa must do (terminate TLS, pass Host, Origin and Referer on
unchanged, set X-Forwarded-For with trusted_proxies to match, wait at
least downstream_timeout, optionally refuse /metrics) and what pixa does
itself; that the state directory needs a persistent volume, what
cache_max_bytes counts and why to set it; the health check for a load
balancer; what SIGTERM does and the exit codes; and what running outside
Docker needs. configs/Caddyfile is the example, as Caddy needs no
settings beyond the host name and pixa's address.

Model: opus-5-5
2026-10-04 15:08:03 +02:00
clawbot 604b51eea6 Test metrics auth, CORS preflight, login logging and metrics (closes #79)
check / check (push) Waiting to run
New tests only. MetricsAuth on its own answers 401 with a challenge
without credentials or with a wrong username or password, and lets the
configured ones through. A CORS preflight request gets the same
Access-Control-Allow-Origin as a GET. A POST / carrying the signing key
leaves the key out of the request log line, and the login handler's own
log lines leave out the submitted key. The metrics middleware on its own
records a request it served; the router records nothing while no metrics
username is set.

Not tested through the router: the basic auth in front of /metrics and
recording with a metrics username set (#180).

The pinned basicauth-go compares the password in constant time.

Model: opus-5-5
2026-10-04 14:39:27 +02:00
clawbot ba5a716223 Test the image route's signature check and error answers (closes #76)
check / check (push) Waiting to run
New tests in internal/handlers, with no network: the status and JSON
error body the image route answers for a missing, wrong, unpadded,
upper-case or expired signature on a host not on the allowlist, or a
valid one sent for its parent domain, a sibling host, a subdomain or
the host with another domain appended; an unparseable path, localhost
as the upstream host, and an upstream error; that an allowlisted host
is served without a signature and another host only with a valid one;
and the answers of /robots.txt and the health check. An expired
signature is answered 401, as the code and README.md say, where the
issue body expected 410. No code changes.

Model: opus-5-5
2026-10-04 13:58:29 +02:00
clawbot c7173c47d8 Return and pass on request IDs, and give /v1/e/ ETag, 304 and HEAD (closes #84)
check / check (push) Waiting to run
Every response carries X-Request-Id, the upstream fetch sends it, and
the "upstream fetched", "image converted" and "image served" lines log
it as request_id. pixa's own RequestID middleware keeps a request's own
ID only when it is at most 64 letters, digits, '-', '_' or '.', and
otherwise makes a random one with crypto/rand, so nothing a client
chooses freely and nothing about the host reaches upstream. /v1/e/ now
sets ETag, answers a matching If-None-Match with 304 and is routed for
HEAD, through notModified, which both image handlers call. No Vary is
added: go-chi/cors already sends Vary: Origin.

Model: opus-5-5
2026-10-04 12:41:54 +02:00
clawbot 363774c058 Document every route, encrypted URLs and the config file search (closes #75)
check / check (push) Waiting to run
README.md "Routes" lists every route pixa registers with its method,
purpose, what it needs and the status codes it answers with, read from
the handlers, says q and fit are part of what is cached, and says the
login and generator forms need HTTPS unless debug is on. A new
"Encrypted URLs" section covers logging in with the signing key, making
a URL on the generator page, how long it lasts, and the 410 once it has
expired. "Configuration" gives the order in which pixa looks for its
config file. config.example.yml now lists db_url and env and gives every
key's default. scripts/manual-test.sh is left to #97.

Model: opus-5-5
2026-10-04 09:58:36 +02:00
clawbot 1616e91a6a Stop cache eviction in progress at shutdown (closes #102)
check / check (push) Waiting to run
StartEviction runs the eviction goroutine with its own context, which
StopEviction cancels in place of the old stop channel, so a pass in
progress stops at its next database call, file, row or eviction
candidate instead of running to completion, and no new pass starts.
StopEviction takes a context: when it ends before the goroutine exits,
StopEviction stops waiting and returns an error wrapping it. The
handlers' stop hook passes fx's stop context, so an eviction still
running at fx's stop deadline fails the stop and the exit code is 1.
A stop logs at most one warning.

Model: opus-5-5
2026-10-04 09:24:42 +02:00
clawbot 6830bdc5de Remove unimplemented Purge and three unused interfaces (closes #73)
check / check (push) Waiting to run
Purge only returned an error and nothing called it, so it is gone from
the ImageCache interface and from Service, along with its error value.
It can be added back when something needs it; eviction reclaims disk
space meanwhile. The SignatureValidator, Allowlist and Storage
interfaces in imgcache.go had no implementers and no users, and
described types that look different from the real ones, so they are
deleted. No behaviour changes.

Model: opus-5-5
2026-10-04 08:41:37 +02:00
clawbot 5b17d1f555 Remove idle host semaphores and delete .meta with its variant (closes #87)
check / check (push) Successful in 10m10s
Each upstream host's semaphore now counts the fetches holding or waiting
for one of its slots, and is removed from hostSems when the last of them
gives its slot back or stops waiting, so a long-running pixad no longer
keeps one semaphore per host it ever fetched from.

VariantStorage.Delete removes the variant's .meta file too, a missing one
not being an error; DeleteWithMeta, which eviction called for that, is
gone.

Model: opus-5-5
2026-10-04 07:11:31 +02:00
clawbot 3a274aaa44 Make the README's storage, formats, TLS and metrics match the code (closes #74)
check / check (push) Successful in 10m10s
"Storage" names the cache directories pixa uses (cache/sources,
cache/metadata, cache/variants) and how files are named in each; the
schema comments name the same paths, and the output_content comment says
the table is not written. Routes and the signature section give one list
of output formats, jpg and original included, and say how those two are
signed. The TLS sentence names allow_http as its exception. "Metrics"
says what exists: generic HTTP and Go runtime metrics at /metrics,
measured and served only when the metrics username and password are set.

Model: opus-5-5
2026-10-04 06:24:36 +02:00
clawbot 00da62db2c Exit with the shutdown's code and wait for image processing (closes #86)
check / check (push) Successful in 9m56s
fx alone handles SIGINT and SIGTERM; the server's own handler, which
only cancelled a context that fx's stop did not wait for, is gone.
fx's Run exits with the shutdown's code: the one a shutdown request
carries, 0 for a signal, 1 when the app fails to start or stop.
A listen error asks fx to shut down with exit code 1. A Sentry DSN that
cannot be used fails the server's start hook, so fx stops what had
already started. The server's stop hook stops the HTTP server, then
waits for the images still being processed, both within
ShutdownTimeout; images still being processed after that are logged and
fail the stop, so the exit code is 1.

Model: opus-5-5
2026-10-04 04:59:30 +02:00
47 changed files with 3141 additions and 600 deletions
+4
View File
@@ -0,0 +1,4 @@
# Every PR adds an entry at the top of TODO.md's Completed Steps; union keeps
# both sides instead of conflicting. Git never reports a conflict here: read
# the merged entries after every merge or rebase.
TODO.md merge=union
+217 -22
View File
@@ -34,6 +34,66 @@ else has a built-in default. A config file mounted at `/etc/pixa/config.yml`
is optional: it is read when present, and an environment variable wins over is optional: it is read when present, and an environment variable wins over
the same setting in it. the same setting in it.
## Deployment
pixa listens on plain HTTP and runs behind a reverse proxy that terminates TLS.
[`configs/Caddyfile`](configs/Caddyfile) is an example for Caddy, chosen because
it is the smallest correct one: Caddy gets the TLS certificate itself and does
everything in this list without further settings. The reverse proxy must:
- terminate TLS, as the login and generator pages work only over HTTPS (see
Routes);
- pass the `Host`, `Origin` and `Referer` headers on unchanged, as pixa refuses
a form from those pages unless `Origin` or `Referer` names the host in `Host`,
and builds encrypted URLs from `Host`;
- set `X-Forwarded-For` to the client's address, with `trusted_proxies` set to
the address pixa sees the proxy's requests come from, so the login limit
counts each client by its own address (see `trusted_proxies` under
Configuration);
- wait for pixa's answer for at least `downstream_timeout` (default `60s`), the
longest pixa takes to fetch, convert and send an image.
It may also refuse `/metrics`, as the example does, so that only a scraper that
reaches pixa directly can read it; pixa itself asks for the metrics username and
password there.
pixa does the rest itself: it checks signatures and encrypted URLs, applies the
allowlist, refuses upstream hosts with private or local addresses, limits login
attempts, upstream response size and image dimensions, and sends the security
headers, `Strict-Transport-Security` included, with every response.
The state directory (`state_dir`, `/var/lib/pixa` in the container) holds the
database and the disk cache:
- It needs a persistent volume: without one, every restart starts with an empty
cache. In the container, the startup script gives the directory to the user
pixa runs as (uid 65532) and sets its mode to `750`; outside it, that user
must be able to write the directory.
- `cache_max_bytes` limits the source and transformed images together. The
database, the metadata files, the `.meta` file beside each transformed image
and files still being written come on top, and eviction runs in the
background, so the cache can pass the limit for a while: leave room on the
volume beyond it.
- Set `cache_max_bytes` for a lasting deployment. Its default, worked out each
time pixa starts, is 75% of the sum of the space free on the volume and the
space the cached images already take, so a restart keeps the limit the cache
had, but anything else that fills or frees space on the volume moves it.
A load balancer's health check can request `/.well-known/healthcheck.json`,
which answers 200 whenever pixa is running, in maintenance mode too (see
`maintenance_mode`).
On SIGTERM or SIGINT pixa stops accepting connections, gives the requests in
progress and the images being processed 5 seconds to finish, and exits: with 0,
or with 1 when images were still being processed after those 5 seconds or
another part of pixa failed to stop. A request not finished by then is cut off.
`docker stop` waits 10 seconds before it kills the container.
Outside Docker, pixa needs libvips (the image has 8.15) and libheif to run, as
it uses libvips through CGO; building it also needs their development files,
`pkg-config` and a C compiler. `script/bootstrap` installs all of these with
nix, apt, brew or apk.
## Running under upaas ## Running under upaas
What the [upaas](https://git.eeqj.de/sneak/upaas) app for pixa needs: What the [upaas](https://git.eeqj.de/sneak/upaas) app for pixa needs:
@@ -49,7 +109,7 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for pixa needs:
- `PIXA_ALLOWLIST_HOSTS`: upstream hosts served without a signature, - `PIXA_ALLOWLIST_HOSTS`: upstream hosts served without a signature,
comma-separated comma-separated
- `PIXA_CACHE_MAX_BYTES`: disk cache limit in bytes; `0` disables it; - `PIXA_CACHE_MAX_BYTES`: disk cache limit in bytes; `0` disables it;
default 75% of free space default 75% of (free space + what the cache holds)
- the rest are in the table under Configuration below - the rest are in the table under Configuration below
- **Health check:** the image's `HEALTHCHECK` requests - **Health check:** the image's `HEALTHCHECK` requests
`/.well-known/healthcheck.json`. upaas reads the container's health 60 `/.well-known/healthcheck.json`. upaas reads the container's health 60
@@ -71,13 +131,17 @@ prevent abuse, and allowlisted source hosts for open access.
### Storage ### Storage
- **Source content**: - **Source content**:
`<statedir>/cache/src-content/<ab>/<cd>/<sha256 of source content>` `<state_dir>/cache/sources/<ab>/<cd>/<sha256 of source content>`
- **Source metadata**: - **Source metadata**:
`<statedir>/cache/src-metadata/<hostname>/<sha256 of path>.json` `<state_dir>/cache/metadata/<hostname>/<sha256 of path and query>.json`
(fetch time, original headers, request, content hash) (host, path and query, content hash, upstream status and headers, fetch time)
- **Database**: `<statedir>/state.sqlite3` (SQLite) - **Database**: `<state_dir>/state.sqlite3` (SQLite)
- **Output documents**: - **Transformed images**:
`<statedir>/cache/dst-content/<ab>/<cd>/<sha256 of output content>` `<state_dir>/cache/variants/<ab>/<cd>/<sha256 of host, path, query, size, format, quality and fit>`,
each with a `.meta` file beside it holding its content type
`<ab>` and `<cd>` are the first and second pairs of characters of the file's
name.
Multiple source paths may reference the same content blob; the Multiple source paths may reference the same content blob; the
database tracks references rather than using filesystem refcounting. database tracks references rather than using filesystem refcounting.
@@ -88,17 +152,96 @@ the metadata file stored beside it.
### Routes ### Routes
pixa answers these routes; any other path answers 404. A path in this list asked
with a method the list does not give answers 405, except `/static/<file>`, which
answers any method as it answers `GET`. A browser's CORS preflight request
(`OPTIONS` with `Origin` and `Access-Control-Request-Method` headers) to any
path under `/v1/` answers 200, in maintenance mode too.
- `GET /` — the login page, or the URL generator page with a login session
(see Encrypted URLs). Needs: nothing. Answers: 200.
- `POST /` — log in with the signing key typed into the login page. Needs: the
login page's form (below). Answers: 303 to `/` with a login session cookie
that lasts 30 days for the right key; 200 with the login page and an error for
a wrong key; 429 over the login limit (below).
- `POST /generate` — make an encrypted URL from the generator page's form.
Needs: a login session and the generator page's form (below); without a login
session it answers 303 to `/`. Answers: 200 with the page showing the URL; 400
with the page naming a field that is not valid; 500 when the URL cannot be
made.
- `GET /logout` — end the login session. Needs: nothing. Answers: 303 to `/`.
- `GET` or `HEAD` `/v1/image/<host>/<path>/<size>.<format>` — an image, fetched,
resized and converted (below). Needs: a signature, unless the host is
allowlisted (see Source Hosts). Answers: 200; 304 when `If-None-Match` matches
the image's `ETag`; 400 for a URL or parameter that is not valid; 401 for a
missing or wrong signature, a missing `exp` or an `exp` in the past; 403 when
the upstream host, or a host it redirects to, is `localhost`, ends in
`.localhost` or `.local`, or has an address in a blocked network (see
`blocked_networks`); 502 when the upstream answered with an error status, and
for 5 minutes after that for the same source URL; 503 when pixa is busy or in
maintenance mode; 500 for any other failure.
- `GET` or `HEAD` `/v1/e/<token>/<name>` — an image through an encrypted URL
(see Encrypted URLs). Needs: nothing but the URL. Answers: 200; 304 when
`If-None-Match` matches the image's `ETag`; 400 for a token that does not
decrypt, or that asks for a size or fit that is not valid; 410 once it has
expired; 504 when the upstream has not sent its response headers within
`upstream_fetch_timeout`, but 500 when that time runs out while the image
itself is still arriving; 403, 502, 503 and 500 as for `/v1/image/`.
- `GET /robots.txt` — asks every crawler to stay away (`Disallow: /`). Needs:
nothing. Answers: 200.
- `GET /.well-known/healthcheck.json` — JSON with `status` (`ok`), `now`,
`uptime_seconds`, `uptime_human`, `version`, `appname` and
`maintenance_mode`. Needs: nothing. Answers: 200, always.
- `GET /static/<file>` — the script the login and generator pages load. Needs:
nothing. Answers: 200, or 404 for a file that does not exist.
- `GET /metrics` — Prometheus metrics (see Architecture). Needs: HTTP basic
authentication with `metrics.username` and `metrics.password`. Answers: 200;
401 without them; 404 when they are not set, as the route then does not exist.
Every response carries an `X-Request-ID` header holding the request's ID, which
a client can quote when reporting a problem: the request's own `X-Request-ID`,
as a reverse proxy in front of pixa may send, when it is at most 64 letters,
digits, `-`, `_` or `.`; otherwise a random one pixa makes for the request,
which tells nothing about the machine or the other requests. pixa's log line for
the request carries the same ID as `request_id`, and so do the lines it logs
when it fetches, converts and serves an image; the fetch sends it to the
upstream host as `X-Request-ID`.
Both `POST` routes accept only a form that pixa's own page served: the page puts
a token in the form and sets a cookie to match, and a request without both is
refused with 403, so another site cannot submit the form from a visitor's
browser. The login and generator pages are meant to be opened over HTTPS: while
`debug` is off, a form sent from a page opened over plain HTTP is refused with
403, and while it is on, so is one sent from a page opened over HTTPS. Plain
HTTP is for development on the browser's own machine: the login session cookie
is always marked `Secure`, and over plain HTTP a browser keeps such a cookie
only for its own machine (`localhost`), if at all. A form is also refused with
403 when the page's host is not the `Host` header pixa receives, so a reverse
proxy in front of pixa must pass that header on unchanged. A form body over
1 MiB is refused with 413. The image routes answer the errors listed for them
with JSON holding `error`, `status` and `timestamp`.
An image URL has this form:
``` ```
/v1/image/<host>/<path>/<size>.<format>?sig=<signature>&exp=<expiration> /v1/image/<host>/<path>/<size>.<format>?sig=<signature>&exp=<expiration>&q=<quality>&fit=<fit>
``` ```
Images are only fetched from origins using TLS with valid certificates. Images are only fetched from origins using TLS with valid certificates, unless
`allow_http` is set: then pixa fetches every image over plain HTTP, which is for
testing only.
A request whose query string cannot be decoded, or gives any parameter more A request whose query string cannot be decoded, or gives any parameter more
than once, is refused with 400. than once, is refused with 400.
- `<format>`: one of `orig`, `png`, `jpeg`, `webp` - `<format>`: one of `orig` (or `original`), `jpeg` (or `jpg`), `png`, `webp`,
`avif`, `gif`
- `<size>`: `orig` or `<width>x<height>` (e.g. `800x600`) - `<size>`: `orig` or `<width>x<height>` (e.g. `800x600`)
- `sig` and `exp`: the signature and its expiry, needed unless the host is
allowlisted (see Signature Specification)
- `q` and `fit`: the output quality and how the image is fitted to `<size>`,
both optional (values under Signature Specification). Both are part of what
is cached, so each value of either is a separate cached image.
An image is served with `Cache-Control: public, max-age=<seconds>, immutable`. An image is served with `Cache-Control: public, max-age=<seconds>, immutable`.
When the URL has an expiry (an `exp`, or the TTL of an encrypted URL), When the URL has an expiry (an `exp`, or the TTL of an encrypted URL),
@@ -113,7 +256,9 @@ source) and one transcode: the first request does the work, and the others wait
for its image or its error, holding no upstream connection or processing slot for its image or its error, holding no upstream connection or processing slot
of their own. A waiting request stops waiting when its own client goes away. of their own. A waiting request stops waiting when its own client goes away.
The work goes on for the others even if the first request's client goes away, The work goes on for the others even if the first request's client goes away,
until that request's `downstream_timeout` ends. until that request's `downstream_timeout` ends. The shared fetch sends the first
request's ID upstream, and the lines logged for the fetch and the transcode
carry that ID.
The login form (`POST /`) is limited to 5 attempts per minute per client The login form (`POST /`) is limited to 5 attempts per minute per client
address, counting an IPv6 client by its /64; an attempt over the limit is address, counting an IPv6 client by its /64; an attempt over the limit is
@@ -132,6 +277,39 @@ its own `X-Forwarded-For`, whether it connects directly or through the proxy,
because its own address is trusted too. Setting `trusted_proxies` to only the because its own address is trusted too. Setting `trusted_proxies` to only the
address pixa sees for requests that come through the proxy closes this. address pixa sees for requests that come through the proxy closes this.
### Encrypted URLs
An encrypted URL is an image URL made on pixa's own web page by someone who
knows the signing key. It works for any upstream host, allowlisted or not,
without a signature, and whoever gets it can neither read the source URL from it
nor change what it asks for.
1. Open `/` in a browser over HTTPS (or over plain HTTP while `debug` is on, see
Routes) and log in with the signing key (`signing_key`). The login session
lasts 30 days, or until `/logout`.
2. On the generator page, give the source image's URL, the width and height, the
format, quality and fit, and how long the URL lasts, then submit the form
(`POST /generate`). Width and height both empty or `0` keep the original
size; if only one of them is empty or `0`, that side is scaled to keep the
image's proportions.
3. The page shows the URL, `https://<host>/v1/e/<token>/img.<format>`, and when
it expires. `<host>` is the host the page was opened on, and the URL starts
with `http` instead while `debug` is on. The name after the token is ignored
and only gives the URL a file extension, `jpg` for `orig`.
The token holds the source's host, path and query and the size, format,
quality, fit and expiry, encrypted with a key derived from `signing_key`. The
source URL's scheme is not kept: the image is fetched like any other (see
Routes), and the blocked networks still apply.
How long the URL lasts is chosen on the page, from 1 minute to 1 year, or
never. The expiry is fixed in the token when the URL is made and cannot be
changed or revoked afterwards. Until then the image is served with a `max-age`
that ends at the expiry (see Routes); after it the URL answers 410
`URL has expired`. A URL made to last forever stops working only when
`signing_key` changes: changing it makes every encrypted URL already handed out
answer 400, and ends every login session.
### Image Metadata ### Image Metadata
pixa decodes and re-encodes every image it serves, and removes all metadata from pixa decodes and re-encodes every image it serves, and removes all metadata from
@@ -173,7 +351,8 @@ Where:
- `query` — source query string, empty string if none - `query` — source query string, empty string if none
- `width` — requested width in pixels, `0` for original - `width` — requested width in pixels, `0` for original
- `height` — requested height in pixels, `0` for original - `height` — requested height in pixels, `0` for original
- `format` — output format (jpeg, png, webp, avif, gif, orig) - `format` — output format, one of those listed under Routes, with `original`
signed as `orig` and `jpg` as `jpeg`
- `expiration` — the URL's `exp` query parameter, the Unix timestamp when - `expiration` — the URL's `exp` query parameter, the Unix timestamp when
the signature expires; a request whose `exp` is not a whole number, an the signature expires; a request whose `exp` is not a whole number, an
empty `exp=` included, is refused with 400 empty `exp=` included, is refused with 400
@@ -229,13 +408,24 @@ startup naming it, as an unknown config key does. The one other accepted
name is `PIXA_CONFIG_PATH`, the config file's path (like `--config`). The name is `PIXA_CONFIG_PATH`, the config file's path (like `--config`). The
variables set by the file's `env:` section are checked the same way. variables set by the file's `env:` section are checked the same way.
pixa reads at most one config file: the one given with `--config` (or `-c`),
otherwise the one `PIXA_CONFIG_PATH` names, otherwise the first of these that
pixa finds: `/etc/pixa/config.yml`, `/etc/pixa/config.yaml`,
`~/.config/pixa/config.yml`, `~/.config/pixa/config.yaml`, then `config.yml`
and `config.yaml` in the working directory. A named file that does not exist,
cannot be read or does not parse aborts startup. Of the files pixa looks for on
its own, only one that does not exist is passed over, without a message. One
that pixa cannot read or parse aborts startup, naming the file. So does one in a
directory pixa may not enter, whether or not it is there, since pixa cannot
tell. With no file, pixa uses the environment and the defaults.
| Variable | Config key | Meaning | | Variable | Config key | Meaning |
| ------------------------------------ | ------------------------------- | ---------------------------------------------------------------------------- | | ------------------------------------ | ------------------------------- | ---------------------------------------------------------------------------- |
| `PIXA_SIGNING_KEY` | `signing_key` | Required: secret for signed and encrypted URLs and login, 32+ characters | | `PIXA_SIGNING_KEY` | `signing_key` | Required: secret for signed and encrypted URLs and login, 32+ characters |
| `PORT` | `port` | Port to listen on; default `8080` | | `PORT` | `port` | Port to listen on; default `8080` |
| `PIXA_STATE_DIR` | `state_dir` | Directory for the database and the disk cache; default `/var/lib/pixa` | | `PIXA_STATE_DIR` | `state_dir` | Directory for the database and the disk cache; default `/var/lib/pixa` |
| `PIXA_DB_URL` | `db_url` | SQLite database URL; default `state.sqlite3` in the state directory | | `PIXA_DB_URL` | `db_url` | SQLite database URL; default `state.sqlite3` in the state directory |
| `PIXA_CACHE_MAX_BYTES` | `cache_max_bytes` | Disk cache limit in bytes; `0` disables it; default 75% of free space | | `PIXA_CACHE_MAX_BYTES` | `cache_max_bytes` | Disk cache limit in bytes; `0` disables it; default 75% of (free + cached) |
| `PIXA_ALLOWLIST_HOSTS` | `allowlist_hosts` | Upstream hosts served without a signature | | `PIXA_ALLOWLIST_HOSTS` | `allowlist_hosts` | Upstream hosts served without a signature |
| `PIXA_BLOCKED_NETWORKS` | `blocked_networks` | CIDR ranges never fetched from, on top of the built-in ones | | `PIXA_BLOCKED_NETWORKS` | `blocked_networks` | CIDR ranges never fetched from, on top of the built-in ones |
| `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 |
@@ -300,8 +490,10 @@ Key settings in more detail:
each), so keep it longer than `upstream_fetch_timeout` plus 20 seconds 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 sum of the free space on
the filesystem containing `<state_dir>/cache/` (minimum 500 MiB) the filesystem containing `<state_dir>/cache/` and the bytes of source and
transformed images the cache already holds, worked out at startup (minimum
500 MiB)
- `upstream_connections` — the most connections to upstream hosts at once, all - `upstream_connections` — the most connections to upstream hosts at once, all
hosts together, on top of `upstream_connections_per_host`; default `64`. A 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 fetch holds its connection until its image has been processed. A fetch that
@@ -314,12 +506,12 @@ Key settings in more detail:
seconds for one to free up; if none does, and `downstream_timeout` has not seconds for one to free up; if none does, and `downstream_timeout` has not
ended first, it is answered 503 the same way ended first, it is answered 503 the same way
- `maintenance_mode` — while `true`, the image routes (`/v1/image/` and - `maintenance_mode` — while `true`, the image routes (`/v1/image/` and
`/v1/e/`) answer every request with 503, a `Retry-After` header and a JSON `/v1/e/`) answer every request for an image with 503, a `Retry-After` header
error body. The health check (`/.well-known/healthcheck.json`) still answers and a JSON error body. The health check (`/.well-known/healthcheck.json`)
200 and reports `"maintenance_mode": true`. It stays 200 because the image's still answers 200 and reports `"maintenance_mode": true`. It stays 200
Docker `HEALTHCHECK` requests it: a 503 there would make the container because the image's Docker `HEALTHCHECK` requests it: a 503 there would make
unhealthy, and upaas marks a deploy failed when its container is unhealthy. the container unhealthy, and upaas marks a deploy failed when its container
The login and URL generator pages and `/metrics` keep working 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.
@@ -330,7 +522,10 @@ See `config.example.yml` for all options with defaults.
- **Image processing**: govips (CGO wrapper for libvips) - **Image processing**: govips (CGO wrapper for libvips)
- **Database**: SQLite via modernc.org/sqlite - **Database**: SQLite via modernc.org/sqlite
- **Static assets**: embedded via `//go:embed` - **Static assets**: embedded via `//go:embed`
- **Metrics**: Prometheus - **Metrics**: Prometheus, at `/metrics`: generic HTTP request metrics
(duration, response size, requests in flight) and the Go runtime and process
metrics; requests are measured and `/metrics` is served only when
`metrics.username` and `metrics.password` are set
- **Logging**: stdlib slog - **Logging**: stdlib slog
## Entrypoints ## Entrypoints
+127 -6
View File
@@ -3,6 +3,8 @@
* branch per issue from `next` * branch per issue from `next`
* do the work in Next Step * do the work in Next Step
* move Next Step to the top of Completed Steps * move Next Step to the top of Completed Steps
* `TODO.md` merges with git's union merge (`.gitattributes`), which never
reports a conflict: read the merged entries after every merge or rebase
* move the top item of Future Steps into Next Step * move the top item of Future Steps into Next Step
* commit (`TODO.md` changes in the same commit as the work) * commit (`TODO.md` changes in the same commit as the work)
* open a PR based on `next` * open a PR based on `next`
@@ -29,6 +31,131 @@ P2: security: referer blacklist
# Completed Steps # Completed Steps
- 2026-10-04 `TODO.md` merges with git's union merge (closes #190): a root
`.gitattributes`, copied from `sneak/prompts`, marks it `merge=union`, so two
branches that each add an entry at the top of Completed Steps merge without a
conflict and keep both entries. Git now never reports a conflict in
`TODO.md`: a real one keeps both versions of the lines, and two entries that
share an identical line can end up one inside the other, which a rebase can
do to an entry already on `next`. The Workflow above says to read the merged
entries after every merge or rebase.
- 2026-10-04 the default `cache_max_bytes` no longer shrinks as the cache fills
(closes #184): for an omitted key, the cache works out the limit when it
opens, after the database is open, as 75% of the sum of the free space on the
filesystem containing `<state_dir>/cache/` and what the cache already holds by
its own size accounting, at least 500 MiB, so a cache filled to its limit
keeps that limit across a restart. The computation and its tests moved from
`internal/config` to `internal/imgcache`; the config only records whether the
key was set.
- 2026-10-04 `TestEvictionRunsOnPeriodicSchedule` no longer races the evictor
(closes #183): it wrote each variant file and then inserted its accounting row
by hand, and a reconciliation pass between the two adopted the file first, so
the insert failed. It now writes the files only, while holding the test
database's only connection so the evictor's startup pass waits after walking
the empty variant directory; a periodic reconciliation pass then adopts the
files and the eviction pass after it evicts them. No other test in
`internal/imgcache` inserts a row by hand after starting the evictor. Test
only.
- 2026-10-04 a config file pixa cannot read aborts startup (closes #176): of the
places pixa looks for its config file on its own, only one where the file does
not exist is passed over; any other error, such as a directory on the path
that pixa may not enter, aborts startup naming the file, as a file that does
not parse already did.
- 2026-10-04 deployment guide and example Caddy config (closes #89):
"Deployment" in `README.md` says what the reverse proxy in front of pixa must
do (terminate TLS; pass `Host`, `Origin` and `Referer` on unchanged; set
`X-Forwarded-For`, with `trusted_proxies` to match; wait at least
`downstream_timeout`; optionally refuse `/metrics`) and what pixa does itself,
that the state directory needs a persistent volume and what `cache_max_bytes`
counts, the health check for a load balancer, what a stop does and its exit
codes, and what running outside Docker needs; `configs/Caddyfile` is the
example, checked with `caddy validate`.
- 2026-10-04 the metrics basic auth, CORS preflight, request logging and
metrics recording have tests (closes #79): `MetricsAuth` on its own answers
401 with a challenge without credentials or with a wrong username or password
and lets the configured ones through; a preflight request gets `*` for any
origin when `access_control_allow_origin` is `*` and no
`Access-Control-Allow-Origin` from another origin than the configured one; a
`POST /` carrying the signing key leaves no trace of it in the request log
line, and the login handler's own log lines leave out the submitted key; the
metrics middleware on its own records a request it served, and the router
records nothing while no metrics username is set. Not tested: that the router
puts the basic auth in front of `/metrics` and records requests when a
metrics username is set. Only one test per package can set up `/metrics`, and
in `internal/server` that is `TestMaintenanceModeKeepsOtherRoutes`, which
needs the owner's approval to change; #180 holds it. Tests only; the basic
auth library already compares the password in constant time.
- 2026-10-04 the image route's signature check and error answers are tested
(closes #76): new tests in `internal/handlers`, with no network, check the
status and JSON error body for a missing, wrong, unpadded, upper-case or
expired signature on a host not on the allowlist, or a valid one sent for
its parent domain, a sibling host, a subdomain or the host with another
domain appended (401), an unparseable path (400), `localhost` as the
upstream host (403) and an upstream error (502); that an allowlisted host is
served without a signature, another host only with a valid one; and the
answers of `/robots.txt` and the health check. No code changes.
- 2026-10-04 request IDs returned and passed on, and `/v1/e/` revalidates
(closes #84): pixa's own `RequestID` middleware, in place of chi's, gives each
request an ID, its own `X-Request-ID` when that is at most 64 letters, digits,
`-`, `_` or `.` and a random one otherwise, stores it where chi's did and
sends it back as `X-Request-ID` on every response; the upstream fetch sends
that ID, and the "upstream fetched", "image converted" and "image served" log
lines carry it as `request_id`, a fetch shared by several requests carrying
the first request's; `/v1/e/` sets `ETag`, answers a matching `If-None-Match`
with 304 and is routed for `HEAD`, the `ETag` and 304 code being
`notModified`, which `/v1/image/` calls too; its token checks moved unchanged
into `parseImageEncRequest` to keep `HandleImageEnc` within the line limit; no
`Vary` is added, as no response depends on a request header except the image
routes' CORS headers, for which `go-chi/cors` already sends `Vary: Origin`;
`Vary: Accept` is left to #88.
- 2026-10-04 routes, encrypted URLs and config file documented (closes #75):
"Routes" in `README.md` lists every route with its method, purpose, what it
needs and the status codes it answers with, and says `q` and `fit` are part
of what is cached; "Encrypted URLs" covers logging in, making one on the
generator page, how long it lasts and the 410 once it has expired;
"Configuration" gives the order in which pixa looks for its config file;
`config.example.yml` lists `db_url` and `env` and gives every key's default;
`scripts/manual-test.sh` is left to #97.
- 2026-10-04 shutdown stops cache eviction in progress (closes #102):
`StartEviction` runs the eviction goroutine with its own context, which
`StopEviction` cancels, so a pass in progress stops at its next database
call, file, row or eviction candidate instead of running to completion, and
no pass starts after it, so a stop logs at most one warning;
`StopEviction` takes a context and, when that context ends before the
goroutine exits, stops waiting and returns its error; the handlers' stop hook
passes fx's stop context, so an eviction still running when fx's stop
deadline ends fails the stop and makes the exit code 1.
- 2026-10-04 dead code in `internal/imgcache` is gone (closes #73): `Purge`,
which only returned an error and which nothing called, is no longer part of
the `ImageCache` interface or `Service`; the `SignatureValidator`,
`Allowlist` and `Storage` interfaces, which nothing implemented or used, are
deleted. Nothing else changes.
- 2026-10-04 upstream host semaphores and variant `.meta` files no longer
outlive their use (closes #87): the fetcher counts the fetches holding or
waiting for a slot of each upstream host's semaphore and removes the host's
semaphore once none is left, so fetches from many hosts no longer leave one
semaphore each until restart; `VariantStorage.Delete` removes the variant's
`.meta` file along with it, a missing `.meta` file not being an error, and
`DeleteWithMeta`, which eviction called for that, is gone.
- 2026-10-04 `README.md` matches the code (closes #74): "Storage" names the
cache directories pixa uses (`cache/sources`, `cache/metadata`,
`cache/variants`) and how files are named in each, and the comments in
`001_schema.sql` name the same paths; the routes and the signature section
list the same output formats, `jpg` and `original` included; the TLS
sentence names `allow_http` as its exception; "Metrics" says only generic
HTTP and Go runtime metrics exist, measured and served only when the metrics
username and password are set.
- 2026-10-03 shutdown sets the exit code and waits for image processing
(closes #86): fx alone handles SIGINT and SIGTERM, and the server's own
signal handler is gone; fx's `Run` in `cmd/pixad` exits with the shutdown's
code: 0 for a signal, 1 when the HTTP server cannot listen or the app fails
to start or to stop; the server's stop hook, which fx waits for, stops the
HTTP server, waits for the images still being processed, both within 5
seconds, then flushes Sentry; images still being processed after that are
logged with their count and make the exit code 1; a Sentry DSN that cannot be
used fails startup, so the stop hooks of what had already started run,
instead of exiting the process from a goroutine; the eviction loop is left to
#102.
- 2026-10-03 every `script/cibuild` and `script/docker` run executes the checks - 2026-10-03 every `script/cibuild` and `script/docker` run executes the checks
(closes #101): the `Dockerfile` declares `CHECK_EPOCH` above `make fmt-check` (closes #101): the `Dockerfile` declares `CHECK_EPOCH` above `make fmt-check`
and `make lint` in the lint stage and above `make test` in the build stage, and `make lint` in the lint stage and above `make test` in the build stage,
@@ -390,7 +517,6 @@ P2: security: referer blacklist
- P2: HTTP response handling - P2: HTTP response handling
- Last-Modified headers - Last-Modified headers
- Vary header for content negotiation - Vary header for content negotiation
- X-Request-ID propagation
- P2: auto format selection (format=auto based on Accept header) - P2: auto format selection (format=auto based on Accept header)
- P2: configuration - P2: configuration
- YAML config file support - YAML config file support
@@ -400,8 +526,3 @@ P2: security: referer blacklist
- Prometheus performance metrics - Prometheus performance metrics
- integration tests for the image proxy flow - integration tests for the image proxy flow
- load tests to verify the 1k to 5k req/s target - load tests to verify the 1k to 5k req/s target
- P2: documentation
- configuration options
- API endpoints
- deployment guide
- example nginx or caddy reverse proxy config
+5
View File
@@ -4,6 +4,8 @@ package main
import ( import (
"fmt" "fmt"
"os" "os"
"os/signal"
"syscall"
"github.com/spf13/cobra" "github.com/spf13/cobra"
"go.uber.org/fx" "go.uber.org/fx"
@@ -45,6 +47,9 @@ func run(_ *cobra.Command, _ []string) {
_ = os.Setenv("PIXA_CONFIG_PATH", configPath) _ = os.Setenv("PIXA_CONFIG_PATH", configPath)
} }
// A write to a closed stdout or stderr must not end the process.
signal.Ignore(syscall.SIGPIPE)
fx.New( fx.New(
fx.Provide( fx.Provide(
config.New, config.New,
+30 -11
View File
@@ -12,27 +12,38 @@
# Durations are Go duration strings such as 30s or 2m and must be # Durations are Go duration strings such as 30s or 2m and must be
# positive; a bare number has no unit and aborts startup. Sizes are a # positive; a bare number has no unit and aborts startup. Sizes are a
# whole number of bytes. # whole number of bytes.
#
# A key left out takes the default its comment gives.
# Server settings # Port to listen on (default: 8080)
port: 8080 port: 8080
# Debug logging and plain-HTTP local development (default: false)
debug: false debug: false
# While true, the image routes (/v1/image/ and /v1/e/) answer every request # 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 # for an image with 503 and a Retry-After header. The health check keeps
# reports maintenance_mode as true. It stays 200 because the image's Docker # answering 200 and reports maintenance_mode as true. It stays 200 because
# HEALTHCHECK requests it: a 503 there would make the container unhealthy, and # the image's Docker HEALTHCHECK requests it: a 503 there would make the
# upaas marks a deploy failed when its container is unhealthy. # container unhealthy, and upaas marks a deploy failed when its container is
# unhealthy. (default: false)
maintenance_mode: false maintenance_mode: false
# Data directory for SQLite database and cache files # Data directory for SQLite database and cache files
# (default: /var/lib/pixa)
state_dir: ./data state_dir: ./data
# SQLite database URL (default:
# file:<state_dir>/state.sqlite3?_journal_mode=WAL). An empty value aborts
# startup; leave the key out to use the default.
# db_url: "file:./data/state.sqlite3?_journal_mode=WAL"
# Image proxy settings # Image proxy settings
# HMAC signing key for URL signatures (required, at least 32 characters) # HMAC signing key for URL signatures (required, at least 32 characters)
# Generate with: openssl rand -base64 32 # Generate with: openssl rand -base64 32
signing_key: "CHANGE_ME_generate_with_openssl_rand_base64_32" signing_key: "CHANGE_ME_generate_with_openssl_rand_base64_32"
# Hosts that don't require signatures # Hosts that don't require signatures (default: none)
# Use "." prefix for wildcard subdomain matching (e.g., ".example.com" matches "cdn.example.com") # Use "." prefix for wildcard subdomain matching (e.g., ".example.com" matches "cdn.example.com")
allowlist_hosts: allowlist_hosts:
- s3.sneak.cloud - s3.sneak.cloud
@@ -45,7 +56,7 @@ allowlist_hosts:
# SSRF protection. These are added to the always-enforced built-in ranges # SSRF protection. These are added to the always-enforced built-in ranges
# (loopback, RFC 1918 private, link-local, CGNAT, benchmark, NAT64, and # (loopback, RFC 1918 private, link-local, CGNAT, benchmark, NAT64, and
# similar), never replacing them. Each entry must be a valid CIDR in IPv4 # similar), never replacing them. Each entry must be a valid CIDR in IPv4
# or IPv6 form; an invalid entry aborts startup. # or IPv6 form; an invalid entry aborts startup. (default: none)
# blocked_networks: # blocked_networks:
# - 100.64.0.0/10 # - 100.64.0.0/10
# - 2001:db8::/32 # - 2001:db8::/32
@@ -72,6 +83,7 @@ allowlist_hosts:
# - 2001:db8::/32 # - 2001:db8::/32
# Allow HTTP upstream (only for testing, always use HTTPS in production) # Allow HTTP upstream (only for testing, always use HTTPS in production)
# (default: false)
allow_http: false allow_http: false
# Maximum concurrent connections per upstream host (default: 20) # Maximum concurrent connections per upstream host (default: 20)
@@ -116,15 +128,22 @@ access_control_allow_origin: "*"
# Maximum disk cache size in bytes. Explicit values are used exactly as # Maximum disk cache size in bytes. Explicit values are used exactly as
# given; 0 disables the disk cache entirely (every request fetches and # given; 0 disables the disk cache entirely (every request fetches and
# processes uncached). When omitted, the default is 75% of the free # processes uncached). When omitted, the default is 75% of the sum of
# space on the filesystem containing <state_dir>/cache/ at startup, # the free space on the filesystem containing <state_dir>/cache/ and
# the bytes of images the cache already holds, worked out at startup,
# with a minimum of 500 MiB. # with a minimum of 500 MiB.
# cache_max_bytes: 10737418240 # cache_max_bytes: 10737418240
# Sentry error reporting (optional) # Sentry DSN for error reporting (default: empty, which turns it off)
sentry_dsn: "" sentry_dsn: ""
# Metrics endpoint authentication (optional) # Username and password for /metrics, set together (default: unset). Metrics
# are measured and /metrics is served only when both are set.
# metrics: # metrics:
# username: "admin" # username: "admin"
# password: "secret" # password: "secret"
# Environment variables set while this file loads, as described at the top
# (default: none)
# env:
# PIXA_DEBUG: "true"
+17
View File
@@ -0,0 +1,17 @@
# Example Caddy config for running pixa behind Caddy; see "Deployment" in
# README.md. Replace images.example.com with pixa's public host name, and
# 127.0.0.1:8080 with the address Caddy reaches pixa on.
#
# Caddy gets and renews the TLS certificate for the host name, passes the
# Host, Origin and Referer headers on unchanged, sets X-Forwarded-For to the
# client's address, and waits for pixa's answer with no time limit of its
# own, so pixa's downstream_timeout is what ends a slow request.
images.example.com
# pixa asks for metrics.username and metrics.password on /metrics. This
# line also keeps it off the public address, for a scraper that reaches
# pixa directly; remove it to read /metrics through Caddy.
respond /metrics 404
reverse_proxy 127.0.0.1:8080
+13 -154
View File
@@ -1,26 +1,10 @@
package config package config
import ( import (
"errors"
"log/slog"
"os"
"path/filepath"
"strings" "strings"
"testing" "testing"
) )
// Static errors returned by the stub free-space probes below.
var (
errTestStatfsFailed = errors.New("statfs failed")
errTestProbeNotExpected = errors.New("probe must not be called")
)
// discardLogger returns a logger that swallows all output, for tests
// that exercise code paths which log.
func discardLogger() *slog.Logger {
return slog.New(slog.DiscardHandler)
}
// TestCacheMaxBytesExplicitValueUsedWithoutFloor verifies that an // TestCacheMaxBytesExplicitValueUsedWithoutFloor verifies that an
// explicitly configured cache_max_bytes value is used exactly as // explicitly configured cache_max_bytes value is used exactly as
// given: the 500 MiB floor applies only to the computed default, never // given: the 500 MiB floor applies only to the computed default, never
@@ -151,155 +135,30 @@ func TestCacheMaxBytesInvalidValuesAbortStartup(t *testing.T) {
} }
} }
// TestComputeDefaultCacheMaxBytesUses75PercentOfFreeSpace verifies the // TestCacheMaxBytesExplicitIsRecorded verifies that an omitted
// computed default is 75% of the probed free space when that exceeds // cache_max_bytes is recorded as not explicit, so the cache works out
// the floor. // the default when it opens, and that an explicit zero is recorded as
func TestComputeDefaultCacheMaxBytesUses75PercentOfFreeSpace(t *testing.T) { // explicit, so it disables the disk cache instead.
func TestCacheMaxBytesExplicitIsRecorded(t *testing.T) {
t.Parallel() t.Parallel()
// 4 GiB free -> 3 GiB default. signingKeyLine := "signing_key: " + validTestSigningKey + "\n"
probe := func(string) (uint64, error) { return 4294967296, nil }
got, err := ComputeDefaultCacheMaxBytes(t.TempDir(), probe) omitted, err := configFromYAML(t, signingKeyLine)
if err != nil {
t.Fatalf("ComputeDefaultCacheMaxBytes returned error: %v", err)
}
if got != 3221225472 {
t.Errorf("ComputeDefaultCacheMaxBytes = %d, want 3221225472 (75%% of 4 GiB)",
got)
}
}
// TestComputeDefaultCacheMaxBytesAppliesFloorToComputedDefault
// verifies that when 75% of free space is below 500 MiB, the computed
// default is floored at DefaultCacheMaxBytesFloor.
func TestComputeDefaultCacheMaxBytesAppliesFloorToComputedDefault(t *testing.T) {
t.Parallel()
cases := []struct {
name string
freeBytes uint64
}{
{name: "100 MiB free", freeBytes: 104857600},
{name: "zero free", freeBytes: 0},
{name: "just below floor threshold", freeBytes: 699050665},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
probe := func(string) (uint64, error) { return tc.freeBytes, nil }
got, err := ComputeDefaultCacheMaxBytes(t.TempDir(), probe)
if err != nil {
t.Fatalf("ComputeDefaultCacheMaxBytes returned error: %v", err)
}
if got != DefaultCacheMaxBytesFloor {
t.Errorf("ComputeDefaultCacheMaxBytes = %d, want floor %d",
got, DefaultCacheMaxBytesFloor)
}
})
}
}
// TestComputeDefaultCacheMaxBytesPropagatesProbeError verifies that a
// failing free-space probe produces an error naming the config key,
// instead of a silently wrong default.
func TestComputeDefaultCacheMaxBytesPropagatesProbeError(t *testing.T) {
t.Parallel()
probe := func(string) (uint64, error) { return 0, errTestStatfsFailed }
_, err := ComputeDefaultCacheMaxBytes(t.TempDir(), probe)
if err == nil {
t.Fatal("probe failure must produce an error, got nil")
}
t.Logf("got expected error: %v", err)
if !strings.Contains(err.Error(), keyCacheMaxBytes) {
t.Errorf("error %q does not name the config key cache_max_bytes", err.Error())
}
}
// TestResolveCacheMaxBytesComputesDefaultWhenOmitted verifies that an
// omitted cache_max_bytes key resolves to the computed default, that
// the probe is pointed at <state_dir>/cache/ (which must be created
// first so statfs measures the right filesystem), and that the result
// lands on the Config.
func TestResolveCacheMaxBytesComputesDefaultWhenOmitted(t *testing.T) {
t.Parallel()
c, err := configFromYAML(t, "signing_key: "+validTestSigningKey+"\n")
if err != nil { if err != nil {
t.Fatalf("minimal config should be valid, got error: %v", err) t.Fatalf("minimal config should be valid, got error: %v", err)
} }
c.StateDir = t.TempDir() if omitted.CacheMaxBytesExplicit {
wantCacheDir := filepath.Join(c.StateDir, "cache") t.Error("omitted cache_max_bytes recorded as explicit")
var probedPath string
// 4 GiB free -> 3 GiB default.
probe := func(path string) (uint64, error) {
probedPath = path
return 4294967296, nil
} }
err = c.resolveCacheMaxBytes(discardLogger(), probe) zero, err := configFromYAML(t, signingKeyLine+"cache_max_bytes: 0\n")
if err != nil { if err != nil {
t.Fatalf("resolveCacheMaxBytes returned error: %v", err) t.Fatalf("cache_max_bytes: 0 must be accepted, got error: %v", err)
} }
if c.CacheMaxBytes != 3221225472 { if !zero.CacheMaxBytesExplicit {
t.Errorf("CacheMaxBytes = %d, want computed default 3221225472", t.Error("cache_max_bytes: 0 not recorded as explicit")
c.CacheMaxBytes)
}
if probedPath != wantCacheDir {
t.Errorf("free space probed at %q, want cache directory %q",
probedPath, wantCacheDir)
}
info, err := os.Stat(wantCacheDir)
if err != nil || !info.IsDir() {
t.Errorf("cache directory %q was not created before probing: info=%v err=%v",
wantCacheDir, info, err)
}
}
// TestResolveCacheMaxBytesDoesNotOverrideExplicitValue verifies that
// an explicitly configured value survives resolution untouched and
// that the free-space probe is never consulted for it.
func TestResolveCacheMaxBytesDoesNotOverrideExplicitValue(t *testing.T) {
t.Parallel()
yamlContent := "signing_key: " + validTestSigningKey + "\ncache_max_bytes: 1024\n"
c, err := configFromYAML(t, yamlContent)
if err != nil {
t.Fatalf("explicit cache_max_bytes must be accepted, got error: %v", err)
}
c.StateDir = t.TempDir()
probe := func(string) (uint64, error) {
t.Error("free-space probe must not be consulted for explicit values")
return 0, errTestProbeNotExpected
}
err = c.resolveCacheMaxBytes(discardLogger(), probe)
if err != nil {
t.Fatalf("resolveCacheMaxBytes returned error: %v", err)
}
if c.CacheMaxBytes != 1024 {
t.Errorf("CacheMaxBytes = %d, want explicit 1024 (no floor, no recompute)",
c.CacheMaxBytes)
} }
} }
-116
View File
@@ -1,116 +0,0 @@
package config
import (
"fmt"
"log/slog"
"math"
"os"
"path/filepath"
"syscall"
)
// DefaultCacheMaxBytesFloor is the minimum computed default for the
// cache_max_bytes setting: 500 MiB. The floor applies only to the
// computed default (when the key is omitted from the configuration),
// never to explicitly configured values.
const DefaultCacheMaxBytesFloor int64 = 524288000
// cacheDirPerms is the permission mode for the cache directory created
// before probing free space, matching the state directory permissions.
const cacheDirPerms = 0o750
// freeSpaceFractionNumerator and freeSpaceFractionDenominator express
// the 75% share of free space used for the computed default limit as
// integer arithmetic (dividing before multiplying avoids overflow).
const (
freeSpaceFractionNumerator uint64 = 3
freeSpaceFractionDenominator uint64 = 4
)
// FreeSpaceProbeFunc reports the number of free bytes available on the
// filesystem containing path. It is a function type so tests can
// inject a fake probe instead of depending on the host disk.
type FreeSpaceProbeFunc func(path string) (uint64, error)
// defaultFreeSpaceProbe reports free filesystem bytes via statfs on
// the given path, as available to unprivileged processes.
func defaultFreeSpaceProbe(path string) (uint64, error) {
var stat syscall.Statfs_t
err := syscall.Statfs(path, &stat)
if err != nil {
return 0, err
}
if stat.Bsize < 0 {
return 0, fmt.Errorf("%w %d for %q", errNegativeBlockSize, stat.Bsize, path)
}
blockSize := uint64(stat.Bsize)
return stat.Bavail * blockSize, nil
}
// ComputeDefaultCacheMaxBytes returns the default cache size limit for
// the filesystem containing cacheDir: 75% of the free bytes reported
// by probe, with a floor of DefaultCacheMaxBytesFloor.
func ComputeDefaultCacheMaxBytes(
cacheDir string, probe FreeSpaceProbeFunc,
) (int64, error) {
freeBytes, err := probe(cacheDir)
if err != nil {
return 0, fmt.Errorf("config key %q: cannot determine free space for %q: %w",
"cache_max_bytes", cacheDir, err)
}
computed := freeBytes / freeSpaceFractionDenominator * freeSpaceFractionNumerator
computed = min(computed, math.MaxInt64)
// gosec cannot see that min() above bounds computed, so it reads
// this conversion as potentially overflowing. It cannot: computed is
// at most math.MaxInt64 on every path here.
//nolint:gosec // G115: clamped to MaxInt64 by min above
limit := int64(computed)
limit = max(limit, DefaultCacheMaxBytesFloor)
return limit, nil
}
// resolveCacheMaxBytes finalizes CacheMaxBytes after state_dir
// validation: an explicitly configured value is kept as-is (no floor
// applies), while an omitted key receives the computed default based
// on free space in <state_dir>/cache/. The cache directory is created
// first so statfs measures the filesystem that will actually hold the
// cache. The effective limit is logged either way.
func (c *Config) resolveCacheMaxBytes(
log *slog.Logger, probe FreeSpaceProbeFunc,
) error {
if !c.cacheMaxBytesExplicit {
cacheDir := filepath.Join(c.StateDir, "cache")
err := os.MkdirAll(cacheDir, cacheDirPerms)
if err != nil {
return fmt.Errorf("config key %q: cannot create cache directory %q: %w",
keyCacheMaxBytes, cacheDir, err)
}
limit, err := ComputeDefaultCacheMaxBytes(cacheDir, probe)
if err != nil {
return err
}
c.CacheMaxBytes = limit
log.Info("computed default cache size limit from free space",
"cache_max_bytes", limit,
"cache_dir", cacheDir,
)
}
log.Info("effective cache size limit",
"cache_max_bytes", c.CacheMaxBytes,
"cache_disabled", c.CacheMaxBytes == 0,
)
return nil
}
+43 -30
View File
@@ -4,6 +4,7 @@ package config
import ( import (
"errors" "errors"
"fmt" "fmt"
"io/fs"
"log/slog" "log/slog"
"math" "math"
"net/netip" "net/netip"
@@ -14,6 +15,7 @@ import (
"sort" "sort"
"strconv" "strconv"
"strings" "strings"
"syscall"
"time" "time"
"git.eeqj.de/sneak/smartconfig" "git.eeqj.de/sneak/smartconfig"
@@ -91,9 +93,7 @@ var (
errMustBeSetTogether = errors.New("must be set together") errMustBeSetTogether = errors.New("must be set together")
errMustNotBeNegative = errors.New("must not be negative") errMustNotBeNegative = errors.New("must not be negative")
errOverflowsInt64 = errors.New("overflows a 64-bit integer") errOverflowsInt64 = errors.New("overflows a 64-bit integer")
errNegativeBlockSize = errors.New( errValueNull = errors.New(
"statfs reported negative block size")
errValueNull = errors.New(
"value is null; omit the key entirely to use the default") "value is null; omit the key entirely to use the default")
errValuesNull = errors.New( errValuesNull = errors.New(
"value is null; omit a key entirely to use its default") "value is null; omit a key entirely to use its default")
@@ -169,18 +169,19 @@ type Config struct {
// address, and an explicit list replaces the default. // address, and an explicit list replaces the default.
TrustedProxies []netip.Prefix TrustedProxies []netip.Prefix
// CacheMaxBytes is the disk cache size limit in bytes. Zero // CacheMaxBytes is the disk cache size limit in bytes. Only an
// disables the disk cache entirely. When cache_max_bytes is // explicit zero (CacheMaxBytesExplicit true) disables the disk
// omitted from the configuration, this holds the computed default // cache. Zero with CacheMaxBytesExplicit false means
// (75% of free space on the filesystem containing // cache_max_bytes was omitted, and the cache works out the default
// <state_dir>/cache/, floored at DefaultCacheMaxBytesFloor). // limit when it opens.
CacheMaxBytes int64 CacheMaxBytes int64
// cacheMaxBytesExplicit records whether cache_max_bytes was // CacheMaxBytesExplicit records whether cache_max_bytes was
// explicitly set, in the environment or the configuration file. // explicitly set, in the environment or the configuration file.
// Explicit values are used exactly as given; only an omitted key // Explicit values are used exactly as given; for an omitted key the
// gets the computed default (and its floor) in resolveCacheMaxBytes. // cache works out the default limit when it opens (see
cacheMaxBytesExplicit bool // imgcache.CacheConfig.UseDefaultMaxBytes).
CacheMaxBytesExplicit bool
} }
// New creates a new Config instance from the environment and the // New creates a new Config instance from the environment and the
@@ -217,9 +218,13 @@ func New(_ fx.Lifecycle, params Params) (*Config, error) {
return nil, err return nil, err
} }
err = c.resolveCacheMaxBytes(log, defaultFreeSpaceProbe) // An omitted cache_max_bytes is worked out and logged when the
if err != nil { // cache opens.
return nil, err if c.CacheMaxBytesExplicit {
log.Info("effective cache size limit",
"cache_max_bytes", c.CacheMaxBytes,
"cache_disabled", c.CacheMaxBytes == 0,
)
} }
if c.Debug { if c.Debug {
@@ -298,11 +303,11 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
TrustedProxies: trustedProxies, TrustedProxies: trustedProxies,
} }
// The computed default for cache_max_bytes needs a validated // The default for an omitted cache_max_bytes is worked out when
// state_dir, so it is resolved later (resolveCacheMaxBytes); here // the cache opens; here we only record whether the operator set
// we only record whether the operator set the key explicitly. // the key explicitly.
if _, present := lookupValue(sc, keyCacheMaxBytes); present { if _, present := lookupValue(sc, keyCacheMaxBytes); present {
c.cacheMaxBytesExplicit = true c.CacheMaxBytesExplicit = true
} }
// Build DBURL from StateDir if not explicitly set. The derived URL // Build DBURL from StateDir if not explicitly set. The derived URL
@@ -778,19 +783,27 @@ func loadConfigFile(log *slog.Logger, appName string) (*smartconfig.Config, erro
for _, path := range configPaths { for _, path := range configPaths {
cleanPath := filepath.Clean(path) cleanPath := filepath.Clean(path)
// Only a config file that does not exist is skipped, including
// one whose path runs through a file, such as under a HOME of
// /dev/null. One that cannot be read or does not parse is a
// fatal startup error.
_, statErr := os.Stat(cleanPath) _, statErr := os.Stat(cleanPath)
if statErr == nil { if errors.Is(statErr, fs.ErrNotExist) || errors.Is(statErr, syscall.ENOTDIR) {
// A config file that exists but does not parse is a fatal continue
// startup error, never something to skip over.
sc, err := smartconfig.NewFromConfigPath(path)
if err != nil {
return nil, fmt.Errorf("failed to parse config file %s: %w", path, err)
}
log.Info("loaded config file", "path", path)
return sc, nil
} }
if statErr != nil {
return nil, fmt.Errorf("failed to read config file %s: %w", path, statErr)
}
sc, err := smartconfig.NewFromConfigPath(path)
if err != nil {
return nil, fmt.Errorf("failed to parse config file %s: %w", path, err)
}
log.Info("loaded config file", "path", path)
return sc, nil
} }
return nil, nil //nolint:nilnil // nil config is valid (use defaults) return nil, nil //nolint:nilnil // nil config is valid (use defaults)
@@ -564,6 +564,116 @@ func TestMalformedConfigFileAbortsStartup(t *testing.T) {
t.Logf("got expected error: %v", err) t.Logf("got expected error: %v", err)
} }
// TestConfigFileInDirectoryPixaMayNotEnterAbortsStartup checks that a
// config file pixa cannot read because it may not enter its directory
// aborts startup instead of being passed over.
func TestConfigFileInDirectoryPixaMayNotEnterAbortsStartup(t *testing.T) {
if os.Geteuid() == 0 {
t.Skip("root may enter any directory")
}
home := t.TempDir()
configDir := filepath.Join(home, ".config", "pixa-test-nonexistent-app")
configPath := filepath.Join(configDir, "config.yml")
err := os.MkdirAll(configDir, 0o700)
if err != nil {
t.Fatalf("failed to create config directory: %v", err)
}
err = os.WriteFile(configPath, []byte(signingKeyLine), 0o600)
if err != nil {
t.Fatalf("failed to write config: %v", err)
}
err = os.Chmod(configDir, 0)
if err != nil {
t.Fatalf("failed to remove the config directory's permissions: %v", err)
}
// Give the directory back its permissions so t.TempDir can remove it.
t.Cleanup(func() {
//nolint:gosec // G302: a directory needs its execute bit to be removed
_ = os.Chmod(configDir, 0o700)
})
// The ~/.config candidate is the only one that exists: the appname
// rules out /etc, and the working directory is empty.
t.Setenv("PIXA_CONFIG_PATH", "")
t.Setenv("HOME", home)
t.Chdir(t.TempDir())
log := slog.New(slog.DiscardHandler)
sc, err := loadConfigFile(log, "pixa-test-nonexistent-app")
if err == nil {
t.Fatalf("config file pixa cannot read must abort startup, got config: %v",
sc)
}
t.Logf("got expected error: %v", err)
if !strings.Contains(err.Error(), configPath) {
t.Errorf("error %q does not name the config file %s", err.Error(), configPath)
}
}
// TestConfigFileLinkingToItselfAbortsStartup checks that a config file
// pixa cannot read for a reason other than not existing aborts startup,
// as root too: a symbolic link to itself fails with "too many levels of
// symbolic links".
func TestConfigFileLinkingToItselfAbortsStartup(t *testing.T) {
workDir := t.TempDir()
err := os.Symlink("config.yml", filepath.Join(workDir, "config.yml"))
if err != nil {
t.Fatalf("failed to create symbolic link: %v", err)
}
// Only the working directory's config.yml is there: the appname rules
// out /etc, and HOME is empty.
t.Setenv("PIXA_CONFIG_PATH", "")
t.Setenv("HOME", t.TempDir())
t.Chdir(workDir)
log := slog.New(slog.DiscardHandler)
sc, err := loadConfigFile(log, "pixa-test-nonexistent-app")
if err == nil {
t.Fatalf("config file pixa cannot read must abort startup, got config: %v",
sc)
}
t.Logf("got expected error: %v", err)
if !strings.Contains(err.Error(), "config.yml") {
t.Errorf("error %q does not name the config file config.yml", err.Error())
}
}
// TestConfigPathThroughFileIsPassedOver checks that a config file path
// that runs through a file, such as one under a HOME of /dev/null, is
// passed over like one that does not exist, since no file can be there.
func TestConfigPathThroughFileIsPassedOver(t *testing.T) {
// No config file is there: the appname rules out /etc, HOME is
// /dev/null, and the working directory is empty.
t.Setenv("PIXA_CONFIG_PATH", "")
t.Setenv("HOME", os.DevNull)
t.Chdir(t.TempDir())
log := slog.New(slog.DiscardHandler)
sc, err := loadConfigFile(log, "pixa-test-nonexistent-app")
if err != nil {
t.Fatalf("a config path through a file must be passed over, got error: %v",
err)
}
if sc != nil {
t.Errorf("expected no config file, got config: %v", sc)
}
}
func TestEnsureStateDirCreatesDirectory(t *testing.T) { func TestEnsureStateDirCreatesDirectory(t *testing.T) {
t.Parallel() t.Parallel()
+1 -1
View File
@@ -98,7 +98,7 @@ func TestEnvironmentSetsEveryKey(t *testing.T) {
UpstreamConnections: 10, UpstreamConnections: 10,
MaxConcurrentProcessing: 3, 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")},
TrustedProxies: []netip.Prefix{netip.MustParsePrefix("192.0.2.0/24")}, TrustedProxies: []netip.Prefix{netip.MustParsePrefix("192.0.2.0/24")},
AccessControlAllowOrigin: "https://app.example.com", AccessControlAllowOrigin: "https://app.example.com",
+4 -3
View File
@@ -2,7 +2,7 @@
-- Creates all tables for the pixa caching image proxy -- Creates all tables for the pixa caching image proxy
-- Source content blobs -- Source content blobs
-- Files stored at: cache/src-content/<ab>/<cd>/<sha256> -- Files stored at: cache/sources/<ab>/<cd>/<sha256>
-- last_accessed_at is NULL until the first LRU touch; eviction falls -- last_accessed_at is NULL until the first LRU touch; eviction falls
-- back to fetched_at for rows that have never been touched. -- back to fetched_at for rows that have never been touched.
CREATE TABLE IF NOT EXISTS source_content ( CREATE TABLE IF NOT EXISTS source_content (
@@ -16,7 +16,7 @@ CREATE INDEX IF NOT EXISTS idx_source_content_last_accessed
ON source_content(last_accessed_at); ON source_content(last_accessed_at);
-- Source URL metadata - maps URLs to content hashes -- Source URL metadata - maps URLs to content hashes
-- JSON stored at: cache/src-metadata/<hostname>/<path_hash>.json -- JSON stored at: cache/metadata/<hostname>/<path_hash>.json
CREATE TABLE IF NOT EXISTS source_metadata ( CREATE TABLE IF NOT EXISTS source_metadata (
id INTEGER PRIMARY KEY AUTOINCREMENT, id INTEGER PRIMARY KEY AUTOINCREMENT,
source_host TEXT NOT NULL, source_host TEXT NOT NULL,
@@ -56,7 +56,8 @@ CREATE INDEX IF NOT EXISTS idx_variant_content_last_accessed
ON variant_content(last_accessed_at); ON variant_content(last_accessed_at);
-- Output/transformed content blobs -- Output/transformed content blobs
-- Files stored at: cache/dst-content/<ab>/<cd>/<sha256> -- Not written: transformed images are stored in cache/variants and
-- tracked in variant_content above.
CREATE TABLE IF NOT EXISTS output_content ( CREATE TABLE IF NOT EXISTS output_content (
content_hash TEXT PRIMARY KEY, content_hash TEXT PRIMARY KEY,
content_type TEXT NOT NULL, content_type TEXT NOT NULL,
@@ -0,0 +1,59 @@
package handlers
import (
"bytes"
"log/slog"
"net/http"
"net/http/httptest"
"net/url"
"strings"
"testing"
"sneak.berlin/go/pixa/internal/config"
"sneak.berlin/go/pixa/internal/session"
)
// TestLoginLogLeavesOutSubmittedKey verifies that the log lines for a
// failed and for a successful login do not contain the submitted key.
func TestLoginLogLeavesOutSubmittedKey(t *testing.T) {
t.Parallel()
const wrongKey = "wrong-signing-key-fedcba9876543210"
var buf bytes.Buffer
sessMgr, err := session.NewManager(testSigningKey)
if err != nil {
t.Fatalf("session.NewManager() error = %v", err)
}
h := &Handlers{
log: slog.New(slog.NewJSONHandler(&buf, nil)),
config: &config.Config{SigningKey: testSigningKey},
sessMgr: sessMgr,
}
submittedKeys := []string{wrongKey, testSigningKey}
for _, key := range submittedKeys {
form := url.Values{loginKeyField: {key}}
req := httptest.NewRequestWithContext(
t.Context(), http.MethodPost, "/",
strings.NewReader(form.Encode()))
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
h.handleLoginPost(httptest.NewRecorder(), req)
}
for _, msg := range []string{"failed login attempt", "successful login"} {
if !strings.Contains(buf.String(), msg) {
t.Fatalf("log missing %q; got %q", msg, buf.String())
}
}
for _, key := range submittedKeys {
if strings.Contains(buf.String(), key) {
t.Errorf("log contains submitted key %q; got %q", key, buf.String())
}
}
}
@@ -0,0 +1,147 @@
package handlers
import (
"os"
"path/filepath"
"testing"
"go.uber.org/fx/fxtest"
"sneak.berlin/go/pixa/internal/config"
"sneak.berlin/go/pixa/internal/database"
"sneak.berlin/go/pixa/internal/globals"
"sneak.berlin/go/pixa/internal/logger"
)
// TestNewCacheConfigFromCacheMaxBytes checks the cache configuration
// built from cache_max_bytes: omitted, the cache works out the default
// limit; 0 turns the disk cache off; a positive value is the limit,
// unchanged.
func TestNewCacheConfigFromCacheMaxBytes(t *testing.T) {
t.Parallel()
const oneGiB = 1 << 30
cases := []struct {
name string
cacheMaxBytes int64
cacheMaxBytesExplicit bool
wantMaxBytes int64
wantUseDefaultMaxBytes bool
wantDisableDiskCache bool
}{
{
name: "cache_max_bytes omitted",
cacheMaxBytes: 0,
cacheMaxBytesExplicit: false,
wantMaxBytes: 0,
wantUseDefaultMaxBytes: true,
wantDisableDiskCache: false,
},
{
name: "cache_max_bytes: 0",
cacheMaxBytes: 0,
cacheMaxBytesExplicit: true,
wantMaxBytes: 0,
wantUseDefaultMaxBytes: false,
wantDisableDiskCache: true,
},
{
name: "cache_max_bytes: 1 GiB",
cacheMaxBytes: oneGiB,
cacheMaxBytesExplicit: true,
wantMaxBytes: oneGiB,
wantUseDefaultMaxBytes: false,
wantDisableDiskCache: false,
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
cfg := &config.Config{
CacheMaxBytes: tc.cacheMaxBytes,
CacheMaxBytesExplicit: tc.cacheMaxBytesExplicit,
}
got := newCacheConfig(cfg, nil)
t.Logf("MaxBytes = %d, UseDefaultMaxBytes = %v, DisableDiskCache = %v",
got.MaxBytes, got.UseDefaultMaxBytes, got.DisableDiskCache)
if got.MaxBytes != tc.wantMaxBytes {
t.Errorf("MaxBytes = %d, want %d", got.MaxBytes, tc.wantMaxBytes)
}
if got.UseDefaultMaxBytes != tc.wantUseDefaultMaxBytes {
t.Errorf("UseDefaultMaxBytes = %v, want %v",
got.UseDefaultMaxBytes, tc.wantUseDefaultMaxBytes)
}
if got.DisableDiskCache != tc.wantDisableDiskCache {
t.Errorf("DisableDiskCache = %v, want %v",
got.DisableDiskCache, tc.wantDisableDiskCache)
}
})
}
}
// TestDiskCacheOffOnlyForExplicitZeroCacheMaxBytes starts the handlers
// once with cache_max_bytes omitted and once with cache_max_bytes: 0,
// and checks by whether the cache directories were created that the
// disk cache is on in the first case and off in the second.
func TestDiskCacheOffOnlyForExplicitZeroCacheMaxBytes(t *testing.T) {
t.Parallel()
cases := []struct {
name string
cacheMaxBytesExplicit bool
wantDiskCache bool
}{
{name: "cache_max_bytes omitted", cacheMaxBytesExplicit: false, wantDiskCache: true},
{name: "cache_max_bytes: 0", cacheMaxBytesExplicit: true, wantDiskCache: false},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
stateDir := t.TempDir()
cfg := &config.Config{
SigningKey: testSigningKey,
StateDir: stateDir,
DBURL: "file:" + filepath.Join(stateDir, "state.sqlite3"),
CacheMaxBytes: 0,
CacheMaxBytesExplicit: tc.cacheMaxBytesExplicit,
}
lc := fxtest.NewLifecycle(t)
log, err := logger.New(lc, logger.Params{Globals: &globals.Globals{}})
if err != nil {
t.Fatalf("logger.New() error = %v", err)
}
db, err := database.New(lc, database.Params{Logger: log, Config: cfg})
if err != nil {
t.Fatalf("database.New() error = %v", err)
}
_, err = New(lc, Params{Logger: log, Database: db, Config: cfg})
if err != nil {
t.Fatalf("New() error = %v", err)
}
lc.RequireStart()
t.Cleanup(lc.RequireStop)
_, err = os.Stat(filepath.Join(stateDir, "cache", "variants"))
gotDiskCache := err == nil
if gotDiskCache != tc.wantDiskCache {
t.Errorf("cache directories created = %v, want %v",
gotDiskCache, tc.wantDiskCache)
}
})
}
}
+28 -22
View File
@@ -58,41 +58,47 @@ func New(lc fx.Lifecycle, params Params) (*Handlers, error) {
} }
lc.Append(fx.Hook{ lc.Append(fx.Hook{
// The eviction goroutine must outlive OnStart, so it cannot //nolint:contextcheck // the eviction loop outlives OnStart; OnStop cancels it
// inherit this hook's context. It makes its own instead, which
// leaves it uncancellable: an in-flight pass runs to completion
// during OnStop regardless of the shutdown deadline. Making the
// loop cancellable changes shutdown semantics and is tracked
// separately in issue #102, rather than being folded into the
// lint-conformance change that surfaced it.
//nolint:contextcheck // see issue #102
OnStart: func(_ context.Context) error { OnStart: func(_ context.Context) error {
return s.initImageService() return s.initImageService()
}, },
OnStop: func(_ context.Context) error { OnStop: func(ctx context.Context) error {
if s.imgCache != nil { if s.imgCache == nil {
s.imgCache.StopEviction() return nil
} }
return nil return s.imgCache.StopEviction(ctx)
}, },
}) })
return s, nil return s, nil
} }
// WaitForProcessing waits until no image is being processed, or until ctx
// ends, and returns how many images were still being processed then.
func (s *Handlers) WaitForProcessing(ctx context.Context) int {
return s.imgSvc.WaitForProcessing(ctx)
}
// newCacheConfig builds the image cache's configuration from cfg.
// cache_max_bytes: 0 disables the disk cache entirely; any other value
// is the eviction limit in bytes; when it is omitted, the cache works
// out the default limit itself.
func newCacheConfig(cfg *config.Config, log *slog.Logger) imgcache.CacheConfig {
return imgcache.CacheConfig{
StateDir: cfg.StateDir,
CacheTTL: imgcache.DefaultCacheTTL,
NegativeTTL: imgcache.DefaultNegativeTTL,
MaxBytes: cfg.CacheMaxBytes,
UseDefaultMaxBytes: !cfg.CacheMaxBytesExplicit,
DisableDiskCache: cfg.CacheMaxBytesExplicit && cfg.CacheMaxBytes == 0,
Logger: log,
}
}
// initImageService initializes the image cache and service. // initImageService initializes the image cache and service.
func (s *Handlers) initImageService() error { func (s *Handlers) initImageService() error {
// Create the cache. cache_max_bytes: 0 disables the disk cache cache, err := imgcache.NewCache(s.db.DB(), newCacheConfig(s.config, s.log))
// entirely; any other value is the eviction limit in bytes.
cache, err := imgcache.NewCache(s.db.DB(), imgcache.CacheConfig{
StateDir: s.config.StateDir,
CacheTTL: imgcache.DefaultCacheTTL,
NegativeTTL: imgcache.DefaultNegativeTTL,
MaxBytes: s.config.CacheMaxBytes,
DisableDiskCache: s.config.CacheMaxBytes == 0,
Logger: s.log,
})
if err != nil { if err != nil {
return err return err
} }
+23 -11
View File
@@ -10,6 +10,7 @@ import (
"time" "time"
"github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5"
"github.com/go-chi/chi/v5/middleware"
"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/imageprocessor"
@@ -247,6 +248,25 @@ func cacheControl(expires time.Time) string {
return fmt.Sprintf("public, max-age=%d, immutable", int64(maxAge/time.Second)) return fmt.Sprintf("public, max-age=%d, immutable", int64(maxAge/time.Second))
} }
// notModified sets the ETag header to etag and, when the request's
// If-None-Match is that ETag, answers 304 Not Modified. It reports whether it
// answered. An empty etag sets no header and never answers.
func notModified(w http.ResponseWriter, r *http.Request, etag string) bool {
if etag == "" {
return false
}
w.Header().Set("ETag", etag)
if r.Header.Get("If-None-Match") != etag {
return false
}
w.WriteHeader(http.StatusNotModified)
return true
}
// writeImageResponse writes headers and streams the image content, // writeImageResponse writes headers and streams the image content,
// handling conditional and HEAD requests. // handling conditional and HEAD requests.
func (s *Handlers) writeImageResponse( func (s *Handlers) writeImageResponse(
@@ -265,17 +285,8 @@ func (s *Handlers) writeImageResponse(
w.Header().Set("Cache-Control", cacheControl(req.Expires)) w.Header().Set("Cache-Control", cacheControl(req.Expires))
w.Header().Set("X-Pixa-Cache", string(resp.CacheStatus)) w.Header().Set("X-Pixa-Cache", string(resp.CacheStatus))
if resp.ETag != "" { if notModified(w, r, resp.ETag) {
w.Header().Set("ETag", resp.ETag) return
// Check for conditional request (If-None-Match)
if ifNoneMatch := r.Header.Get("If-None-Match"); ifNoneMatch != "" {
if ifNoneMatch == resp.ETag {
w.WriteHeader(http.StatusNotModified)
return
}
}
} }
// Handle HEAD request - return headers only // Handle HEAD request - return headers only
@@ -298,6 +309,7 @@ func (s *Handlers) writeImageResponse(
// Log cache status and timing after serving // Log cache status and timing after serving
duration := time.Since(startTime) duration := time.Since(startTime)
s.log.Info("image served", s.log.Info("image served",
"request_id", middleware.GetReqID(r.Context()),
"cache_key", cacheKey, "cache_key", cacheKey,
"cache_status", resp.CacheStatus, "cache_status", resp.CacheStatus,
"duration_ms", duration.Milliseconds(), "duration_ms", duration.Milliseconds(),
@@ -23,8 +23,10 @@ const photoPath = "/images/photo.jpg"
// newSignedHostServer returns a router for both image routes, and the Handlers // newSignedHostServer returns a router for both image routes, and the Handlers
// behind it, whose fetcher serves a JPEG at photoPath on signedHost. signedHost // behind it, whose fetcher serves a JPEG at photoPath on signedHost. signedHost
// is not on the allowlist, so a /v1/image/ URL for it is served only with a // is not on the allowlist, so a /v1/image/ URL for it is served only with a
// valid signature. // valid signature. The handlers and the image service log to log.
func newSignedHostServer(t *testing.T) (*Handlers, http.Handler) { func newSignedHostServer(
t *testing.T, log *slog.Logger,
) (*Handlers, http.Handler) {
t.Helper() t.Helper()
cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{ cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{
@@ -44,6 +46,7 @@ func newSignedHostServer(t *testing.T) (*Handlers, http.Handler) {
signedHost + photoPath: &fstest.MapFile{Data: jpegData}, signedHost + photoPath: &fstest.MapFile{Data: jpegData},
}), }),
SigningKey: testSigningKey, SigningKey: testSigningKey,
Logger: log,
}) })
if err != nil { if err != nil {
t.Fatalf("imgcache.NewService() error = %v", err) t.Fatalf("imgcache.NewService() error = %v", err)
@@ -55,7 +58,7 @@ func newSignedHostServer(t *testing.T) (*Handlers, http.Handler) {
} }
h := &Handlers{ h := &Handlers{
log: slog.New(slog.DiscardHandler), log: log,
imgSvc: svc, imgSvc: svc,
encGen: encGen, encGen: encGen,
} }
@@ -103,7 +106,7 @@ func getMaxAge(t *testing.T, srv http.Handler, target string) int {
func TestHandleImage_SignedURL_MaxAgeEndsAtExp(t *testing.T) { func TestHandleImage_SignedURL_MaxAgeEndsAtExp(t *testing.T) {
t.Parallel() t.Parallel()
h, srv := newSignedHostServer(t) h, srv := newSignedHostServer(t, slog.New(slog.DiscardHandler))
signedURL, err := h.imgSvc.GenerateSignedURL("", &imgcache.ImageRequest{ signedURL, err := h.imgSvc.GenerateSignedURL("", &imgcache.ImageRequest{
SourceHost: signedHost, SourceHost: signedHost,
@@ -179,7 +182,7 @@ func TestHandleImageEnc_MaxAge(t *testing.T) {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
t.Parallel() t.Parallel()
h, srv := newSignedHostServer(t) h, srv := newSignedHostServer(t, slog.New(slog.DiscardHandler))
token, err := h.encGen.Generate(&encurl.Payload{ token, err := h.encGen.Generate(&encurl.Payload{
SourceHost: signedHost, SourceHost: signedHost,
@@ -0,0 +1,267 @@
package handlers
import (
"encoding/json"
"fmt"
"image/color"
"log/slog"
"net/http"
"net/http/httptest"
"strings"
"testing"
"testing/fstest"
"time"
"github.com/go-chi/chi/v5"
"sneak.berlin/go/pixa/internal/httpfetcher"
"sneak.berlin/go/pixa/internal/imgcache"
"sneak.berlin/go/pixa/internal/signature"
)
// allowlistedHost is the only host on the allowlist of the image route
// newImageRoute builds.
const allowlistedHost = "allowed.example.com"
// newImageRoute returns the image route of a Handlers whose service fetches
// with fetcher and checks signatures with testSigningKey.
func newImageRoute(t *testing.T, fetcher httpfetcher.Fetcher) http.Handler {
t.Helper()
cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{
StateDir: t.TempDir(),
CacheTTL: time.Hour,
NegativeTTL: 5 * time.Minute,
})
if err != nil {
t.Fatalf("failed to create cache: %v", err)
}
svc, err := imgcache.NewService(&imgcache.ServiceConfig{
Cache: cache,
Fetcher: fetcher,
SigningKey: testSigningKey,
Allowlist: []string{allowlistedHost},
})
if err != nil {
t.Fatalf("failed to create service: %v", err)
}
h := &Handlers{imgSvc: svc, log: slog.New(slog.DiscardHandler)}
r := chi.NewRouter()
r.Get("/v1/image/*", h.HandleImage())
return r
}
// newPhotoFetcher returns a mock fetcher that serves a JPEG at photoPath on
// each of hosts, and answers any other URL with an upstream error.
func newPhotoFetcher(t *testing.T, hosts ...string) *httpfetcher.MockFetcher {
t.Helper()
photo := &fstest.MapFile{
Data: generateTestJPEG(t, 100, 100, color.RGBA{255, 0, 0, 255}),
}
files := fstest.MapFS{}
for _, host := range hosts {
files[host+photoPath] = photo
}
return httpfetcher.NewMock(files)
}
// photoURL returns the image route URL of photoPath on host, as a 50x50 JPEG.
func photoURL(host string) string {
return "/v1/image/" + host + photoPath + "/50x50.jpeg"
}
// photoURLWithSig returns photoURL(host) with sig and expires as its sig and
// exp.
func photoURLWithSig(host, sig string, expires time.Time) string {
return fmt.Sprintf("%s?sig=%s&exp=%d", photoURL(host), sig, expires.Unix())
}
// photoSignature returns the signature of photoURL(host) at the default
// quality and fit, made with key and expiring at expires.
func photoSignature(key, host string, expires time.Time) string {
return signature.New(key).Sign(&signature.Request{
SourceHost: host,
SourcePath: photoPath,
Width: 50,
Height: 50,
Format: string(imgcache.FormatJPEG),
Quality: 85,
FitMode: string(imgcache.FitCover),
Expires: expires,
})
}
// sendGet sends a GET for target to route and returns the response.
func sendGet(
t *testing.T, route http.Handler, target string,
) *httptest.ResponseRecorder {
t.Helper()
req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, target, nil)
rec := httptest.NewRecorder()
route.ServeHTTP(rec, req)
t.Logf("GET %s: %d", target, rec.Code)
return rec
}
// checkErrorBody checks that rec has status wantStatus and the JSON error body
// the image route sends: wantError, wantStatus and the time in RFC 3339.
func checkErrorBody(
t *testing.T, rec *httptest.ResponseRecorder, wantStatus int, wantError string,
) {
t.Helper()
if rec.Code != wantStatus {
t.Errorf("status = %d, want %d", rec.Code, wantStatus)
}
if ct := rec.Header().Get("Content-Type"); ct != "application/json" {
t.Errorf("Content-Type = %q, want application/json", ct)
}
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("decoding response body: %v", err)
}
if body.Error != wantError || body.Status != wantStatus {
t.Errorf("body error and status = %q %d, want %q %d",
body.Error, body.Status, wantError, wantStatus)
}
_, err = time.Parse(time.RFC3339, body.Timestamp)
if err != nil {
t.Errorf("body timestamp: %v", err)
}
}
// TestHandleImage_ErrorAnswers checks the status and the JSON error body the
// image route answers each request below with. The JPEG at photoPath exists on
// signedHost and on each host below that differs from it, so a request refused
// with 401 would otherwise be served.
func TestHandleImage_ErrorAnswers(t *testing.T) {
t.Parallel()
// A signature for signedHost must not verify for any of these.
parentHost := "example.com"
siblingHost := "other.example.com"
subdomainHost := "img." + signedHost
appendedHost := signedHost + ".example.net"
photos := newPhotoFetcher(t,
signedHost, parentHost, siblingHost, subdomainHost, appendedHost)
// The real fetcher refuses localhost before any lookup or connection.
realFetcher := httpfetcher.New(httpfetcher.DefaultConfig())
exp := time.Now().Add(time.Hour)
expired := time.Now().Add(-time.Hour)
sig := photoSignature(testSigningKey, signedHost, exp)
otherKeySig := photoSignature("another-signing-key", signedHost, exp)
expiredSig := photoSignature(testSigningKey, signedHost, expired)
localhostSig := photoSignature(testSigningKey, "localhost", exp)
// The error every request refused for its signature gets.
const unauthorized = "unauthorized"
tests := []struct {
name string
fetcher httpfetcher.Fetcher
target string
wantStatus int
wantError string
}{
{"no sig or exp", photos, photoURL(signedHost),
http.StatusUnauthorized, unauthorized},
{"exp but no sig", photos,
fmt.Sprintf("%s?exp=%d", photoURL(signedHost), exp.Unix()),
http.StatusUnauthorized, unauthorized},
{"sig made with another key", photos,
photoURLWithSig(signedHost, otherKeySig, exp),
http.StatusUnauthorized, unauthorized},
{"sig without its = padding", photos,
photoURLWithSig(signedHost, strings.TrimRight(sig, "="), exp),
http.StatusUnauthorized, unauthorized},
{"sig in upper case", photos,
photoURLWithSig(signedHost, strings.ToUpper(sig), exp),
http.StatusUnauthorized, unauthorized},
{"expired sig", photos, photoURLWithSig(signedHost, expiredSig, expired),
http.StatusUnauthorized, unauthorized},
{"sig sent for the parent domain", photos,
photoURLWithSig(parentHost, sig, exp),
http.StatusUnauthorized, unauthorized},
{"sig sent for a sibling host", photos,
photoURLWithSig(siblingHost, sig, exp),
http.StatusUnauthorized, unauthorized},
{"sig sent for a subdomain", photos,
photoURLWithSig(subdomainHost, sig, exp),
http.StatusUnauthorized, unauthorized},
{"sig sent with another domain appended", photos,
photoURLWithSig(appendedHost, sig, exp),
http.StatusUnauthorized, unauthorized},
{"unparseable path", photos,
"/v1/image/" + allowlistedHost + photoPath + "/big.jpeg",
http.StatusBadRequest, "invalid image URL: invalid size format"},
{"blocked upstream address", realFetcher,
photoURLWithSig("localhost", localhostSig, exp),
http.StatusForbidden, "forbidden"},
{"upstream error", photos,
"/v1/image/" + allowlistedHost + "/images/missing.jpg/50x50.jpeg",
http.StatusBadGateway, "upstream error"},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
rec := sendGet(t, newImageRoute(t, tt.fetcher), tt.target)
checkErrorBody(t, rec, tt.wantStatus, tt.wantError)
})
}
}
// TestHandleImage_AllowlistOrSignature checks that the image route serves an
// image without a signature for a host on the allowlist only, and for another
// host only with a valid signature.
func TestHandleImage_AllowlistOrSignature(t *testing.T) {
t.Parallel()
photos := newPhotoFetcher(t, allowlistedHost, signedHost)
exp := time.Now().Add(time.Hour)
sig := photoSignature(testSigningKey, signedHost, exp)
tests := []struct {
name string
target string
wantStatus int
}{
{"allowlisted host, no sig", photoURL(allowlistedHost), http.StatusOK},
{"other host, no sig", photoURL(signedHost), http.StatusUnauthorized},
{"other host, valid sig", photoURLWithSig(signedHost, sig, exp),
http.StatusOK},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
rec := sendGet(t, newImageRoute(t, photos), tt.target)
if rec.Code != tt.wantStatus {
t.Errorf("status = %d, want %d", rec.Code, tt.wantStatus)
}
})
}
}
+65 -37
View File
@@ -9,6 +9,7 @@ import (
"time" "time"
"github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5"
"github.com/go-chi/chi/v5/middleware"
"sneak.berlin/go/pixa/internal/encurl" "sneak.berlin/go/pixa/internal/encurl"
"sneak.berlin/go/pixa/internal/httpfetcher" "sneak.berlin/go/pixa/internal/httpfetcher"
@@ -24,43 +25,8 @@ func (s *Handlers) HandleImageEnc() http.HandlerFunc {
ctx := r.Context() ctx := r.Context()
start := time.Now() start := time.Now()
// Extract token from URL req, ok := s.parseImageEncRequest(w, r)
token := chi.URLParam(r, "token") if !ok {
if token == "" {
s.respondError(w, "missing token", http.StatusBadRequest)
return
}
// Decrypt and validate the payload
payload, err := s.encGen.Parse(token)
if err != nil {
if errors.Is(err, encurl.ErrExpired) {
s.log.Debug("encrypted URL expired", "error", err)
s.respondError(w, "URL has expired", http.StatusGone)
return
}
s.log.Debug("failed to decrypt URL", "error", err)
s.respondError(w, "invalid encrypted URL", http.StatusBadRequest)
return
}
// Convert payload to ImageRequest
req := payload.ToImageRequest()
// Apply the same dimension and fit-mode bounds as the plain image
// route: a sealed payload is trusted for its origin, not for staying
// within limits, so an over-limit size or unknown fit mode is a 400
// here rather than an out-of-memory or a 500 from the processor.
err = imgcache.ValidateImageRequest(req)
if err != nil {
s.log.Debug("encrypted URL failed validation", "error", err)
s.respondError(w, "invalid encrypted URL: "+err.Error(),
http.StatusBadRequest)
return return
} }
@@ -94,6 +60,17 @@ func (s *Handlers) HandleImageEnc() http.HandlerFunc {
w.Header().Set("Cache-Control", cacheControl(req.Expires)) w.Header().Set("Cache-Control", cacheControl(req.Expires))
w.Header().Set("X-Pixa-Cache", string(resp.CacheStatus)) w.Header().Set("X-Pixa-Cache", string(resp.CacheStatus))
if notModified(w, r, resp.ETag) {
return
}
// A HEAD request gets the headers only
if r.Method == http.MethodHead {
w.WriteHeader(http.StatusOK)
return
}
// Stream the response // Stream the response
written, err := io.Copy(w, resp.Content) written, err := io.Copy(w, resp.Content)
if err != nil { if err != nil {
@@ -105,6 +82,7 @@ func (s *Handlers) HandleImageEnc() http.HandlerFunc {
// Log completion // Log completion
duration := time.Since(start) duration := time.Since(start)
s.log.Info("image served", s.log.Info("image served",
"request_id", middleware.GetReqID(ctx),
"cache_key", imgcache.CacheKey(req), "cache_key", imgcache.CacheKey(req),
"host", req.SourceHost, "host", req.SourceHost,
"path", req.SourcePath, "path", req.SourcePath,
@@ -116,6 +94,56 @@ func (s *Handlers) HandleImageEnc() http.HandlerFunc {
} }
} }
// parseImageEncRequest decrypts the token of an encrypted image URL into an
// ImageRequest and checks it. On a token that is missing, does not decrypt,
// has expired or asks for something not valid, it writes an error response
// and returns false.
func (s *Handlers) parseImageEncRequest(
w http.ResponseWriter, r *http.Request,
) (*imgcache.ImageRequest, bool) {
// Extract token from URL
token := chi.URLParam(r, "token")
if token == "" {
s.respondError(w, "missing token", http.StatusBadRequest)
return nil, false
}
// Decrypt and validate the payload
payload, err := s.encGen.Parse(token)
if err != nil {
if errors.Is(err, encurl.ErrExpired) {
s.log.Debug("encrypted URL expired", "error", err)
s.respondError(w, "URL has expired", http.StatusGone)
return nil, false
}
s.log.Debug("failed to decrypt URL", "error", err)
s.respondError(w, "invalid encrypted URL", http.StatusBadRequest)
return nil, false
}
// Convert payload to ImageRequest
req := payload.ToImageRequest()
// Apply the same dimension and fit-mode bounds as the plain image
// route: a sealed payload is trusted for its origin, not for staying
// within limits, so an over-limit size or unknown fit mode is a 400
// here rather than an out-of-memory or a 500 from the processor.
err = imgcache.ValidateImageRequest(req)
if err != nil {
s.log.Debug("encrypted URL failed validation", "error", err)
s.respondError(w, "invalid encrypted URL: "+err.Error(),
http.StatusBadRequest)
return nil, false
}
return req, true
}
// handleImageError converts image service errors to HTTP responses. // handleImageError converts image service errors to HTTP responses.
func (s *Handlers) handleImageError(w http.ResponseWriter, err error) { func (s *Handlers) handleImageError(w http.ResponseWriter, err error) {
switch { switch {
@@ -96,3 +96,70 @@ func TestHandleImageEnc_InvalidFitMode_Returns400(t *testing.T) {
t.Fatalf("status = %d, want %d", rec.Code, http.StatusBadRequest) t.Fatalf("status = %d, want %d", rec.Code, http.StatusBadRequest)
} }
} }
// TestHandleImageEnc_IfNoneMatch_Returns304 verifies that an image served
// through an encrypted URL carries an ETag, and that a request whose
// If-None-Match is that ETag is answered 304 Not Modified with no body.
func TestHandleImageEnc_IfNoneMatch_Returns304(t *testing.T) {
t.Parallel()
h, srv := newSignedHostServer(t, slog.New(slog.DiscardHandler))
target := encPhotoURL(t, h)
rec := httptest.NewRecorder()
srv.ServeHTTP(rec, httptest.NewRequestWithContext(
t.Context(), http.MethodGet, target, nil))
etag := rec.Header().Get("ETag")
t.Logf("GET: %d, ETag %q", rec.Code, etag)
if rec.Code != http.StatusOK || etag == "" {
t.Fatalf("GET: status = %d, ETag = %q, want %d and an ETag",
rec.Code, etag, http.StatusOK)
}
req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, target, nil)
req.Header.Set("If-None-Match", etag)
rec = httptest.NewRecorder()
srv.ServeHTTP(rec, req)
t.Logf("GET with If-None-Match: %d, %d body bytes", rec.Code, rec.Body.Len())
if rec.Code != http.StatusNotModified || rec.Body.Len() != 0 {
t.Errorf("status = %d with %d body bytes, want %d with none",
rec.Code, rec.Body.Len(), http.StatusNotModified)
}
}
// TestHandleImageEnc_HEAD_ReturnsHeadersOnly verifies that HEAD on an
// encrypted URL is answered 200 with the headers GET sends and no body.
func TestHandleImageEnc_HEAD_ReturnsHeadersOnly(t *testing.T) {
t.Parallel()
h, _ := newSignedHostServer(t, slog.New(slog.DiscardHandler))
r := chi.NewRouter()
r.Head("/v1/e/{token}/*", h.HandleImageEnc())
rec := httptest.NewRecorder()
r.ServeHTTP(rec, httptest.NewRequestWithContext(
t.Context(), http.MethodHead, encPhotoURL(t, h), nil))
t.Logf("HEAD: %d, headers %v, %d body bytes",
rec.Code, rec.Header(), rec.Body.Len())
if rec.Code != http.StatusOK {
t.Fatalf("status = %d, want %d", rec.Code, http.StatusOK)
}
for _, name := range []string{
"Content-Type", "Content-Length", "Cache-Control", "ETag",
} {
if rec.Header().Get(name) == "" {
t.Errorf("HEAD response has no %s", name)
}
}
if rec.Body.Len() != 0 {
t.Errorf("HEAD response body has %d bytes, want none", rec.Body.Len())
}
}
@@ -0,0 +1,131 @@
package handlers
import (
"bytes"
"context"
"encoding/json"
"io"
"log/slog"
"net/http"
"net/http/httptest"
"testing"
"time"
"github.com/go-chi/chi/v5/middleware"
"sneak.berlin/go/pixa/internal/encurl"
"sneak.berlin/go/pixa/internal/imgcache"
)
// signedPhotoURL returns a signed /v1/image/ URL, valid for a minute, for the
// JPEG at photoPath on signedHost at 50x50, made with h's image service.
func signedPhotoURL(t *testing.T, h *Handlers) string {
t.Helper()
signedURL, err := h.imgSvc.GenerateSignedURL("", &imgcache.ImageRequest{
SourceHost: signedHost,
SourcePath: photoPath,
Size: imgcache.Size{Width: 50, Height: 50},
Format: imgcache.FormatJPEG,
}, time.Minute)
if err != nil {
t.Fatalf("GenerateSignedURL() error = %v", err)
}
return signedURL
}
// encPhotoURL returns an encrypted /v1/e/ URL, which never expires, for the
// JPEG at photoPath on signedHost at 50x50, made with h's generator.
func encPhotoURL(t *testing.T, h *Handlers) string {
t.Helper()
token, err := h.encGen.Generate(&encurl.Payload{
SourceHost: signedHost,
SourcePath: photoPath,
Width: 50,
Height: 50,
Format: imgcache.FormatJPEG,
})
if err != nil {
t.Fatalf("Generate() error = %v", err)
}
return "/v1/e/" + token + "/img.jpg"
}
// requestIDByMessage reads the JSON log lines in logs and returns the
// request_id of each line, by its message.
func requestIDByMessage(t *testing.T, logs io.Reader) map[string]string {
t.Helper()
logged := make(map[string]string)
dec := json.NewDecoder(logs)
for dec.More() {
var line map[string]any
err := dec.Decode(&line)
if err != nil {
t.Fatalf("decoding log line: %v", err)
}
msg, _ := line["msg"].(string)
requestID, _ := line["request_id"].(string)
logged[msg] = requestID
}
return logged
}
// TestImageLogLinesCarryRequestID verifies that the lines logged when an image
// is fetched, converted and served through either image route carry the
// request's ID as request_id, as the request log line does, so they can be
// found from it.
func TestImageLogLinesCarryRequestID(t *testing.T) {
t.Parallel()
const requestID = "test-request-id"
imageURLs := map[string]func(*testing.T, *Handlers) string{
"/v1/image/": signedPhotoURL,
"/v1/e/": encPhotoURL,
}
for route, imageURL := range imageURLs {
t.Run(route, func(t *testing.T) {
t.Parallel()
var logs bytes.Buffer
h, srv := newSignedHostServer(t,
slog.New(slog.NewJSONHandler(&logs, nil)))
ctx := context.WithValue(t.Context(),
middleware.RequestIDKey, requestID)
rec := httptest.NewRecorder()
srv.ServeHTTP(rec, httptest.NewRequestWithContext(
ctx, http.MethodGet, imageURL(t, h), nil))
if rec.Code != http.StatusOK {
t.Fatalf("status = %d, want %d", rec.Code, http.StatusOK)
}
t.Logf("logged:\n%s", logs.String())
logged := requestIDByMessage(t, &logs)
for _, msg := range []string{
"upstream fetched", "image converted", "image served",
} {
got, ok := logged[msg]
if !ok {
t.Errorf("no %q line logged", msg)
} else if got != requestID {
t.Errorf("%q line has request_id %q, want %q",
msg, got, requestID)
}
}
})
}
}
@@ -0,0 +1,90 @@
package handlers
import (
"encoding/json"
"log/slog"
"net/http"
"testing"
"go.uber.org/fx/fxtest"
"sneak.berlin/go/pixa/internal/config"
"sneak.berlin/go/pixa/internal/globals"
"sneak.berlin/go/pixa/internal/healthcheck"
"sneak.berlin/go/pixa/internal/logger"
)
// TestHandleRobotsTxt checks that /robots.txt asks every crawler to stay off
// the whole site.
func TestHandleRobotsTxt(t *testing.T) {
t.Parallel()
h := &Handlers{log: slog.New(slog.DiscardHandler)}
rec := sendGet(t, h.HandleRobotsTxt(), "/robots.txt")
if rec.Code != http.StatusOK {
t.Errorf("status = %d, want %d", rec.Code, http.StatusOK)
}
if ct := rec.Header().Get("Content-Type"); ct != "text/plain" {
t.Errorf("Content-Type = %q, want text/plain", ct)
}
want := "User-agent: *\nDisallow: /\n"
if rec.Body.String() != want {
t.Errorf("body = %q, want %q", rec.Body.String(), want)
}
}
// TestHandleHealthCheck checks that the health check answers 200 with status
// ok, the app's name and version, now, uptime_seconds, uptime_human and
// maintenance_mode, which is true here: the health check stays 200 while
// maintenance mode is on.
func TestHandleHealthCheck(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)
}
hc, err := healthcheck.New(lc, healthcheck.Params{
Globals: &globals.Globals{Appname: "pixad", Version: "v1.2.3"},
Config: &config.Config{MaintenanceMode: true},
Logger: log,
})
if err != nil {
t.Fatalf("healthcheck.New() error = %v", err)
}
h := &Handlers{hc: hc, log: slog.New(slog.DiscardHandler)}
rec := sendGet(t, h.HandleHealthCheck(), "/.well-known/healthcheck.json")
if rec.Code != http.StatusOK {
t.Errorf("status = %d, want %d", rec.Code, http.StatusOK)
}
if ct := rec.Header().Get("Content-Type"); ct != "application/json" {
t.Errorf("Content-Type = %q, want application/json", ct)
}
var body map[string]any
err = json.NewDecoder(rec.Body).Decode(&body)
if err != nil {
t.Fatalf("decoding response body: %v", err)
}
if body["status"] != "ok" || body["appname"] != "pixad" ||
body["version"] != "v1.2.3" || body["maintenance_mode"] != true {
t.Errorf("body = %v, want status ok, appname pixad, version v1.2.3 "+
"and maintenance_mode true", body)
}
for _, key := range []string{"now", "uptime_seconds", "uptime_human"} {
if _, ok := body[key]; !ok {
t.Errorf("body = %v, has no %s", body, key)
}
}
}
+17 -1
View File
@@ -191,7 +191,23 @@ func fetchBody(t *testing.T, f *HTTPFetcher, path string) string {
// semLen reports how many per-host semaphore slots are currently held. // semLen reports how many per-host semaphore slots are currently held.
func semLen(f *HTTPFetcher, host string) int { func semLen(f *HTTPFetcher, host string) int {
return len(f.getHostSemaphore(host)) f.hostSemMu.Lock()
defer f.hostSemMu.Unlock()
sem, ok := f.hostSems[host]
if !ok {
return 0
}
return len(sem.slots)
}
// hostSemCount reports how many hosts have a semaphore in hostSems.
func hostSemCount(f *HTTPFetcher) int {
f.hostSemMu.Lock()
defer f.hostSemMu.Unlock()
return len(f.hostSems)
} }
func TestFetchRedirectToPrivateIPBlocked(t *testing.T) { func TestFetchRedirectToPrivateIPBlocked(t *testing.T) {
+53 -8
View File
@@ -18,6 +18,8 @@ import (
"strings" "strings"
"sync" "sync"
"time" "time"
"github.com/go-chi/chi/v5/middleware"
) )
// Fetcher configuration constants. // Fetcher configuration constants.
@@ -160,10 +162,13 @@ func DefaultConfig() *Config {
// HTTPFetcher implements Fetcher with SSRF protection and connection limits // HTTPFetcher implements Fetcher with SSRF protection and connection limits
// per host and for all hosts together. // 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 holds the semaphore of each host with a fetch holding or
hostSemMu sync.Mutex // protects hostSems map // waiting for one of its slots; the entry is removed when the host's
// last such fetch gives its slot back or stops waiting.
hostSems map[string]*hostSemaphore
hostSemMu sync.Mutex // protects hostSems and each entry's count
// allHostsSemaphore has one slot per connection allowed to all hosts // allHostsSemaphore has one slot per connection allowed to all hosts
// together (config.MaxConnections). // together (config.MaxConnections).
allHostsSemaphore chan struct{} allHostsSemaphore chan struct{}
@@ -171,6 +176,14 @@ type HTTPFetcher struct {
connectionWaitTimeout time.Duration connectionWaitTimeout time.Duration
} }
// hostSemaphore is one host's connection slots
// (config.MaxConnectionsPerHost) and the number of fetches holding or
// waiting for one of them.
type hostSemaphore struct {
slots chan struct{}
count int
}
// New creates a new HTTPFetcher with SSRF protection. // New creates a new HTTPFetcher with SSRF protection.
func New(config *Config) *HTTPFetcher { func New(config *Config) *HTTPFetcher {
if config == nil { if config == nil {
@@ -211,7 +224,7 @@ 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]*hostSemaphore),
allHostsSemaphore: make(chan struct{}, config.MaxConnections), allHostsSemaphore: make(chan struct{}, config.MaxConnections),
connectionWaitTimeout: ConnectionWaitTimeout, connectionWaitTimeout: ConnectionWaitTimeout,
} }
@@ -256,6 +269,13 @@ func (f *HTTPFetcher) Fetch(ctx context.Context, url string) (*FetchResult, erro
req.Header.Set("User-Agent", f.config.UserAgent) req.Header.Set("User-Agent", f.config.UserAgent)
req.Header.Set("Accept", strings.Join(f.config.AllowedContentTypes, ", ")) req.Header.Set("Accept", strings.Join(f.config.AllowedContentTypes, ", "))
// The ID of the request this fetch serves, so the fetch can be found in
// the upstream host's logs
requestID := middleware.GetReqID(ctx)
if requestID != "" {
req.Header.Set(middleware.RequestIDHeader, requestID)
}
// Use httptrace to capture connection details // Use httptrace to capture connection details
var remoteAddr string var remoteAddr string
@@ -307,6 +327,8 @@ func (f *HTTPFetcher) acquireConnection(
select { select {
case hostSem <- struct{}{}: case hostSem <- struct{}{}:
case <-ctx.Done(): case <-ctx.Done():
f.putHostSemaphore(host)
return nil, ctx.Err() return nil, ctx.Err()
} }
@@ -314,32 +336,55 @@ func (f *HTTPFetcher) acquireConnection(
case f.allHostsSemaphore <- struct{}{}: case f.allHostsSemaphore <- struct{}{}:
case <-time.After(f.connectionWaitTimeout): case <-time.After(f.connectionWaitTimeout):
<-hostSem <-hostSem
f.putHostSemaphore(host)
return nil, ErrTooManyConnections return nil, ErrTooManyConnections
case <-ctx.Done(): case <-ctx.Done():
<-hostSem <-hostSem
f.putHostSemaphore(host)
return nil, ctx.Err() return nil, ctx.Err()
} }
return func() { return func() {
<-hostSem <-hostSem
f.putHostSemaphore(host)
<-f.allHostsSemaphore <-f.allHostsSemaphore
}, nil }, nil
} }
// getHostSemaphore returns the semaphore for a host, creating it if necessary. // getHostSemaphore returns the semaphore for a host, creating it if
// necessary, and counts the caller among the fetches using it. The caller
// calls putHostSemaphore once it holds no slot and waits for none.
func (f *HTTPFetcher) getHostSemaphore(host string) chan struct{} { func (f *HTTPFetcher) getHostSemaphore(host string) chan struct{} {
f.hostSemMu.Lock() f.hostSemMu.Lock()
defer f.hostSemMu.Unlock() defer f.hostSemMu.Unlock()
sem, ok := f.hostSems[host] sem, ok := f.hostSems[host]
if !ok { if !ok {
sem = make(chan struct{}, f.config.MaxConnectionsPerHost) sem = &hostSemaphore{
slots: make(chan struct{}, f.config.MaxConnectionsPerHost),
}
f.hostSems[host] = sem f.hostSems[host] = sem
} }
return sem sem.count++
return sem.slots
}
// putHostSemaphore stops counting the caller among the fetches using the
// host's semaphore, and removes the semaphore when no fetch uses it.
func (f *HTTPFetcher) putHostSemaphore(host string) {
f.hostSemMu.Lock()
defer f.hostSemMu.Unlock()
sem := f.hostSems[host]
sem.count--
if sem.count == 0 {
delete(f.hostSems, host)
}
} }
// buildResult validates the upstream response and assembles a FetchResult // buildResult validates the upstream response and assembles a FetchResult
@@ -5,6 +5,7 @@ import (
"errors" "errors"
"net" "net"
"strconv" "strconv"
"sync"
"testing" "testing"
"time" "time"
) )
@@ -119,6 +120,98 @@ func TestFetchFreesHostSlotWhenContextEndsWaitingForConnection(t *testing.T) {
} }
} }
// TestFetchRemovesIdleHostSemaphores checks that a host's semaphore is
// removed once no fetch holds or waits for one of its slots: after 100
// concurrent fetches from 50 hosts have all finished, no semaphore is left.
func TestFetchRemovesIdleHostSemaphores(t *testing.T) {
t.Parallel()
srv := startUpstream(t)
f, _ := newServerFetcher(t, srv, nil)
ctx := testContext(t)
var wg sync.WaitGroup
for i := range 100 {
wg.Go(func() {
res, err := f.Fetch(ctx, imageURLOnPort(1+i%50))
if err != nil {
t.Errorf("Fetch() error = %v", err)
return
}
_ = res.Content.Close()
})
}
wg.Wait()
if n := hostSemCount(f); n != 0 {
t.Errorf("%d host semaphores left after every fetch finished, want 0", n)
}
}
// TestFetchRemovesHostSemaphoreWhenNoConnection checks that a fetch that
// ends without a connection leaves no semaphore behind: when it is refused
// after waiting for a connection shared by all hosts, when its context ends
// while it waits for its host's slot, and when its context ends while it
// waits for a connection shared by all hosts, long before the 10 second
// wait timeout.
func TestFetchRemovesHostSemaphoreWhenNoConnection(t *testing.T) {
t.Parallel()
srv := startUpstream(t)
cfg := DefaultConfig()
cfg.MaxConnections = 1
cfg.MaxConnectionsPerHost = 1
f, _ := newServerFetcher(t, srv, cfg)
f.connectionWaitTimeout = 100 * time.Millisecond
open, err := f.Fetch(testContext(t), imageURLOnPort(81))
if err != nil {
t.Fatalf("first Fetch() error = %v", err)
}
_, err = f.Fetch(testContext(t), imageURLOnPort(82))
if !errors.Is(err, ErrTooManyConnections) {
t.Fatalf("Fetch() from another host: error = %v, "+
"want ErrTooManyConnections", err)
}
ctx, cancel := context.WithTimeout(t.Context(), 100*time.Millisecond)
defer cancel()
_, err = f.Fetch(ctx, imageURLOnPort(81))
if !errors.Is(err, context.DeadlineExceeded) {
t.Fatalf("Fetch() from the busy host: error = %v, "+
"want context.DeadlineExceeded", err)
}
// Back to the 10 second wait, so the next fetch's context ends first.
f.connectionWaitTimeout = ConnectionWaitTimeout
ctx, cancel = context.WithTimeout(t.Context(), 100*time.Millisecond)
defer cancel()
_, err = f.Fetch(ctx, imageURLOnPort(83))
if !errors.Is(err, context.DeadlineExceeded) {
t.Fatalf("Fetch() from a host with nothing open: error = %v, "+
"want context.DeadlineExceeded", err)
}
err = open.Content.Close()
if err != nil {
t.Fatalf("close first body: %v", err)
}
if n := hostSemCount(f); n != 0 {
t.Errorf("%d host semaphores left after every fetch finished, want 0", n)
}
}
// TestFetchReleasesConnectionOnError checks that a fetch that fails after // TestFetchReleasesConnectionOnError checks that a fetch that fails after
// taking its connection gives it back: with MaxConnections at 1, the slot // taking its connection gives it back: with MaxConnections at 1, the slot
// must be free after the failure and the next fetch must succeed. // must be free after the failure and the next fetch must succeed.
@@ -0,0 +1,50 @@
package httpfetcher
import (
"context"
"io"
"net/http"
"net/http/httptest"
"testing"
"github.com/go-chi/chi/v5/middleware"
)
// TestFetchSendsRequestID verifies that a fetch sends the ID of the request
// it serves, which the RequestID middleware stores in the request context,
// to the upstream host as X-Request-Id, so the fetch can be found in that
// host's logs.
func TestFetchSendsRequestID(t *testing.T) {
t.Parallel()
const requestID = "test-request-id"
received := make(chan string, 1)
srv := httptest.NewServer(http.HandlerFunc(
func(w http.ResponseWriter, r *http.Request) {
received <- r.Header.Get("X-Request-Id")
w.Header().Set("Content-Type", contentTypeJPEG)
_, _ = io.WriteString(w, imagePayload)
}))
t.Cleanup(srv.Close)
f, _ := newServerFetcher(t, srv, nil)
ctx := context.WithValue(testContext(t), middleware.RequestIDKey, requestID)
res, err := f.Fetch(ctx, upstreamURL("/image"))
if err != nil {
t.Fatalf("Fetch() error = %v", err)
}
_ = res.Content.Close()
got := <-received
t.Logf("upstream received X-Request-Id %q", got)
if got != requestID {
t.Errorf("upstream X-Request-Id = %q, want %q", got, requestID)
}
}
+25
View File
@@ -331,6 +331,31 @@ func FormatToMIME(format Format) string {
} }
} }
// WaitForProcessing waits until no image is being processed, or until ctx
// ends, and returns how many images were still being processed then. It
// waits by taking each slot in processingSemaphore as it frees up until it
// holds them all, or until ctx ends, then gives back the slots it took.
func (p *ImageProcessor) WaitForProcessing(ctx context.Context) int {
taken := 0
defer func() {
for range taken {
<-p.processingSemaphore
}
}()
for taken < cap(p.processingSemaphore) {
select {
case p.processingSemaphore <- struct{}{}:
taken++
case <-ctx.Done():
return len(p.processingSemaphore) - taken
}
}
return 0
}
// acquireSlot takes a slot in processingSemaphore, waiting at most // acquireSlot takes a slot in processingSemaphore, waiting at most
// processingWaitTimeout for one to free up, and returns the func that gives // 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 // it back. A free slot is taken even when ctx has ended; only the wait for
@@ -300,3 +300,69 @@ func TestProcessReleasesSlotOnError(t *testing.T) {
}) })
} }
} }
// 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)
}
}
+33 -8
View File
@@ -11,7 +11,6 @@ import (
"io" "io"
"log/slog" "log/slog"
"path/filepath" "path/filepath"
"sync"
"time" "time"
lru "github.com/hashicorp/golang-lru/v2" lru "github.com/hashicorp/golang-lru/v2"
@@ -38,11 +37,16 @@ type CacheConfig struct {
NegativeTTL time.Duration NegativeTTL time.Duration
// MaxBytes is the disk cache size limit in bytes that eviction // MaxBytes is the disk cache size limit in bytes that eviction
// enforces. Zero means no limit is enforced (no eviction). The // enforces. Zero means no limit is enforced (no eviction).
// config layer supplies the computed default when the operator
// omits cache_max_bytes.
MaxBytes int64 MaxBytes int64
// UseDefaultMaxBytes makes NewCache replace MaxBytes with the
// default limit: 75% of the sum of the space free on the filesystem
// holding the cache and the bytes the cache already holds, at least
// DefaultCacheMaxBytesFloor. The config layer sets this when the
// operator omits cache_max_bytes.
UseDefaultMaxBytes bool
// DisableDiskCache turns the disk cache off entirely: no cache // DisableDiskCache turns the disk cache off entirely: no cache
// directories are created, lookups always miss, stores are // directories are created, lookups always miss, stores are
// no-ops, and no eviction machinery runs. The config layer sets // no-ops, and no eviction machinery runs. The config layer sets
@@ -69,11 +73,11 @@ type Cache struct {
// Eviction machinery. The channels are created in NewCache so // Eviction machinery. The channels are created in NewCache so
// stores can signal write pressure without racing StartEviction. // stores can signal write pressure without racing StartEviction.
// evictionCancel, set by StartEviction, cancels the eviction
// goroutine's context.
evictionPressure chan struct{} evictionPressure chan struct{}
evictionStop chan struct{}
evictionDone chan struct{} evictionDone chan struct{}
evictionStarted bool evictionCancel context.CancelFunc
evictionStopOnce sync.Once
// metaCache holds the content types of the variants most recently // metaCache holds the content types of the variants most recently
// stored or served, so a hit does not read the variant's .meta file. // stored or served, so a hit does not read the variant's .meta file.
@@ -96,6 +100,14 @@ type Cache struct {
// NewCache creates a new cache instance. // NewCache creates a new cache instance.
func NewCache(db *sql.DB, config CacheConfig) (*Cache, error) { func NewCache(db *sql.DB, config CacheConfig) (*Cache, error) {
return newCache(db, config, defaultFreeSpaceProbe)
}
// newCache is NewCache with the free-space probe passed in, so tests
// can fake the free space the default limit is worked out from.
func newCache(
db *sql.DB, config CacheConfig, probe FreeSpaceProbeFunc,
) (*Cache, error) {
log := config.Logger log := config.Logger
if log == nil { if log == nil {
log = slog.Default() log = slog.Default()
@@ -112,7 +124,6 @@ func NewCache(db *sql.DB, config CacheConfig) (*Cache, error) {
log: log, log: log,
disabled: config.DisableDiskCache, disabled: config.DisableDiskCache,
evictionPressure: make(chan struct{}, 1), evictionPressure: make(chan struct{}, 1),
evictionStop: make(chan struct{}),
evictionDone: make(chan struct{}), evictionDone: make(chan struct{}),
metaCache: metaCache, metaCache: metaCache,
contentLocks: newContentLock(), contentLocks: newContentLock(),
@@ -147,6 +158,20 @@ func NewCache(db *sql.DB, config CacheConfig) (*Cache, error) {
c.variants = variants c.variants = variants
c.srcMetadata = srcMetadata c.srcMetadata = srcMetadata
if config.UseDefaultMaxBytes {
limit, err := c.computeDefaultMaxBytes(context.Background(), probe)
if err != nil {
return nil, err
}
c.config.MaxBytes = limit
log.Info("computed default cache size limit from free space and cache contents",
"cache_max_bytes", limit,
"cache_dir", filepath.Join(config.StateDir, "cache"),
)
}
return c, nil return c, nil
} }
+89
View File
@@ -0,0 +1,89 @@
package imgcache
import (
"context"
"errors"
"fmt"
"math"
"path/filepath"
"syscall"
)
// DefaultCacheMaxBytesFloor is the minimum computed default for the
// cache_max_bytes setting: 500 MiB. The floor applies only to the
// computed default (when the key is omitted from the configuration),
// never to explicitly configured values.
const DefaultCacheMaxBytesFloor int64 = 524288000
// freeSpaceFractionNumerator and freeSpaceFractionDenominator express
// the 75% share used for the computed default limit as integer
// arithmetic (dividing before multiplying avoids overflow).
const (
freeSpaceFractionNumerator uint64 = 3
freeSpaceFractionDenominator uint64 = 4
)
var errNegativeBlockSize = errors.New("statfs reported negative block size")
// FreeSpaceProbeFunc reports the number of free bytes available on the
// filesystem containing path. It is a function type so tests can
// inject a fake probe instead of depending on the host disk.
type FreeSpaceProbeFunc func(path string) (uint64, error)
// defaultFreeSpaceProbe reports free filesystem bytes via statfs on
// the given path, as available to unprivileged processes.
func defaultFreeSpaceProbe(path string) (uint64, error) {
var stat syscall.Statfs_t
err := syscall.Statfs(path, &stat)
if err != nil {
return 0, err
}
if stat.Bsize < 0 {
return 0, fmt.Errorf("%w %d for %q", errNegativeBlockSize, stat.Bsize, path)
}
blockSize := uint64(stat.Bsize)
return stat.Bavail * blockSize, nil
}
// computeDefaultMaxBytes returns the default cache size limit: 75% of
// the sum of the free bytes probe reports for <state_dir>/cache/ and
// the bytes the cache already holds, with a floor of
// DefaultCacheMaxBytesFloor. Counting what the cache holds keeps the
// limit from shrinking as the cache fills.
func (c *Cache) computeDefaultMaxBytes(
ctx context.Context, probe FreeSpaceProbeFunc,
) (int64, error) {
cacheDir := filepath.Join(c.config.StateDir, "cache")
freeBytes, err := probe(cacheDir)
if err != nil {
return 0, fmt.Errorf(
"default cache_max_bytes: cannot determine free space for %q: %w",
cacheDir, err)
}
usedBytes, err := c.UsageBytes(ctx)
if err != nil {
return 0, err
}
// Both terms are at most math.MaxInt64, so the sum cannot overflow.
//nolint:gosec // G115: UsageBytes sums file sizes, never negative
spaceBytes := min(freeBytes, math.MaxInt64) + uint64(usedBytes)
computed := spaceBytes / freeSpaceFractionDenominator * freeSpaceFractionNumerator
computed = min(computed, math.MaxInt64)
// gosec cannot see that min() above bounds computed, so it reads
// this conversion as potentially overflowing. It cannot: computed is
// at most math.MaxInt64 on every path here.
//nolint:gosec // G115: clamped to MaxInt64 by min above
limit := int64(computed)
limit = max(limit, DefaultCacheMaxBytesFloor)
return limit, nil
}
@@ -0,0 +1,188 @@
package imgcache
import (
"errors"
"os"
"path/filepath"
"strings"
"testing"
)
// Static errors returned by the stub free-space probes below.
var (
errTestStatfsFailed = errors.New("statfs failed")
errTestProbeNotExpected = errors.New("probe must not be called")
)
// TestComputeDefaultMaxBytesCountsWhatTheCacheHolds verifies that the
// default limit is 75% of the free space plus what the cache already
// holds, so a cache filled to its limit keeps that limit across a
// restart instead of shrinking to 75% of the space left free.
func TestComputeDefaultMaxBytesCountsWhatTheCacheHolds(t *testing.T) {
t.Parallel()
cache, _ := newEvictionTestCache(t, 1<<30)
// Empty cache, 4 GiB free -> 3 GiB default.
got, err := cache.computeDefaultMaxBytes(t.Context(),
func(string) (uint64, error) { return 4294967296, nil })
if err != nil {
t.Fatalf("computeDefaultMaxBytes returned error: %v", err)
}
t.Logf("default for an empty cache with 4 GiB free: %d", got)
if got != 3221225472 {
t.Errorf("default for an empty cache = %d, want 3221225472 (75%% of 4 GiB)",
got)
}
// The cache now holds those 3 GiB, which leaves 1 GiB free.
_, err = cache.db.ExecContext(t.Context(),
`INSERT INTO variant_content (cache_key, size_bytes, content_type)
VALUES (?, ?, ?)`,
string(testVariantKeyOne), 3221225472, testContentTypeWebP,
)
if err != nil {
t.Fatalf("failed to insert variant accounting row: %v", err)
}
got, err = cache.computeDefaultMaxBytes(t.Context(),
func(string) (uint64, error) { return 1073741824, nil })
if err != nil {
t.Fatalf("computeDefaultMaxBytes returned error: %v", err)
}
t.Logf("default for a cache holding 3 GiB with 1 GiB free: %d", got)
if got != 3221225472 {
t.Errorf("default for a cache holding 3 GiB with 1 GiB free = %d, "+
"want 3221225472 (75%% of 1 GiB + 3 GiB)", got)
}
}
// TestComputeDefaultMaxBytesAppliesFloor verifies that when 75% of the
// free space plus what the cache holds is below 500 MiB, the default
// is floored at DefaultCacheMaxBytesFloor.
func TestComputeDefaultMaxBytesAppliesFloor(t *testing.T) {
t.Parallel()
cases := []struct {
name string
freeBytes uint64
}{
{name: "100 MiB free", freeBytes: 104857600},
{name: "zero free", freeBytes: 0},
{name: "just below floor threshold", freeBytes: 699050665},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
cache, _ := newEvictionTestCache(t, 1<<30)
got, err := cache.computeDefaultMaxBytes(t.Context(),
func(string) (uint64, error) { return tc.freeBytes, nil })
if err != nil {
t.Fatalf("computeDefaultMaxBytes returned error: %v", err)
}
if got != DefaultCacheMaxBytesFloor {
t.Errorf("computeDefaultMaxBytes = %d, want floor %d",
got, DefaultCacheMaxBytesFloor)
}
})
}
}
// TestComputeDefaultMaxBytesPropagatesProbeError verifies that a
// failing free-space probe produces an error naming cache_max_bytes,
// instead of a silently wrong default.
func TestComputeDefaultMaxBytesPropagatesProbeError(t *testing.T) {
t.Parallel()
cache, _ := newEvictionTestCache(t, 1<<30)
_, err := cache.computeDefaultMaxBytes(t.Context(),
func(string) (uint64, error) { return 0, errTestStatfsFailed })
if err == nil {
t.Fatal("probe failure must produce an error, got nil")
}
t.Logf("got expected error: %v", err)
if !strings.Contains(err.Error(), "cache_max_bytes") {
t.Errorf("error %q does not name cache_max_bytes", err.Error())
}
}
// TestNewCacheComputesDefaultMaxBytesWhenAsked verifies that with
// UseDefaultMaxBytes set, the cache's limit becomes the computed
// default, and that the probe is pointed at <state_dir>/cache/, which
// must be created first so statfs measures the right filesystem.
func TestNewCacheComputesDefaultMaxBytesWhenAsked(t *testing.T) {
t.Parallel()
stateDir := t.TempDir()
wantCacheDir := filepath.Join(stateDir, "cache")
var probedPath string
// 4 GiB free -> 3 GiB default.
probe := func(path string) (uint64, error) {
probedPath = path
info, err := os.Stat(path)
if err != nil || !info.IsDir() {
t.Errorf("cache directory %q was not created before probing: info=%v err=%v",
path, info, err)
}
return 4294967296, nil
}
cache, err := newCache(evictionTestDB(t), CacheConfig{
StateDir: stateDir,
UseDefaultMaxBytes: true,
}, probe)
if err != nil {
t.Fatalf("newCache returned error: %v", err)
}
if cache.config.MaxBytes != 3221225472 {
t.Errorf("MaxBytes = %d, want computed default 3221225472",
cache.config.MaxBytes)
}
if probedPath != wantCacheDir {
t.Errorf("free space probed at %q, want cache directory %q",
probedPath, wantCacheDir)
}
}
// TestNewCacheKeepsExplicitMaxBytes verifies that without
// UseDefaultMaxBytes the cache keeps MaxBytes exactly as given and
// never consults the free-space probe.
func TestNewCacheKeepsExplicitMaxBytes(t *testing.T) {
t.Parallel()
probe := func(string) (uint64, error) {
t.Error("free-space probe must not be consulted for explicit values")
return 0, errTestProbeNotExpected
}
cache, err := newCache(evictionTestDB(t), CacheConfig{
StateDir: t.TempDir(),
MaxBytes: 1024,
}, probe)
if err != nil {
t.Fatalf("newCache returned error: %v", err)
}
if cache.config.MaxBytes != 1024 {
t.Errorf("MaxBytes = %d, want explicit 1024 (no floor, no recompute)",
cache.config.MaxBytes)
}
}
+65 -24
View File
@@ -117,7 +117,10 @@ func (c *Cache) EvictToLimit(ctx context.Context) error {
// evictBatch fetches one batch of LRU candidates across variants and // evictBatch fetches one batch of LRU candidates across variants and
// source blobs and evicts them oldest-first until excessBytes are // source blobs and evicts them oldest-first until excessBytes are
// freed or the batch is exhausted. It returns the bytes freed. // freed or the batch is exhausted. It returns the bytes freed. A
// candidate that fails once ctx is cancelled (every one started after
// that fails at its first database call) ends the batch with ctx's
// error, without a warning.
func (c *Cache) evictBatch(ctx context.Context, excessBytes int64) (int64, error) { func (c *Cache) evictBatch(ctx context.Context, excessBytes int64) (int64, error) {
candidates, err := c.evictionCandidates(ctx) candidates, err := c.evictionCandidates(ctx)
if err != nil { if err != nil {
@@ -133,6 +136,10 @@ func (c *Cache) evictBatch(ctx context.Context, excessBytes int64) (int64, error
err := c.evictCandidate(ctx, candidate) err := c.evictCandidate(ctx, candidate)
if err != nil { if err != nil {
if ctx.Err() != nil {
return freed, ctx.Err()
}
c.log.Warn("failed to evict cache entry", c.log.Warn("failed to evict cache entry",
"cache_key", candidate.cacheKey, "cache_key", candidate.cacheKey,
"content_hash", candidate.contentHash, "content_hash", candidate.contentHash,
@@ -283,7 +290,7 @@ func (c *Cache) evictVariant(ctx context.Context, cacheKey VariantKey) error {
c.metaCache.Remove(cacheKey) c.metaCache.Remove(cacheKey)
err = c.variants.DeleteWithMeta(cacheKey) err = c.variants.Delete(cacheKey)
if err != nil { if err != nil {
return err return err
} }
@@ -424,37 +431,44 @@ func (c *Cache) notifyWritePressure() {
// startup and again on every periodic tick thereafter, and evicts to // startup and again on every periodic tick thereafter, and evicts to
// the configured limit on the given periodic interval and on // the configured limit on the given periodic interval and on
// write-pressure notifications. It is a no-op on a disabled cache or // write-pressure notifications. It is a no-op on a disabled cache or
// when already started. // when already started. The goroutine outlives the caller, so it runs
// with its own context, which StopEviction cancels.
func (c *Cache) StartEviction(interval time.Duration) { func (c *Cache) StartEviction(interval time.Duration) {
if c.disabled || c.evictionStarted { if c.disabled || c.evictionCancel != nil {
return return
} }
c.evictionStarted = true ctx, cancel := context.WithCancel(context.Background())
c.evictionCancel = cancel
go c.evictionLoop(interval) go c.evictionLoop(ctx, interval)
} }
// StopEviction stops the background eviction goroutine and waits for // StopEviction cancels the background eviction goroutine, which
// it to exit. It is safe to call when eviction was never started, and // interrupts a pass in progress, and waits for it to exit or for ctx to
// safe to call more than once. // end, whichever comes first. In the second case it returns an error
func (c *Cache) StopEviction() { // wrapping ctx's error. It is safe to call when eviction was never
if !c.evictionStarted { // started, and safe to call more than once.
return func (c *Cache) StopEviction(ctx context.Context) error {
if c.evictionCancel == nil {
return nil
} }
c.evictionStopOnce.Do(func() { c.evictionCancel()
close(c.evictionStop)
<-c.evictionDone select {
}) case <-c.evictionDone:
return nil
case <-ctx.Done():
return fmt.Errorf("cache eviction still running: %w", ctx.Err())
}
} }
// evictionLoop is the body of the background eviction goroutine. // evictionLoop is the body of the background eviction goroutine. It
func (c *Cache) evictionLoop(interval time.Duration) { // returns when ctx is cancelled, and starts no pass after that.
func (c *Cache) evictionLoop(ctx context.Context, interval time.Duration) {
defer close(c.evictionDone) defer close(c.evictionDone)
ctx := context.Background()
c.runReconciliationPass(ctx) c.runReconciliationPass(ctx)
c.runEvictionPass(ctx) c.runEvictionPass(ctx)
@@ -463,7 +477,7 @@ func (c *Cache) evictionLoop(interval time.Duration) {
for { for {
select { select {
case <-c.evictionStop: case <-ctx.Done():
return return
case <-ticker.C: case <-ticker.C:
// Reconciliation walks the cache directories, so it only // Reconciliation walks the cache directories, so it only
@@ -484,8 +498,13 @@ func (c *Cache) evictionLoop(interval time.Duration) {
} }
// runEvictionPass runs one eviction pass, logging failures instead of // runEvictionPass runs one eviction pass, logging failures instead of
// propagating them (the loop must keep running). // propagating them (the loop must keep running). It does nothing once
// ctx is cancelled.
func (c *Cache) runEvictionPass(ctx context.Context) { func (c *Cache) runEvictionPass(ctx context.Context) {
if ctx.Err() != nil {
return
}
err := c.EvictToLimit(ctx) err := c.EvictToLimit(ctx)
if err != nil { if err != nil {
c.log.Warn("cache eviction pass failed", "error", err) c.log.Warn("cache eviction pass failed", "error", err)
@@ -493,8 +512,13 @@ func (c *Cache) runEvictionPass(ctx context.Context) {
} }
// runReconciliationPass runs one reconciliation pass, logging failures // runReconciliationPass runs one reconciliation pass, logging failures
// instead of propagating them (the loop must keep running). // instead of propagating them (the loop must keep running). It does
// nothing once ctx is cancelled.
func (c *Cache) runReconciliationPass(ctx context.Context) { func (c *Cache) runReconciliationPass(ctx context.Context) {
if ctx.Err() != nil {
return
}
err := c.reconcileAccounting(ctx) err := c.reconcileAccounting(ctx)
if err != nil { if err != nil {
c.log.Warn("cache accounting reconciliation failed", "error", err) c.log.Warn("cache accounting reconciliation failed", "error", err)
@@ -511,7 +535,8 @@ func (c *Cache) runReconciliationPass(ctx context.Context) {
// know (and rows whose files are gone), and sweeps stale temp files // know (and rows whose files are gone), and sweeps stale temp files
// left behind by crashed writes. Running it periodically, not just // left behind by crashed writes. Running it periodically, not just
// once, bounds how long such drift can accumulate unaccounted for on a // once, bounds how long such drift can accumulate unaccounted for on a
// long-running process to one eviction interval. // long-running process to one eviction interval. Once ctx is cancelled,
// it stops at the next file or row and returns ctx's error.
func (c *Cache) reconcileAccounting(ctx context.Context) error { func (c *Cache) reconcileAccounting(ctx context.Context) error {
if c.disabled { if c.disabled {
return nil return nil
@@ -546,6 +571,10 @@ func (c *Cache) reconcileVariantFiles(ctx context.Context) error {
return filepath.WalkDir( return filepath.WalkDir(
c.variants.baseDir, c.variants.baseDir,
func(path string, entry fs.DirEntry, err error) error { func(path string, entry fs.DirEntry, err error) error {
if ctx.Err() != nil {
return ctx.Err()
}
if err != nil || entry.IsDir() { if err != nil || entry.IsDir() {
return err return err
} }
@@ -634,6 +663,10 @@ func (c *Cache) reconcileVariantRows(ctx context.Context) error {
} }
for _, key := range keys { for _, key := range keys {
if ctx.Err() != nil {
return ctx.Err()
}
if c.variants.Exists(key) { if c.variants.Exists(key) {
continue continue
} }
@@ -699,6 +732,10 @@ func (c *Cache) reconcileSourceFiles(ctx context.Context) error {
return filepath.WalkDir( return filepath.WalkDir(
c.srcContent.baseDir, c.srcContent.baseDir,
func(path string, entry fs.DirEntry, err error) error { func(path string, entry fs.DirEntry, err error) error {
if ctx.Err() != nil {
return ctx.Err()
}
if err != nil || entry.IsDir() { if err != nil || entry.IsDir() {
return err return err
} }
@@ -760,6 +797,10 @@ func (c *Cache) reconcileSourceRows(ctx context.Context) error {
} }
for _, hash := range hashes { for _, hash := range hashes {
if ctx.Err() != nil {
return ctx.Err()
}
if c.srcContent.Exists(hash) { if c.srcContent.Exists(hash) {
continue continue
} }
+297 -19
View File
@@ -4,9 +4,12 @@ import (
"bytes" "bytes"
"context" "context"
"database/sql" "database/sql"
"errors"
"io/fs" "io/fs"
"log/slog"
"os" "os"
"path/filepath" "path/filepath"
"strings"
"testing" "testing"
"time" "time"
@@ -657,7 +660,7 @@ func TestEvictionRunsUnderWritePressure(t *testing.T) {
// An interval far longer than the test ensures only write // An interval far longer than the test ensures only write
// pressure can trigger eviction here. // pressure can trigger eviction here.
cache.StartEviction(time.Hour) cache.StartEviction(time.Hour)
defer cache.StopEviction() defer func() { _ = cache.StopEviction(t.Context()) }()
keys := []VariantKey{ keys := []VariantKey{
testVariantKeyOne, testVariantKeyTwo, testVariantKeyThree, testVariantKeyOne, testVariantKeyTwo, testVariantKeyThree,
@@ -678,6 +681,11 @@ func TestEvictionRunsUnderWritePressure(t *testing.T) {
assertNoDanglingReferences(t, cache) assertNoDanglingReferences(t, cache)
} }
// TestEvictionRunsOnPeriodicSchedule writes three variant files straight
// to disk, bypassing StoreVariant, so they have no accounting rows and no
// write-pressure notification fires. Only a periodic reconciliation pass
// can then adopt them, and only the eviction pass that follows it can
// evict them.
func TestEvictionRunsOnPeriodicSchedule(t *testing.T) { func TestEvictionRunsOnPeriodicSchedule(t *testing.T) {
t.Parallel() t.Parallel()
@@ -685,12 +693,28 @@ func TestEvictionRunsOnPeriodicSchedule(t *testing.T) {
cache, _ := newEvictionTestCache(t, limit) cache, _ := newEvictionTestCache(t, limit)
// Start the evictor while the cache is empty, then create tracked // Hold the test database's only connection, so the startup pass
// over-limit state WITHOUT going through the store methods, so no // waits for it after walking the still empty variant directory: the
// write-pressure notification fires and only the periodic ticker // files written while it waits are first seen by a periodic pass.
// can trigger eviction. conn, err := cache.db.Conn(t.Context())
if err != nil {
t.Fatalf("failed to take the database connection: %v", err)
}
defer func() { _ = conn.Close() }()
cache.StartEviction(100 * time.Millisecond) cache.StartEviction(100 * time.Millisecond)
defer cache.StopEviction() defer func() { _ = cache.StopEviction(t.Context()) }()
deadline := time.Now().Add(5 * time.Second)
for cache.db.Stats().WaitCount == 0 {
if time.Now().After(deadline) {
t.Fatal("the startup pass never waited for the database")
}
time.Sleep(10 * time.Millisecond)
}
keys := []VariantKey{ keys := []VariantKey{
testVariantKeyOne, testVariantKeyTwo, testVariantKeyThree, testVariantKeyOne, testVariantKeyTwo, testVariantKeyThree,
@@ -700,25 +724,43 @@ func TestEvictionRunsOnPeriodicSchedule(t *testing.T) {
for i, key := range keys { for i, key := range keys {
content := bytes.Repeat([]byte{fills[i]}, 1000) content := bytes.Repeat([]byte{fills[i]}, 1000)
_, err := cache.variants.Store(key, bytes.NewReader(content), "image/webp") _, err = cache.variants.Store(key, bytes.NewReader(content), "image/webp")
if err != nil { if err != nil {
t.Fatalf("failed to store variant file: %v", err) t.Fatalf("failed to store variant file: %v", err)
} }
}
_, err = cache.db.ExecContext(t.Context(), _ = conn.Close()
`INSERT INTO variant_content (cache_key, size_bytes, content_type)
VALUES (?, ?, ?)`, // Only one of the 1000-byte files fits under the limit: wait until
string(key), len(content), "image/webp", // the evictor has removed the other two.
) stored := len(keys)
if err != nil { deadline = time.Now().Add(5 * time.Second)
t.Fatalf("failed to insert variant accounting row: %v", err)
for stored > 1 && time.Now().Before(deadline) {
time.Sleep(25 * time.Millisecond)
stored = 0
for _, key := range keys {
if cache.variants.Exists(key) {
stored++
}
} }
} }
usage := waitForUsageAtOrBelow(t, cache, limit, 5*time.Second) if stored > 1 {
t.Fatalf("periodic schedule did not trigger eviction: %d of %d "+
"variant files still on disk, want at most 1", stored, len(keys))
}
usage, err := cache.UsageBytes(t.Context())
if err != nil {
t.Fatalf("UsageBytes failed: %v", err)
}
if usage > limit { if usage > limit {
t.Errorf("periodic schedule did not trigger eviction: usage = %d, want <= %d", t.Errorf("usage after eviction = %d, want <= %d", usage, limit)
usage, limit)
} }
assertNoDanglingReferences(t, cache) assertNoDanglingReferences(t, cache)
@@ -751,7 +793,7 @@ func TestStartEvictionReconcilesAccountingWithDisk(t *testing.T) {
} }
cache.StartEviction(time.Hour) cache.StartEviction(time.Hour)
defer cache.StopEviction() defer func() { _ = cache.StopEviction(t.Context()) }()
deadline := time.Now().Add(5 * time.Second) deadline := time.Now().Add(5 * time.Second)
@@ -808,7 +850,7 @@ func TestPeriodicReconciliationAdoptsFileThatAppearsAfterStartup(t *testing.T) {
const interval = 100 * time.Millisecond const interval = 100 * time.Millisecond
cache.StartEviction(interval) cache.StartEviction(interval)
defer cache.StopEviction() defer func() { _ = cache.StopEviction(t.Context()) }()
// Let startup reconciliation run and settle on an empty cache // Let startup reconciliation run and settle on an empty cache
// before introducing the untracked file, so the adoption we assert // before introducing the untracked file, so the adoption we assert
@@ -862,6 +904,242 @@ func TestPeriodicReconciliationAdoptsFileThatAppearsAfterStartup(t *testing.T) {
} }
} }
// TestStopEvictionInterruptsPassInProgress holds the test database's
// only connection, so the startup reconciliation pass waits for it, and
// checks that StopEviction stops that pass instead of waiting for the
// connection to come free, and that the stop logs one warning: the
// interrupted reconciliation's, with no eviction pass started after it.
func TestStopEvictionInterruptsPassInProgress(t *testing.T) {
t.Parallel()
cache, _ := newEvictionTestCache(t, 1<<30)
var logBuf bytes.Buffer
cache.log = slog.New(slog.NewJSONHandler(&logBuf, nil))
conn, err := cache.db.Conn(t.Context())
if err != nil {
t.Fatalf("failed to take the database connection: %v", err)
}
defer func() { _ = conn.Close() }()
cache.StartEviction(time.Hour)
// The pass is in progress once it waits for the connection.
deadline := time.Now().Add(5 * time.Second)
for cache.db.Stats().WaitCount == 0 {
if time.Now().After(deadline) {
t.Fatal("the reconciliation pass never waited for the database")
}
time.Sleep(10 * time.Millisecond)
}
ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second)
defer cancel()
err = cache.StopEviction(ctx)
t.Logf("StopEviction() error = %v", err)
if err != nil {
t.Fatalf("StopEviction() error = %v, want nil: the pass waiting for "+
"the database did not stop", err)
}
t.Logf("log output: %s", logBuf.String())
warnings := strings.Count(logBuf.String(), `"level":"WARN"`)
if warnings != 1 {
t.Errorf("the stop logged %d warnings, want 1", warnings)
}
}
// TestEvictToLimitStopsAtNextCandidateOnceCancelled cancels the context
// while the oldest of three source blobs is being evicted, and checks that
// EvictToLimit then returns context.Canceled without evicting the other
// two or logging a warning for either of them.
func TestEvictToLimitStopsAtNextCandidateOnceCancelled(t *testing.T) {
t.Parallel()
cache, _ := newEvictionTestCache(t, 1)
var logBuf bytes.Buffer
cache.log = slog.New(slog.NewJSONHandler(&logBuf, nil))
hashes := []ContentHash{
storeEvictionTestSource(t, cache, "cancel.example.com", "/a.jpg",
bytes.Repeat([]byte{0x61}, 1000)),
storeEvictionTestSource(t, cache, "cancel.example.com", "/b.jpg",
bytes.Repeat([]byte{0x62}, 1000)),
storeEvictionTestSource(t, cache, "cancel.example.com", "/c.jpg",
bytes.Repeat([]byte{0x63}, 1000)),
}
base := time.Now().Add(-time.Hour)
for i, hash := range hashes {
setSourceLastAccessed(t, cache, hash, base.Add(time.Duration(i)*time.Minute))
}
ctx, cancel := context.WithCancel(t.Context())
defer cancel()
cache.evictSourceBlobTestHook = func(ContentHash) { cancel() }
err := cache.EvictToLimit(ctx)
t.Logf("EvictToLimit() error = %v", err)
t.Logf("log output: %s", logBuf.String())
if !errors.Is(err, context.Canceled) {
t.Errorf("EvictToLimit() error = %v, want context.Canceled", err)
}
if cache.srcContent.Exists(hashes[0]) {
t.Errorf("source blob %s, evicted when the context was cancelled, "+
"is still on disk", hashes[0])
}
for _, hash := range hashes[1:] {
if !cache.srcContent.Exists(hash) {
t.Errorf("source blob %s was evicted after the context was cancelled", hash)
}
}
if strings.Contains(logBuf.String(), `"level":"WARN"`) {
t.Errorf("EvictToLimit logged a warning after the context was cancelled")
}
assertNoDanglingReferences(t, cache)
}
// TestStopEvictionReturnsWhenItsContextEnds pauses an eviction pass where
// cancellation cannot reach it, after a source blob's rows are deleted and
// before its file is removed, and checks that StopEviction returns its
// context's error when that context ends instead of waiting for the pass.
// Once the pass goes on, the goroutine exits and no row points at a
// missing file.
func TestStopEvictionReturnsWhenItsContextEnds(t *testing.T) {
t.Parallel()
cache, _ := newEvictionTestCache(t, 1)
paused := make(chan struct{})
resume := make(chan struct{})
cache.evictSourceBlobTestHook = func(ContentHash) {
close(paused)
<-resume
}
hash := storeEvictionTestSource(t, cache, "stop.example.com", "/a.jpg",
bytes.Repeat([]byte{0x61}, 1000))
cache.StartEviction(time.Hour)
select {
case <-paused:
case <-time.After(5 * time.Second):
t.Fatal("the eviction pass never reached the source blob")
}
ctx, cancel := context.WithTimeout(t.Context(), 50*time.Millisecond)
defer cancel()
err := cache.StopEviction(ctx)
t.Logf("StopEviction() error = %v", err)
if !errors.Is(err, context.DeadlineExceeded) {
t.Errorf("StopEviction() error = %v, want context.DeadlineExceeded", err)
}
close(resume)
err = cache.StopEviction(t.Context())
if err != nil {
t.Fatalf("second StopEviction() error = %v, want nil", err)
}
assertNoDanglingReferences(t, cache)
if cache.srcContent.Exists(hash) {
t.Errorf("source blob %s is still on disk after its rows were deleted", hash)
}
}
// TestReconciliationWalksStopOnceCancelled checks that both directory
// walks of a reconciliation pass return the context's error once it is
// cancelled, leaving in place a stale temp file they would otherwise
// remove.
func TestReconciliationWalksStopOnceCancelled(t *testing.T) {
t.Parallel()
cache, _ := newEvictionTestCache(t, 1<<30)
ctx, cancel := context.WithCancel(t.Context())
cancel()
staleTime := time.Now().Add(-2 * staleTempFileAge)
walks := map[string]func(context.Context) error{
cache.variants.baseDir: cache.reconcileVariantFiles,
cache.srcContent.baseDir: cache.reconcileSourceFiles,
}
for dir, walk := range walks {
tempFile := filepath.Join(dir, tempFilePrefix+"stale")
err := os.WriteFile(tempFile, []byte("partial"), 0o600)
if err != nil {
t.Fatalf("failed to write temp file: %v", err)
}
err = os.Chtimes(tempFile, staleTime, staleTime)
if err != nil {
t.Fatalf("failed to backdate temp file: %v", err)
}
err = walk(ctx)
t.Logf("walk of %s: error = %v", dir, err)
if !errors.Is(err, context.Canceled) {
t.Errorf("walk of %s: error = %v, want context.Canceled", dir, err)
}
_, err = os.Stat(tempFile)
if err != nil {
t.Errorf("walk of %s went on after cancellation: %v", dir, err)
}
}
}
// TestReconciliationPassLogsNoWarningOnceCancelled checks that a
// reconciliation pass run with an already cancelled context logs no
// warning, so a periodic tick the loop takes after a stop adds no
// warning to the one from the pass the stop interrupted.
func TestReconciliationPassLogsNoWarningOnceCancelled(t *testing.T) {
t.Parallel()
cache, _ := newEvictionTestCache(t, 1<<30)
var logBuf bytes.Buffer
cache.log = slog.New(slog.NewJSONHandler(&logBuf, nil))
ctx, cancel := context.WithCancel(t.Context())
cancel()
cache.runReconciliationPass(ctx)
t.Logf("log output: %s", logBuf.String())
if strings.Contains(logBuf.String(), `"level":"WARN"`) {
t.Errorf("runReconciliationPass logged a warning with a cancelled context")
}
}
// TestEvictSourceBlobExcludesConcurrentStoreOfIdenticalContent exercises // TestEvictSourceBlobExcludesConcurrentStoreOfIdenticalContent exercises
// the exact TOCTOU window between evictSourceBlob's row-deletion // the exact TOCTOU window between evictSourceBlob's row-deletion
// transaction commit and its content file unlink: a concurrent // transaction commit and its content file unlink: a concurrent
-30
View File
@@ -5,7 +5,6 @@ import (
"context" "context"
"errors" "errors"
"io" "io"
"net/url"
"time" "time"
) )
@@ -154,9 +153,6 @@ type ImageCache interface {
// Warm pre-fetches and caches an image without returning it // Warm pre-fetches and caches an image without returning it
Warm(ctx context.Context, req *ImageRequest) error Warm(ctx context.Context, req *ImageRequest) error
// Purge removes a cached image
Purge(ctx context.Context, req *ImageRequest) error
// Stats returns cache statistics // Stats returns cache statistics
Stats(ctx context.Context) (*CacheStats, error) Stats(ctx context.Context) (*CacheStats, error)
} }
@@ -176,29 +172,3 @@ type CacheStats struct {
// HitRate is HitCount / (HitCount + MissCount) // HitRate is HitCount / (HitCount + MissCount)
HitRate float64 HitRate float64
} }
// SignatureValidator validates request signatures
type SignatureValidator interface {
// Validate checks if the signature is valid for the request
Validate(req *ImageRequest) error
// Generate creates a signature for a request
Generate(req *ImageRequest) string
}
// Allowlist checks if a URL is allowlisted (no signature required)
type Allowlist interface {
// IsAllowlisted returns true if the URL doesn't require a signature
IsAllowlisted(u *url.URL) bool
}
// Storage handles persistent storage of cached content
type Storage interface {
// Store saves content and returns its hash
Store(ctx context.Context, content io.Reader) (hash string, err error)
// Load retrieves content by hash
Load(ctx context.Context, hash string) (io.ReadCloser, error)
// Delete removes content by hash
Delete(ctx context.Context, hash string) error
// Exists checks if content exists
Exists(ctx context.Context, hash string) (bool, error)
}
+12 -9
View File
@@ -13,6 +13,7 @@ import (
"github.com/dustin/go-humanize" "github.com/dustin/go-humanize"
"github.com/getsentry/sentry-go" "github.com/getsentry/sentry-go"
"github.com/go-chi/chi/v5/middleware"
"golang.org/x/sync/singleflight" "golang.org/x/sync/singleflight"
"sneak.berlin/go/pixa/internal/allowlist" "sneak.berlin/go/pixa/internal/allowlist"
"sneak.berlin/go/pixa/internal/httpfetcher" "sneak.berlin/go/pixa/internal/httpfetcher"
@@ -56,11 +57,10 @@ type ServiceConfig struct {
Logger *slog.Logger Logger *slog.Logger
} }
// Static errors for service construction and unimplemented operations. // Static errors for service construction.
var ( var (
errCacheRequired = errors.New("cache is required") errCacheRequired = errors.New("cache is required")
errSigningKeyRequired = errors.New("signing key is required") errSigningKeyRequired = errors.New("signing key is required")
errPurgeNotImplemented = errors.New("purge not implemented")
) )
// NewService creates a new image service. // NewService creates a new image service.
@@ -189,16 +189,17 @@ func (s *Service) Warm(ctx context.Context, req *ImageRequest) error {
return err return err
} }
// Purge removes a cached image. Purging is not implemented yet.
func (s *Service) Purge(_ context.Context, _ *ImageRequest) error {
return errPurgeNotImplemented
}
// Stats returns cache statistics. // Stats returns cache statistics.
func (s *Service) Stats(ctx context.Context) (*CacheStats, error) { func (s *Service) Stats(ctx context.Context) (*CacheStats, error) {
return s.cache.Stats(ctx) return s.cache.Stats(ctx)
} }
// WaitForProcessing waits until no image is being processed, or until ctx
// ends, and returns how many images were still being processed then.
func (s *Service) WaitForProcessing(ctx context.Context) int {
return s.processor.WaitForProcessing(ctx)
}
// ValidateRequest validates the request signature if required. // ValidateRequest validates the request signature if required.
func (s *Service) ValidateRequest(req *ImageRequest) error { func (s *Service) ValidateRequest(req *ImageRequest) error {
// Check if host is allowed (no signature required) // Check if host is allowed (no signature required)
@@ -461,6 +462,7 @@ func (s *Service) fetchAndProcess(
// Log upstream fetch details // Log upstream fetch details
s.log.Info("upstream fetched", s.log.Info("upstream fetched",
"request_id", middleware.GetReqID(ctx),
"host", req.SourceHost, "host", req.SourceHost,
"path", req.SourcePath, "path", req.SourcePath,
"bytes", fetchBytes, "bytes", fetchBytes,
@@ -545,6 +547,7 @@ func (s *Service) processAndStore(
} }
s.log.Info("image converted", s.log.Info("image converted",
"request_id", middleware.GetReqID(ctx),
"host", req.SourceHost, "host", req.SourceHost,
"path", req.SourcePath, "path", req.SourcePath,
"src_format", processResult.InputFormat, "src_format", processResult.InputFormat,
+3 -13
View File
@@ -564,7 +564,8 @@ func (s *VariantStorage) Exists(key VariantKey) bool {
return err == nil return err == nil
} }
// Delete removes content at the given key. // Delete removes the content at the given key together with its .meta
// sidecar file. A missing file is not an error.
func (s *VariantStorage) Delete(key VariantKey) error { func (s *VariantStorage) Delete(key VariantKey) error {
path := s.keyToPath(key) path := s.keyToPath(key)
@@ -573,18 +574,7 @@ func (s *VariantStorage) Delete(key VariantKey) error {
return fmt.Errorf("failed to delete content: %w", err) return fmt.Errorf("failed to delete content: %w", err)
} }
return nil metaPath := path + ".meta"
}
// DeleteWithMeta removes the content at the given key together with
// its .meta sidecar file. A missing file is not an error.
func (s *VariantStorage) DeleteWithMeta(key VariantKey) error {
err := s.Delete(key)
if err != nil {
return err
}
metaPath := s.keyToPath(key) + ".meta"
err = os.Remove(metaPath) err = os.Remove(metaPath)
if err != nil && !os.IsNotExist(err) { if err != nil && !os.IsNotExist(err) {
@@ -438,3 +438,73 @@ func TestVariantStorage_StoreLogsFailedMetaWrite(t *testing.T) {
t.Errorf("log missing %s; got %q", want, logBuf.String()) t.Errorf("log missing %s; got %q", want, logBuf.String())
} }
} }
// storeTestVariant stores one variant, with its .meta file, in a new
// VariantStorage and returns the storage and the variant's key.
func storeTestVariant(t *testing.T) (*VariantStorage, VariantKey) {
t.Helper()
storage, err := NewVariantStorage(t.TempDir(), slog.New(slog.DiscardHandler))
if err != nil {
t.Fatalf("NewVariantStorage() error = %v", err)
}
key := CacheKey(&ImageRequest{SourceHost: testHostCDN, SourcePath: testPathCat})
_, err = storage.Store(key, bytes.NewReader([]byte("variant data")), "image/webp")
if err != nil {
t.Fatalf("Store() error = %v", err)
}
return storage, key
}
// TestVariantStorage_DeleteRemovesMeta verifies that Delete removes the
// variant's .meta file along with the variant file.
func TestVariantStorage_DeleteRemovesMeta(t *testing.T) {
t.Parallel()
storage, key := storeTestVariant(t)
metaPath := storage.keyToPath(key) + ".meta"
_, err := os.Stat(metaPath)
if err != nil {
t.Fatalf("Store() wrote no .meta file: %v", err)
}
err = storage.Delete(key)
if err != nil {
t.Fatalf("Delete() error = %v", err)
}
if storage.Exists(key) {
t.Error("Exists() = true after delete, want false")
}
_, err = os.Stat(metaPath)
if !os.IsNotExist(err) {
t.Errorf(".meta file left after Delete() (stat err=%v)", err)
}
}
// TestVariantStorage_DeleteWithoutMeta verifies that Delete succeeds for a
// variant whose .meta file is missing.
func TestVariantStorage_DeleteWithoutMeta(t *testing.T) {
t.Parallel()
storage, key := storeTestVariant(t)
err := os.Remove(storage.keyToPath(key) + ".meta")
if err != nil {
t.Fatalf("removing .meta file: %v", err)
}
err = storage.Delete(key)
if err != nil {
t.Fatalf("Delete() error = %v, want nil", err)
}
if storage.Exists(key) {
t.Error("Exists() = true after delete, want false")
}
}
+29
View File
@@ -2,9 +2,12 @@
package middleware package middleware
import ( import (
"context"
"crypto/rand"
"log/slog" "log/slog"
"net/http" "net/http"
"net/netip" "net/netip"
"regexp"
"time" "time"
basicauth "github.com/99designs/basicauth-go" basicauth "github.com/99designs/basicauth-go"
@@ -115,6 +118,32 @@ func (s *Middleware) RateLimit(
}) })
} }
// requestIDPattern is what a request's own X-Request-Id must look like to be
// kept as its ID: 1 to 64 letters, digits, '-', '_' or '.'.
var requestIDPattern = regexp.MustCompile(`^[A-Za-z0-9._-]{1,64}$`)
// RequestID returns a middleware that gives each request an ID and sends it as
// the X-Request-Id response header, so a client can quote it when reporting a
// problem. The ID is the request's own X-Request-Id when that matches
// requestIDPattern, and otherwise a random one, which tells nothing about the
// machine or the traffic. It is stored in the request context under chi's
// RequestIDKey, where the logging middleware, the handlers and the upstream
// fetch read it.
func (s *Middleware) RequestID() func(http.Handler) http.Handler {
return func(next http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
id := r.Header.Get(middleware.RequestIDHeader)
if !requestIDPattern.MatchString(id) {
id = rand.Text()
}
w.Header().Set(middleware.RequestIDHeader, id)
ctx := context.WithValue(r.Context(), middleware.RequestIDKey, id)
next.ServeHTTP(w, r.WithContext(ctx))
})
}
}
type loggingResponseWriter struct { type loggingResponseWriter struct {
http.ResponseWriter http.ResponseWriter
@@ -1,11 +1,16 @@
package middleware package middleware
import ( import (
"bytes"
"log/slog" "log/slog"
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"net/url"
"strings"
"testing" "testing"
"github.com/prometheus/client_golang/prometheus/promhttp"
"sneak.berlin/go/pixa/internal/config" "sneak.berlin/go/pixa/internal/config"
) )
@@ -56,6 +61,203 @@ func TestCORSAnswersWithConfiguredOrigin(t *testing.T) {
} }
} }
// TestCORSAnswersPreflightWithConfiguredOrigin checks that the CORS
// middleware answers a preflight request, which the CORS library handles
// apart from other requests, the same way: "*" lets any origin read
// responses and a single origin lets only that origin read them.
func TestCORSAnswersPreflightWithConfiguredOrigin(t *testing.T) {
t.Parallel()
const appOrigin = "https://app.example.com"
cases := []struct {
configured string
requestOrigin string
want string
}{
{"*", "https://any.example.com", "*"},
{appOrigin, appOrigin, appOrigin},
{appOrigin, "https://other.example.com", ""},
}
testHandler := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
})
for _, tc := range cases {
mw := &Middleware{
log: slog.Default(),
config: &config.Config{AccessControlAllowOrigin: tc.configured},
}
handler := mw.CORS()(testHandler)
// An OPTIONS request naming the method it asks about is the
// preflight a browser sends before some cross-origin requests.
req := httptest.NewRequestWithContext(
t.Context(), http.MethodOptions, "/v1/image/example.com/a.jpg/1x1.png", nil)
req.Header.Set("Origin", tc.requestOrigin)
req.Header.Set("Access-Control-Request-Method", http.MethodGet)
rec := httptest.NewRecorder()
handler.ServeHTTP(rec, req)
got := rec.Header().Get("Access-Control-Allow-Origin")
if got != tc.want {
t.Errorf("configured %q, preflight from %q: "+
"Access-Control-Allow-Origin = %q, want %q",
tc.configured, tc.requestOrigin, got, tc.want)
}
}
}
// TestMetricsAuthRequiresConfiguredCredentials checks that MetricsAuth on
// its own answers 401 with a challenge to a request without credentials or
// with a wrong username or password, and lets a request with the configured
// username and password through. That the router puts it in front of
// /metrics is not tested.
func TestMetricsAuthRequiresConfiguredCredentials(t *testing.T) {
t.Parallel()
const (
username = "metricsuser"
password = "metricspass"
challenge = `Basic realm="metrics"`
)
// An empty username stands for a request sent without credentials.
cases := []struct {
name string
username string
password string
wantReached bool
}{
{"no credentials", "", "", false},
{"wrong username", "someone", password, false},
{"wrong password", username, "wrongpass", false},
{"configured credentials", username, password, true},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
mw := &Middleware{
log: slog.Default(),
config: &config.Config{
MetricsUsername: username,
MetricsPassword: password,
},
}
reached := false
handler := mw.MetricsAuth()(http.HandlerFunc(
func(http.ResponseWriter, *http.Request) {
reached = true
}))
req := httptest.NewRequestWithContext(
t.Context(), http.MethodGet, "/metrics", nil)
if tc.username != "" {
req.SetBasicAuth(tc.username, tc.password)
}
rec := httptest.NewRecorder()
handler.ServeHTTP(rec, req)
if reached != tc.wantReached {
t.Fatalf("request reached /metrics = %v, want %v",
reached, tc.wantReached)
}
if tc.wantReached {
return
}
if rec.Code != http.StatusUnauthorized {
t.Errorf("status = %d, want %d",
rec.Code, http.StatusUnauthorized)
}
if got := rec.Header().Get("WWW-Authenticate"); got != challenge {
t.Errorf("WWW-Authenticate = %q, want %q", got, challenge)
}
})
}
}
// TestMetricsRecordsServedRequest checks that the metrics middleware
// records a request it served, so /metrics reports it. It is the only test
// in this package that sets up the metrics middleware, which registers with
// the process-wide Prometheus registry and can do so only once.
func TestMetricsRecordsServedRequest(t *testing.T) {
t.Parallel()
// The line /metrics shows once one GET /test has been served.
const want = `http_request_duration_seconds_count{` +
`code="200",handler="/test",method="GET",service=""} 1`
mw := &Middleware{log: slog.Default(), config: &config.Config{}}
handler := mw.Metrics()(http.HandlerFunc(
func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
}))
handler.ServeHTTP(httptest.NewRecorder(), httptest.NewRequestWithContext(
t.Context(), http.MethodGet, "/test", nil))
rec := httptest.NewRecorder()
promhttp.Handler().ServeHTTP(rec, httptest.NewRequestWithContext(
t.Context(), http.MethodGet, "/metrics", nil))
if !strings.Contains(rec.Body.String(), want) {
t.Errorf("/metrics does not report the GET /test served; "+
"want the line %q in:\n%s", want, rec.Body.String())
}
}
// TestLoggingLeavesOutSubmittedSigningKey checks that a login, a POST /
// whose form carries the signing key, leaves no trace of the key in the
// request's log line.
func TestLoggingLeavesOutSubmittedSigningKey(t *testing.T) {
t.Parallel()
const signingKey = "test-signing-key-0123456789abcdef"
var buf bytes.Buffer
mw := newTestMiddleware(t, &buf)
// The handler reads the key from the form, as the login handler does.
handler := mw.Logging()(http.HandlerFunc(
func(w http.ResponseWriter, r *http.Request) {
if got := r.FormValue("key"); got != signingKey {
t.Errorf("key in form = %q, want %q", got, signingKey)
}
w.WriteHeader(http.StatusSeeOther)
}))
form := url.Values{"key": {signingKey}}
req := httptest.NewRequestWithContext(t.Context(), http.MethodPost, "/",
strings.NewReader(form.Encode()))
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
handler.ServeHTTP(httptest.NewRecorder(), req)
if !strings.Contains(buf.String(), `"method":"POST"`) {
t.Fatalf("no log line for the request; got %q", buf.String())
}
if strings.Contains(buf.String(), signingKey) {
t.Errorf("log output contains the signing key; got %q", buf.String())
}
}
func TestSecurityHeaders(t *testing.T) { func TestSecurityHeaders(t *testing.T) {
t.Parallel() t.Parallel()
@@ -0,0 +1,118 @@
package middleware
import (
"log/slog"
"net/http"
"net/http/httptest"
"os"
"strings"
"testing"
"github.com/go-chi/chi/v5/middleware"
"sneak.berlin/go/pixa/internal/config"
)
// sendRequestID sends a request through the RequestID middleware, carrying
// incoming as its X-Request-Id unless that is empty. It returns the ID the
// next handler found in the request context, which the upstream fetch sends
// and the log lines carry, and the X-Request-Id of the response.
func sendRequestID(t *testing.T, incoming string) (string, string) {
t.Helper()
mw := &Middleware{log: slog.Default(), config: &config.Config{}}
var inContext string
handler := mw.RequestID()(http.HandlerFunc(
func(_ http.ResponseWriter, r *http.Request) {
inContext = middleware.GetReqID(r.Context())
}))
req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
if incoming != "" {
req.Header.Set("X-Request-Id", incoming)
}
rec := httptest.NewRecorder()
handler.ServeHTTP(rec, req)
return inContext, rec.Header().Get("X-Request-Id")
}
// TestRequestIDKeepsShortPlainID verifies that a request's own X-Request-Id of
// at most 64 letters, digits, '-', '_' or '.' is kept as its ID.
func TestRequestIDKeepsShortPlainID(t *testing.T) {
t.Parallel()
for _, incoming := range []string{
"client-request-id",
"A1_b2.c3-d4",
strings.Repeat("a", 64),
} {
inContext, inResponse := sendRequestID(t, incoming)
if inContext != incoming || inResponse != incoming {
t.Errorf("incoming %q: context has %q, response %q, want both %q",
incoming, inContext, inResponse, incoming)
}
}
}
// TestRequestIDReplacesLongOrUnusualID verifies that a request's own
// X-Request-Id that is over 64 characters or holds anything but letters,
// digits, '-', '_' or '.' is neither sent back nor sent upstream: the request
// gets a fresh ID instead.
func TestRequestIDReplacesLongOrUnusualID(t *testing.T) {
t.Parallel()
for _, incoming := range []string{
strings.Repeat("a", 65),
strings.Repeat("a", 9000),
"has space",
"a/b",
"a,b",
"<script>",
"ünicode",
} {
inContext, inResponse := sendRequestID(t, incoming)
t.Logf("incoming %.20q: made up %q", incoming, inResponse)
if inResponse == "" || inResponse == incoming {
t.Errorf("incoming %.20q: response has %q, want a fresh ID",
incoming, inResponse)
}
if inContext != inResponse {
t.Errorf("incoming %.20q: context has %q, want the response's %q",
incoming, inContext, inResponse)
}
}
}
// TestRequestIDMadeUpTellsNothing verifies that the ID made up for a request
// that sent none differs for every request and does not hold the host name.
func TestRequestIDMadeUpTellsNothing(t *testing.T) {
t.Parallel()
hostname, err := os.Hostname()
if err != nil {
t.Fatalf("os.Hostname() error = %v", err)
}
firstInContext, first := sendRequestID(t, "")
_, second := sendRequestID(t, "")
t.Logf("host %q, made up %q and %q", hostname, first, second)
if first == "" || first == second {
t.Errorf("made up %q and %q, want two different IDs", first, second)
}
if firstInContext != first {
t.Errorf("context has %q, want the response's %q", firstInContext, first)
}
if strings.Contains(first, hostname) {
t.Errorf("made-up ID %q holds the host name %q", first, hostname)
}
}
+8 -6
View File
@@ -5,6 +5,8 @@ import (
"fmt" "fmt"
"net/http" "net/http"
"time" "time"
"go.uber.org/fx"
) )
// HTTP server configuration constants. // HTTP server configuration constants.
@@ -36,19 +38,19 @@ func (s *Server) newHTTPServer() *http.Server {
} }
} }
// serveUntilShutdown serves on s.httpServer until it is shut down. When it
// stops for any other reason, such as its port being in use, it asks fx to
// shut down with exit code 1.
func (s *Server) serveUntilShutdown() { func (s *Server) serveUntilShutdown() {
s.httpServer = s.newHTTPServer()
s.SetupRoutes()
s.log.Info("http begin listen", "listenaddr", s.httpServer.Addr) s.log.Info("http begin listen", "listenaddr", s.httpServer.Addr)
err := s.httpServer.ListenAndServe() err := s.httpServer.ListenAndServe()
if err != nil && !errors.Is(err, http.ErrServerClosed) { if err != nil && !errors.Is(err, http.ErrServerClosed) {
s.log.Error("listen error", "error", err) s.log.Error("listen error", "error", err)
if s.cancelFunc != nil { err = s.shutdowner.Shutdown(fx.ExitCode(1))
s.cancelFunc() if err != nil {
s.log.Error("shutdown request failed", "error", err)
} }
} }
} }
+32
View File
@@ -0,0 +1,32 @@
package server
import (
"net/http"
"net/http/httptest"
"strings"
"testing"
"github.com/prometheus/client_golang/prometheus/promhttp"
)
// TestNoMetricsRecordedWithoutMetricsUsername checks that with no metrics
// username set the router records nothing about the requests it serves.
// /metrics is not served then, so the process-wide Prometheus registry is
// read directly. TestMaintenanceModeKeepsOtherRoutes records into the same
// registry, but never a GET /robots.txt.
func TestNoMetricsRecordedWithoutMetricsUsername(t *testing.T) {
t.Parallel()
s := newTestServer(t)
s.ServeHTTP(httptest.NewRecorder(), httptest.NewRequestWithContext(
t.Context(), http.MethodGet, "/robots.txt", nil))
rec := httptest.NewRecorder()
promhttp.Handler().ServeHTTP(rec, httptest.NewRequestWithContext(
t.Context(), http.MethodGet, "/metrics", nil))
if strings.Contains(rec.Body.String(), `handler="/robots.txt"`) {
t.Errorf("with no metrics username GET /robots.txt was recorded:\n%s",
rec.Body.String())
}
}
@@ -0,0 +1,58 @@
package server
import (
"net/http"
"net/http/httptest"
"testing"
)
// requestIDHeader is the header that carries a request's ID.
const requestIDHeader = "X-Request-Id"
// TestResponsesCarryRequestID verifies that every response, whatever its route
// and status, carries the request's ID as X-Request-Id, so a client can quote
// it when reporting a problem: one pixa made up when the request brought none,
// and the request's own X-Request-Id when it brought one.
func TestResponsesCarryRequestID(t *testing.T) {
t.Parallel()
const clientRequestID = "client-request-id"
s := newTestServer(t)
paths := []string{
"/robots.txt",
"/no-such-path",
unsignedImagePath,
encryptedImagePath,
}
for _, path := range paths {
t.Run(path, func(t *testing.T) {
t.Parallel()
rec := httptest.NewRecorder()
s.ServeHTTP(rec, httptest.NewRequestWithContext(
t.Context(), http.MethodGet, path, nil))
t.Logf("status %d, %s %q",
rec.Code, requestIDHeader, rec.Header().Get(requestIDHeader))
if rec.Header().Get(requestIDHeader) == "" {
t.Errorf("response has no %s", requestIDHeader)
}
req := httptest.NewRequestWithContext(
t.Context(), http.MethodGet, path, nil)
req.Header.Set(requestIDHeader, clientRequestID)
rec = httptest.NewRecorder()
s.ServeHTTP(rec, req)
got := rec.Header().Get(requestIDHeader)
if got != clientRequestID {
t.Errorf("%s = %q, want the request's own %q",
requestIDHeader, got, clientRequestID)
}
})
}
}
+2 -1
View File
@@ -28,7 +28,7 @@ func (s *Server) SetupRoutes() {
s.router = chi.NewRouter() s.router = chi.NewRouter()
s.router.Use(middleware.Recoverer) s.router.Use(middleware.Recoverer)
s.router.Use(middleware.RequestID) s.router.Use(s.mw.RequestID())
s.router.Use(s.mw.ClientIP()) s.router.Use(s.mw.ClientIP())
s.router.Use(s.mw.SecurityHeaders()) s.router.Use(s.mw.SecurityHeaders())
s.router.Use(s.mw.Logging()) s.router.Use(s.mw.Logging())
@@ -97,6 +97,7 @@ func (s *Server) SetupRoutes() {
// The trailing filename (e.g., /img.jpg) is ignored but helps // The trailing filename (e.g., /img.jpg) is ignored but helps
// browsers with content type // browsers with content type
r.Get("/e/{token}/*", s.h.HandleImageEnc()) r.Get("/e/{token}/*", s.h.HandleImageEnc())
r.Head("/e/{token}/*", s.h.HandleImageEnc())
}) })
}) })
+27
View File
@@ -0,0 +1,27 @@
package server
import (
"net/http"
"net/http/httptest"
"testing"
)
// TestEncryptedImageRouteAnswersHEAD verifies that HEAD on the encrypted image
// route reaches its handler, as GET does, instead of being answered 405 Method
// Not Allowed. The handler refuses a token it cannot decrypt with 400, so that
// status shows the request got through.
func TestEncryptedImageRouteAnswersHEAD(t *testing.T) {
t.Parallel()
s := newTestServer(t)
rec := httptest.NewRecorder()
s.ServeHTTP(rec, httptest.NewRequestWithContext(
t.Context(), http.MethodHead, encryptedImagePath, nil))
t.Logf("status %d", rec.Code)
if rec.Code != http.StatusBadRequest {
t.Errorf("status = %d, want %d from the encrypted image handler",
rec.Code, http.StatusBadRequest)
}
}
+52 -63
View File
@@ -3,12 +3,10 @@ package server
import ( import (
"context" "context"
"errors"
"fmt" "fmt"
"log/slog" "log/slog"
"net/http" "net/http"
"os"
"os/signal"
"syscall"
"time" "time"
"github.com/getsentry/sentry-go" "github.com/getsentry/sentry-go"
@@ -27,6 +25,10 @@ const (
SentryFlushTimeout = 2 * time.Second SentryFlushTimeout = 2 * time.Second
) )
// errStillProcessing is returned by the server's stop hook when images are
// still being processed once ShutdownTimeout has passed.
var errStillProcessing = errors.New("images still being processed at shutdown")
// Params defines dependencies for Server. // Params defines dependencies for Server.
type Params struct { type Params struct {
fx.In fx.In
@@ -36,6 +38,7 @@ type Params struct {
Config *config.Config Config *config.Config
Middleware *middleware.Middleware Middleware *middleware.Middleware
Handlers *handlers.Handlers Handlers *handlers.Handlers
Shutdowner fx.Shutdowner
} }
// Server is the main HTTP server. // Server is the main HTTP server.
@@ -45,59 +48,58 @@ type Server struct {
globals *globals.Globals globals *globals.Globals
mw *middleware.Middleware mw *middleware.Middleware
h *handlers.Handlers h *handlers.Handlers
shutdowner fx.Shutdowner
startupTime time.Time startupTime time.Time
exitCode int
sentryEnabled bool sentryEnabled bool
cancelFunc context.CancelFunc
httpServer *http.Server httpServer *http.Server
router *chi.Mux router *chi.Mux
} }
// New creates a new Server instance. // New creates a new Server instance. Its start hook starts Sentry and the
// HTTP server; its stop hook, which fx runs on SIGINT, SIGTERM or a
// shutdown request, shuts them down.
func New(lc fx.Lifecycle, params Params) (*Server, error) { func New(lc fx.Lifecycle, params Params) (*Server, error) {
s := &Server{ s := &Server{
log: params.Logger.Get(), log: params.Logger.Get(),
config: params.Config, config: params.Config,
globals: params.Globals, globals: params.Globals,
mw: params.Middleware, mw: params.Middleware,
h: params.Handlers, h: params.Handlers,
shutdowner: params.Shutdowner,
} }
lc.Append(fx.Hook{ lc.Append(fx.Hook{
OnStart: func(ctx context.Context) error { OnStart: func(_ context.Context) error {
s.startupTime = time.Now() s.startupTime = time.Now()
go s.Run(context.WithoutCancel(ctx))
return nil err := s.enableSentry()
}, if err != nil {
OnStop: func(_ context.Context) error { return err
if s.cancelFunc != nil {
s.cancelFunc()
} }
s.SetupRoutes()
s.httpServer = s.newHTTPServer()
go s.serveUntilShutdown()
return nil return nil
}, },
OnStop: s.cleanShutdown,
}) })
return s, nil return s, nil
} }
// Run starts the server.
func (s *Server) Run(ctx context.Context) {
s.enableSentry()
s.serve(ctx)
}
// MaintenanceMode returns whether maintenance mode is enabled. // MaintenanceMode returns whether maintenance mode is enabled.
func (s *Server) MaintenanceMode() bool { func (s *Server) MaintenanceMode() bool {
return s.config.MaintenanceMode return s.config.MaintenanceMode
} }
func (s *Server) enableSentry() { func (s *Server) enableSentry() error {
s.sentryEnabled = false s.sentryEnabled = false
if s.config.SentryDSN == "" { if s.config.SentryDSN == "" {
return return nil
} }
err := sentry.Init(sentry.ClientOptions{ err := sentry.Init(sentry.ClientOptions{
@@ -105,55 +107,42 @@ func (s *Server) enableSentry() {
Release: fmt.Sprintf("%s-%s", s.globals.Appname, s.globals.Version), Release: fmt.Sprintf("%s-%s", s.globals.Appname, s.globals.Version),
}) })
if err != nil { if err != nil {
s.log.Error("sentry init failure", "error", err) return fmt.Errorf("sentry init failure: %w", err)
os.Exit(1)
} }
s.log.Info("sentry error reporting activated") s.log.Info("sentry error reporting activated")
s.sentryEnabled = true s.sentryEnabled = true
return nil
} }
func (s *Server) serve(ctx context.Context) int { // cleanShutdown stops the HTTP server, waits for the images still being
ctx, cancelFunc := context.WithCancel(ctx) // processed, then flushes Sentry. The first two share ShutdownTimeout. It
s.cancelFunc = cancelFunc // returns errStillProcessing when images are still being processed after
// that, as their work is abandoned.
func (s *Server) cleanShutdown(ctx context.Context) error {
s.log.Info("shutting down")
go func() { ctxShutdown, shutdownCancel := context.WithTimeout(ctx, ShutdownTimeout)
c := make(chan os.Signal, 1)
signal.Ignore(syscall.SIGPIPE)
signal.Notify(c, os.Interrupt, syscall.SIGTERM)
sig := <-c
s.log.Info("signal received", "signal", sig)
if s.cancelFunc != nil {
s.cancelFunc()
}
}()
go s.serveUntilShutdown()
<-ctx.Done()
s.cleanShutdown(ctx)
return s.exitCode
}
func (s *Server) cleanShutdown(ctx context.Context) {
s.exitCode = 0
ctxShutdown, shutdownCancel := context.WithTimeout(
context.WithoutCancel(ctx), ShutdownTimeout)
defer shutdownCancel() defer shutdownCancel()
if s.httpServer != nil { err := s.httpServer.Shutdown(ctxShutdown)
err := s.httpServer.Shutdown(ctxShutdown) if err != nil {
if err != nil { s.log.Error("server clean shutdown failed", "error", err)
s.log.Error("server clean shutdown failed", "error", err)
}
} }
stillProcessing := s.h.WaitForProcessing(ctxShutdown)
if s.sentryEnabled { if s.sentryEnabled {
sentry.Flush(SentryFlushTimeout) sentry.Flush(SentryFlushTimeout)
} }
if stillProcessing > 0 {
s.log.Error("images still being processed at shutdown",
"count", stillProcessing)
return errStillProcessing
}
return nil
} }
+96
View File
@@ -0,0 +1,96 @@
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")
}
}