6 Commits
Author SHA1 Message Date
clawbot 989da78681 Keep WebP, AVIF and JPEG XL encodes at the limits under 40 seconds (closes #272)
check / check (push) Waiting to run
WebP is saved at effort 2 and JPEG XL at effort 4, and an AVIF output of
more than 16,777,216 pixels is refused with 400 before anything is
decoded, as AVIF is already at the lowest effort govips can set. With
one libvips thread, at any quality, 8192x8192 random pixels with an
alpha channel take up to about 23 seconds as WebP and 27 as JPEG XL, and
4096x4096, 8192x2048 or 2048x8192 up to about 34 as AVIF. Resizing with
fit=cover already decodes the input, so the output's dimensions are now
worked out before resizing, in outputDimensions. Reading the input moves
into readInput to keep Process within the linter's limits.
TestImageProcessor_WebPAtDefaultEffort becomes TestImageProcessor_WebPAtEffort2, as #280 (comment) allows.

Model: opus-5-5
2026-10-20 15:19:07 +02:00
clawbot 4fb6350aa1 Answer 415 for a source whose first bytes do not match its Content-Type (closes #253)
check / check (push) Canceled after 0s
A body whose first bytes are not those of the format its Content-Type
names answered 500 on both image routes, was logged as pixa's own
error, and was fetched again on every request. Both routes now answer
415 for each error the first-bytes check returns, and the image service
logs the refused source at warning level. The failure also gets a
negative cache entry, as an upstream error status does, so the source
URL is not fetched again for 5 minutes, and meanwhile an image that
would need that fetch answers 502. README.md says so.

The case of TestService_Get_ConcurrentMissesShareAFailure that expected
such a source to be fetched again now expects the negative cache, as
#280 (comment) allows.

Model: opus-5-5
2026-10-20 14:40:51 +02:00
clawbot cde9c0d10d Let a bug fix change the test that pins the behavior the bug calls wrong (closes #280)
check / check (push) Canceled after 0s
AGENTS.md required the owner's approval for every change to an existing
test. A bug fix may now change an existing test without asking when that
test pins exactly the behavior the filed bug calls wrong, and the change
moves it to the fixed behavior and nothing else. Any other change to an
existing test still needs approval. The owner approved this on the issue.

Model: opus-5-5
2026-10-20 14:25:07 +02:00
clawbot 957ec85f97 Re-vendor the canonical files from sneak/prompts at 0653579 (closes #221)
check / check (push) Canceled after 0s
REPO_POLICIES.md, script/cibuild, script/docker, script/lint and
script/test are their copies at that commit. In every Dockerfile stage
the system package install and the Go module download now come before
ARG CACHEBUST, so check builds keep those layers until the base image,
the install line, go.mod or go.sum change; every step that lints, tests
or compiles still reruns. golangci-lint run also mounts golangci-lint's
cache. The stages install their packages with apt-get and apk directly
instead of through script/bootstrap --cgo. The test phase installs no
recommended packages and names libheif's AVIF encoder,
libheif-plugin-aomenc, which the tests use.

Model: opus-5-5
2026-10-19 06:11:43 +02:00
clawbot 930e43961e Read a trusted_proxies entry in the IPv4-mapped form as its IPv4 range (closes #303)
check / check (push) Canceled after 0s
An entry such as ::ffff:10.0.0.0/104 in trusted_proxies was accepted
but trusted nothing, because the client address is matched as the IPv4
address an IPv4-mapped address holds. The conversion that blocked_networks
gained in #301 now lives in
parseCIDRList, which reads both settings: such an entry is read as the
IPv4 range it covers, and one shorter than /96 stops startup with an
error naming the setting being read and quoting the entry as written.
README.md says so for trusted_proxies.

Model: opus-5-5
2026-10-19 05:29:05 +02:00
clawbot 9eacfa6406 Space reconciliation passes by ten times the last pass's time (closes #250)
check / check (push) Canceled after 0s
The reconciliation pass reads a row and stats a file for every cached
file, and it ran on every eviction tick, so on a large cache a pass
longer than the tick was followed by the next one at once. It still runs
at startup and is considered on each tick, but it now starts no sooner
than ten times as long after the last pass ended as that pass took, so
it takes at most about a tenth of the time however large the cache.
Eviction and the removal of expired negative cache rows still run on
every tick. The comments that bound the drift, and README.md, give the
new bound. No existing test changed.

Model: opus-5-5
2026-10-19 05:00:48 +02:00
21 changed files with 901 additions and 239 deletions
+5 -1
View File
@@ -14,7 +14,11 @@ These rules MUST be followed at all times, it is very important.
without asking first. In almost all cases, the code should be changed, NOT the without asking first. In almost all cases, the code should be changed, NOT the
tests. If you think the test needs to be changed, make your case for that and tests. If you think the test needs to be changed, make your case for that and
ask for permission to proceed, then stop. You need explicit user approval to ask for permission to proceed, then stop. You need explicit user approval to
modify existing tests. (You do not need user approval for writing NEW tests.) modify existing tests. A bug fix may change an existing test without asking
when that test pins exactly the behavior the filed bug calls wrong, and the
change moves it to the fixed behavior and nothing else; any other change to an
existing test still needs explicit user approval. (You do not need user
approval for writing NEW tests.)
- When linting, assume the linter config is CORRECT, and that each item output - When linting, assume the linter config is CORRECT, and that each item output
by the linter is something that legitimately needs fixing in the code. by the linter is something that legitimately needs fixing in the code.
+31 -22
View File
@@ -1,12 +1,16 @@
# Every stage declares ARG CACHEBUST first: script/ passes a new value # In every stage ARG CACHEBUST comes right after the steps that install
# on every build, which reruns every RUN step after it. The steps that # system packages and download Go's modules, or right after FROM in a
# compile mount Go's build cache, which survives from build to build. # stage with neither, and every step that lints, tests or compiles
# comes after it. script/ passes a new value on every build, which
# reruns every RUN step after it. Docker keeps the layers above it
# until the base image, their own lines or go.mod and go.sum change.
# The steps that compile mount Go's build cache, and golangci-lint run
# also mounts golangci-lint's; both survive from build to build.
# Lint phase. script/lint builds it alone. The linter is run directly: # Lint phase. script/lint builds it alone. The linter is run directly:
# `make lint` and script/lint are themselves a docker build. # `make lint` and script/lint are themselves a docker build.
# golangci/golangci-lint:v2.14.0, 2026-10-15 # golangci/golangci-lint:v2.14.0, 2026-10-15
FROM golangci/golangci-lint@sha256:ad862ba6b3798cbe0fd9fd7408d498fd74fbd2623a92406b2fd3898faf0bf98f AS lint FROM golangci/golangci-lint@sha256:ad862ba6b3798cbe0fd9fd7408d498fd74fbd2623a92406b2fd3898faf0bf98f AS lint
ARG CACHEBUST
# The linter compiles every package, and govips needs the libvips # The linter compiles every package, and govips needs the libvips
# headers for that. REPO_POLICIES.md has the lint phase install them # headers for that. REPO_POLICIES.md has the lint phase install them
@@ -18,25 +22,29 @@ RUN apt-get update \
WORKDIR /src WORKDIR /src
COPY go.mod go.sum ./ COPY go.mod go.sum ./
RUN go mod download RUN go mod download
ARG CACHEBUST
COPY . . COPY . .
RUN --mount=type=cache,target=/root/.cache/go-build \ RUN --mount=type=cache,target=/root/.cache/go-build \
--mount=type=cache,target=/root/.cache/golangci-lint \
golangci-lint run --config .golangci.yml ./... golangci-lint run --config .golangci.yml ./...
# Test phase. script/test builds it alone. -race needs cgo and so a C # Test phase. script/test builds it alone. -race needs cgo and so a C
# compiler, which the Debian Go image ships and the alpine one does not. # compiler, which the Debian Go image ships and the alpine one does not.
# golang:1.25.4-trixie, 2026-10-15 # golang:1.25.4-trixie, 2026-10-15
FROM golang@sha256:a02d35efc036053fdf0da8c15919276bf777a80cbfda6a35c5e9f087e652adfc AS test FROM golang@sha256:a02d35efc036053fdf0da8c15919276bf777a80cbfda6a35c5e9f087e652adfc AS test
ARG CACHEBUST
# govips compiles against the libvips and libheif headers, and the tests
# need libvips' JPEG XL support; libvips-dev brings all three. The tests
# also encode AVIF, which needs libheif's AVIF encoder,
# libheif-plugin-aomenc: Debian only recommends it, so it is named here.
RUN apt-get update \
&& apt-get install -y --no-install-recommends libvips-dev libheif-plugin-aomenc \
&& rm -rf /var/lib/apt/lists/*
WORKDIR /src WORKDIR /src
# script/bootstrap --cgo installs the build dependencies (the libvips
# and libheif headers, and libvips' JPEG XL support, which the tests
# need) and downloads the Go modules.
COPY script/ ./script/
COPY go.mod go.sum ./ COPY go.mod go.sum ./
RUN script/bootstrap --cgo RUN go mod download
ARG CACHEBUST
COPY . . COPY . .
# Without -v first; on a failure, again with -v for the details, and # Without -v first; on a failure, again with -v for the details, and
@@ -52,22 +60,23 @@ RUN --mount=type=cache,target=/root/.cache/go-build \
go test -count=1 -timeout 90s -race -parallel 4 -v ./...; exit 1; } go test -count=1 -timeout 90s -race -parallel 4 -v ./...; exit 1; }
# Build stage. Nothing is wanted from the two phases above: these copies # Build stage. Nothing is wanted from the two phases above: these copies
# make BuildKit build them first, so this stage runs only when lint and # make BuildKit build them first, so nothing after them runs unless lint
# test passed. # and test passed.
# golang:1.25.4-alpine3.22, 2026-02-25; the runtime stage uses Alpine 3.22 too # golang:1.25.4-alpine3.22, 2026-02-25; the runtime stage uses Alpine 3.22 too
FROM golang:1.25.4-alpine3.22@sha256:d3f0cf7723f3429e3f9ed846243970b20a2de7bae6a5b66fc5914e228d831bbb AS builder FROM golang:1.25.4-alpine3.22@sha256:d3f0cf7723f3429e3f9ed846243970b20a2de7bae6a5b66fc5914e228d831bbb AS builder
# git for the version, and for govips a C compiler, pkg-config and the
# libvips and libheif headers
RUN apk add --no-cache git build-base pkgconfig vips-dev libheif-dev
WORKDIR /src
COPY go.mod go.sum ./
RUN go mod download
ARG CACHEBUST ARG CACHEBUST
COPY --from=lint /src/go.sum /dev/null COPY --from=lint /src/go.sum /dev/null
COPY --from=test /src/go.sum /dev/null COPY --from=test /src/go.sum /dev/null
WORKDIR /src
# Build dependencies, git among them, and Go modules, as in the test
# phase
COPY script/ ./script/
COPY go.mod go.sum ./
RUN script/bootstrap --cgo
# A tar-stream context keeps the sender's file owners, which git refuses. # A tar-stream context keeps the sender's file owners, which git refuses.
RUN git config --system --add safe.directory /src RUN git config --system --add safe.directory /src
@@ -102,7 +111,6 @@ RUN --mount=type=cache,target=/root/.cache/go-build \
# the libvips and musl it was built with. # the libvips and musl it was built with.
# alpine:3.22, 2026-10-08 # alpine:3.22, 2026-10-08
FROM alpine:3.22@sha256:5291449c3df73caf6ed85e649dec1b9e818b39a5d8c871e97afc13e9cd5e8fa8 FROM alpine:3.22@sha256:5291449c3df73caf6ed85e649dec1b9e818b39a5d8c871e97afc13e9cd5e8fa8
ARG CACHEBUST
# Install runtime dependencies only. vips-jxl is libvips' JPEG XL # Install runtime dependencies only. vips-jxl is libvips' JPEG XL
# support, without which pixad does not start. # support, without which pixad does not start.
@@ -113,6 +121,7 @@ RUN apk add --no-cache \
ca-certificates \ ca-certificates \
tzdata \ tzdata \
su-exec su-exec
ARG CACHEBUST
# Copy binary from builder # Copy binary from builder
COPY --from=builder /pixad /usr/local/bin/pixad COPY --from=builder /pixad /usr/local/bin/pixad
+63 -36
View File
@@ -102,8 +102,8 @@ it uses libvips through CGO. pixad does not start unless libvips has its JPEG XL
support, which on Alpine is the `vips-jxl` package and which the nix and brew support, which on Alpine is the `vips-jxl` package and which the nix and brew
packages of libvips include, as do the apt ones from Debian 12 and Ubuntu 24.04 packages of libvips include, as do the apt ones from Debian 12 and Ubuntu 24.04
on. Building pixa also needs the development files of libvips and libheif, on. Building pixa also needs the development files of libvips and libheif,
`pkg-config` and a C compiler. `script/bootstrap --cgo` installs all of these, `pkg-config` and a C compiler. `script/bootstrap --cgo` installs all of these;
as the `Dockerfile` does where it compiles pixa. Plain `script/bootstrap`, which the `Dockerfile` installs its own packages. Plain `script/bootstrap`, which
`script/setup` and `script/cibuild` run, installs git, make and Go, and Node, `script/setup` and `script/cibuild` run, installs git, make and Go, and Node,
Yarn and the prettier pinned in `yarn.lock` for formatting the markdown, but Yarn and the prettier pinned in `yarn.lock` for formatting the markdown, but
none of the C libraries: the checks compile pixa in Docker. Docker itself must none of the C libraries: the checks compile pixa in Docker. Docker itself must
@@ -199,7 +199,11 @@ into place, so a crash leaves no file under its own name with part of its data.
When pixa starts, and every five minutes after, it removes each cached source or When pixa starts, and every five minutes after, it removes each cached source or
transformed image whose file is empty, or not the size the database records for transformed image whose file is empty, or not the size the database records for
it, with a warning naming the file; the next request for it fetches the source it, with a warning naming the file; the next request for it fetches the source
or converts the image again. or converts the image again. Looking for those files reads the database once for
every cached file, so pixa skips each five-minute look that would start less
than ten times as long after the last look ended as that look took. After a look
that took one minute, the next one starts 10 to 15 minutes after that look
ended.
pixa's target is 1-5k r/s, which has not been measured at that rate (see Load pixa's target is 1-5k r/s, which has not been measured at that rate (see Load
Test). Toward it, pixa keeps in memory the content types of the 10,000 Test). Toward it, pixa keeps in memory the content types of the 10,000
@@ -243,9 +247,11 @@ path under `/v1/` answers 200, in maintenance mode too.
redirects to, is `localhost`, ends in `.localhost` or `.local`, or has an redirects to, is `localhost`, ends in `.localhost` or `.local`, or has an
address in a blocked network (see `blocked_networks`); 429 over address in a blocked network (see `blocked_networks`); 429 over
`image_requests_per_minute` (below); 415 when the upstream answers with a `image_requests_per_minute` (below); 415 when the upstream answers with a
`Content-Type` that is not one of the source formats (below), or with an image `Content-Type` that is not one of the source formats (below), with a body
whose first bytes libvips reads as another format; 502 when the upstream whose first bytes are not those of the format its `Content-Type` names, or
answered with an error status, and for 5 minutes after that for an image of with an image whose first bytes libvips reads as another format; 502 when the
upstream answered with an error status, and for 5 minutes after that, or after
a body whose first bytes are not those of its `Content-Type`, for an image of
the same source URL that pixa can neither serve from its cache nor make from the same source URL that pixa can neither serve from its cache nor make from
the source image still in its cache; 503 when pixa is busy or in maintenance the source image still in its cache; 503 when pixa is busy or in maintenance
mode; 500 for any other failure. mode; 500 for any other failure.
@@ -351,11 +357,22 @@ either side may be; so is a size with `fit=outside`, which scales the image to
cover the size and so past it on one side, when that side would come out cover the size and so past it on one side, when that side would come out
above 8192. above 8192.
An AVIF image of more than 16,777,216 pixels, the number 4096x4096 and 8192x2048
have, is refused with 400 before anything is decoded, as its encode could take
more than 40 seconds (see below). That includes `orig` of an AVIF source, and
`auto` when it chooses AVIF, so a client whose `Accept` names `image/avif` but
not `image/jxl` is refused such a size.
The source image may be JPEG, PNG, GIF, WebP, AVIF or JPEG XL. The upstream's The source image may be JPEG, PNG, GIF, WebP, AVIF or JPEG XL. The upstream's
`Content-Type` must name its format, and the image's first bytes must match it. `Content-Type` must name its format, and the image's first bytes must match it.
An upstream answering with any other `Content-Type`, SVG included, is refused An upstream answering with any other `Content-Type`, SVG included, is refused
with 415, and so is an image whose first bytes libvips reads as another format, with 415. So is a body whose first bytes are not those of the format its
such as an SVG whose first bytes pass the check for AVIF. `Content-Type` names, such as a JPEG sent as `image/png`, text sent as an image,
or an empty body. pixa then does not fetch that source URL again for 5 minutes,
as after an upstream error status, and meanwhile answers 502 for an image that
would need that fetch. An image whose first bytes libvips reads as another
format, such as an SVG whose first bytes pass the check for AVIF, is refused
with 415 too.
With the format `auto`, pixa chooses the format for each request from its With the format `auto`, pixa chooses the format for each request from its
`Accept` header, in this order: `Accept` header, in this order:
@@ -400,14 +417,18 @@ transcode that stops this way is answered 503 with the error
ended. govips gives pixa no way to stop libvips once an encode has begun, so an ended. govips gives pixa no way to stop libvips once an encode has begun, so an
encode keeps its slot until it finishes, which can be after encode keeps its slot until it finishes, which can be after
`downstream_timeout`. The limit of 8192 on each side of the source and of the `downstream_timeout`. The limit of 8192 on each side of the source and of the
output bounds how long that takes. With one libvips thread, as pixad runs it, output, and for AVIF the limit of 16,777,216 pixels, bound how long that takes.
encoding 8192x8192 random pixels takes up to about 1 second as JPEG, 15 as PNG, With one libvips thread, as pixad runs it, encoding 8192x8192 random pixels
16 as GIF, 63 as WebP, 110 as AVIF and 96 as JPEG XL, the slowest of 8 or 16 takes up to about 1 second as JPEG, 15 as PNG and 16 as GIF, the slowest of 8 or
bits per sample, with or without an alpha channel. WebP, AVIF and JPEG XL are 16 bits per sample, with or without an alpha channel. The WebP and JPEG XL
that slow only with an alpha channel: 8-bit RGB takes about 44, 53 and 28 encoders run below libvips' default effort, and AVIF at the lowest effort govips
seconds. Decoding such a source adds up to about 11 seconds. A WebP, AVIF or can set, so that no encode takes more than 40 seconds of the default
JPEG XL encode can therefore hold its slot past the default `downstream_timeout` `downstream_timeout` of 60: at any quality and 8 or 16 bits per sample,
of 60 seconds. 8192x8192 random pixels with an alpha channel take up to about 23 seconds as
WebP and 27 as JPEG XL, and 4096x4096, 8192x2048 or 2048x8192 up to about 34 as
AVIF, which at quality 85 would take about 96 at 8192x8192. Without an alpha
channel they take up to about 16, 13 and 22 seconds. Decoding such a source adds
up to about 11 seconds.
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
@@ -459,13 +480,13 @@ nor change what it asks for.
sent with an empty format, or none, also makes a JPEG XL URL. Width and sent with an empty format, or none, also makes a JPEG XL URL. Width and
height both empty or `0` keep the original size; if only one of them is empty 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. A URL whose or `0`, that side is scaled to keep the image's proportions. A URL whose
scaled side would come out above 8192 answers 400, and so does a source URL scaled side would come out above 8192 answers 400, and so does an AVIF URL of
whose path holds a `..` segment. So does a source URL that the image URL more than 16,777,216 pixels or a source URL whose path holds a `..` segment.
would fetch written another way, and the page then shows the URL pixa would So does a source URL that the image URL would fetch written another way, and
fetch: one with a user name and password, which pixa does not send, or one the page then shows the URL pixa would fetch: one with a user name and
whose path holds `%2F`, fetched as `/`, or `(`, fetched as `%28`, as pixa password, which pixa does not send, or one whose path holds `%2F`, fetched as
keeps the source path decoded and fetches it percent-encoded again (see `/`, or `(`, fetched as `%28`, as pixa keeps the source path decoded and
Routes). fetches it percent-encoded again (see Routes).
3. The page shows the URL, `https://<host>/v1/e/<token>/img.<format>`, and when 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 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 with `http` instead while `debug` is on. The name after the token is ignored
@@ -833,19 +854,25 @@ provide:
`script/precommit` `script/precommit`
Every `docker build` in these scripts passes a new value of the `CACHEBUST` Every `docker build` in these scripts passes a new value of the `CACHEBUST`
build argument, which every stage of the `Dockerfile` declares first, so every build argument, which every stage of the `Dockerfile` declares, so every `RUN`
`RUN` step, the lint and test phases included, runs on every build instead of step after that line, the lint and test steps included, runs on every build
coming from the build cache. Go's build cache is kept from build to build: the instead of coming from the build cache. Above the line are the steps that
steps that compile mount it, so a package whose inputs are unchanged is not install system packages and download the Go modules; Docker keeps their layers
compiled again, and both `go test` runs pass `-count=1` so that no stored test until the base image, the step itself, `go.mod` or `go.sum` changes. Go's build
result is replayed. The `test` phase runs on the Debian Go image, with Debian's cache is kept from build to build: the steps that compile mount it, so a package
libvips and libheif; the image pixa ships is built and runs on Alpine. whose inputs are unchanged is not compiled again, and both `go test` runs pass
`script/check`, `script/cibuild`, `script/docker`, `script/lint`, `script/test`, `-count=1` so that no stored test result is replayed. The lint step also mounts
`script/setup` and `script/install-precommit` are the standard copies from golangci-lint's cache, which reuses a package's findings only when its files,
`sneak/prompts`, kept identical to them. `script/fmt` and `script/fmt-check` are those of the packages it imports, `go.mod`, the linters and their settings in
the standard copies with pixa's `gofmt` step kept before prettier. prettier `.golangci.yml`, and the golangci-lint version are all unchanged. The `test`
formats the markdown only, not the HTML templates, as it cannot parse a Go phase runs on the Debian Go image, with Debian's libvips and libheif; the image
template action inside a tag. pixa ships is built and runs on Alpine. `script/check`, `script/cibuild`,
`script/docker`, `script/lint`, `script/test`, `script/setup` and
`script/install-precommit` are the standard copies from `sneak/prompts`, kept
identical to them. `script/fmt` and `script/fmt-check` are the standard copies
with pixa's `gofmt` step kept before prettier. prettier formats the markdown
only, not the HTML templates, as it cannot parse a Go template action inside a
tag.
## Load Test ## Load Test
+68 -35
View File
@@ -1,6 +1,6 @@
--- ---
title: Repository Policies title: Repository Policies
last_modified: 2026-10-15 last_modified: 2026-10-18
--- ---
This document covers repository structure, tooling, and workflow standards. Code This document covers repository structure, tooling, and workflow standards. Code
@@ -159,8 +159,11 @@ style conventions are in separate documents:
one's findings, and a host-global lock in `$TMPDIR` makes concurrent runs one's findings, and a host-global lock in `$TMPDIR` makes concurrent runs
exit non-zero with `parallel golangci-lint is running` — a status a caller exit non-zero with `parallel golangci-lint is running` — a status a caller
cannot tell from real findings. Both have produced wrong verdicts in this cannot tell from real findings. Both have produced wrong verdicts in this
org, in both directions. A container has its own golangci-lint cache, its org, in both directions. A container has its own `TMPDIR`, so the lock is
own `TMPDIR` and a digest-pinned binary, so neither is reachable. out of reach. The lint phase does keep golangci-lint's cache between check
builds (below), but it lints every checkout at `/src`, and a finding is
taken from that cache only for the same files, settings and golangci-lint
version.
- **Any build that runs checks is given a new `CACHEBUST` value.** Docker - **Any build that runs checks is given a new `CACHEBUST` value.** Docker
invalidates a `COPY` layer only when the copied content changes, so on an invalidates a `COPY` layer only when the copied content changes, so on an
@@ -170,15 +173,21 @@ style conventions are in separate documents:
id, so no two builds share a value, not even the three in one id, so no two builds share a value, not even the three in one
`script/cibuild`. `script/lint`, `script/test`, `script/cibuild` and `script/cibuild`. `script/lint`, `script/test`, `script/cibuild` and
`script/docker` are the four, and there is no fifth — `script/check` runs the `script/docker` are the four, and there is no fifth — `script/check` runs the
two gate phases and `script/fmt-check`, and builds no image of its own. Every two gate phases and `script/fmt-check`, and builds no image of its own. In
stage of the `Dockerfile` declares `ARG CACHEBUST` as its first line after every stage of the `Dockerfile`, `ARG CACHEBUST` comes right after the steps
`FROM`, and a new value reruns every `RUN` step after that line; `COPY` steps that install system packages and download Go's modules, or right after `FROM`
are compared by content and need nothing. A stage without the `ARG` line is in a stage with neither, and every step that lints, tests or compiles comes
served from cache on an unchanged tree, so a repository re-vendors the scripts after it. A step that installs anything else as well, such as a
and adds the `ARG` lines in one commit. The scripts do not pass `--no-cache`: `script/bootstrap` that also installs node packages, comes after it too. A new
it reruns every step too, but it also empties every cache mount, and Go's value reruns every `RUN` step after that line; `COPY` steps are compared by
build cache, which the canonical Go `Dockerfile` below mounts so that compiled content and need nothing. Docker keeps the layers above the line until the
packages survive between check builds, would start empty every time. A bare base image, their own lines or the files they copy change. A stage without the
`ARG` line is served from cache on an unchanged tree, so a repository
re-vendors the scripts and adds the `ARG` lines in one commit. The scripts do
not pass `--no-cache`: it reruns every step too, but it also builds the layers
above the line again and empties every cache mount, so Go's build cache and
golangci-lint's, which the canonical Go `Dockerfile` below mounts so that they
survive between check builds, would start empty every time. A bare
`docker build .` is not evidence that anything ran: a sub-second build `docker build .` is not evidence that anything ran: a sub-second build
reporting success is a cache hit, not a result. Never invalidate by pruning — reporting success is a cache hit, not a result. Never invalidate by pruning —
`docker builder prune` and friends destroy a build cache shared with every `docker builder prune` and friends destroy a build cache shared with every
@@ -194,19 +203,25 @@ style conventions are in separate documents:
`Dockerfile`: `Dockerfile`:
```dockerfile ```dockerfile
# Every stage declares ARG CACHEBUST first: script/ passes a new value # In every stage ARG CACHEBUST comes right after the steps that install
# on every build, which reruns every RUN step after it. The steps that # system packages and download Go's modules, or right after FROM in a
# compile mount Go's build cache, which survives from build to build. # stage with neither, and every step that lints, tests or compiles
# comes after it. script/ passes a new value on every build, which
# reruns every RUN step after it. Docker keeps the layers above it
# until the base image, their own lines or go.mod and go.sum change.
# The steps that compile mount Go's build cache, and golangci-lint run
# also mounts golangci-lint's; both survive from build to build.
# Lint phase # Lint phase
# golangci/golangci-lint:v2.x.x, YYYY-MM-DD # golangci/golangci-lint:v2.x.x, YYYY-MM-DD
FROM golangci/golangci-lint@sha256:... AS lint FROM golangci/golangci-lint@sha256:... AS lint
ARG CACHEBUST
WORKDIR /src WORKDIR /src
COPY go.mod go.sum ./ COPY go.mod go.sum ./
RUN go mod download RUN go mod download
ARG CACHEBUST
COPY . . COPY . .
RUN --mount=type=cache,target=/root/.cache/go-build \ RUN --mount=type=cache,target=/root/.cache/go-build \
--mount=type=cache,target=/root/.cache/golangci-lint \
golangci-lint run --config .golangci.yml ./... golangci-lint run --config .golangci.yml ./...
# Test phase. -race needs cgo and so a C compiler, which the Debian Go # Test phase. -race needs cgo and so a C compiler, which the Debian Go
@@ -214,10 +229,10 @@ style conventions are in separate documents:
# cache also holds test results, which go test would otherwise replay. # cache also holds test results, which go test would otherwise replay.
# golang:1.x, YYYY-MM-DD # golang:1.x, YYYY-MM-DD
FROM golang@sha256:... AS test FROM golang@sha256:... AS test
ARG CACHEBUST
WORKDIR /src WORKDIR /src
COPY go.mod go.sum ./ COPY go.mod go.sum ./
RUN go mod download RUN go mod download
ARG CACHEBUST
COPY . . COPY . .
RUN --mount=type=cache,target=/root/.cache/go-build \ RUN --mount=type=cache,target=/root/.cache/go-build \
go test -count=1 -timeout 90s -race -cover ./... || \ go test -count=1 -timeout 90s -race -cover ./... || \
@@ -225,19 +240,19 @@ style conventions are in separate documents:
go test -count=1 -timeout 90s -race -v ./...; exit 1; } go test -count=1 -timeout 90s -race -v ./...; exit 1; }
# Build stage. Nothing is wanted from either phase above; the copies # Build stage. Nothing is wanted from either phase above; the copies
# are what make BuildKit build them first, so this stage cannot run # are what make BuildKit build them first, so nothing after them can
# unless lint and test passed. # run unless lint and test passed.
# golang:1.x-alpine, YYYY-MM-DD # golang:1.x-alpine, YYYY-MM-DD
FROM golang@sha256:... AS builder FROM golang@sha256:... AS builder
ARG CACHEBUST
COPY --from=lint /src/go.sum /dev/null
COPY --from=test /src/go.sum /dev/null
RUN apk add --no-cache git RUN apk add --no-cache git
# A tar-stream context keeps the sender's file owners, which git refuses.
RUN git config --system --add safe.directory /src
WORKDIR /src WORKDIR /src
COPY go.mod go.sum ./ COPY go.mod go.sum ./
RUN go mod download RUN go mod download
ARG CACHEBUST
COPY --from=lint /src/go.sum /dev/null
COPY --from=test /src/go.sum /dev/null
# A tar-stream context keeps the sender's file owners, which git refuses.
RUN git config --system --add safe.directory /src
COPY . . COPY . .
# The VERSION build arg when one is given, otherwise # The VERSION build arg when one is given, otherwise
@@ -274,16 +289,26 @@ style conventions are in separate documents:
- Keep the runtime stage last, and if you add a stage after it, give it the - Keep the runtime stage last, and if you add a stage after it, give it the
same two copies. A plain `docker build .` builds the last stage's chain same two copies. A plain `docker build .` builds the last stage's chain
and nothing else. and nothing else.
- Only Go's build cache is mounted (`/root/.cache/go-build`, where Go keeps - Check builds keep these, and none of them can hand back a stale result:
it for root), and only on the steps that compile: `golangci-lint run`, - The layers above `ARG CACHEBUST`: the system packages and Go's
both `go test` runs and `go build`. It keeps compiled packages between modules. Docker installs the packages again when the base image or the
check builds, and Go reuses one only when all its inputs are unchanged, so install line changes, and downloads the modules again when `go.mod` or
it saves the compile without skipping a check. Nothing else is kept: `go.sum` changes. Go checks every module against `go.sum` as it
golangci-lint's own cache would replay stored findings in place of a lint downloads it, and no later build can change a kept layer. The modules
run, and Go builds from whatever files a kept module cache holds without are kept in a layer rather than a cache mount because a cache mount is
checking them again (only `go mod verify` does), so a file changed there shared by every build on the host, and Go builds from whatever files
would go unnoticed. Go's build cache also holds test results, which is why it holds without checking them again (only `go mod verify` does), so a
both `go test` runs carry `-count=1`. file changed there would go unnoticed.
- Go's build cache (`/root/.cache/go-build`, where Go keeps it for
root), mounted on the steps that compile: `golangci-lint run`, both
`go test` runs and `go build`. Go reuses a compiled package only when
all its inputs are unchanged. The cache also holds test results, which
is why both `go test` runs carry `-count=1`.
- golangci-lint's cache (`/root/.cache/golangci-lint`, where the pinned
image keeps it), mounted on `golangci-lint run`. golangci-lint reuses
a package's findings only when the package's files and those of every
package it imports, `go.mod`, the linters and their settings in
`.golangci.yml`, and the golangci-lint version are all unchanged.
- If the project uses `//go:embed` directives that reference build artifacts - If the project uses `//go:embed` directives that reference build artifacts
(e.g. a web frontend compiled in a separate stage), the lint phase must (e.g. a web frontend compiled in a separate stage), the lint phase must
create placeholder files so the embed directives resolve. Example: create placeholder files so the embed directives resolve. Example:
@@ -292,12 +317,20 @@ style conventions are in separate documents:
in the lint phase. The `golangci/golangci-lint` image is Debian-based and in the lint phase. The `golangci/golangci-lint` image is Debian-based and
has no `apk`, so install with `apt-get` under the Debian package name has no `apk`, so install with `apt-get` under the Debian package name
(`libvips-dev`, where alpine says `vips-dev`), and delete the package (`libvips-dev`, where alpine says `vips-dev`), and delete the package
lists in the same `RUN`, so the layer does not keep them: lists in the same `RUN`, so the layer does not keep them. The test phase,
on the Debian Go image, installs the libraries its tests need the same
way. Either install goes right after `FROM`, above `ARG CACHEBUST`, so
Docker keeps it until the base image or the install line changes:
```dockerfile ```dockerfile
FROM golangci/golangci-lint@sha256:... AS lint
RUN apt-get update \ RUN apt-get update \
&& apt-get install -y --no-install-recommends libvips-dev \ && apt-get install -y --no-install-recommends libvips-dev \
&& rm -rf /var/lib/apt/lists/* && rm -rf /var/lib/apt/lists/*
WORKDIR /src
COPY go.mod go.sum ./
RUN go mod download
ARG CACHEBUST
``` ```
- `.dockerignore` lets `.git` into the build context. It keeps out every git - `.dockerignore` lets `.git` into the build context. It keeps out every git
+31
View File
@@ -30,10 +30,41 @@ P2: security: per-origin rate limiting
# Completed Steps # Completed Steps
- 2026-10-20 a WebP, AVIF or JPEG XL encode at the limits takes at most about 34
seconds (closes #272): with one libvips thread, 8192x8192 random pixels with
an alpha channel took about 61 seconds to encode as WebP, 81 as JPEG XL and,
at quality 85, 96 as AVIF, so with the default `downstream_timeout` of 60
seconds such an image was never delivered, yet its encode held a processing
slot to the end. WebP is now saved at effort 2 and JPEG XL at effort 4, which
take up to about 23 and 27 seconds at any quality. AVIF was already at the
lowest effort govips can set, so an AVIF output of more than 16,777,216 pixels
is refused with 400 before anything is decoded; 4096x4096, 8192x2048 or
2048x8192 takes up to about 34 seconds at any quality. Resizing with
`fit=cover` already decodes the input, so the output's dimensions are worked
out before resizing. `README.md` gives the limit and the new times.
- 2026-10-20 a source whose first bytes do not match its `Content-Type` answers
415 (closes #253): a body whose first bytes are not those of the format its
`Content-Type` names, such as a JPEG sent as `image/png`, text, or a body
under 12 bytes, answered 500 on both image routes and was logged as an error
of pixa's, and each request for it fetched the whole body again. Both routes
now answer 415, and the image service logs it at warning level as a refused
source. It also gets a negative cache entry, as an upstream error status does,
so the source URL is not fetched again for 5 minutes, and meanwhile an image
that would need that fetch answers 502.
- 2026-10-20 `AGENTS.md`: a bug fix may change an existing test without asking
when that test pins exactly the behavior the filed bug calls wrong, and the
change moves it to the fixed behavior and nothing else (closes #280).
- 2026-10-18 re-vendored from `sneak/prompts` commit `0653579` (closes #221):
each `Dockerfile` stage installs its system packages and downloads the Go
modules above `ARG CACHEBUST`, so check builds keep them, and the lint step
keeps golangci-lint's cache.
- 2026-10-19 a `trusted_proxies` entry in the IPv4-mapped form trusted nothing; - 2026-10-19 a `trusted_proxies` entry in the IPv4-mapped form trusted nothing;
it is now read as the IPv4 range it covers by the same code that reads it is now read as the IPv4 range it covers by the same code that reads
`blocked_networks`, and such an entry shorter than `/96` aborts startup `blocked_networks`, and such an entry shorter than `/96` aborts startup
(closes #303). (closes #303).
- 2026-10-18 the next reconciliation pass waits at least ten times as long as
the last one took, so passes on a large cache no longer run back to back
(closes #250).
- 2026-10-18 a `blocked_networks` entry in the IPv4-mapped form refused nothing; - 2026-10-18 a `blocked_networks` entry in the IPv4-mapped form refused nothing;
`::ffff:10.0.0.0/104` is now read as `10.0.0.0/8`, and such an entry shorter `::ffff:10.0.0.0/104` is now read as `10.0.0.0/8`, and such an entry shorter
than `/96` aborts startup (closes #301). than `/96` aborts startup (closes #301).
@@ -0,0 +1,49 @@
package handlers
import (
"fmt"
"log/slog"
"net/http"
"net/http/httptest"
"strings"
"testing"
"sneak.berlin/go/pixa/internal/imageprocessor"
"sneak.berlin/go/pixa/internal/imgcache"
)
// TestAVIFOutputTooLargeAnswers400 checks that respondImageError and
// handleImageError, which answer the errors of the two image routes, answer
// 400 naming the AVIF pixel limit when the image processor refuses an AVIF
// output above it, wrapped as the service wraps it. The request's source only
// goes into a log line, so it is left empty.
func TestAVIFOutputTooLargeAnswers400(t *testing.T) {
t.Parallel()
h := &Handlers{log: slog.New(slog.DiscardHandler)}
err := fmt.Errorf("image processing failed: %w: 4097x4096",
imageprocessor.ErrAVIFOutputTooLarge)
wantBody := fmt.Sprintf("above %d pixels", imageprocessor.MaxAVIFOutputPixels)
plain := httptest.NewRecorder()
h.respondImageError(plain, &imgcache.ImageRequest{}, err)
encrypted := httptest.NewRecorder()
h.handleImageError(encrypted, err)
for name, rec := range map[string]*httptest.ResponseRecorder{
"respondImageError": plain, "handleImageError": encrypted,
} {
t.Logf("%s: status = %d, body = %q", name, rec.Code, rec.Body.String())
if rec.Code != http.StatusBadRequest {
t.Errorf("%s: status = %d, want %d",
name, rec.Code, http.StatusBadRequest)
}
if !strings.Contains(rec.Body.String(), wantBody) {
t.Errorf("%s: body = %q, want it to say %q",
name, rec.Body.String(), wantBody)
}
}
}
@@ -0,0 +1,180 @@
package handlers
import (
"bytes"
"encoding/json"
"image/color"
"log/slog"
"net/http"
"slices"
"testing"
"testing/fstest"
"time"
"github.com/go-chi/chi/v5"
"sneak.berlin/go/pixa/internal/encurl"
"sneak.berlin/go/pixa/internal/httpfetcher"
"sneak.berlin/go/pixa/internal/imgcache"
)
// mismatchedSourcePath is the path of the source that
// TestFirstBytesNotMatchingContentType serves on allowlistedHost. The mock
// fetcher names a file's Content-Type from its extension, so the source is
// sent as image/png.
const mismatchedSourcePath = "/images/source.png"
// newRoutesFetchingThrough returns both image routes, and the Handlers behind
// them, whose image service fetches through fetcher and has allowlistedHost on
// its allowlist. The handlers and the image service log to log.
func newRoutesFetchingThrough(
t *testing.T, fetcher httpfetcher.Fetcher, log *slog.Logger,
) (*Handlers, http.Handler) {
t.Helper()
cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{
StateDir: t.TempDir(),
NegativeTTL: 5 * time.Minute,
})
if err != nil {
t.Fatalf("imgcache.NewCache() error = %v", err)
}
svc, err := imgcache.NewService(&imgcache.ServiceConfig{
Cache: cache,
Fetcher: fetcher,
SigningKey: testSigningKey,
Allowlist: []string{allowlistedHost},
Logger: log,
})
if err != nil {
t.Fatalf("imgcache.NewService() error = %v", err)
}
encGen, err := encurl.NewGenerator(testSigningKey)
if err != nil {
t.Fatalf("encurl.NewGenerator() error = %v", err)
}
h := &Handlers{log: log, imgSvc: svc, encGen: encGen}
r := chi.NewRouter()
r.Get("/v1/image/*", h.HandleImage())
r.Get("/v1/e/{token}/*", h.HandleImageEnc())
return h, r
}
// mismatchedSourcePlainURL returns the /v1/image/ URL of mismatchedSourcePath
// on allowlistedHost at 50x50 as a JPEG.
func mismatchedSourcePlainURL(*testing.T, *Handlers) string {
return "/v1/image/" + allowlistedHost + mismatchedSourcePath + "/50x50.jpeg"
}
// mismatchedSourceEncURL returns an encrypted /v1/e/ URL, made with h's
// generator, of mismatchedSourcePath on allowlistedHost at 50x50 as a JPEG.
func mismatchedSourceEncURL(t *testing.T, h *Handlers) string {
t.Helper()
token, err := h.encGen.Generate(&encurl.Payload{
SourceHost: allowlistedHost,
SourcePath: mismatchedSourcePath,
Width: 50,
Height: 50,
Format: imgcache.FormatJPEG,
})
if err != nil {
t.Fatalf("Generate() error = %v", err)
}
return "/v1/e/" + token + "/img.jpg"
}
// loggedLevels reads the JSON log lines in logs and returns the messages
// logged at each level.
func loggedLevels(t *testing.T, logs *bytes.Buffer) map[string][]string {
t.Helper()
t.Logf("logged:\n%s", logs.String())
messages := make(map[string][]string)
dec := json.NewDecoder(logs)
for dec.More() {
var line struct {
Level string `json:"level"`
Msg string `json:"msg"`
}
err := dec.Decode(&line)
if err != nil {
t.Fatalf("decoding log line: %v", err)
}
messages[line.Level] = append(messages[line.Level], line.Msg)
}
return messages
}
// TestFirstBytesNotMatchingContentType checks that both image routes answer
// 415 for an upstream body whose first bytes are not those of the format its
// Content-Type names, and log it as a refused source, not as an error of
// pixa's. The next request for that source must answer 502 without fetching
// it again, as after an upstream error status.
func TestFirstBytesNotMatchingContentType(t *testing.T) {
t.Parallel()
bodies := map[string][]byte{
"JPEG sent as image/png": generateTestJPEG(t, 10, 10, color.Black),
"text sent as image/png": []byte("This is just text, not a PNG"),
"body too short to name a format": []byte("short"),
}
// The source's URL on each image route.
sourceURLs := map[string]func(*testing.T, *Handlers) string{
"plain URL": mismatchedSourcePlainURL,
"encrypted URL": mismatchedSourceEncURL,
}
for bodyName, body := range bodies {
for urlName, sourceURL := range sourceURLs {
t.Run(urlName+", "+bodyName, func(t *testing.T) {
t.Parallel()
fetcher := &countingFetcher{
Fetcher: httpfetcher.NewMock(fstest.MapFS{
allowlistedHost + mismatchedSourcePath: &fstest.MapFile{
Data: body,
},
}),
}
var logs bytes.Buffer
h, routes := newRoutesFetchingThrough(t, fetcher,
slog.New(slog.NewJSONHandler(&logs, nil)))
target := sourceURL(t, h)
checkErrorBody(t, sendGet(t, routes, target),
http.StatusUnsupportedMediaType, "unsupported source format")
levels := loggedLevels(t, &logs)
if len(levels[slog.LevelError.String()]) != 0 {
t.Errorf("logged at error level: %q",
levels[slog.LevelError.String()])
}
if !slices.Contains(levels[slog.LevelWarn.String()], "source refused") {
t.Error(`no "source refused" line logged at warning level`)
}
checkErrorBody(t, sendGet(t, routes, target),
http.StatusBadGateway, "upstream error")
if n := fetcher.fetches.Load(); n != 1 {
t.Errorf("upstream fetched %d times, want 1", n)
}
})
}
}
}
+21
View File
@@ -14,6 +14,7 @@ import (
"sneak.berlin/go/pixa/internal/httpfetcher" "sneak.berlin/go/pixa/internal/httpfetcher"
"sneak.berlin/go/pixa/internal/imageprocessor" "sneak.berlin/go/pixa/internal/imageprocessor"
"sneak.berlin/go/pixa/internal/imgcache" "sneak.berlin/go/pixa/internal/imgcache"
"sneak.berlin/go/pixa/internal/magic"
) )
// HandleImage handles the main image proxy route: // HandleImage handles the main image proxy route:
@@ -220,6 +221,18 @@ func (s *Handlers) HandleTooManyRequests() http.HandlerFunc {
func (s *Handlers) respondImageError( func (s *Handlers) respondImageError(
w http.ResponseWriter, req *imgcache.ImageRequest, err error, w http.ResponseWriter, req *imgcache.ImageRequest, err error,
) { ) {
// An upstream body whose first bytes are not those of the format its
// Content-Type names is the upstream's error, not pixa's: the image
// service has logged it as a refused source.
if errors.Is(err, magic.ErrMagicByteMismatch) ||
errors.Is(err, magic.ErrUnknownFormat) ||
errors.Is(err, magic.ErrNotEnoughData) {
s.respondError(w, "unsupported source format",
http.StatusUnsupportedMediaType)
return
}
s.log.Error("failed to get image", s.log.Error("failed to get image",
"host", req.SourceHost, "host", req.SourceHost,
"path", req.SourcePath, "path", req.SourcePath,
@@ -264,6 +277,14 @@ func (s *Handlers) respondImageError(
return return
} }
if errors.Is(err, imageprocessor.ErrAVIFOutputTooLarge) {
s.respondError(w, fmt.Sprintf("size too large for AVIF: the output "+
"would be above %d pixels", imageprocessor.MaxAVIFOutputPixels),
http.StatusBadRequest)
return
}
s.respondError(w, "internal error", http.StatusInternalServerError) s.respondError(w, "internal error", http.StatusInternalServerError)
} }
+9 -1
View File
@@ -15,6 +15,7 @@ import (
"sneak.berlin/go/pixa/internal/httpfetcher" "sneak.berlin/go/pixa/internal/httpfetcher"
"sneak.berlin/go/pixa/internal/imageprocessor" "sneak.berlin/go/pixa/internal/imageprocessor"
"sneak.berlin/go/pixa/internal/imgcache" "sneak.berlin/go/pixa/internal/imgcache"
"sneak.berlin/go/pixa/internal/magic"
) )
// HandleImageEnc handles requests to /v1/e/{token}/* for encrypted // HandleImageEnc handles requests to /v1/e/{token}/* for encrypted
@@ -163,13 +164,20 @@ func (s *Handlers) handleImageError(w http.ResponseWriter, err error) {
s.respondError(w, "server busy, try again later", s.respondError(w, "server busy, try again later",
http.StatusServiceUnavailable) http.StatusServiceUnavailable)
case errors.Is(err, httpfetcher.ErrInvalidContentType), case errors.Is(err, httpfetcher.ErrInvalidContentType),
errors.Is(err, imageprocessor.ErrUnsupportedInputFormat): errors.Is(err, imageprocessor.ErrUnsupportedInputFormat),
errors.Is(err, magic.ErrMagicByteMismatch),
errors.Is(err, magic.ErrUnknownFormat),
errors.Is(err, magic.ErrNotEnoughData):
s.respondError(w, "unsupported source format", s.respondError(w, "unsupported source format",
http.StatusUnsupportedMediaType) http.StatusUnsupportedMediaType)
case errors.Is(err, imageprocessor.ErrOutputTooLarge): case errors.Is(err, imageprocessor.ErrOutputTooLarge):
s.respondError(w, fmt.Sprintf("size too large for the source: the side "+ s.respondError(w, fmt.Sprintf("size too large for the source: the side "+
"computed from its proportions is above %d", "computed from its proportions is above %d",
imageprocessor.MaxOutputDimension), http.StatusBadRequest) imageprocessor.MaxOutputDimension), http.StatusBadRequest)
case errors.Is(err, imageprocessor.ErrAVIFOutputTooLarge):
s.respondError(w, fmt.Sprintf("size too large for AVIF: the output "+
"would be above %d pixels", imageprocessor.MaxAVIFOutputPixels),
http.StatusBadRequest)
default: default:
s.log.Error("image request failed", "error", err) s.log.Error("image request failed", "error", err)
s.respondError(w, "internal error", http.StatusInternalServerError) s.respondError(w, "internal error", http.StatusInternalServerError)
@@ -0,0 +1,84 @@
package imageprocessor
import (
"bytes"
"encoding/binary"
"errors"
"os"
"testing"
)
// TestImageProcessor_RefusesAVIFAboveMaxPixels verifies that an AVIF output
// of more than MaxAVIFOutputPixels is refused with ErrAVIFOutputTooLarge
// before anything is decoded, whatever size and fit give it, the source's own
// size included, and that an AVIF output of at most that many pixels, or one
// in another format, is not. One input is a 64x32 PNG cut off in its pixel
// data. The other is testdata/red.avif, a 4x4 AVIF, with the size in its
// header changed to 4097x4096. libvips reads each input's size from its
// header but fails when it decodes its pixel data, so a request that gets
// that failure was not refused, and a refusal came before any decoding.
func TestImageProcessor_RefusesAVIFAboveMaxPixels(t *testing.T) {
t.Parallel()
whole := createTestPNG(t, 64, 32)
cutOff := whole[:len(whole)/2]
largeAVIF, err := os.ReadFile("testdata/red.avif")
if err != nil {
t.Fatalf("failed to read test AVIF: %v", err)
}
// The ispe box gives the width and then the height, 4 bytes each, after
// the box's type and its 4 bytes of version and flags
ispe := bytes.Index(largeAVIF, []byte("ispe"))
binary.BigEndian.PutUint32(largeAVIF[ispe+8:], 4097)
binary.BigEndian.PutUint32(largeAVIF[ispe+12:], 4096)
tests := []struct {
name string
input []byte
format Format
width, height int
fit FitMode
refused bool
}{
{"AVIF at the limit", cutOff, FormatAVIF, 4096, 4096, FitCover, false},
{"AVIF above the limit", cutOff, FormatAVIF, 4096, 4097, FitCover, true},
{"WebP above the AVIF limit", cutOff, FormatWebP, 4096, 4097, FitCover, false},
// FitInside does not enlarge, so the output stays 64x32
{"AVIF inside a larger size", cutOff, FormatAVIF, 8192, 8192, FitInside, false},
// FitOutside scales 64x32 by 128 to cover 8192x1: 8192x4096
{"AVIF outside a smaller size", cutOff, FormatAVIF, 8192, 1, FitOutside, true},
// With no size, the output keeps the source's 4097x4096
{"AVIF of the source's size", largeAVIF, FormatAVIF, 0, 0, FitCover, true},
{"orig of an AVIF source", largeAVIF, FormatOriginal, 0, 0, FitCover, true},
{"JPEG of the source's size", largeAVIF, FormatJPEG, 0, 0, FitCover, false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
_, err := New(Params{}).Process(t.Context(), bytes.NewReader(tt.input),
&Request{
Size: Size{Width: tt.width, Height: tt.height},
Format: tt.format,
FitMode: tt.fit,
})
t.Logf("Process() error = %v", err)
if tt.refused {
if !errors.Is(err, ErrAVIFOutputTooLarge) {
t.Errorf("Process() error = %v, want %v", err, ErrAVIFOutputTooLarge)
}
return
}
if err == nil || errors.Is(err, ErrAVIFOutputTooLarge) {
t.Errorf("Process() error = %v, want the failure to decode "+
"the input", err)
}
})
}
}
@@ -77,10 +77,10 @@ func TestImageProcessor_PNGIsCompressed(t *testing.T) {
} }
} }
// TestImageProcessor_WebPAtDefaultEffort verifies that WebP is saved at // TestImageProcessor_WebPAtEffort2 verifies that WebP is saved at effort 2,
// libvips' default effort, 4: the output is the size of the same image saved // not libvips' default of 4: the output is the size of the same image saved
// at that effort. // at effort 2.
func TestImageProcessor_WebPAtDefaultEffort(t *testing.T) { func TestImageProcessor_WebPAtEffort2(t *testing.T) {
t.Parallel() t.Parallel()
input := createTestJPEG(t, 200, 150) input := createTestJPEG(t, 200, 150)
@@ -90,14 +90,14 @@ func TestImageProcessor_WebPAtDefaultEffort(t *testing.T) {
want, _, err := decode(t, input).ExportWebp(&vips.WebpExportParams{ want, _, err := decode(t, input).ExportWebp(&vips.WebpExportParams{
StripMetadata: true, StripMetadata: true,
Quality: 85, Quality: 85,
ReductionEffort: 4, ReductionEffort: 2,
}) })
if err != nil { if err != nil {
t.Fatalf("ExportWebp() error = %v", err) t.Fatalf("ExportWebp() error = %v", err)
} }
if size != int64(len(want)) { if size != int64(len(want)) {
t.Errorf("WebP output is %d bytes, want %d, the size at effort 4", t.Errorf("WebP output is %d bytes, want %d, the size at effort 2",
size, len(want)) size, len(want))
} }
} }
@@ -143,3 +143,29 @@ func TestImageProcessor_AVIFAtEffort1(t *testing.T) {
"and 8 bits", size, len(want)) "and 8 bits", size, len(want))
} }
} }
// TestImageProcessor_JPEGXLAtEffort4 verifies that JPEG XL is saved at
// effort 4, not libvips' default of 7: the output is the size of the same
// image saved at effort 4, with the distance exportJXL gives quality 85.
func TestImageProcessor_JPEGXLAtEffort4(t *testing.T) {
t.Parallel()
input := createTestJPEG(t, 200, 150)
size := processedSize(t, input, &Request{Format: FormatJXL, Quality: 85})
params := vips.NewJxlExportParams()
params.Quality = 85
params.Distance = jxlDistance(85)
params.Effort = 4
want, _, err := decode(t, input).ExportJxl(params)
if err != nil {
t.Fatalf("ExportJxl() error = %v", err)
}
if size != int64(len(want)) {
t.Errorf("JPEG XL output is %d bytes, want %d, the size at effort 4",
size, len(want))
}
}
+146 -81
View File
@@ -142,6 +142,19 @@ const MaxOutputDimension = 8192
// requested size. // requested size.
var ErrOutputTooLarge = errors.New("output image dimensions exceed maximum") var ErrOutputTooLarge = errors.New("output image dimensions exceed maximum")
// MaxAVIFOutputPixels is the most pixels an AVIF output may have, so that an
// encode takes at most 40 seconds of the default downstream_timeout of 60
// seconds; AVIF is already at the lowest effort govips can set (avifEffort).
// With one thread, as pixad runs libvips, saving 4096x4096, 8192x2048 or
// 2048x8192 random pixels with an alpha channel takes up to about 34
// seconds, the slowest at any quality and at 8 or 16 bits per sample, and
// 5120x4096 at 8 bits already takes about 36.
const MaxAVIFOutputPixels = 4096 * 4096
// ErrAVIFOutputTooLarge is returned, before anything is decoded, when an AVIF
// output would have more than MaxAVIFOutputPixels pixels.
var ErrAVIFOutputTooLarge = errors.New("AVIF output has too many pixels")
// ErrUnsupportedOutputFormat is returned when the requested output format is // ErrUnsupportedOutputFormat is returned when the requested output format is
// not supported. // not supported.
var ErrUnsupportedOutputFormat = errors.New("unsupported output format") var ErrUnsupportedOutputFormat = errors.New("unsupported output format")
@@ -210,7 +223,8 @@ func New(params Params) *ImageProcessor {
// ErrTooManyImages. When ctx has ended once Process has a slot, or before it // ErrTooManyImages. When ctx has ended once Process has a slot, or before it
// encodes, it returns ctx's error and gives the slot back. govips gives pixa // encodes, it returns ctx's error and gives the slot back. govips gives pixa
// no way to stop libvips once an encode has begun, so an encode keeps its // no way to stop libvips once an encode has begun, so an encode keeps its
// slot until it finishes. // slot until it finishes. An AVIF output of more than MaxAVIFOutputPixels is
// refused with ErrAVIFOutputTooLarge before anything is decoded.
func (p *ImageProcessor) Process( func (p *ImageProcessor) Process(
ctx context.Context, ctx context.Context,
input io.Reader, input io.Reader,
@@ -223,18 +237,9 @@ func (p *ImageProcessor) Process(
defer release() defer release()
// Read input with a size limit to prevent unbounded memory consumption. data, err := p.readInput(input)
// We read at most maxInputBytes+1 so we can detect if the input exceeds
// the limit without consuming additional memory.
limited := io.LimitReader(input, p.maxInputBytes+1)
data, err := io.ReadAll(limited)
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to read input: %w", err) return nil, err
}
if int64(len(data)) > p.maxInputBytes {
return nil, ErrInputDataTooLarge
} }
// govips picks the libvips loader from the input's first bytes with // govips picks the libvips loader from the input's first bytes with
@@ -277,20 +282,28 @@ func (p *ImageProcessor) Process(
return nil, err return nil, err
} }
// Resize if needed
if targetWidth != origWidth || targetHeight != origHeight {
err := p.resize(img, targetWidth, targetHeight, req.FitMode)
if err != nil {
return nil, fmt.Errorf("failed to resize: %w", err)
}
}
// orig is the source's own format; encode refuses an empty format // orig is the source's own format; encode refuses an empty format
outputFormat := req.Format outputFormat := req.Format
if outputFormat == FormatOriginal { if outputFormat == FormatOriginal {
outputFormat = sourceFormat outputFormat = sourceFormat
} }
// Determine output dimensions; too large ones are refused before
// resizing, which with FitCover already decodes the input
outputWidth, outputHeight, err := outputDimensions(origWidth, origHeight,
targetWidth, targetHeight, req.FitMode, outputFormat)
if err != nil {
return nil, err
}
// Resize if needed
if targetWidth != origWidth || targetHeight != origHeight {
err := p.resize(img, outputWidth, outputHeight, req.FitMode)
if err != nil {
return nil, fmt.Errorf("failed to resize: %w", err)
}
}
// Encode to target format // Encode to target format
output, err := p.encode(ctx, img, outputFormat, req.Quality) output, err := p.encode(ctx, img, outputFormat, req.Quality)
if err != nil { if err != nil {
@@ -445,6 +458,23 @@ func (p *ImageProcessor) acquireSlot(ctx context.Context) (func(), error) {
return release, nil return release, nil
} }
// readInput reads input with a size limit to prevent unbounded memory
// consumption, refusing more than maxInputBytes with ErrInputDataTooLarge. It
// reads at most maxInputBytes+1 so it can detect if the input exceeds the
// limit without consuming additional memory.
func (p *ImageProcessor) readInput(input io.Reader) ([]byte, error) {
data, err := io.ReadAll(io.LimitReader(input, p.maxInputBytes+1))
if err != nil {
return nil, fmt.Errorf("failed to read input: %w", err)
}
if int64(len(data)) > p.maxInputBytes {
return nil, ErrInputDataTooLarge
}
return data, nil
}
// detectFormat returns the format string for a govips image type. // detectFormat returns the format string for a govips image type.
func (p *ImageProcessor) detectFormat(imageType vips.ImageType) string { func (p *ImageProcessor) detectFormat(imageType vips.ImageType) string {
switch imageType { switch imageType {
@@ -469,9 +499,67 @@ func (p *ImageProcessor) detectFormat(imageType vips.ImageType) string {
} }
} }
// resize resizes the image according to the fit mode. With FitOutside one // outputDimensions returns the dimensions resize gives an image of imgWidth x
// side can come out larger than requested; when it would be above // imgHeight for the target dimensions and fit mode, without resizing it, and
// MaxOutputDimension, resize returns ErrOutputTooLarge without resizing. // refuses an output too large for format. With FitOutside one side can come
// out larger than the target; when it would be above MaxOutputDimension,
// outputDimensions returns ErrOutputTooLarge. An AVIF output of more than
// MaxAVIFOutputPixels is ErrAVIFOutputTooLarge.
func outputDimensions(
imgWidth, imgHeight, width, height int, fit FitMode, format Format,
) (int, int, error) {
scaleW := float64(width) / float64(imgWidth)
scaleH := float64(height) / float64(imgHeight)
outputWidth, outputHeight := width, height
switch fit {
case FitCover, FitFill, "":
// Cover crops, and fill stretches, to exactly the target dimensions
case FitContain:
// Fit within the target dimensions, maintaining aspect ratio. Scaled
// by the smaller scale, the short side of a thin image can round
// down to 0, which libvips refuses as a width, so each side is at
// least 1.
scale := min(scaleW, scaleH)
outputWidth = max(1, int(float64(imgWidth)*scale))
outputHeight = max(1, int(float64(imgHeight)*scale))
case FitInside:
// Same as contain, but only shrink
scale := min(scaleW, scaleH, 1)
outputWidth = max(1, int(float64(imgWidth)*scale))
outputHeight = max(1, int(float64(imgHeight)*scale))
case FitOutside:
// The smallest dimension fits, the other may exceed the target. A
// side scaled to 1 can come out just under 1 in floating point, as
// 49 times 1/49 does, and round down to 0, which libvips refuses as
// a width, so each side is at least 1.
scale := max(scaleW, scaleH)
outputWidth = max(1, int(float64(imgWidth)*scale))
outputHeight = max(1, int(float64(imgHeight)*scale))
default:
return 0, 0, fmt.Errorf("%w: %s", ErrInvalidFitMode, fit)
}
// The side FitOutside scales past the target is held to the maximum too
if max(outputWidth, outputHeight) > MaxOutputDimension {
return 0, 0, fmt.Errorf("%w: %dx%d is above %d", ErrOutputTooLarge,
outputWidth, outputHeight, MaxOutputDimension)
}
if format == FormatAVIF && outputWidth*outputHeight > MaxAVIFOutputPixels {
return 0, 0, fmt.Errorf("%w: %dx%d is above %d pixels",
ErrAVIFOutputTooLarge, outputWidth, outputHeight, MaxAVIFOutputPixels)
}
return outputWidth, outputHeight, nil
}
// resize resizes the image to width x height, the dimensions
// outputDimensions gives for the fit mode.
func (p *ImageProcessor) resize( func (p *ImageProcessor) resize(
img *vips.ImageRef, width, height int, fit FitMode, img *vips.ImageRef, width, height int, fit FitMode,
) error { ) error {
@@ -480,58 +568,20 @@ func (p *ImageProcessor) resize(
// Crop the centre and resize to fill exact dimensions (default) // Crop the centre and resize to fill exact dimensions (default)
return cropCentreAndResize(img, width, height) return cropCentreAndResize(img, width, height)
case FitContain:
// Resize to fit within dimensions, maintaining aspect ratio. Scaled
// by the smaller scale, the short side of a thin image can round
// down to 0, which libvips refuses as a width, so each side is at
// least 1.
imgW, imgH := img.Width(), img.Height()
scaleW := float64(width) / float64(imgW)
scaleH := float64(height) / float64(imgH)
scale := min(scaleW, scaleH)
newW := max(1, int(float64(imgW)*scale))
newH := max(1, int(float64(imgH)*scale))
return img.Thumbnail(newW, newH, vips.InterestingNone)
case FitFill: case FitFill:
// Resize to exact dimensions (may distort) // Resize to exact dimensions (may distort)
return img.ThumbnailWithSize(width, height, vips.InterestingNone, vips.SizeForce) return img.ThumbnailWithSize(width, height, vips.InterestingNone, vips.SizeForce)
case FitInside: case FitInside:
// Same as contain, but only shrink // outputDimensions keeps the dimensions of an image that already fits
if img.Width() <= width && img.Height() <= height { if img.Width() == width && img.Height() == height {
return nil // Already fits return nil
} }
imgW, imgH := img.Width(), img.Height() return img.Thumbnail(width, height, vips.InterestingNone)
scaleW := float64(width) / float64(imgW)
scaleH := float64(height) / float64(imgH)
scale := min(scaleW, scaleH)
newW := max(1, int(float64(imgW)*scale))
newH := max(1, int(float64(imgH)*scale))
return img.Thumbnail(newW, newH, vips.InterestingNone) case FitContain, FitOutside:
return img.Thumbnail(width, height, vips.InterestingNone)
case FitOutside:
// Resize so smallest dimension fits, may exceed target on other
// dimension. A side scaled to 1 can come out just under 1 in
// floating point, as 49 times 1/49 does, and round down to 0, which
// libvips refuses as a width, so each side is at least 1.
imgW, imgH := img.Width(), img.Height()
scaleW := float64(width) / float64(imgW)
scaleH := float64(height) / float64(imgH)
scale := max(scaleW, scaleH)
newW := max(1, int(float64(imgW)*scale))
newH := max(1, int(float64(imgH)*scale))
// The dimension that exceeds the target is held to the maximum too
if newW > MaxOutputDimension || newH > MaxOutputDimension {
return fmt.Errorf("%w: %dx%d is above %d", ErrOutputTooLarge,
newW, newH, MaxOutputDimension)
}
return img.Thumbnail(newW, newH, vips.InterestingNone)
default: default:
return fmt.Errorf("%w: %s", ErrInvalidFitMode, fit) return fmt.Errorf("%w: %s", ErrInvalidFitMode, fit)
@@ -667,11 +717,16 @@ func exportGIF(img *vips.ImageRef) ([]byte, error) {
return output, err return output, err
} }
// webpEffort is libvips' default WebP effort, from 0 (fastest) to 6. // webpEffort is the WebP effort, from 0 (fastest) to 6. It is below libvips'
const webpEffort = 4 // default of 4 so that an encode takes at most 40 seconds of the default
// downstream_timeout of 60 seconds. With one thread, as pixad runs libvips,
// saving an 8192x8192 image of random pixels with an alpha channel takes up
// to about 23 seconds at 2, and up to about 16 without one, at any quality;
// with an alpha channel, 3 takes about 50 seconds and 4 about 61.
const webpEffort = 2
// exportWebP encodes img as lossy WebP at quality and libvips' default // exportWebP encodes img as lossy WebP at quality and webpEffort, without
// effort, without metadata. // metadata.
func exportWebP(img *vips.ImageRef, quality int) ([]byte, error) { func exportWebP(img *vips.ImageRef, quality int) ([]byte, error) {
output, _, err := img.ExportWebp(&vips.WebpExportParams{ output, _, err := img.ExportWebp(&vips.WebpExportParams{
StripMetadata: true, StripMetadata: true,
@@ -683,12 +738,13 @@ func exportWebP(img *vips.ImageRef, quality int) ([]byte, error) {
} }
// avifEffort is the AVIF effort, from 0 (fastest) to 9; 1 is the lowest // avifEffort is the AVIF effort, from 0 (fastest) to 9; 1 is the lowest
// govips can set. With one thread, as pixad runs libvips, 1 takes about 51 // govips can set. With one thread, as pixad runs libvips, 1 takes about 52
// seconds to save an 8192x8192 RGB image of random pixels, and about 110 // seconds to save an 8192x8192 RGB image of random pixels at quality 85,
// with an alpha channel, the worst case, against the default // and about 96 with an alpha channel, against the default
// downstream_timeout of 60 seconds. On an image of milder noise, which 1 // downstream_timeout of 60 seconds, so an AVIF output is held to
// saves in about 12 seconds, 2 takes nearly a minute and libvips' default, // MaxAVIFOutputPixels. On an image of milder noise, which 1 saves in about
// 4, takes minutes. // 12 seconds, 2 takes nearly a minute and libvips' default, 4, takes
// minutes.
const avifEffort = 1 const avifEffort = 1
// avifBitdepth is the AVIF bit depth, 8 bits per sample for every image. // avifBitdepth is the AVIF bit depth, 8 bits per sample for every image.
@@ -715,10 +771,18 @@ func exportAVIF(img *vips.ImageRef, quality int) ([]byte, error) {
// JPEG that names none. // JPEG that names none.
const jxlResolution = 72 / 25.4 const jxlResolution = 72 / 25.4
// exportJXL encodes img as JPEG XL at quality, with libvips' default effort // jxlEffort is the JPEG XL effort, from 1 (fastest) to 9. It is below
// and without metadata. govips sends libvips a distance, the JPEG XL // libvips' default of 7 so that an encode takes at most 40 seconds of the
// encoder's own measure of quality, along with the quality, and libvips then // default downstream_timeout of 60 seconds. With one thread, as pixad runs
// uses the distance alone, so the quality is also given as a distance. // libvips, saving an 8192x8192 image of random pixels with an alpha channel
// takes up to about 27 seconds at 4, and up to about 13 without one, at any
// quality; with an alpha channel, 5 takes about 43 seconds and 7 about 81.
const jxlEffort = 4
// exportJXL encodes img as JPEG XL at quality and jxlEffort, without
// metadata. govips sends libvips a distance, the JPEG XL encoder's own
// measure of quality, along with the quality, and libvips then uses the
// distance alone, so the quality is also given as a distance.
func exportJXL(img *vips.ImageRef, quality int) ([]byte, error) { func exportJXL(img *vips.ImageRef, quality int) ([]byte, error) {
// libvips converts a CMYK image to sRGB before it saves WebP, AVIF or // libvips converts a CMYK image to sRGB before it saves WebP, AVIF or
// PNG, but cannot save one as JPEG XL. encode has already converted // PNG, but cannot save one as JPEG XL. encode has already converted
@@ -757,6 +821,7 @@ func exportJXL(img *vips.ImageRef, quality int) ([]byte, error) {
params := vips.NewJxlExportParams() params := vips.NewJxlExportParams()
params.Quality = quality params.Quality = quality
params.Distance = jxlDistance(quality) params.Distance = jxlDistance(quality)
params.Effort = jxlEffort
output, _, err := toSave.ExportJxl(params) output, _, err := toSave.ExportJxl(params)
if err != nil { if err != nil {
@@ -331,8 +331,7 @@ func TestService_Get_FirstRequestLeavingKeepsTheWork(t *testing.T) {
// TestService_Get_ConcurrentMissesShareAFailure has several requests for an // TestService_Get_ConcurrentMissesShareAFailure has several requests for an
// image that cannot be served arrive while its fetch is held: the one fetch // image that cannot be served arrive while its fetch is held: the one fetch
// answers all of them with its error. The request after them is answered from // answers all of them with its error. The request after them is answered from
// the negative cache when the failure is kept there, and fetches again when it // the negative cache, where the failure is kept.
// is not.
func TestService_Get_ConcurrentMissesShareAFailure(t *testing.T) { func TestService_Get_ConcurrentMissesShareAFailure(t *testing.T) {
t.Parallel() t.Parallel()
@@ -346,8 +345,8 @@ func TestService_Get_ConcurrentMissesShareAFailure(t *testing.T) {
{"upstream answers 404, kept in the negative cache", {"upstream answers 404, kept in the negative cache",
"/images/missing.jpg", httpfetcher.ErrUpstreamError, "/images/missing.jpg", httpfetcher.ErrUpstreamError,
ErrNegativeCached, 1}, ErrNegativeCached, 1},
{"source fails the magic byte check, not kept", {"source fails the magic byte check, kept in the negative cache",
"/images/text.png", magic.ErrUnknownFormat, magic.ErrUnknownFormat, 2}, "/images/text.png", magic.ErrUnknownFormat, ErrNegativeCached, 1},
} }
for _, tc := range tests { for _, tc := range tests {
+46 -22
View File
@@ -18,6 +18,14 @@ import (
// write-pressure wakeups triggered by stores. // write-pressure wakeups triggered by stores.
const DefaultEvictionInterval = 5 * time.Minute const DefaultEvictionInterval = 5 * time.Minute
// reconciliationWaitFactor is how many times as long as a
// reconciliation pass took the eviction loop waits at least, after that
// pass ends, before it starts the next one. A pass reads a row and stats a
// file for every cached file, so its time grows with the cache; waiting
// ten times as long keeps reconciliation to at most a tenth of the time,
// however large the cache is.
const reconciliationWaitFactor = 10
// evictionBatchSize is how many LRU candidates of each class (variants // evictionBatchSize is how many LRU candidates of each class (variants
// and source blobs) one eviction pass fetches from the database. // and source blobs) one eviction pass fetches from the database.
const evictionBatchSize = 100 const evictionBatchSize = 100
@@ -438,10 +446,12 @@ func (c *Cache) notifyWritePressure() {
} }
// StartEviction launches the background eviction goroutine, which // StartEviction launches the background eviction goroutine, which
// reconciles the database accounting with the cache directories and
// removes the expired negative cache rows at startup and again on every // removes the expired negative cache rows at startup and again on every
// periodic tick thereafter, and evicts to the configured limit on the // periodic tick thereafter, reconciles the database accounting with the
// given periodic interval and on write-pressure notifications. On a // cache directories at startup and again on the first periodic tick
// that comes at least ten times as long after a reconciliation pass
// ended as that pass took, and evicts to the configured limit on the given
// periodic interval and on write-pressure notifications. On a
// disabled cache it only removes the expired negative cache rows, as // disabled cache it only removes the expired negative cache rows, as
// the negative cache is used either way. It is a no-op when already // the negative cache is used either way. It is a no-op when already
// started. The goroutine outlives the caller, so it runs with its own // started. The goroutine outlives the caller, so it runs with its own
@@ -482,7 +492,7 @@ func (c *Cache) StopEviction(ctx context.Context) error {
func (c *Cache) evictionLoop(ctx context.Context, interval time.Duration) { func (c *Cache) evictionLoop(ctx context.Context, interval time.Duration) {
defer close(c.evictionDone) defer close(c.evictionDone)
c.runReconciliationPass(ctx) nextReconciliation := c.runReconciliationPass(ctx)
c.runCleanExpired(ctx) c.runCleanExpired(ctx)
c.runEvictionPass(ctx) c.runEvictionPass(ctx)
@@ -494,16 +504,20 @@ func (c *Cache) evictionLoop(ctx context.Context, interval time.Duration) {
case <-ctx.Done(): case <-ctx.Done():
return return
case <-ticker.C: case <-ticker.C:
// Reconciliation walks the cache directories, so it only // Reconciliation walks the cache directories and reads a
// runs on the periodic ticker rather than on every // row for every file, so it runs only on the periodic
// write-pressure wakeup, keeping it off the per-store hot // ticker rather than on every write-pressure wakeup, and
// path. Reusing the eviction interval itself (rather than a // only on the first tick at or after nextReconciliation,
// separate, longer one) is a deliberate choice: it is the // which is ten times as long after the last pass ended as
// simplest option that still bounds how long a store's // that pass took. Passes then never run back to back,
// best-effort accounting insert can stay silently // however long one takes. A file whose best-effort
// unaccounted for to one interval, on a process that is // accounting insert failed in a store stays unaccounted for
// already running this loop regardless. // until the next pass, up to about one eviction interval
c.runReconciliationPass(ctx) // plus ten times the last pass's time.
if !time.Now().Before(nextReconciliation) {
nextReconciliation = c.runReconciliationPass(ctx)
}
c.runCleanExpired(ctx) c.runCleanExpired(ctx)
case <-c.evictionPressure: case <-c.evictionPressure:
} }
@@ -527,17 +541,25 @@ 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). It does // instead of propagating them (the loop must keep running), and returns
// nothing once ctx is cancelled. // the earliest time the next pass may start: reconciliationWaitFactor
func (c *Cache) runReconciliationPass(ctx context.Context) { // times as long after this pass ended as it took. It does nothing once
// ctx is cancelled.
func (c *Cache) runReconciliationPass(ctx context.Context) time.Time {
if ctx.Err() != nil { if ctx.Err() != nil {
return return time.Now()
} }
started := time.Now()
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)
} }
ended := time.Now()
return ended.Add(reconciliationWaitFactor * ended.Sub(started))
} }
// runCleanExpired runs CleanExpired, logging a failure instead of // runCleanExpired runs CleanExpired, logging a failure instead of
@@ -556,7 +578,8 @@ func (c *Cache) runCleanExpired(ctx context.Context) {
// reconcileAccounting synchronizes the database size accounting with // reconcileAccounting synchronizes the database size accounting with
// the actual contents of the cache directories. It runs at startup and // the actual contents of the cache directories. It runs at startup and
// again on every periodic eviction tick thereafter, off the request // then on the first periodic eviction tick that comes at least ten times
// as long after the last pass ended as that pass took, off the request
// hot path: it adopts variant files that predate the accounting table // hot path: it adopts variant files that predate the accounting table
// (or whose accounting insert failed, e.g. StoreVariant's best-effort // (or whose accounting insert failed, e.g. StoreVariant's best-effort
// insert under transient DB contention), drops accounting rows whose // insert under transient DB contention), drops accounting rows whose
@@ -566,9 +589,10 @@ func (c *Cache) runCleanExpired(ctx context.Context) {
// files left behind by crashed writes, and last checks the total cache // files left behind by crashed writes, and last checks the total cache
// usage against the tables. It reads the tables a page at a time. // usage against the tables. It reads the tables a page at a time.
// Running it periodically, not just once, bounds how long such drift can // Running it periodically, not just once, bounds how long such drift can
// accumulate unaccounted for on a long-running process to one eviction // accumulate unaccounted for on a long-running process to about one
// interval. Once ctx is cancelled, it stops at the next file or row and // eviction interval plus ten times the last pass's time.
// returns ctx's error. // 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
@@ -918,6 +918,89 @@ func TestPeriodicReconciliationAdoptsFileThatAppearsAfterStartup(t *testing.T) {
} }
} }
// TestReconciliationPassesAreSpacedByTenTimesTheirTime makes the hook
// called after every read of the reconciliation pass sleep 10 ms, runs
// the eviction loop on a 10 ms interval, and records when each of those
// calls starts and ends. A pass starts before its first call and ends
// after its last, so the time from the first call's start to the last
// call's end is at most the pass's time. The next pass's first call must
// start at least ten times that long after the last call ended.
func TestReconciliationPassesAreSpacedByTenTimesTheirTime(t *testing.T) {
t.Parallel()
cache, _ := newEvictionTestCache(t, 1<<30)
// A pass made here, before the loop starts, counts the reads one
// pass makes on this empty cache, so the reads recorded below can be
// split into passes.
readsPerPass := 0
cache.reconciliationReadTestHook = func(int) { readsPerPass++ }
err := cache.reconcileAccounting(t.Context())
if err != nil {
t.Fatalf("reconcileAccounting failed: %v", err)
}
t.Logf("one reconciliation pass makes %d reads", readsPerPass)
if readsPerPass == 0 {
t.Fatal("a reconciliation pass made no read")
}
// The pass at startup and two after it.
const passes = 3
type read struct{ start, end time.Time }
// The channel holds every read the test looks at, so none of those
// is dropped; reads after them are, and never block the loop.
reads := make(chan read, passes*readsPerPass)
cache.reconciliationReadTestHook = func(int) {
start := time.Now()
time.Sleep(10 * time.Millisecond)
select {
case reads <- read{start: start, end: time.Now()}:
default:
}
}
cache.StartEviction(10 * time.Millisecond)
defer func() { _ = cache.StopEviction(t.Context()) }()
recorded := make([]read, 0, passes*readsPerPass)
timeout := time.After(15 * time.Second)
for len(recorded) < passes*readsPerPass {
select {
case r := <-reads:
recorded = append(recorded, r)
case <-timeout:
t.Fatalf("saw %d reconciliation reads in 15 s, want %d (%d passes)",
len(recorded), passes*readsPerPass, passes)
}
}
for pass := range passes - 1 {
first := recorded[pass*readsPerPass]
last := recorded[(pass+1)*readsPerPass-1]
next := recorded[(pass+1)*readsPerPass]
passTime := last.end.Sub(first.start)
wait := next.start.Sub(last.end)
t.Logf("pass %d took at least %v, and the next pass started %v after it",
pass+1, passTime, wait)
if wait < 10*passTime {
t.Errorf("pass %d took at least %v, and the next pass started %v "+
"after it, want at least %v", pass+1, passTime, wait, 10*passTime)
}
}
}
// TestStopEvictionInterruptsPassInProgress holds the test database's // TestStopEvictionInterruptsPassInProgress holds the test database's
// only connection, so the startup reconciliation pass waits for it, and // only connection, so the startup reconciliation pass waits for it, and
// checks that StopEviction stops that pass instead of waiting for the // checks that StopEviction stops that pass instead of waiting for the
+23 -8
View File
@@ -458,13 +458,7 @@ func (s *Service) fetchAndProcess(
if err != nil { if err != nil {
// Store negative cache for certain errors // Store negative cache for certain errors
if isNegativeCacheable(err) { if isNegativeCacheable(err) {
statusCode := extractStatusCode(err) s.storeNegative(ctx, req, extractStatusCode(err), err)
storeErr := s.cache.StoreNegative(ctx, req, statusCode, err.Error())
if storeErr != nil {
s.log.Warn("failed to store negative cache entry",
"host", req.SourceHost, "path", req.SourcePath, "error", storeErr)
}
} }
return nil, fmt.Errorf("upstream fetch failed: %w", err) return nil, fmt.Errorf("upstream fetch failed: %w", err)
@@ -510,9 +504,16 @@ func (s *Service) fetchAndProcess(
"cipher", fetchResult.TLSCipherSuite, "cipher", fetchResult.TLSCipherSuite,
) )
// Validate magic bytes match content type // Validate magic bytes match content type. A body that does not is the
// upstream's error, so it gets a negative cache entry, as an upstream
// error status does.
err = magic.ValidateMagicBytes(sourceData, fetchResult.ContentType) err = magic.ValidateMagicBytes(sourceData, fetchResult.ContentType)
if err != nil { if err != nil {
s.log.Warn("source refused", "request_id", middleware.GetReqID(ctx),
"host", req.SourceHost, "path", req.SourcePath,
"content_type", fetchResult.ContentType, "error", err)
s.storeNegative(ctx, req, httpStatusUnsupportedMediaType, err)
return nil, fmt.Errorf("content validation failed: %w", err) return nil, fmt.Errorf("content validation failed: %w", err)
} }
@@ -528,6 +529,19 @@ func (s *Service) fetchAndProcess(
) )
} }
// storeNegative stores a negative cache entry for req's source, whose fetch
// failed with err, so the source is not fetched again until the entry
// expires. A failure to store the entry is only logged.
func (s *Service) storeNegative(
ctx context.Context, req *ImageRequest, statusCode int, err error,
) {
storeErr := s.cache.StoreNegative(ctx, req, statusCode, err.Error())
if storeErr != nil {
s.log.Warn("failed to store negative cache entry",
"host", req.SourceHost, "path", req.SourcePath, "error", storeErr)
}
}
// processedVariant is a variant as processAndStore made it. Each request that // processedVariant is a variant as processAndStore made it. Each request that
// shared its processing serves it through a reader of its own. // shared its processing serves it through a reader of its own.
type processedVariant struct { type processedVariant struct {
@@ -644,6 +658,7 @@ func signatureRequest(req *ImageRequest) *signature.Request {
const ( const (
httpStatusBadGateway = 502 httpStatusBadGateway = 502
httpStatusInternalError = 500 httpStatusInternalError = 500
httpStatusUnsupportedMediaType = 415
) )
// isNegativeCacheable returns true if the error should be cached. // isNegativeCacheable returns true if the error should be cached.
+2 -3
View File
@@ -19,9 +19,8 @@
# CGO image libraries (pkg-config, vips # CGO image libraries (pkg-config, vips
# with its JPEG XL support, libheif) for # with its JPEG XL support, libheif) for
# the govips bindings instead of Node: to # the govips bindings instead of Node: to
# compile pixa, in the Dockerfile's test # compile pixa outside Docker, as
# phase and build stage, which format # `make build` does
# nothing
set -eu set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
+5 -4
View File
@@ -2,11 +2,12 @@
# script/cibuild: run the CI build. It bootstraps first: a CI runner # script/cibuild: run the CI build. It bootstraps first: a CI runner
# checks out and runs this and nothing else, and script/fmt-check runs # checks out and runs this and nothing else, and script/fmt-check runs
# the formatter on the host, which a pristine checkout cannot do. # the formatter on the host, which a pristine checkout cannot do.
# A new CACHEBUST value reruns every RUN step, since a cached gate phase # A new CACHEBUST value reruns every RUN step after the ARG CACHEBUST
# is a check that did not run; unlike --no-cache, it keeps Go's build cache. # line, since a cached gate phase is a check that did not run; unlike
# --no-cache, it keeps the layers above that line and the cache mounts.
# This relies on every stage of the Dockerfile declaring ARG CACHEBUST # This relies on every stage of the Dockerfile declaring ARG CACHEBUST
# first; a stage without that line is served from cache on an unchanged # before every step that lints, tests or compiles; a stage without that
# tree, with no warning. # line is served from cache on an unchanged tree, with no warning.
set -eu set -eu
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
+5 -4
View File
@@ -1,11 +1,12 @@
#!/bin/sh #!/bin/sh
# script/docker: build the Docker image tagged with the project name. # script/docker: build the Docker image tagged with the project name.
# Identical in all repos; the tag comes from script/projectname. # Identical in all repos; the tag comes from script/projectname.
# A new CACHEBUST value reruns every RUN step, since a cached gate phase # A new CACHEBUST value reruns every RUN step after the ARG CACHEBUST
# is a check that did not run; unlike --no-cache, it keeps Go's build cache. # line, since a cached gate phase is a check that did not run; unlike
# --no-cache, it keeps the layers above that line and the cache mounts.
# This relies on every stage of the Dockerfile declaring ARG CACHEBUST # This relies on every stage of the Dockerfile declaring ARG CACHEBUST
# first; a stage without that line is served from cache on an unchanged # before every step that lints, tests or compiles; a stage without that
# tree, with no warning. # line is served from cache on an unchanged tree, with no warning.
set -eu set -eu
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
+7 -5
View File
@@ -5,11 +5,13 @@
# make its answer untrustworthy. # make its answer untrustworthy.
# #
# The phase is not the last stage in the file, so it is built only when # The phase is not the last stage in the file, so it is built only when
# --target names it. A new CACHEBUST value reruns every RUN step, since # --target names it. A new CACHEBUST value reruns every RUN step after
# a cached lint layer is a lint that did not run; unlike --no-cache, it # the ARG CACHEBUST line, since a cached lint layer is a lint that did
# keeps Go's build cache. This relies on every stage of the Dockerfile # not run; unlike --no-cache, it keeps the layers above that line and
# declaring ARG CACHEBUST first; a stage without that line is served # the cache mounts. This relies on every stage of the Dockerfile
# from cache on an unchanged tree, with no warning. # declaring ARG CACHEBUST before every step that lints, tests or
# compiles; a stage without that line is served from cache on an
# unchanged tree, with no warning.
# --output type=cacheonly writes no image, since nothing uses one. # --output type=cacheonly writes no image, since nothing uses one.
set -eu set -eu
+6 -5
View File
@@ -3,11 +3,12 @@
# and this builds that phase alone, on the same terms as script/lint: # and this builds that phase alone, on the same terms as script/lint:
# --target because a phase that is not the last stage is built only when # --target because a phase that is not the last stage is built only when
# named, and a new CACHEBUST value because a cached test layer is a test # named, and a new CACHEBUST value because a cached test layer is a test
# that did not run; unlike --no-cache, it keeps Go's build cache. This # that did not run; unlike --no-cache, it keeps the layers above the ARG
# relies on every stage of the Dockerfile declaring ARG CACHEBUST first; # CACHEBUST line and the cache mounts. This relies on every stage of the
# a stage without that line is served from cache on an unchanged tree, # Dockerfile declaring ARG CACHEBUST before every step that lints, tests
# with no warning. --output type=cacheonly writes no image, since # or compiles; a stage without that line is served from cache on an
# nothing uses one. # unchanged tree, with no warning. --output type=cacheonly writes no
# image, since nothing uses one.
set -eu set -eu
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)" SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"