10 Commits
Author SHA1 Message Date
clawbot 69f65d4302 Remove expired negative cache rows in the eviction loop (closes #244)
check / check (push) Waiting to run
A negative cache row was deleted only when its URL was requested
again, so a client asking for a new missing path each time grew the
database without limit. The eviction goroutine now removes expired rows
at startup and on every tick, so a row stays about NegativeTTL plus one
eviction interval. It deletes at most 1000 rows per statement, so a
request on the one database connection waits behind one batch at most.
The goroutine now also runs when the disk cache is disabled, since the
negative cache is used either way.

Model: opus-5-5
2026-10-15 15:02:27 +02:00
clawbot 2790125cb2 Serve a cached variant while its source URL is in the negative cache (closes #245)
check / check (push) Waiting to run
Get checked the negative cache before the variant cache, so after one
failed fetch of a source every variant of it answered 502 for five
minutes, also one on disk or one that could be made from the cached
source. The check now sits at the start of fetchAndProcess, which runs
only when no cached source can be used. A refusal from the negative
cache now counts as a miss.

Model: opus-5-5
2026-10-15 13:45:25 +02:00
clawbot c23585dd6d Lower-case the image URL's host and require a signature for a port (closes #246)
check / check (push) Waiting to run
With `cdn.example.com` allowlisted, other letter cases and a host with a port also passed the allowlist; each spelling was fetched and cached on its own with its own per-host connection limit, and a port let an unsigned URL reach any port of the host.

The image URL parser now lower-cases the host and refuses one holding `@` with 400. A host followed by `:`, with or without a port (read as Go's URL parser reads it, unbracketed IPv6 included), never counts as allowlisted, so it needs a signature. The host is signed lower-cased, as the URL writes it, so a signature made over an upper-case spelling is refused with 401.

Model: opus-5-5
2026-10-15 12:31:14 +02:00
clawbot 5d1e3ea871 Escape : and % in the signed and cached path and query (closes #247)
check / check (push) Waiting to run
The signed data and the cache key joined host, path and query with ":" and no escaping, so path /a:b with no query and path /a with query b: gave the same string: a URL signed for https://h/a:b verified for https://h/a?b:, and the two shared a cached image. Both now write each % in the path and the query as %25 and each : as %3A before joining them. The host never holds / and the path always starts with /, so the joined string reads back one way only. A path and query holding neither character are signed and keyed as before, so the existing known-answer vectors and the README example are unchanged; new vectors cover the escaped cases. Signature Specification in README.md states the escaping.

Model: opus-5-5
2026-10-15 09:37:01 +02:00
clawbot 761ade2e36 Accept a chunked upstream body of exactly the maximum size (closes #248)
check / check (push) Waiting to run
Once upstream_max_response_size bytes were read, the fetcher refused the next read without looking for the end of the body. Go's HTTP client returns the end of a body sent with a Content-Length together with its last bytes, but the end of a chunked body can come in a later read, so a chunked body of exactly the limit was refused after it was downloaded whole and the image routes answered 500. Once the limit is used up, the fetcher now reads one more byte: if the body has ended it is accepted, and if there is more it is refused with ErrResponseTooLarge.

Model: opus-5-5
2026-10-15 07:44:09 +02:00
clawbot 88e1b1cdfa Allowlist no host in the example config (closes #249)
check / check (push) Waiting to run
The example config allowlisted github.com and user-images.githubusercontent.com, where anyone can put an image, so a deployment set up as Getting Started says served every image there to anyone without a signature. The example now allowlists nothing and shows one commented entry under example.com. README.md says in Getting Started and in "Running under upaas" that every image on an allowlisted host is served without a signature, so an allowlisted host must be one the operator controls. A new test loads the example config and fails if it allowlists any host.

Model: opus-5-5
2026-10-15 06:26:23 +02:00
clawbot afb5dc5d0d Refuse a fit=outside size whose output exceeds the maximum (closes #257)
check / check (push) Successful in 15m51s
fit=outside scales the source to cover the requested size, and the side
scaled past it was held to no maximum: 1x8192?fit=outside of a 3000x600
source was resized to 40960x8192 and encoded. resize now refuses such an
output with ErrOutputTooLarge before resizing, so both image routes answer
it 400 with the error a size with one side 0 already gets. The other fit
modes make the requested size or one within it. README.md and TODO.md
say so.

Model: opus-5-5
2026-10-13 10:44:45 +02:00
clawbot d8760c40a1 Refuse a size whose computed side exceeds the maximum (closes #241)
check / check (push) Successful in 14m47s
A side given as 0 is computed from the source's proportions, and
nothing held it to 8192: 0x8192 of a 3000x600 source was resized to
40960x8192 and encoded. Process now refuses such a size with
ErrOutputTooLarge once it knows the source's dimensions, before the
resize and the encode, and both image routes answer it 400 with the
JSON error body. It is a processing error, not a failed fetch, so it
is never negative-cached. imgcache.MaxDimension is now
imageprocessor.MaxOutputDimension, so the request limit and the output
limit are one value. README.md says it under Routes and Encrypted URLs.

Model: opus-5-5
2026-10-13 09:44:44 +02:00
clawbot ca512fdb4f Remove the unused cache lifetime setting (closes #69)
check / check (push) Canceled after 0s
CacheConfig.CacheTTL was set to DefaultCacheTTL (24 hours) and never
read, so cached images never expired. Remove the field, the constant
and the line that set it. Cached source and transformed images stay
until eviction removes them to keep the cache within cache_max_bytes.

A new test caches a source and a transformed image, sets every time
pixa stores for them two days back (database rows, metadata files and
file times), and checks that both are still served from the cache
without a new upstream fetch.

Existing tests lose only the CacheTTL line of their cache config.

Model: opus-5-5
2026-10-13 07:58:55 +02:00
clawbot 52f69d3082 Refuse SVG sources and never save an unknown input as JPEG (closes #68)
check / check (push) Canceled after 0s
The upstream fetch no longer accepts image/svg+xml, and both image
routes answer 415, not 500, for any upstream Content-Type outside the
source formats. Before decoding, the image processor names the input's
format from govips' DetermineImageType and refuses, with 415, input
whose first bytes libvips reads as anything but the six source formats,
so an SVG passing the first-bytes check for AVIF never reaches librsvg.
orig of such an input is refused instead of being saved as JPEG.
README.md drops SVG from the source formats.

SVG detection stays in internal/magic, unused on the request path, for
#168. An AVIF image sequence
(brand avis) answers 415.

Model: opus-5-5
2026-10-13 07:13:17 +02:00
39 changed files with 1959 additions and 115 deletions
+43 -12
View File
@@ -26,6 +26,11 @@ make docker
docker run -p 8080:8080 -e PIXA_SIGNING_KEY="$(openssl rand -base64 32)" pixa:latest docker run -p 8080:8080 -e PIXA_SIGNING_KEY="$(openssl rand -base64 32)" pixa:latest
``` ```
The example config allowlists no host (`allowlist_hosts`), so pixa serves an
image only for a signed or encrypted URL. Every image on an allowlisted host is
served to anyone without a signature, at any size and in any format, so
allowlist only hosts you control, where nobody else can put images.
A container takes its settings from environment variables (see Configuration A container takes its settings from environment variables (see Configuration
below for the list). Only `PIXA_SIGNING_KEY` is required; if it is unset the below for the list). Only `PIXA_SIGNING_KEY` is required; if it is unset the
container exits at startup naming the variable. Everything else has a built-in container exits at startup naming the variable. Everything else has a built-in
@@ -115,7 +120,9 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for pixa needs:
- `PIXA_SIGNING_KEY` (required): secret for signed and encrypted URLs and - `PIXA_SIGNING_KEY` (required): secret for signed and encrypted URLs and
login, 32+ characters, for example from `openssl rand -base64 32` login, 32+ characters, for example from `openssl rand -base64 32`
- `PIXA_ALLOWLIST_HOSTS`: upstream hosts served without a signature, - `PIXA_ALLOWLIST_HOSTS`: upstream hosts served without a signature,
comma-separated comma-separated; default none. Every image on a listed host is served to
anyone without a signature, so list only hosts you control, where nobody
else can put images.
- `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 + what the cache holds) 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
@@ -190,9 +197,13 @@ path under `/v1/` answers 200, in maintenance mode too.
the cache and the upstream fetch; 403 when the upstream host, or a host it the cache and the upstream fetch; 403 when the upstream host, or a host it
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); 502 when the upstream answered with an `image_requests_per_minute` (below); 415 when the upstream answers with a
error status, and for 5 minutes after that for the same source URL; 503 when `Content-Type` that is not one of the source formats (below), or with an image
pixa is busy or in maintenance mode; 500 for any other failure. whose first bytes libvips reads as another format; 502 when the upstream
answered with an error status, and for 5 minutes after that for an image of
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
mode; 500 for any other failure.
- `GET` or `HEAD` `/v1/e/<token>/<name>` — an image through an encrypted URL - `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 (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 `If-None-Match` matches the image's `ETag`; 400 for a token that does not
@@ -200,7 +211,7 @@ path under `/v1/` answers 200, in maintenance mode too.
expired; 504 when the upstream has not sent its response headers within 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 `upstream_fetch_timeout`, but 500 when that time runs out while the image
itself is still arriving; 400 for an `Accept` header that is not valid, and itself is still arriving; 400 for an `Accept` header that is not valid, and
406, 403, 429, 502, 503 and 500, as for `/v1/image/`. 406, 403, 415, 429, 502, 503 and 500, as for `/v1/image/`.
- `GET /robots.txt` — asks every crawler to stay away (`Disallow: /`). Needs: - `GET /robots.txt` — asks every crawler to stay away (`Disallow: /`). Needs:
nothing. Answers: 200. nothing. Answers: 200.
- `GET /.well-known/healthcheck.json` — JSON with `status` (`ok`), `now`, - `GET /.well-known/healthcheck.json` — JSON with `status` (`ok`), `now`,
@@ -260,9 +271,17 @@ once, is refused with 400.
both optional (values under Signature Specification). Both are part of what is both optional (values under Signature Specification). Both are part of what is
cached, so each value of either is a separate cached image. cached, so each value of either is a separate cached image.
The source image may be JPEG, PNG, GIF, WebP, AVIF, JPEG XL or SVG. The A size with one side `0` has that side scaled to keep the source's proportions,
upstream's `Content-Type` must name its format, and the image's first bytes must and is refused with 400 when that side would come out above 8192, the most
match it. 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
above 8192.
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.
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,
such as an SVG whose first bytes pass the check for AVIF.
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:
@@ -340,7 +359,8 @@ nor change what it asks for.
(`POST /generate`). The format is JPEG XL unless another is chosen; a form (`POST /generate`). The format is JPEG XL unless another is chosen; a form
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. or `0`, that side is scaled to keep the image's proportions. A URL whose
scaled side would come out above 8192 answers 400.
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
@@ -399,9 +419,14 @@ HMAC-SHA256(secret, "host:path:query:width:height:format:expiration:quality:fit"
Where: Where:
- `host` — source origin hostname (e.g. `cdn.example.com`) - `host` — source host as the URL writes it up to the first `/`, in lower case,
- `path` — source path (e.g. `/photos/cat.jpg`) keeping an IPv6 address's brackets and any `:` and port after it (e.g.
- `query` — source query string, empty string if none `cdn.example.com`, `cdn.example.com:8443`, `[2001:db8::1]:8443`)
- `path` — source path (e.g. `/photos/cat.jpg`), with each `%` written as `%25`
and each `:` as `%3A`, so `/photos/a:b.jpg` is signed as `/photos/a%3Ab.jpg`
- `query` — source query string, empty string if none, with `%` and `:` written
as in `path`; neither then holds a `:`, so path `/a:b` with no query and path
`/a` with query `b:` are signed differently
- `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, one of those listed under Routes, with `original` - `format` — output format, one of those listed under Routes, with `original`
@@ -451,6 +476,12 @@ entry that is neither a host name (letters, digits, hyphens, underscores and
dots, with at most one leading dot) nor an IP address, such as one with a port dots, with at most one leading dot) nor an IP address, such as one with a port
or a `*.` wildcard, aborts startup. or a `*.` wildcard, aborts startup.
pixa lower-cases the host of a `/v1/image/` URL before anything else uses it, so
`CDN.Example.com` is matched, fetched, cached and counted against
`upstream_connections_per_host` as `cdn.example.com`. Such a URL whose host has
a port, as in `cdn.example.com:8443`, or a `:` with no port, needs a signature
even when the host is allowlisted. A host holding `@` is refused with 400.
### Configuration ### Configuration
Every setting can be given as an environment variable, in a YAML config file Every setting can be given as an environment variable, in a YAML config file
+94
View File
@@ -30,6 +30,100 @@ P2: security: per-origin rate limiting
# Completed Steps # Completed Steps
- 2026-10-15 expired negative cache rows are removed on a schedule (closes
#244): the row of a failed fetch, which makes pixa answer 502 for that source
URL for 5 minutes, was deleted only when the same URL was requested again
after that, so a client asking for a new missing path each time added rows
that stayed for good. The eviction goroutine now calls `CleanExpired` at
startup and on each periodic tick, every 5 minutes in pixad, so a row stays at
most about 10 minutes. `CleanExpired` deletes at most 1000 rows per statement,
so a request waits behind 1000 deletions at most, however many rows have
expired. The goroutine runs with the disk cache disabled
(`cache_max_bytes: 0`) too, where it does nothing else, as the negative cache
is used either way.
- 2026-10-15 a cached variant is served while its source URL is in the negative
cache (closes #245): `Get` checked the negative cache before the variant
cache, so once a fetch of the source failed, as one does when another variant
misses after the source was evicted and the upstream is down, every variant of
that source answered 502 for 5 minutes, also one on disk or one that could be
made from the cached source. The negative cache is now checked only just
before a fetch from the upstream, so a cached variant is served and a variant
not in the cache is made from the cached source when there is one. `README.md`
says so under `/v1/image/`. A refusal from the negative cache now counts as a
miss, as any other request that misses the variant cache does.
- 2026-10-15 the host of a `/v1/image/` URL is lower-cased, and one with a port
needs a signature (closes #246): with `cdn.example.com` allowlisted,
`CDN.Example.com` and `cdn.example.com:8443` passed the allowlist too, and
each spelling was fetched and cached on its own and had connections of its own
under `upstream_connections_per_host`. The image URL parser now lower-cases
the host and refuses one holding `@`, which a URL reads as a user name and
password, with 400. A host with a port, or with a `:` and no port, needs a
signature even when it is allowlisted. That is checked in the image service,
not in `allowlist.IsAllowed`, which also matches `referer_blocklist`, where a
`Referer` is blocked whatever its port. A URL is signed over its host as it
writes it, in lower case, with any IPv6 brackets, `:` and port kept.
- 2026-10-15 a `:` in the source path or query no longer moves where the path
ends and the query begins (closes #247): the signed data and the cache key
joined host, path and query with `:` and no escaping, so a URL signed for
`https://h/a:b` verified for `https://h/a?b:`; the two also shared a cache
key, and whichever was fetched first was served for both. In both, the path
and the query now have each `%` written as `%25` and each `:` as `%3A`; a path
and query holding neither are signed and keyed as before. Signature
Specification in `README.md` says so.
- 2026-10-15 the example config allowlists no host (closes #249): it allowlisted
`github.com` and `user-images.githubusercontent.com`, where anyone can put an
image, so a deployment set up as Getting Started says served those images to
anyone without a signature, at any size and in any format.
`configs/config.example.yml` now leaves `allowlist_hosts` out, with one
commented entry under `example.com`. Getting Started and "Running under upaas"
in `README.md` say that every image on an allowlisted host is served without a
signature, so an allowlisted host must be one the operator controls.
- 2026-10-15 a chunked upstream body of exactly `upstream_max_response_size`
bytes is accepted (closes #248): once the limit was used up, the fetcher's
next read refused the body without looking for its end. Go's HTTP client
returns the end of a body sent with a `Content-Length` together with its last
bytes, but the end of a chunked body can come in a later read, so such a body
was refused after it had been downloaded whole, and the image routes
answered 500. Once the limit is used up, the fetcher now reads one more byte:
if the body has ended it is accepted, and if there is more it is refused with
`ErrResponseTooLarge`, as before.
- 2026-10-13 `fit=outside` no longer makes an output above 8192 on a side
(closes #257): it scales the image to cover the requested size, and the side
scaled past that size was held to no maximum, so `1x8192?fit=outside` of a
3000x600 source was resized to 40960x8192 and encoded. The image processor now
refuses such a size with `ErrOutputTooLarge` before resizing, and both image
routes answer it 400, as for a size with one side `0`. The other fit modes
make the requested size or one within it, so none can go above 8192.
- 2026-10-13 a size with one side `0` no longer makes an output above 8192 on a
side (closes #241): that side is computed from the source's proportions, and
nothing held it to the 8192 a requested side is held to, so `0x8192` of a
3000x600 source was resized to 40960x8192 and encoded. The image processor now
refuses such a size with `ErrOutputTooLarge` once it knows the source's
dimensions, before resizing, and both image routes answer it 400 with the JSON
error body. The refusal is not cached as a failed fetch, so the same source is
still served at other sizes. `imgcache.MaxDimension` is now the image
processor's `MaxOutputDimension`, so the two cannot differ.
- 2026-10-13 the unused cache lifetime is removed (closes #69):
`CacheConfig.CacheTTL`, which pixad set to `DefaultCacheTTL` (24 hours), was
never read, so cached images never expired. The field and the constant are
removed. Cached source and transformed images stay until eviction removes them
to keep the cache within `cache_max_bytes`.
- 2026-10-13 SVG sources are refused, and `orig` is never saved as JPEG in place
of the source's own format (closes #68): the upstream fetch no longer accepts
`image/svg+xml`, which libvips would render. Both image routes answer 415 for
an upstream `Content-Type` that is not one of the source formats, SVG
included, where they answered 500. Before decoding, the image processor
refuses with `ErrUnsupportedInputFormat`, also answered 415, any input that
govips' `DetermineImageType`, which picks the libvips loader from the first
bytes, does not name as a source format. That catches an SVG whose first bytes
pass the first-bytes check for another type, as an XML comment can make them
do for AVIF; an SVG whose first bytes do not pass that check answers 500, as
before, and so does a damaged image in a source format, whose first bytes
`DetermineImageType` names. `orig` is always the source's own format, where a
source the processor could not name was saved as JPEG. `README.md` no longer
lists SVG. The SVG detection in `internal/magic` stays: the request path does
not call it, and it goes with the unused code of
https://git.eeqj.de/sneak/pixa/issues/168.
- 2026-10-12 per-client limit on the image routes (closes #236): the new setting - 2026-10-12 per-client limit on the image routes (closes #236): the new setting
`image_requests_per_minute` (`PIXA_IMAGE_REQUESTS_PER_MINUTE`), default 600, `image_requests_per_minute` (`PIXA_IMAGE_REQUESTS_PER_MINUTE`), default 600,
is how many `GET` and `HEAD` requests one client may send to `/v1/image/` and is how many `GET` and `HEAD` requests one client may send to `/v1/image/` and
+5 -6
View File
@@ -45,15 +45,14 @@ state_dir: ./data
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 (default: none) # Hosts that don't require signatures (default: none)
# Every image on an allowlisted host is served to anyone without a
# signature, at any size and in any format, so list only hosts you
# control, where nobody else can put images.
# 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")
# An entry that is neither a host name nor an IP address (IPv6 without # An entry that is neither a host name nor an IP address (IPv6 without
# brackets), such as one with a port or a "*." wildcard, aborts startup. # brackets), such as one with a port or a "*." wildcard, aborts startup.
allowlist_hosts: # allowlist_hosts:
- s3.sneak.cloud # - images.example.com
- static.sneak.cloud
- sneak.berlin
- github.com
- user-images.githubusercontent.com
# Hosts whose pages may not show pixa's images, written as for # Hosts whose pages may not show pixa's images, written as for
# allowlist_hosts. A request to /v1/image/ or /v1/e/ whose Referer header # allowlist_hosts. A request to /v1/image/ or /v1/e/ whose Referer header
+26
View File
@@ -92,6 +92,32 @@ func TestGetStringSlice_Empty(t *testing.T) {
} }
} }
// TestExampleConfigAllowlistsNoHost loads configs/config.example.yml
// with a real signing key, as Getting Started in README.md has it copied
// and edited, and checks that it allowlists no host: every image on an
// allowlisted host is served to anyone without a signature.
func TestExampleConfigAllowlistsNoHost(t *testing.T) {
t.Setenv("PIXA_SIGNING_KEY", validTestSigningKey)
examplePath := filepath.Join("..", "..", "configs", "config.example.yml")
sc, err := loadTestConfig(examplePath)
if err != nil {
t.Fatalf("failed to load %s: %v", examplePath, err)
}
c, err := newFromSmartConfig(sc)
if err != nil {
t.Fatalf("%s is not a valid config: %v", examplePath, err)
}
t.Logf("allowlist_hosts in %s: %v", examplePath, c.AllowlistHosts)
if len(c.AllowlistHosts) != 0 {
t.Errorf("%s allowlists %v, want no host", examplePath, c.AllowlistHosts)
}
}
// loadTestConfig is a helper to load a config file for testing. // loadTestConfig is a helper to load a config file for testing.
func loadTestConfig(path string) (*smartconfig.Config, error) { func loadTestConfig(path string) (*smartconfig.Config, error) {
return smartconfig.NewFromConfigPath(path) return smartconfig.NewFromConfigPath(path)
+2 -2
View File
@@ -106,7 +106,6 @@ func (s *Handlers) WaitForProcessing(ctx context.Context) int {
func newCacheConfig(cfg *config.Config, log *slog.Logger) imgcache.CacheConfig { func newCacheConfig(cfg *config.Config, log *slog.Logger) imgcache.CacheConfig {
return imgcache.CacheConfig{ return imgcache.CacheConfig{
StateDir: cfg.StateDir, StateDir: cfg.StateDir,
CacheTTL: imgcache.DefaultCacheTTL,
NegativeTTL: imgcache.DefaultNegativeTTL, NegativeTTL: imgcache.DefaultNegativeTTL,
MaxBytes: cfg.CacheMaxBytes, MaxBytes: cfg.CacheMaxBytes,
UseDefaultMaxBytes: !cfg.CacheMaxBytesExplicit, UseDefaultMaxBytes: !cfg.CacheMaxBytesExplicit,
@@ -125,7 +124,8 @@ func (s *Handlers) initImageService() error {
s.imgCache = cache s.imgCache = cache
// Background eviction: startup reconciliation, then periodic and // Background eviction: startup reconciliation, then periodic and
// write-pressure passes. No-op when the disk cache is disabled. // write-pressure passes, and the removal of expired negative cache
// rows, which is all it does when the disk cache is disabled.
cache.StartEviction(imgcache.DefaultEvictionInterval) cache.StartEviction(imgcache.DefaultEvictionInterval)
// Writes the counts requests could not write by their deadline // Writes the counts requests could not write by their deadline
@@ -47,7 +47,6 @@ func setupTestHandler(t *testing.T) *testFixtures {
cache, err := imgcache.NewCache(db, imgcache.CacheConfig{ cache, err := imgcache.NewCache(db, imgcache.CacheConfig{
StateDir: tmpDir, StateDir: tmpDir,
CacheTTL: time.Hour,
NegativeTTL: 5 * time.Minute, NegativeTTL: 5 * time.Minute,
}) })
if err != nil { if err != nil {
+16
View File
@@ -247,6 +247,22 @@ func (s *Handlers) respondImageError(
return return
} }
if errors.Is(err, httpfetcher.ErrInvalidContentType) ||
errors.Is(err, imageprocessor.ErrUnsupportedInputFormat) {
s.respondError(w, "unsupported source format",
http.StatusUnsupportedMediaType)
return
}
if errors.Is(err, imageprocessor.ErrOutputTooLarge) {
s.respondError(w, fmt.Sprintf("size too large for the source: the side "+
"computed from its proportions is above %d",
imageprocessor.MaxOutputDimension), http.StatusBadRequest)
return
}
s.respondError(w, "internal error", http.StatusInternalServerError) s.respondError(w, "internal error", http.StatusInternalServerError)
} }
@@ -31,7 +31,6 @@ func newSignedHostServer(
cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{ cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{
StateDir: t.TempDir(), StateDir: t.TempDir(),
CacheTTL: time.Hour,
NegativeTTL: 5 * time.Minute, NegativeTTL: 5 * time.Minute,
}) })
if err != nil { if err != nil {
@@ -29,7 +29,6 @@ func newImageRoute(t *testing.T, fetcher httpfetcher.Fetcher) http.Handler {
cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{ cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{
StateDir: t.TempDir(), StateDir: t.TempDir(),
CacheTTL: time.Hour,
NegativeTTL: 5 * time.Minute, NegativeTTL: 5 * time.Minute,
}) })
if err != nil { if err != nil {
+8
View File
@@ -161,6 +161,14 @@ func (s *Handlers) handleImageError(w http.ResponseWriter, err error) {
errors.Is(err, imageprocessor.ErrTooManyImages): errors.Is(err, imageprocessor.ErrTooManyImages):
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),
errors.Is(err, imageprocessor.ErrUnsupportedInputFormat):
s.respondError(w, "unsupported source format",
http.StatusUnsupportedMediaType)
case errors.Is(err, imageprocessor.ErrOutputTooLarge):
s.respondError(w, fmt.Sprintf("size too large for the source: the side "+
"computed from its proportions is above %d",
imageprocessor.MaxOutputDimension), 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)
@@ -49,7 +49,6 @@ func newRefererRoutes(t *testing.T) (http.Handler, *Handlers, *countingFetcher)
cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{ cache, err := imgcache.NewCache(setupTestDB(t), imgcache.CacheConfig{
StateDir: t.TempDir(), StateDir: t.TempDir(),
CacheTTL: time.Hour,
NegativeTTL: 5 * time.Minute, NegativeTTL: 5 * time.Minute,
}) })
if err != nil { if err != nil {
@@ -0,0 +1,54 @@
package handlers
import (
"fmt"
"log/slog"
"net/http"
"net/http/httptest"
"strings"
"testing"
"sneak.berlin/go/pixa/internal/httpfetcher"
"sneak.berlin/go/pixa/internal/imageprocessor"
"sneak.berlin/go/pixa/internal/imgcache"
)
// TestUnsupportedSourceFormatAnswers415 checks that both image routes answer
// 415 with a clear error when the upstream answers with a content type pixa
// does not accept, such as SVG, and when orig is asked of a source in a
// format the processor cannot name, wrapped as the service wraps them.
func TestUnsupportedSourceFormatAnswers415(t *testing.T) {
t.Parallel()
h := &Handlers{log: slog.New(slog.DiscardHandler)}
req := &imgcache.ImageRequest{SourceHost: "img.example.com", SourcePath: "/a.svg"}
for _, err := range []error{
fmt.Errorf("upstream fetch failed: %w: image/svg+xml",
httpfetcher.ErrInvalidContentType),
fmt.Errorf("image processing failed: %w: unknown",
imageprocessor.ErrUnsupportedInputFormat),
} {
plain := httptest.NewRecorder()
h.respondImageError(plain, req, err)
encrypted := httptest.NewRecorder()
h.handleImageError(encrypted, err)
for route, rec := range map[string]*httptest.ResponseRecorder{
"HandleImage": plain, "HandleImageEnc": encrypted,
} {
t.Logf("%s for %v: %d %s", route, err, rec.Code, rec.Body.String())
if rec.Code != http.StatusUnsupportedMediaType {
t.Errorf("%s for %v: status = %d, want %d",
route, err, rec.Code, http.StatusUnsupportedMediaType)
}
if !strings.Contains(rec.Body.String(), "unsupported source format") {
t.Errorf("%s for %v: body = %q, want the unsupported source format error",
route, err, rec.Body.String())
}
}
}
}
+134
View File
@@ -9,6 +9,7 @@ import (
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"slices" "slices"
"strconv"
"strings" "strings"
"sync" "sync"
"testing" "testing"
@@ -297,6 +298,139 @@ func TestFetchMaxResponseSizeEnforced(t *testing.T) {
} }
} }
// fetchWithMaxResponseSize serves handler as the fake upstream and fetches
// its /image route with MaxResponseSize set to maxResponseSize.
func fetchWithMaxResponseSize(
t *testing.T,
maxResponseSize int64,
handler http.HandlerFunc,
) *FetchResult {
t.Helper()
srv := httptest.NewServer(handler)
t.Cleanup(srv.Close)
cfg := DefaultConfig()
cfg.MaxResponseSize = maxResponseSize
f, _ := newServerFetcher(t, srv, cfg)
res := fetchImage(t, f, "/image")
t.Cleanup(func() { _ = res.Content.Close() })
return res
}
// TestFetchContentLengthBodyAtMaxResponseSize checks that a body sent with a
// Content-Length is read whole at exactly MaxResponseSize bytes and refused
// at one byte more.
func TestFetchContentLengthBodyAtMaxResponseSize(t *testing.T) {
t.Parallel()
const maxResponseSize = 8
for _, tc := range []struct {
size int
wantErr error
}{
{maxResponseSize, nil},
{maxResponseSize + 1, ErrResponseTooLarge},
} {
t.Run(strconv.Itoa(tc.size)+" bytes", func(t *testing.T) {
t.Parallel()
body := strings.Repeat("x", tc.size)
res := fetchWithMaxResponseSize(t, maxResponseSize,
func(w http.ResponseWriter, _ *http.Request) {
w.Header().Set("Content-Type", contentTypeJPEG)
w.Header().Set("Content-Length", strconv.Itoa(tc.size))
_, _ = io.WriteString(w, body)
})
if res.ContentLength != int64(tc.size) {
t.Fatalf("ContentLength = %d, want %d", res.ContentLength, tc.size)
}
data, err := io.ReadAll(res.Content)
t.Logf("body of %d bytes: read %d, error %v", tc.size, len(data), err)
if !errors.Is(err, tc.wantErr) {
t.Fatalf("read error = %v, want %v", err, tc.wantErr)
}
if err == nil && string(data) != body {
t.Errorf("read %q, want %q", data, body)
}
})
}
}
// TestFetchChunkedBodyAtMaxResponseSize checks that a chunked body is read
// whole at exactly MaxResponseSize bytes and refused at one byte more. The
// upstream sends the end of the body only once the test has read
// MaxResponseSize bytes, so the read after them comes before the end of the
// body has arrived, as it can from a real upstream.
func TestFetchChunkedBodyAtMaxResponseSize(t *testing.T) {
t.Parallel()
const maxResponseSize = 8
for _, tc := range []struct {
size int
wantErr error
}{
{maxResponseSize, nil},
{maxResponseSize + 1, ErrResponseTooLarge},
} {
t.Run(strconv.Itoa(tc.size)+" bytes", func(t *testing.T) {
t.Parallel()
body := strings.Repeat("x", tc.size)
endBody := make(chan struct{})
res := fetchWithMaxResponseSize(t, maxResponseSize,
func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", contentTypeJPEG)
_, _ = io.WriteString(w, body)
// With no Content-Length, flushing sends the body
// chunked; its end is sent when the handler returns.
_ = http.NewResponseController(w).Flush()
select {
case <-endBody:
case <-r.Context().Done():
}
})
if res.ContentLength != -1 {
t.Fatalf("ContentLength = %d, want -1 (chunked)", res.ContentLength)
}
head := make([]byte, maxResponseSize)
_, err := io.ReadFull(res.Content, head)
if err != nil {
t.Fatalf("read the first %d bytes: %v", maxResponseSize, err)
}
close(endBody)
rest, err := io.ReadAll(res.Content)
data := string(head) + string(rest)
t.Logf("body of %d bytes: read %d, error %v", tc.size, len(data), err)
if !errors.Is(err, tc.wantErr) {
t.Fatalf("read error = %v, want %v", err, tc.wantErr)
}
if err == nil && data != body {
t.Errorf("read %q, want %q", data, body)
}
})
}
}
func TestFetchSemaphoreReleasedOnError(t *testing.T) { func TestFetchSemaphoreReleasedOnError(t *testing.T) {
t.Parallel() t.Parallel()
+21 -3
View File
@@ -158,7 +158,6 @@ func DefaultConfig() *Config {
contentTypeWebP, contentTypeWebP,
contentTypeAVIF, contentTypeAVIF,
contentTypeJXL, contentTypeJXL,
contentTypeSVG,
}, },
AllowHTTP: false, AllowHTTP: false,
MaxConnectionsPerHost: DefaultMaxConnectionsPerHost, MaxConnectionsPerHost: DefaultMaxConnectionsPerHost,
@@ -661,14 +660,33 @@ func dialSSRFSafe(
return nil, fmt.Errorf("%w to %s", errConnectFailed, host) return nil, fmt.Errorf("%w to %s", errConnectFailed, host)
} }
// limitedReader wraps a reader and limits the number of bytes read. // limitedReader wraps a reader and refuses a body longer than remaining
// bytes with ErrResponseTooLarge. remaining goes below 0 once the body is
// found to be longer.
type limitedReader struct { type limitedReader struct {
reader io.Reader reader io.Reader
remaining int64 remaining int64
} }
// Read reads at most the remaining bytes. Once they are read, it reads one
// more byte to tell the end of the body from more data: at the end it
// returns the wrapped reader's error, io.EOF; with more data it returns
// ErrResponseTooLarge, as does every later Read.
func (r *limitedReader) Read(p []byte) (int, error) { func (r *limitedReader) Read(p []byte) (int, error) {
if r.remaining <= 0 { if r.remaining < 0 {
return 0, ErrResponseTooLarge
}
if r.remaining == 0 {
var extra [1]byte
n, err := r.reader.Read(extra[:])
r.remaining -= int64(n)
if n == 0 {
return 0, err
}
return 0, ErrResponseTooLarge return 0, ErrResponseTooLarge
} }
@@ -0,0 +1,52 @@
package imageprocessor
import (
"bytes"
"errors"
"testing"
)
// TestImageProcessor_RefusesComputedSideAboveMaximum verifies that a size
// with one side 0, which Process computes from the input's proportions, is
// served when that side comes out at MaxOutputDimension and refused with
// ErrOutputTooLarge when it would be above. A 4096x1 or 1x4096 input given
// 2 for its short side has 8192 for its long side.
func TestImageProcessor_RefusesComputedSideAboveMaximum(t *testing.T) {
t.Parallel()
tests := []struct {
name string
inputWidth, inputHeight int
size Size
wantErr error
}{
{"width at the maximum", 4096, 1, Size{Height: 2}, nil},
{"width above the maximum", 4096, 1, Size{Height: 3}, ErrOutputTooLarge},
{"height at the maximum", 1, 4096, Size{Width: 2}, nil},
{"height above the maximum", 1, 4096, Size{Width: 3}, ErrOutputTooLarge},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
input := createTestJPEG(t, tt.inputWidth, tt.inputHeight)
result, err := New(Params{}).Process(t.Context(),
bytes.NewReader(input),
&Request{Size: tt.size, Format: FormatJPEG, FitMode: FitCover})
if result != nil {
t.Logf("Process() made %dx%d", result.Width, result.Height)
if max(result.Width, result.Height) != MaxOutputDimension {
t.Errorf("Process() made %dx%d, want a side of %d",
result.Width, result.Height, MaxOutputDimension)
}
}
if !errors.Is(err, tt.wantErr) {
t.Errorf("Process() error = %v, want %v", err, tt.wantErr)
}
})
}
}
@@ -0,0 +1,62 @@
package imageprocessor
import (
"bytes"
"errors"
"testing"
)
// TestImageProcessor_RefusesFitOutsideAboveMaximum verifies that FitOutside,
// which scales the input to cover the requested size and so past it on one
// side, is refused with ErrOutputTooLarge when that side would come out above
// MaxOutputDimension, and otherwise makes the scaled size. A 1024x256 or
// 256x1024 input scaled by 8 has 8192 for its long side.
func TestImageProcessor_RefusesFitOutsideAboveMaximum(t *testing.T) {
t.Parallel()
tests := []struct {
name string
inputWidth, inputHeight int
size Size
// wantWidth and wantHeight are the output's, 0 for a refusal
wantWidth, wantHeight int
}{
{"width above the maximum", 3000, 600, Size{Width: 1, Height: 8192}, 0, 0},
{"height above the maximum", 600, 3000, Size{Width: 8192, Height: 1}, 0, 0},
{"width at the maximum", 1024, 256, Size{Width: 1, Height: 2048}, 8192, 2048},
{"height at the maximum", 256, 1024, Size{Width: 2048, Height: 1}, 2048, 8192},
{"within the maximum", 3000, 600, Size{Width: 1, Height: 1200}, 6000, 1200},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
input := createTestJPEG(t, tt.inputWidth, tt.inputHeight)
result, err := New(Params{}).Process(t.Context(),
bytes.NewReader(input),
&Request{Size: tt.size, Format: FormatJPEG, FitMode: FitOutside})
if result != nil {
t.Logf("Process() made %dx%d", result.Width, result.Height)
}
if tt.wantWidth == 0 {
if !errors.Is(err, ErrOutputTooLarge) {
t.Errorf("Process() error = %v, want %v", err, ErrOutputTooLarge)
}
return
}
if err != nil {
t.Fatalf("Process() error = %v", err)
}
if result.Width != tt.wantWidth || result.Height != tt.wantHeight {
t.Errorf("Process() made %dx%d, want %dx%d",
result.Width, result.Height, tt.wantWidth, tt.wantHeight)
}
})
}
}
+73 -30
View File
@@ -133,10 +133,26 @@ var ErrInputTooLarge = errors.New("input image dimensions exceed maximum")
// configured byte limit. // configured byte limit.
var ErrInputDataTooLarge = errors.New("input data exceeds maximum allowed size") var ErrInputDataTooLarge = errors.New("input data exceeds maximum allowed size")
// MaxOutputDimension is the maximum width or height of an output image, and
// so of a requested size (imgcache.MaxDimension).
const MaxOutputDimension = 8192
// ErrOutputTooLarge is returned, before resizing, when the output would have a
// side above MaxOutputDimension: the side of a size with one side 0, computed
// from the input's proportions, or with FitOutside, the side scaled past the
// requested size.
var ErrOutputTooLarge = errors.New("output image dimensions exceed maximum")
// 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")
// ErrUnsupportedInputFormat is returned, before the input is decoded, when
// libvips reads the input's first bytes as another format than the source
// formats, or as none: for example an SVG whose first bytes pass the
// first-bytes check for AVIF.
var ErrUnsupportedInputFormat = errors.New("unsupported input format")
// ErrTooManyImages is returned when MaxConcurrentProcessing images are being // ErrTooManyImages is returned when MaxConcurrentProcessing images are being
// processed and none finishes within ProcessingWaitTimeout. // processed and none finishes within ProcessingWaitTimeout.
var ErrTooManyImages = errors.New("too many images being processed at once") var ErrTooManyImages = errors.New("too many images being processed at once")
@@ -219,6 +235,17 @@ func (p *ImageProcessor) Process(
return nil, ErrInputDataTooLarge return nil, ErrInputDataTooLarge
} }
// govips picks the libvips loader from the input's first bytes with
// DetermineImageType. Input whose first bytes it reads as no source
// format, such as SVG, which librsvg would render, is refused before
// libvips reads it.
inputFormat := p.detectFormat(vips.DetermineImageType(data))
sourceFormat, err := p.formatFromString(inputFormat)
if err != nil {
return nil, err
}
// Decode image // Decode image
img, err := vips.NewImageFromBuffer(data) img, err := vips.NewImageFromBuffer(data)
if err != nil { if err != nil {
@@ -237,16 +264,16 @@ func (p *ImageProcessor) Process(
origWidth := img.Width() origWidth := img.Width()
origHeight := img.Height() origHeight := img.Height()
// Detect input format
inputFormat := p.detectFormat(img)
// Validate input dimensions to prevent DoS via decompression bombs // Validate input dimensions to prevent DoS via decompression bombs
if origWidth > MaxInputDimension || origHeight > MaxInputDimension { if origWidth > MaxInputDimension || origHeight > MaxInputDimension {
return nil, ErrInputTooLarge return nil, ErrInputTooLarge
} }
// Determine target dimensions // Determine target dimensions; too large ones are refused before resizing
targetWidth, targetHeight := targetDimensions(req.Size, origWidth, origHeight) targetWidth, targetHeight, err := targetDimensions(req.Size, origWidth, origHeight)
if err != nil {
return nil, err
}
// Resize if needed // Resize if needed
if targetWidth != origWidth || targetHeight != origHeight { if targetWidth != origWidth || targetHeight != origHeight {
@@ -259,7 +286,7 @@ func (p *ImageProcessor) Process(
// 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 = p.formatFromString(inputFormat) outputFormat = sourceFormat
} }
// Encode to target format // Encode to target format
@@ -282,21 +309,29 @@ func (p *ImageProcessor) Process(
// targetDimensions calculates the output dimensions for a requested size, // targetDimensions calculates the output dimensions for a requested size,
// scaling proportionally when only one dimension is given and keeping the // scaling proportionally when only one dimension is given and keeping the
// original dimensions when both are zero. // original dimensions when both are zero. A side scaled proportionally can
func targetDimensions(size Size, origWidth, origHeight int) (int, int) { // come out above MaxOutputDimension, which is ErrOutputTooLarge.
func targetDimensions(size Size, origWidth, origHeight int) (int, int, error) {
width, height := size.Width, size.Height
switch { switch {
case size.Width == 0 && size.Height == 0: case width == 0 && height == 0:
// Both are 0: keep original size // Both are 0: keep original size
return origWidth, origHeight return origWidth, origHeight, nil
case size.Width == 0: case width == 0:
// Only height specified: calculate width proportionally // Only height specified: calculate width proportionally
return origWidth * size.Height / origHeight, size.Height width = origWidth * height / origHeight
case size.Height == 0: case height == 0:
// Only width specified: calculate height proportionally // Only width specified: calculate height proportionally
return size.Width, origHeight * size.Width / origWidth height = origHeight * width / origWidth
default:
return size.Width, size.Height
} }
if width > MaxOutputDimension || height > MaxOutputDimension {
return 0, 0, fmt.Errorf("%w: %dx%d is above %d", ErrOutputTooLarge,
width, height, MaxOutputDimension)
}
return width, height, nil
} }
// MIME types for the supported image formats. // MIME types for the supported image formats.
@@ -403,10 +438,9 @@ func (p *ImageProcessor) acquireSlot(ctx context.Context) (func(), error) {
} }
} }
// detectFormat returns the format string from a vips image. // detectFormat returns the format string for a govips image type.
func (p *ImageProcessor) detectFormat(img *vips.ImageRef) string { func (p *ImageProcessor) detectFormat(imageType vips.ImageType) string {
format := img.Format() switch imageType {
switch format {
case vips.ImageTypeJPEG: case vips.ImageTypeJPEG:
return "jpeg" return "jpeg"
case vips.ImageTypePNG: case vips.ImageTypePNG:
@@ -428,7 +462,9 @@ func (p *ImageProcessor) detectFormat(img *vips.ImageRef) string {
} }
} }
// resize resizes the image according to the fit mode. // resize resizes the image according to the fit mode. With FitOutside one
// side can come out larger than requested; when it would be above
// MaxOutputDimension, resize returns ErrOutputTooLarge without resizing.
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 {
@@ -476,6 +512,12 @@ func (p *ImageProcessor) resize(
newW := int(float64(imgW) * scale) newW := int(float64(imgW) * scale)
newH := int(float64(imgH) * scale) newH := 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) return img.Thumbnail(newW, newH, vips.InterestingNone)
default: default:
@@ -687,22 +729,23 @@ func jxlDistance(quality int) float64 {
return 53.0/3000.0*q*q - 23.0/20.0*q + 25.0 return 53.0/3000.0*q*q - 23.0/20.0*q + 25.0
} }
// formatFromString converts a format string to Format. // formatFromString converts a format string from detectFormat to Format. A
func (p *ImageProcessor) formatFromString(format string) Format { // format detectFormat could not name is ErrUnsupportedInputFormat.
func (p *ImageProcessor) formatFromString(format string) (Format, error) {
switch format { switch format {
case "jpeg": case "jpeg":
return FormatJPEG return FormatJPEG, nil
case "png": case "png":
return FormatPNG return FormatPNG, nil
case "gif": case "gif":
return FormatGIF return FormatGIF, nil
case "webp": case "webp":
return FormatWebP return FormatWebP, nil
case string(FormatAVIF): case string(FormatAVIF):
return FormatAVIF return FormatAVIF, nil
case string(FormatJXL): case string(FormatJXL):
return FormatJXL return FormatJXL, nil
default: default:
return FormatJPEG return "", fmt.Errorf("%w: %s", ErrUnsupportedInputFormat, format)
} }
} }
@@ -0,0 +1,39 @@
package imageprocessor
import (
"bytes"
"errors"
"testing"
"github.com/davidbyttow/govips/v2/vips"
)
// TestImageProcessor_OrigOfUnnamedFormatRefused verifies that orig of a TIFF
// input, which is not a source format, is refused with
// ErrUnsupportedInputFormat before it is decoded, and not saved as JPEG.
func TestImageProcessor_OrigOfUnnamedFormatRefused(t *testing.T) {
t.Parallel()
img, err := vips.NewImageFromBuffer(createTestPNG(t, 20, 20))
if err != nil {
t.Fatalf("failed to decode test PNG: %v", err)
}
t.Cleanup(img.Close)
tiff, _, err := img.ExportTiff(vips.NewTiffExportParams())
if err != nil {
t.Fatalf("failed to save test TIFF: %v", err)
}
result, err := New(Params{}).Process(
t.Context(), bytes.NewReader(tiff), &Request{Format: FormatOriginal},
)
if result != nil {
t.Logf("Process() returned %s", result.ContentType)
}
if !errors.Is(err, ErrUnsupportedInputFormat) {
t.Errorf("Process() error = %v, want %v", err, ErrUnsupportedInputFormat)
}
}
@@ -0,0 +1,46 @@
package imageprocessor
import (
"bytes"
"io"
"testing"
)
// TestImageProcessor_LoadsEachSourceFormat verifies that an image in each of
// the six source formats, saved by the processor itself, is loaded and saved
// again in its own format (orig).
func TestImageProcessor_LoadsEachSourceFormat(t *testing.T) {
t.Parallel()
proc := New(Params{})
for _, format := range []Format{
FormatJPEG, FormatPNG, FormatGIF, FormatWebP, FormatAVIF, FormatJXL,
} {
saved, err := proc.Process(t.Context(),
bytes.NewReader(createTestPNG(t, 20, 20)), &Request{Format: format})
if err != nil {
t.Fatalf("saving the test image as %s: %v", format, err)
}
source, err := io.ReadAll(saved.Content)
if err != nil {
t.Fatalf("reading the %s image: %v", format, err)
}
result, err := proc.Process(t.Context(), bytes.NewReader(source),
&Request{Format: FormatOriginal})
if err != nil {
t.Errorf("orig of the %s image: %v", format, err)
continue
}
t.Logf("orig of the %s image: %s", format, result.ContentType)
if result.ContentType != FormatToMIME(format) {
t.Errorf("orig of the %s image is %s, want %s",
format, result.ContentType, FormatToMIME(format))
}
}
}
+52 -12
View File
@@ -32,10 +32,15 @@ const httpStatusOK = 200
// variant not among them is served as before, reading its .meta file. // variant not among them is served as before, reading its .meta file.
const metaCacheSize = 10000 const metaCacheSize = 10000
// cleanExpiredBatchSize is the most expired negative cache rows one
// statement of CleanExpired deletes. Each batch is a statement of its
// own, so a request waits behind one batch at most, however many rows
// have expired.
const cleanExpiredBatchSize = 1000
// CacheConfig holds cache configuration. // CacheConfig holds cache configuration.
type CacheConfig struct { type CacheConfig struct {
StateDir string StateDir string
CacheTTL time.Duration
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
@@ -51,8 +56,9 @@ type CacheConfig struct {
// 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 the eviction goroutine only removes the expired
// this when the operator configures cache_max_bytes: 0. // negative cache rows. The config layer sets this when the
// operator configures cache_max_bytes: 0.
DisableDiskCache bool DisableDiskCache bool
// Logger receives accounting and eviction log output. A nil // Logger receives accounting and eviction log output. A nil
@@ -483,17 +489,27 @@ func (c *Cache) GetSourceContent(
return c.srcContent.LoadWithSize(contentHash) return c.srcContent.LoadWithSize(contentHash)
} }
// CleanExpired removes expired entries from the cache. // CleanExpired removes the negative cache rows past their expiry, one
// batch per statement, until none is left. Once ctx ends it starts no
// further batch and returns ctx's error. The eviction goroutine calls it
// at startup and on every periodic tick, so a row stays about
// NegativeTTL plus one eviction interval, whether or not its URL is
// requested again.
func (c *Cache) CleanExpired(ctx context.Context) error { func (c *Cache) CleanExpired(ctx context.Context) error {
// Clean expired negative cache entries for {
_, err := c.db.ExecContext(ctx, ` if ctx.Err() != nil {
DELETE FROM negative_cache WHERE expires_at < CURRENT_TIMESTAMP return ctx.Err()
`) }
if err != nil {
return fmt.Errorf("failed to clean negative cache: %w", err)
}
return nil deleted, err := c.cleanExpiredBatch(ctx)
if err != nil {
return err
}
if deleted < cleanExpiredBatchSize {
return nil
}
}
} }
// Stats returns cache statistics. // Stats returns cache statistics.
@@ -757,3 +773,27 @@ func (c *Cache) checkNegativeCache(
return true, nil return true, nil
} }
// cleanExpiredBatch deletes up to cleanExpiredBatchSize negative cache
// rows past their expiry in one statement, and returns how many it
// deleted. DELETE ... LIMIT needs a SQLite build option, so the subquery
// picks the rows.
func (c *Cache) cleanExpiredBatch(ctx context.Context) (int64, error) {
result, err := c.db.ExecContext(ctx, `
DELETE FROM negative_cache WHERE id IN (
SELECT id FROM negative_cache
WHERE expires_at < CURRENT_TIMESTAMP
LIMIT ?
)
`, cleanExpiredBatchSize)
if err != nil {
return 0, fmt.Errorf("failed to clean negative cache: %w", err)
}
deleted, err := result.RowsAffected()
if err != nil {
return 0, fmt.Errorf("failed to count cleaned negative cache rows: %w", err)
}
return deleted, nil
}
-2
View File
@@ -102,7 +102,6 @@ func setupTestCache(t *testing.T) *Cache {
cache, err := NewCache(db, CacheConfig{ cache, err := NewCache(db, CacheConfig{
StateDir: tmpDir, StateDir: tmpDir,
CacheTTL: time.Hour,
NegativeTTL: 5 * time.Minute, NegativeTTL: 5 * time.Minute,
}) })
if err != nil { if err != nil {
@@ -241,7 +240,6 @@ func TestCache_NegativeCacheExpiry(t *testing.T) {
// Very short negative TTL for testing // Very short negative TTL for testing
cache, err := NewCache(db, CacheConfig{ cache, err := NewCache(db, CacheConfig{
StateDir: tmpDir, StateDir: tmpDir,
CacheTTL: time.Hour,
NegativeTTL: 1 * time.Millisecond, NegativeTTL: 1 * time.Millisecond,
}) })
if err != nil { if err != nil {
+25 -7
View File
@@ -437,14 +437,16 @@ 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 at // reconciles the database accounting with the cache directories and
// startup and again on every periodic tick thereafter, and evicts to // removes the expired negative cache rows at startup and again on every
// the configured limit on the given periodic interval and on // periodic tick thereafter, and evicts to the configured limit on the
// write-pressure notifications. It is a no-op on a disabled cache or // given periodic interval and on write-pressure notifications. On a
// when already started. The goroutine outlives the caller, so it runs // disabled cache it only removes the expired negative cache rows, as
// with its own context, which StopEviction cancels. // the negative cache is used either way. It is a no-op when already
// started. The goroutine outlives the caller, so it runs with its own
// context, which StopEviction cancels.
func (c *Cache) StartEviction(interval time.Duration) { func (c *Cache) StartEviction(interval time.Duration) {
if c.disabled || c.evictionCancel != nil { if c.evictionCancel != nil {
return return
} }
@@ -480,6 +482,7 @@ func (c *Cache) evictionLoop(ctx context.Context, interval time.Duration) {
defer close(c.evictionDone) defer close(c.evictionDone)
c.runReconciliationPass(ctx) c.runReconciliationPass(ctx)
c.runCleanExpired(ctx)
c.runEvictionPass(ctx) c.runEvictionPass(ctx)
ticker := time.NewTicker(interval) ticker := time.NewTicker(interval)
@@ -500,6 +503,7 @@ func (c *Cache) evictionLoop(ctx context.Context, interval time.Duration) {
// unaccounted for to one interval, on a process that is // unaccounted for to one interval, on a process that is
// already running this loop regardless. // already running this loop regardless.
c.runReconciliationPass(ctx) c.runReconciliationPass(ctx)
c.runCleanExpired(ctx)
case <-c.evictionPressure: case <-c.evictionPressure:
} }
@@ -535,6 +539,20 @@ func (c *Cache) runReconciliationPass(ctx context.Context) {
} }
} }
// runCleanExpired runs CleanExpired, logging a failure instead of
// propagating it (the loop must keep running). It does nothing once ctx
// is cancelled.
func (c *Cache) runCleanExpired(ctx context.Context) {
if ctx.Err() != nil {
return
}
err := c.CleanExpired(ctx)
if err != nil {
c.log.Warn("negative cache cleanup failed", "error", err)
}
}
// reconcileAccounting synchronizes the database size accounting with // 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 // again on every periodic eviction tick thereafter, off the request
@@ -70,7 +70,6 @@ func newEvictionTestCache(t *testing.T, maxBytes int64) (*Cache, string) {
// backwards compatibility with existing fixtures. // backwards compatibility with existing fixtures.
cache, err := NewCache(db, CacheConfig{ cache, err := NewCache(db, CacheConfig{
StateDir: tmpDir, StateDir: tmpDir,
CacheTTL: time.Hour,
NegativeTTL: 5 * time.Minute, NegativeTTL: 5 * time.Minute,
MaxBytes: maxBytes, MaxBytes: maxBytes,
DisableDiskCache: maxBytes == 0, DisableDiskCache: maxBytes == 0,
-1
View File
@@ -4,6 +4,5 @@ import "time"
// CacheConfig defaults. // CacheConfig defaults.
const ( const (
DefaultCacheTTL = 24 * time.Hour
DefaultNegativeTTL = 5 * time.Minute DefaultNegativeTTL = 5 * time.Minute
) )
@@ -3,6 +3,7 @@ package imgcache
import ( import (
"context" "context"
"errors" "errors"
"strconv"
"testing" "testing"
"time" "time"
) )
@@ -15,7 +16,6 @@ func TestNegativeCache_StoreAndCheck(t *testing.T) {
cache, err := NewCache(db, CacheConfig{ cache, err := NewCache(db, CacheConfig{
StateDir: dir, StateDir: dir,
CacheTTL: time.Hour,
NegativeTTL: 5 * time.Minute, NegativeTTL: 5 * time.Minute,
}) })
if err != nil { if err != nil {
@@ -63,7 +63,6 @@ func TestNegativeCache_Expired(t *testing.T) {
cache, err := NewCache(db, CacheConfig{ cache, err := NewCache(db, CacheConfig{
StateDir: dir, StateDir: dir,
CacheTTL: time.Hour,
NegativeTTL: 1 * time.Millisecond, // very short TTL NegativeTTL: 1 * time.Millisecond, // very short TTL
}) })
if err != nil { if err != nil {
@@ -96,6 +95,307 @@ func TestNegativeCache_Expired(t *testing.T) {
} }
} }
// TestService_Get_ServesCachedVariantOfNegativeCachedURL caches a variant of
// the test photo, then keeps a failed fetch of the photo's URL in the negative
// cache, as a failed fetch for another variant does once the source has been
// evicted. The cached variant is still served, and a variant not in the cache
// is made from the cached source. Once the source is evicted, a variant not in
// the cache needs a fetch and is refused.
func TestService_Get_ServesCachedVariantOfNegativeCachedURL(t *testing.T) {
t.Parallel()
svc, fixtures := SetupTestService(t)
resp, err := svc.Get(t.Context(), photoVariant(fixtures, 85, FitCover))
if err != nil {
t.Fatalf("Get() first request error = %v", err)
}
_ = resp.Content.Close()
err = svc.cache.StoreNegative(t.Context(), photoVariant(fixtures, 85, FitCover),
httpStatusBadGateway, "upstream answered 503")
if err != nil {
t.Fatalf("StoreNegative() error = %v", err)
}
resp, err = svc.Get(t.Context(), photoVariant(fixtures, 85, FitCover))
if err != nil {
t.Fatalf("Get() of the cached variant error = %v", err)
}
_ = resp.Content.Close()
if resp.CacheStatus != CacheHit {
t.Errorf("cached variant CacheStatus = %q, want %q",
resp.CacheStatus, CacheHit)
}
resp, err = svc.Get(t.Context(), photoVariant(fixtures, 40, FitCover))
t.Logf("Get() of a variant made from the cached source: error = %v", err)
if err != nil {
t.Fatalf("Get() of a variant made from the cached source error = %v", err)
}
_ = resp.Content.Close()
contentHash, _, err := svc.cache.LookupSource(t.Context(),
photoVariant(fixtures, 85, FitCover))
if err != nil || contentHash == "" {
t.Fatalf("LookupSource() = %q, %v, want the cached source", contentHash, err)
}
err = svc.cache.evictSourceBlob(t.Context(), contentHash)
if err != nil {
t.Fatalf("evictSourceBlob() error = %v", err)
}
_, err = svc.Get(t.Context(), photoVariant(fixtures, 60, FitCover))
t.Logf("Get() of a variant whose source was evicted: error = %v", err)
if !errors.Is(err, ErrNegativeCached) {
t.Errorf("Get() of a variant whose source was evicted error = %v, want %v",
err, ErrNegativeCached)
}
}
// TestEvictionLoopRemovesExpiredNegativeCacheRows stores a failed fetch
// that expires a second later and starts the eviction loop with a short
// interval: a later tick must remove its row although its URL is never
// requested again, and must keep a row that has not expired. It runs
// with the disk cache enabled and disabled, as the negative cache is
// used either way.
func TestEvictionLoopRemovesExpiredNegativeCacheRows(t *testing.T) {
t.Parallel()
tests := []struct {
name string
disableDiskCache bool
}{
{name: "disk cache enabled", disableDiskCache: false},
{name: "disk cache disabled", disableDiskCache: true},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
cache, err := NewCache(evictionTestDB(t), CacheConfig{
StateDir: t.TempDir(),
NegativeTTL: time.Second,
DisableDiskCache: tt.disableDiskCache,
})
if err != nil {
t.Fatalf("failed to create cache: %v", err)
}
expired := &ImageRequest{
SourceHost: testHostExample, SourcePath: "/gone.jpg",
}
err = cache.StoreNegative(t.Context(), expired, 404, "not found")
if err != nil {
t.Fatalf("StoreNegative failed: %v", err)
}
const notExpiredPath = "/later.jpg"
_, err = cache.db.ExecContext(t.Context(), `
INSERT INTO negative_cache
(source_host, source_path, status_code, expires_at)
VALUES (?, ?, 404, datetime('now', '+1 hour'))
`, testHostExample, notExpiredPath)
if err != nil {
t.Fatalf("failed to insert a row that has not expired: %v", err)
}
started := time.Now()
cache.StartEviction(100 * time.Millisecond)
defer func() { _ = cache.StopEviction(t.Context()) }()
for negativeCacheRowCount(t, cache, expired.SourcePath) > 0 {
if time.Since(started) > 10*time.Second {
t.Fatal("the expired row is still in negative_cache")
}
time.Sleep(50 * time.Millisecond)
}
t.Logf("the expired row was removed %s after the loop started",
time.Since(started))
if negativeCacheRowCount(t, cache, notExpiredPath) != 1 {
t.Error("the row that has not expired was removed")
}
})
}
}
// negativeCacheRowCount returns how many negative_cache rows hold
// sourcePath.
func negativeCacheRowCount(tb testing.TB, cache *Cache, sourcePath string) int {
tb.Helper()
var count int
err := cache.db.QueryRowContext(tb.Context(),
`SELECT COUNT(*) FROM negative_cache WHERE source_path = ?`,
sourcePath,
).Scan(&count)
if err != nil {
tb.Fatalf("failed to count negative_cache rows: %v", err)
}
return count
}
// TestCleanExpiredDeletesInBatches stores one expired negative cache row
// more than one batch holds, and one row that has not expired. One
// statement must delete one batch and no more, so a request waits behind
// one batch at most; CleanExpired must then remove the expired row left
// over and keep the row that has not expired.
func TestCleanExpiredDeletesInBatches(t *testing.T) {
t.Parallel()
cache, err := NewCache(evictionTestDB(t), CacheConfig{StateDir: t.TempDir()})
if err != nil {
t.Fatalf("failed to create cache: %v", err)
}
ctx := t.Context()
insertExpiredNegativeCacheRows(t, cache, cleanExpiredBatchSize+1)
const notExpiredPath = "/later.jpg"
_, err = cache.db.ExecContext(ctx, `
INSERT INTO negative_cache
(source_host, source_path, status_code, expires_at)
VALUES (?, ?, 404, datetime('now', '+1 hour'))
`, testHostExample, notExpiredPath)
if err != nil {
t.Fatalf("failed to insert a row that has not expired: %v", err)
}
deleted, err := cache.cleanExpiredBatch(ctx)
if err != nil {
t.Fatalf("cleanExpiredBatch failed: %v", err)
}
t.Logf("one statement deleted %d of %d expired rows",
deleted, cleanExpiredBatchSize+1)
if deleted != cleanExpiredBatchSize {
t.Errorf("one statement deleted %d rows, want %d",
deleted, cleanExpiredBatchSize)
}
err = cache.CleanExpired(ctx)
if err != nil {
t.Fatalf("CleanExpired failed: %v", err)
}
if count := negativeCacheTableRowCount(t, cache); count != 1 {
t.Errorf("negative_cache holds %d rows after CleanExpired, want 1", count)
}
if negativeCacheRowCount(t, cache, notExpiredPath) != 1 {
t.Error("the row that has not expired was removed")
}
}
// TestCleanExpiredRemovesMoreThanOneBatch stores one expired negative
// cache row more than one batch holds and calls CleanExpired once: it
// must remove them all, so it fails if CleanExpired stops after its
// first batch.
func TestCleanExpiredRemovesMoreThanOneBatch(t *testing.T) {
t.Parallel()
cache, err := NewCache(evictionTestDB(t), CacheConfig{StateDir: t.TempDir()})
if err != nil {
t.Fatalf("failed to create cache: %v", err)
}
insertExpiredNegativeCacheRows(t, cache, cleanExpiredBatchSize+1)
before := negativeCacheTableRowCount(t, cache)
err = cache.CleanExpired(t.Context())
if err != nil {
t.Fatalf("CleanExpired failed: %v", err)
}
after := negativeCacheTableRowCount(t, cache)
t.Logf("CleanExpired removed %d of %d expired rows, %d per batch",
before-after, before, cleanExpiredBatchSize)
if after != 0 {
t.Errorf("negative_cache holds %d rows after CleanExpired, want 0", after)
}
}
// TestCleanExpiredStopsOnceContextEnds runs CleanExpired with a context
// that has already ended: it must return the context's error and delete
// no row.
func TestCleanExpiredStopsOnceContextEnds(t *testing.T) {
t.Parallel()
cache, err := NewCache(evictionTestDB(t), CacheConfig{StateDir: t.TempDir()})
if err != nil {
t.Fatalf("failed to create cache: %v", err)
}
insertExpiredNegativeCacheRows(t, cache, 1)
ctx, cancel := context.WithCancel(t.Context())
cancel()
err = cache.CleanExpired(ctx)
if !errors.Is(err, context.Canceled) {
t.Errorf("CleanExpired() error = %v, want context.Canceled", err)
}
if count := negativeCacheTableRowCount(t, cache); count != 1 {
t.Errorf("negative_cache holds %d rows, want the expired row kept", count)
}
}
// insertExpiredNegativeCacheRows inserts count negative_cache rows that
// expired an hour ago, each for a different source path.
func insertExpiredNegativeCacheRows(t *testing.T, cache *Cache, count int) {
t.Helper()
for i := range count {
_, err := cache.db.ExecContext(t.Context(), `
INSERT INTO negative_cache
(source_host, source_path, status_code, expires_at)
VALUES (?, ?, 404, datetime('now', '-1 hour'))
`, testHostExample, "/expired/"+strconv.Itoa(i)+".jpg")
if err != nil {
t.Fatalf("failed to insert an expired row: %v", err)
}
}
}
// negativeCacheTableRowCount returns how many rows negative_cache holds.
func negativeCacheTableRowCount(tb testing.TB, cache *Cache) int {
tb.Helper()
var count int
err := cache.db.QueryRowContext(tb.Context(),
`SELECT COUNT(*) FROM negative_cache`,
).Scan(&count)
if err != nil {
tb.Fatalf("failed to count negative_cache rows: %v", err)
}
return count
}
func TestService_Get_ReturnsErrorForNegativeCachedURL(t *testing.T) { func TestService_Get_ReturnsErrorForNegativeCachedURL(t *testing.T) {
t.Parallel() t.Parallel()
+31 -17
View File
@@ -9,6 +9,7 @@ import (
"log/slog" "log/slog"
"net/url" "net/url"
"runtime/debug" "runtime/debug"
"strings"
"time" "time"
"github.com/dustin/go-humanize" "github.com/dustin/go-humanize"
@@ -129,25 +130,13 @@ func NewService(cfg *ServiceConfig) (*Service, error) {
var ErrNegativeCached = errors.New("request is in negative cache (recently failed)") var ErrNegativeCached = errors.New("request is in negative cache (recently failed)")
// Get retrieves a processed image, fetching and processing if necessary. // Get retrieves a processed image, fetching and processing if necessary.
// While its source URL is in the negative cache, a cached variant is still
// served and a variant not in the cache is still made from a cached source;
// only a request that needs the source fetched is refused.
func (s *Service) Get(ctx context.Context, req *ImageRequest) (*ImageResponse, error) { func (s *Service) Get(ctx context.Context, req *ImageRequest) (*ImageResponse, error) {
// Propagate AllowHTTP setting to the request // Propagate AllowHTTP setting to the request
req.AllowHTTP = s.allowHTTP req.AllowHTTP = s.allowHTTP
// Check negative cache first - skip fetching for recently-failed URLs
negHit, err := s.cache.checkNegativeCache(ctx, req)
if err != nil {
s.log.Warn("negative cache check failed", "error", err)
}
if negHit {
s.log.Debug("negative cache hit",
"host", req.SourceHost,
"path", req.SourcePath,
)
return nil, fmt.Errorf("%w: %w", httpfetcher.ErrUpstreamError, ErrNegativeCached)
}
// Check variant cache first (disk only, no DB) // Check variant cache first (disk only, no DB)
result, err := s.cache.Lookup(ctx, req) result, err := s.cache.Lookup(ctx, req)
if err != nil { if err != nil {
@@ -218,7 +207,16 @@ func (s *Service) ValidateRequest(req *ImageRequest) error {
return fmt.Errorf("invalid source URL: %w", err) return fmt.Errorf("invalid source URL: %w", err)
} }
if s.allowlist.IsAllowed(parsedURL) { // An allowlist entry names no port, so a host followed by ":", with or
// without a port, needs a signature. The port is read from the parsed
// URL, as the fetcher reads it: in an IPv6 address written without
// brackets, such as 2001:db8::1:8443, it is what follows the last ":".
// This is checked here and not in allowlist.IsAllowed, which also matches
// referer_blocklist, where a Referer must be blocked whatever its port.
hostHasPort := parsedURL.Port() != "" ||
strings.HasSuffix(parsedURL.Host, ":")
if !hostHasPort && s.allowlist.IsAllowed(parsedURL) {
return nil return nil
} }
@@ -423,12 +421,28 @@ func (s *Service) processFromSourceOrFetch(
// fetchAndProcess fetches from upstream, processes, and caches the result. // fetchAndProcess fetches from upstream, processes, and caches the result.
// It counts the fetch with the bytes read from upstream, including when // It counts the fetch with the bytes read from upstream, including when
// reading the response or a later step fails. // reading the response or a later step fails. It fetches nothing while the
// source URL is in the negative cache.
func (s *Service) fetchAndProcess( func (s *Service) fetchAndProcess(
ctx context.Context, ctx context.Context,
req *ImageRequest, req *ImageRequest,
cacheKey VariantKey, cacheKey VariantKey,
) (*processedVariant, error) { ) (*processedVariant, error) {
// Check negative cache - skip fetching for recently-failed URLs
negHit, err := s.cache.checkNegativeCache(ctx, req)
if err != nil {
s.log.Warn("negative cache check failed", "error", err)
}
if negHit {
s.log.Debug("negative cache hit",
"host", req.SourceHost,
"path", req.SourcePath,
)
return nil, fmt.Errorf("%w: %w", httpfetcher.ErrUpstreamError, ErrNegativeCached)
}
// Fetch from upstream // Fetch from upstream
sourceURL := req.SourceURL() sourceURL := req.SourceURL()
+259
View File
@@ -2,7 +2,9 @@ package imgcache
import ( import (
"context" "context"
"encoding/json"
"io" "io"
"os"
"testing" "testing"
"time" "time"
@@ -248,6 +250,97 @@ func TestService_ValidateRequest_SignatureExactHostMatch(t *testing.T) {
} }
} }
// TestService_ValidateRequest_HostWithPortNeedsSignature verifies that a host
// followed by a port, or by a colon with no port, matches no allowlist entry,
// so that a request for it without a signature is refused, while the same
// host with no port is allowlisted.
func TestService_ValidateRequest_HostWithPortNeedsSignature(t *testing.T) {
t.Parallel()
svc, _ := SetupTestService(t,
WithSigningKey(testSigningKey),
WithAllowlist(testHostCDN, "203.0.113.5", "2001:db8::1"),
)
tests := []struct {
host string
allowlisted bool
}{
{testHostCDN, true},
{"cdn.example.com:8443", false},
{"cdn.example.com:443", false},
{"cdn.example.com:", false},
{"203.0.113.5", true},
{"203.0.113.5:8080", false},
{"[2001:db8::1]", true},
{"[2001:db8::1]:8443", false},
}
for _, tt := range tests {
t.Run(tt.host, func(t *testing.T) {
t.Parallel()
req := &ImageRequest{
SourceHost: tt.host,
SourcePath: testPathCat,
Size: Size{Width: 50, Height: 50},
Format: FormatJPEG,
Quality: 85,
FitMode: FitCover,
}
err := svc.ValidateRequest(req)
t.Logf("ValidateRequest() for host %q with no signature: %v",
tt.host, err)
if allowlisted := err == nil; allowlisted != tt.allowlisted {
t.Errorf("host %q served without a signature = %v, want %v",
tt.host, allowlisted, tt.allowlisted)
}
})
}
}
// TestService_ValidateRequest_UnbracketedIPv6PortNeedsSignature verifies that
// an allowlisted IPv6 address written without brackets and followed by a
// port, or by a colon with no port, needs a signature. Go's URL parser, which
// the fetcher uses, reads 2001:db8::1:8443 as the host 2001:db8::1 with the
// port 8443.
func TestService_ValidateRequest_UnbracketedIPv6PortNeedsSignature(
t *testing.T,
) {
t.Parallel()
svc, _ := SetupTestService(t,
WithSigningKey(testSigningKey),
WithAllowlist("2001:db8::1"),
)
for _, host := range []string{"2001:db8::1:8443", "2001:db8::1:"} {
t.Run(host, func(t *testing.T) {
t.Parallel()
req := &ImageRequest{
SourceHost: host,
SourcePath: testPathCat,
Size: Size{Width: 50, Height: 50},
Format: FormatJPEG,
Quality: 85,
FitMode: FitCover,
}
err := svc.ValidateRequest(req)
t.Logf("ValidateRequest() for host %q with no signature: %v",
host, err)
if err == nil {
t.Errorf("host %q served without a signature, want refused",
host)
}
})
}
}
func TestService_Get_InvalidFile(t *testing.T) { func TestService_Get_InvalidFile(t *testing.T) {
t.Parallel() t.Parallel()
@@ -430,6 +523,172 @@ func TestService_Get_Caching(t *testing.T) {
} }
} }
// TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo caches a source and
// a variant of it and sets back two days every time pixa stored for them.
// For the source: fetched_at in source_metadata, fetched_at and
// last_accessed_at in source_content, the fetched_at in its metadata file,
// and the access and modification times of its file and of its metadata
// file. For the variant: created_at and last_accessed_at in variant_content,
// the created_at in its .meta file, and the access and modification times of
// its file and of its .meta file. It then checks that the variant is still
// served from the cache and that a new variant is made from the cached source
// without fetching it again.
//
// Left at the present: the files' status change and birth times, which a
// program cannot set, and the times of the directories holding the files,
// which other entries share. The source's stored response headers are empty,
// as the mock fetcher sends none. expires_at, etag and last_modified in
// source_metadata and in the metadata file are never written, nor are the
// request_cache and output_content tables.
func TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo(t *testing.T) {
t.Parallel()
svc, fixtures := SetupTestService(t)
variant := photoVariant(fixtures, 85, FitCover)
resp, err := svc.Get(t.Context(), variant)
if err != nil {
t.Fatalf("Get() first request error = %v", err)
}
_ = resp.Content.Close()
twoDaysAgo := time.Now().AddDate(0, 0, -2)
backdateSource(t, svc.cache, variant, twoDaysAgo)
backdateVariant(t, svc.cache, variant, twoDaysAgo)
resp, err = svc.Get(t.Context(), variant)
if err != nil {
t.Fatalf("Get() of the cached variant error = %v", err)
}
_ = resp.Content.Close()
if resp.CacheStatus != CacheHit {
t.Errorf("cached variant CacheStatus = %q, want %q",
resp.CacheStatus, CacheHit)
}
// Another quality is a new variant, made from the cached source
resp, err = svc.Get(t.Context(), photoVariant(fixtures, 40, FitCover))
if err != nil {
t.Fatalf("Get() of a new variant error = %v", err)
}
_ = resp.Content.Close()
fetches := readCacheStatsCounters(t, svc.cache).upstreamFetchCount
if fetches != 1 {
t.Errorf("upstream fetches = %d, want 1", fetches)
}
}
// backdateSource sets every time pixa stored for req's cached source to when,
// as listed on TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo.
func backdateSource(
t *testing.T, cache *Cache, req *ImageRequest, when time.Time,
) {
t.Helper()
pathHash := HashPath(req.SourcePath + "?" + req.SourceQuery)
meta, err := cache.srcMetadata.Load(req.SourceHost, pathHash)
if err != nil {
t.Fatalf("reading the source's metadata file: %v", err)
}
meta.FetchedAt = when.Unix()
err = cache.srcMetadata.Store(req.SourceHost, pathHash, meta)
if err != nil {
t.Fatalf("writing the source's metadata file: %v", err)
}
dbTime := when.UTC().Format(sqliteTimestampFormat)
updateOneRow(t, cache, `
UPDATE source_metadata SET fetched_at = ?
WHERE source_host = ? AND source_path = ? AND source_query = ?
`, dbTime, req.SourceHost, req.SourcePath, req.SourceQuery)
updateOneRow(t, cache,
`UPDATE source_content SET fetched_at = ? WHERE content_hash = ?`,
dbTime, meta.ContentHash)
setSourceLastAccessed(t, cache, ContentHash(meta.ContentHash), when)
for _, path := range []string{
cache.srcContent.hashToPath(ContentHash(meta.ContentHash)),
cache.srcMetadata.metaPath(req.SourceHost, pathHash),
} {
err = os.Chtimes(path, when, when)
if err != nil {
t.Fatalf("setting back the times of %s: %v", path, err)
}
}
}
// backdateVariant sets every time pixa stored for the cached variant req asks
// for to when, as listed on
// TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo.
func backdateVariant(
t *testing.T, cache *Cache, req *ImageRequest, when time.Time,
) {
t.Helper()
key := CacheKey(req)
content, size, contentType, err := cache.variants.LoadWithMeta(key)
if err != nil || contentType == "" {
t.Fatalf("reading the variant's .meta file: content type %q, error %v",
contentType, err)
}
_ = content.Close()
meta, err := json.Marshal(VariantMeta{
ContentType: contentType,
Size: size,
CreatedAt: when.Unix(),
})
if err != nil {
t.Fatalf("encoding the variant's .meta file: %v", err)
}
path := cache.variants.keyToPath(key)
err = os.WriteFile(path+variantMetaSuffix, meta, StorageFilePerm)
if err != nil {
t.Fatalf("writing the variant's .meta file: %v", err)
}
updateOneRow(t, cache,
`UPDATE variant_content SET created_at = ? WHERE cache_key = ?`,
when.UTC().Format(sqliteTimestampFormat), string(key))
setVariantLastAccessed(t, cache, key, when)
for _, file := range []string{path, path + variantMetaSuffix} {
err = os.Chtimes(file, when, when)
if err != nil {
t.Fatalf("setting back the times of %s: %v", file, err)
}
}
}
// updateOneRow runs the UPDATE statement query and fails the test unless it
// changed exactly one row.
func updateOneRow(t *testing.T, cache *Cache, query string, args ...any) {
t.Helper()
result, err := cache.db.ExecContext(t.Context(), query, args...)
if err != nil {
t.Fatalf("%s: %v", query, err)
}
rows, err := result.RowsAffected()
if err != nil || rows != 1 {
t.Fatalf("%s: rows = %d, error = %v, want 1 row", query, rows, err)
}
}
func TestService_Get_DifferentSizes(t *testing.T) { func TestService_Get_DifferentSizes(t *testing.T) {
t.Parallel() t.Parallel()
-5
View File
@@ -43,7 +43,6 @@ func TestStats_HitRateIsRatio(t *testing.T) {
cache, err := NewCache(db, CacheConfig{ cache, err := NewCache(db, CacheConfig{
StateDir: dir, StateDir: dir,
CacheTTL: time.Hour,
NegativeTTL: 5 * time.Minute, NegativeTTL: 5 * time.Minute,
}) })
if err != nil { if err != nil {
@@ -91,7 +90,6 @@ func TestStats_ZeroCounts(t *testing.T) {
cache, err := NewCache(db, CacheConfig{ cache, err := NewCache(db, CacheConfig{
StateDir: dir, StateDir: dir,
CacheTTL: time.Hour,
NegativeTTL: 5 * time.Minute, NegativeTTL: 5 * time.Minute,
}) })
if err != nil { if err != nil {
@@ -120,7 +118,6 @@ func TestStats_LogsFailedCountQueries(t *testing.T) {
cache, err := NewCache(db, CacheConfig{ cache, err := NewCache(db, CacheConfig{
StateDir: t.TempDir(), StateDir: t.TempDir(),
CacheTTL: time.Hour,
NegativeTTL: 5 * time.Minute, NegativeTTL: 5 * time.Minute,
Logger: slog.New(slog.NewJSONHandler(&logBuf, nil)), Logger: slog.New(slog.NewJSONHandler(&logBuf, nil)),
}) })
@@ -162,7 +159,6 @@ func TestIncrementStats_LogsFailedUpdates(t *testing.T) {
cache, err := NewCache(db, CacheConfig{ cache, err := NewCache(db, CacheConfig{
StateDir: t.TempDir(), StateDir: t.TempDir(),
CacheTTL: time.Hour,
NegativeTTL: 5 * time.Minute, NegativeTTL: 5 * time.Minute,
Logger: slog.New(slog.NewJSONHandler(&logBuf, nil)), Logger: slog.New(slog.NewJSONHandler(&logBuf, nil)),
}) })
@@ -232,7 +228,6 @@ func TestStats_DisabledCacheReportsNoItems(t *testing.T) {
disabled, err := NewCache(enabled.db, CacheConfig{ disabled, err := NewCache(enabled.db, CacheConfig{
StateDir: t.TempDir(), StateDir: t.TempDir(),
CacheTTL: time.Hour,
NegativeTTL: 5 * time.Minute, NegativeTTL: 5 * time.Minute,
DisableDiskCache: true, DisableDiskCache: true,
}) })
+13 -2
View File
@@ -10,6 +10,7 @@ import (
"log/slog" "log/slog"
"os" "os"
"path/filepath" "path/filepath"
"strings"
"time" "time"
) )
@@ -371,11 +372,13 @@ func HashPath(path string) PathHash {
// CacheKey generates a unique key for a request variant. // CacheKey generates a unique key for a request variant.
// Format: sha256(host:path:query:width:height:format:quality:fit_mode) // Format: sha256(host:path:query:width:height:format:quality:fit_mode)
// The path and query go through escapePercentAndColon, so that a ":" in
// either cannot be taken for the one between them.
func CacheKey(req *ImageRequest) VariantKey { func CacheKey(req *ImageRequest) VariantKey {
data := fmt.Sprintf("%s:%s:%s:%d:%d:%s:%d:%s", data := fmt.Sprintf("%s:%s:%s:%d:%d:%s:%d:%s",
req.SourceHost, req.SourceHost,
req.SourcePath, escapePercentAndColon(req.SourcePath),
req.SourceQuery, escapePercentAndColon(req.SourceQuery),
req.Size.Width, req.Size.Width,
req.Size.Height, req.Size.Height,
req.Format, req.Format,
@@ -387,6 +390,14 @@ func CacheKey(req *ImageRequest) VariantKey {
return VariantKey(hex.EncodeToString(h[:])) return VariantKey(hex.EncodeToString(h[:]))
} }
// escapePercentAndColon returns s with each "%" written as "%25" and each
// ":" as "%3A", so that the result holds no ":".
func escapePercentAndColon(s string) string {
s = strings.ReplaceAll(s, "%", "%25")
return strings.ReplaceAll(s, ":", "%3A")
}
// VariantStorage handles key-based file storage for processed image variants. // VariantStorage handles key-based file storage for processed image variants.
// Files are stored at: <basedir>/<ab>/<cd>/<cache_key> // Files are stored at: <basedir>/<ab>/<cd>/<cache_key>
// Metadata is stored at: <basedir>/<ab>/<cd>/<cache_key>.meta // Metadata is stored at: <basedir>/<ab>/<cd>/<cache_key>.meta
@@ -407,6 +407,62 @@ func TestCacheKey(t *testing.T) {
} }
} }
// TestCacheKey_ColonOrPercentInPathOrQuery checks that two different source
// URLs whose path or query holds ":" or "%" get different keys.
func TestCacheKey_ColonOrPercentInPathOrQuery(t *testing.T) {
t.Parallel()
cases := []struct {
name string
path1, query1 string
path2, query2 string
}{
{
name: "colon moved from path to query",
path1: "/a:b", query1: "",
path2: "/a", query2: "b:",
},
{
name: "colon in path and its percent-encoding",
path1: "/a:b", query1: "",
path2: "/a%3Ab", query2: "",
},
}
for _, tt := range cases {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
key1 := CacheKey(&ImageRequest{
SourceHost: testHostCDN,
SourcePath: tt.path1,
SourceQuery: tt.query1,
Size: Size{Width: 800, Height: 600},
Format: FormatWebP,
Quality: 85,
FitMode: FitCover,
})
key2 := CacheKey(&ImageRequest{
SourceHost: testHostCDN,
SourcePath: tt.path2,
SourceQuery: tt.query2,
Size: Size{Width: 800, Height: 600},
Format: FormatWebP,
Quality: 85,
FitMode: FitCover,
})
t.Logf("CacheKey() of path %q query %q = %s", tt.path1, tt.query1, key1)
t.Logf("CacheKey() of path %q query %q = %s", tt.path2, tt.query2, key2)
if key1 == key2 {
t.Errorf("CacheKey() gave path %q query %q and path %q query %q "+
"the same key", tt.path1, tt.query1, tt.path2, tt.query2)
}
})
}
}
// TestVariantStorage_StoreLogsFailedMetaWrite verifies that a .meta write // TestVariantStorage_StoreLogsFailedMetaWrite verifies that a .meta write
// that fails is logged at warn and the store still succeeds. // that fails is logged at warn and the store still succeeds.
func TestVariantStorage_StoreLogsFailedMetaWrite(t *testing.T) { func TestVariantStorage_StoreLogsFailedMetaWrite(t *testing.T) {
@@ -185,7 +185,6 @@ func SetupTestService(
cache, err := NewCache(db, CacheConfig{ cache, err := NewCache(db, CacheConfig{
StateDir: tmpDir, StateDir: tmpDir,
CacheTTL: time.Hour,
NegativeTTL: 5 * time.Minute, NegativeTTL: 5 * time.Minute,
}) })
if err != nil { if err != nil {
+17 -2
View File
@@ -7,12 +7,15 @@ import (
"regexp" "regexp"
"strconv" "strconv"
"strings" "strings"
"sneak.berlin/go/pixa/internal/imageprocessor"
) )
// URL parsing errors. // URL parsing errors.
var ( var (
ErrInvalidPath = errors.New("invalid image path") ErrInvalidPath = errors.New("invalid image path")
ErrMissingHost = errors.New("missing source host") ErrMissingHost = errors.New("missing source host")
ErrInvalidHost = errors.New("invalid source host")
ErrMissingSize = errors.New("missing size specification") ErrMissingSize = errors.New("missing size specification")
ErrInvalidSize = errors.New("invalid size format") ErrInvalidSize = errors.New("invalid size format")
ErrInvalidFormat = errors.New("invalid or unsupported format") ErrInvalidFormat = errors.New("invalid or unsupported format")
@@ -20,8 +23,9 @@ var (
ErrPathTraversal = errors.New("path traversal detected") ErrPathTraversal = errors.New("path traversal detected")
) )
// MaxDimension is the maximum allowed width or height. // MaxDimension is the maximum allowed width or height, the same as the image
const MaxDimension = 8192 // processor's maximum for an output image.
const MaxDimension = imageprocessor.MaxOutputDimension
// ValidateDimension checks one requested width or height; name ("width" or // ValidateDimension checks one requested width or height; name ("width" or
// "height") appears in the error. 0 means "original size" and is valid. // "height") appears in the error. 0 means "original size" and is valid.
@@ -140,6 +144,17 @@ func parseImageComponents(remainder string) (*ParsedURL, error) {
return nil, ErrMissingHost return nil, ErrMissingHost
} }
// A URL reads what comes before "@" as a user name and password, and the
// rest as the host
if strings.Contains(host, "@") {
return nil, fmt.Errorf("%w: %q holds @", ErrInvalidHost, host)
}
// Host names do not depend on letter case, so the allowlist, the upstream
// connection limits, the cache and the signature see one spelling of each
// host
host = strings.ToLower(host)
// Extract query string if present in path // Extract query string if present in path
if qIndex := strings.Index(path, "?"); qIndex != -1 { if qIndex := strings.Index(path, "?"); qIndex != -1 {
query = path[qIndex+1:] query = path[qIndex+1:]
@@ -234,6 +234,49 @@ func TestParseImagePath(t *testing.T) {
} }
} }
// TestParseImagePath_LowerCasesHost verifies that the host is lower-cased, so
// that every spelling of its letter case is the same host to the allowlist,
// the upstream connection limits and the cache, and that the path keeps its
// case.
func TestParseImagePath_LowerCasesHost(t *testing.T) {
t.Parallel()
got, err := ParseImagePath("CDN.Example.COM/Photos/Cat.jpg/800x600.webp")
if err != nil {
t.Fatalf("ParseImagePath() error = %v", err)
}
t.Logf("parsed host %q, path %q", got.Host, got.Path)
if got.Host != testHostCDN {
t.Errorf("Host = %q, want %q", got.Host, testHostCDN)
}
if got.Path != "/Photos/Cat.jpg" {
t.Errorf("Path = %q, want %q", got.Path, "/Photos/Cat.jpg")
}
}
// TestParseImagePath_RefusesUserinfo verifies that a host holding "@" is
// refused: a URL reads what comes before the "@" as a user name and password
// and the rest as the host.
func TestParseImagePath_RefusesUserinfo(t *testing.T) {
t.Parallel()
for _, input := range []string{
"user@cdn.example.com/photos/cat.jpg/800x600.webp",
"user:password@cdn.example.com/photos/cat.jpg/800x600.webp",
} {
got, err := ParseImagePath(input)
t.Logf("ParseImagePath(%q) = %+v, error = %v", input, got, err)
if !errors.Is(err, ErrInvalidHost) {
t.Errorf("ParseImagePath(%q) error = %v, want %v",
input, err, ErrInvalidHost)
}
}
}
func TestParsedURL_ToImageRequest(t *testing.T) { func TestParsedURL_ToImageRequest(t *testing.T) {
t.Parallel() t.Parallel()
@@ -0,0 +1,153 @@
package server
import (
"bytes"
"encoding/json"
"image/jpeg"
"net/http"
"net/http/httptest"
"testing"
"sneak.berlin/go/pixa/internal/encurl"
"sneak.berlin/go/pixa/internal/imgcache"
)
// TestImageProxyRefusesComputedSideAboveMaximum requests sizes of a 3000x600
// and a 600x3000 PNG through both image routes. A size with one side 0, whose
// side computed from the source's proportions would be above 8192, is refused
// with 400 and the JSON error body. The sizes after those refusals are served
// as before, which also shows that a refusal is not cached as a failed fetch of
// the source: orig, a size with both sides given, and a size with one side 0
// whose computed side stays within 8192.
func TestImageProxyRefusesComputedSideAboveMaximum(t *testing.T) {
t.Parallel()
// The upstream paths of the two sources
const (
widePath = "/wide.png"
tallPath = "/tall.png"
)
wide := encodeTestPNG(t, 3000, 600)
tall := encodeTestPNG(t, 600, 3000)
upstream := httptest.NewServer(http.HandlerFunc(
func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "image/png")
if r.URL.Path == tallPath {
_, _ = w.Write(tall)
return
}
_, _ = w.Write(wide)
}))
t.Cleanup(upstream.Close)
s, _, _ := startImageProxy(t, upstream)
generator, err := encurl.NewGenerator(testSigningKey)
if err != nil {
t.Fatalf("encurl.NewGenerator: %v", err)
}
tests := []struct {
path string
// size is the size of the /v1/image/ URL, and width and height are
// those of the encrypted URL
size string
width, height int
// wantWidth and wantHeight are the served image's, 0 for a refusal
wantWidth, wantHeight int
}{
{widePath, "0x8192", 0, 8192, 0, 0},
{tallPath, "8192x0", 8192, 0, 0, 0},
{widePath, "orig", 0, 0, 3000, 600},
{widePath, "300x200", 300, 200, 300, 200},
{widePath, "0x300", 0, 300, 1500, 300},
{tallPath, "300x0", 300, 0, 300, 1500},
}
for _, tt := range tests {
token, err := generator.Generate(&encurl.Payload{
SourceHost: upstreamHost, SourcePath: tt.path,
Width: tt.width, Height: tt.height, Format: imgcache.FormatJPEG,
})
if err != nil {
t.Fatalf("encurl Generate: %v", err)
}
for _, target := range []string{
"/v1/image/" + upstreamHost + tt.path + "/" + tt.size + ".jpeg",
"/v1/e/" + token + "/img.jpg",
} {
rec := httptest.NewRecorder()
s.ServeHTTP(rec, httptest.NewRequestWithContext(
t.Context(), http.MethodGet, target, nil))
t.Logf("GET %s: %d, Content-Type %s",
target, rec.Code, rec.Header().Get("Content-Type"))
if tt.wantWidth == 0 {
checkSizeRefused(t, rec)
} else {
checkServedSize(t, rec, tt.wantWidth, tt.wantHeight)
}
}
}
}
// checkSizeRefused checks that rec answers 400 with the JSON error body that
// refuses a size whose side computed from the source's proportions is above
// 8192.
func checkSizeRefused(t *testing.T, rec *httptest.ResponseRecorder) {
t.Helper()
const wantError = "size too large for the source: " +
"the side computed from its proportions is above 8192"
var body struct {
Error string `json:"error"`
Status int `json:"status"`
}
err := json.Unmarshal(rec.Body.Bytes(), &body)
if err != nil {
t.Errorf("status %d, body is not JSON: %v", rec.Code, err)
return
}
if rec.Code != http.StatusBadRequest || body.Status != http.StatusBadRequest ||
body.Error != wantError {
t.Errorf("status %d, body %+v, want %d and the error %q",
rec.Code, body, http.StatusBadRequest, wantError)
}
}
// checkServedSize checks that rec answers 200 with a JPEG of wantWidth by
// wantHeight.
func checkServedSize(
t *testing.T, rec *httptest.ResponseRecorder, wantWidth, wantHeight int,
) {
t.Helper()
if rec.Code != http.StatusOK {
t.Errorf("status %d, want %d; body %s",
rec.Code, http.StatusOK, rec.Body.String())
return
}
served, err := jpeg.DecodeConfig(bytes.NewReader(rec.Body.Bytes()))
if err != nil {
t.Errorf("decoding the served JPEG: %v", err)
return
}
if served.Width != wantWidth || served.Height != wantHeight {
t.Errorf("served %dx%d, want %dx%d",
served.Width, served.Height, wantWidth, wantHeight)
}
}
@@ -0,0 +1,91 @@
package server
import (
"fmt"
"net/http"
"net/http/httptest"
"testing"
"sneak.berlin/go/pixa/internal/encurl"
"sneak.berlin/go/pixa/internal/imgcache"
)
// TestImageProxyRefusesFitOutsideAboveMaximum requests sizes with fit=outside
// through both image routes. fit=outside scales the source to cover the size,
// and so past it on one side: a size for which that side would be above 8192
// is refused with 400 and the JSON error body, whether it is the width or the
// height, and a size for which it stays within 8192 is served at the scaled
// size.
func TestImageProxyRefusesFitOutsideAboveMaximum(t *testing.T) {
t.Parallel()
// The upstream paths of the sources
const (
widePath = "/wide.png"
rowPath = "/row.png"
columnPath = "/column.png"
)
sources := map[string][]byte{
widePath: encodeTestPNG(t, 3000, 600),
rowPath: encodeTestPNG(t, 8192, 1),
columnPath: encodeTestPNG(t, 1, 8192),
}
upstream := httptest.NewServer(http.HandlerFunc(
func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "image/png")
_, _ = w.Write(sources[r.URL.Path])
}))
t.Cleanup(upstream.Close)
s, _, _ := startImageProxy(t, upstream)
generator, err := encurl.NewGenerator(testSigningKey)
if err != nil {
t.Fatalf("encurl.NewGenerator: %v", err)
}
tests := []struct {
path string
width, height int
// wantWidth and wantHeight are the served image's, 0 for a refusal
wantWidth, wantHeight int
}{
{widePath, 1, 8192, 0, 0},
{rowPath, 1, 8192, 0, 0},
{columnPath, 8192, 1, 0, 0},
{widePath, 1, 1200, 6000, 1200},
}
for _, tt := range tests {
token, err := generator.Generate(&encurl.Payload{
SourceHost: upstreamHost, SourcePath: tt.path,
Width: tt.width, Height: tt.height, Format: imgcache.FormatJPEG,
FitMode: imgcache.FitOutside,
})
if err != nil {
t.Fatalf("encurl Generate: %v", err)
}
size := fmt.Sprintf("%dx%d", tt.width, tt.height)
for _, target := range []string{
"/v1/image/" + upstreamHost + tt.path + "/" + size +
".jpeg?fit=outside",
"/v1/e/" + token + "/img.jpg",
} {
rec := httptest.NewRecorder()
s.ServeHTTP(rec, httptest.NewRequestWithContext(
t.Context(), http.MethodGet, target, nil))
t.Logf("GET %s: %d, Content-Type %s",
target, rec.Code, rec.Header().Get("Content-Type"))
if tt.wantWidth == 0 {
checkSizeRefused(t, rec)
} else {
checkServedSize(t, rec, tt.wantWidth, tt.wantHeight)
}
}
}
}
@@ -0,0 +1,71 @@
package server
import (
"io"
"net/http"
"net/http/httptest"
"strings"
"testing"
"sneak.berlin/go/pixa/internal/encurl"
"sneak.berlin/go/pixa/internal/imgcache"
)
// svgServedAsAVIF is an SVG whose first bytes pass the first-bytes check for
// AVIF: its XML comment puts "ftypavis" at bytes 4 to 11, where an AVIF file
// has its ftyp box and brand. libvips would load it as SVG.
const svgServedAsAVIF = `<!--ftypavis--><svg xmlns="http://www.w3.org/2000/svg" ` +
`width="64" height="48"><rect width="64" height="48"/></svg>`
// TestImageProxyRefusesSVGServedAsAVIF requests svgServedAsAVIF, which the
// upstream answers as image/avif, through both image routes, resized as PNG
// and in its own format (orig). Each request is answered 415, as for an SVG
// source, never with an image.
func TestImageProxyRefusesSVGServedAsAVIF(t *testing.T) {
t.Parallel()
upstream := httptest.NewServer(http.HandlerFunc(
func(w http.ResponseWriter, _ *http.Request) {
w.Header().Set("Content-Type", "image/avif")
_, _ = io.WriteString(w, svgServedAsAVIF)
}))
t.Cleanup(upstream.Close)
s, _, _ := startImageProxy(t, upstream)
generator, err := encurl.NewGenerator(testSigningKey)
if err != nil {
t.Fatalf("encurl.NewGenerator: %v", err)
}
encryptedURL := func(format imgcache.ImageFormat) string {
token, err := generator.Generate(&encurl.Payload{
SourceHost: upstreamHost, SourcePath: "/drawing.avif",
Width: 32, Height: 24, Format: format,
})
if err != nil {
t.Fatalf("encurl Generate: %v", err)
}
return "/v1/e/" + token + "/drawing." + string(format)
}
for _, target := range []string{
"/v1/image/" + upstreamHost + "/drawing.avif/32x24.png",
"/v1/image/" + upstreamHost + "/drawing.avif/64x48.orig",
encryptedURL(imgcache.FormatPNG),
encryptedURL(imgcache.FormatOriginal),
} {
rec := httptest.NewRecorder()
s.ServeHTTP(rec, httptest.NewRequestWithContext(
t.Context(), http.MethodGet, target, nil))
t.Logf("GET %s: %d, Content-Type %s, body %q",
target, rec.Code, rec.Header().Get("Content-Type"), rec.Body.String())
if rec.Code != http.StatusUnsupportedMediaType ||
!strings.Contains(rec.Body.String(), "unsupported source format") {
t.Errorf("GET %s: %d %q, want %d unsupported source format",
target, rec.Code, rec.Body.String(), http.StatusUnsupportedMediaType)
}
}
}
+41
View File
@@ -0,0 +1,41 @@
package server
import (
"io"
"net/http"
"net/http/httptest"
"testing"
)
// TestImageProxyRefusesSVG requests an image the upstream answers as SVG,
// in its own format (orig) and resized as PNG, through pixa's router,
// upstream fetcher and image processor. pixa does not accept SVG as a
// source, so each request is answered 415, never with an image.
func TestImageProxyRefusesSVG(t *testing.T) {
t.Parallel()
upstream := httptest.NewServer(http.HandlerFunc(
func(w http.ResponseWriter, _ *http.Request) {
w.Header().Set("Content-Type", "image/svg+xml")
_, _ = io.WriteString(w, `<svg xmlns="http://www.w3.org/2000/svg" `+
`width="64" height="48"><rect width="64" height="48"/></svg>`)
}))
t.Cleanup(upstream.Close)
s, _, _ := startImageProxy(t, upstream)
for _, sizeFormat := range []string{"64x48.orig", "32x24.png"} {
target := "/v1/image/" + upstreamHost + "/drawing.svg/" + sizeFormat
rec := httptest.NewRecorder()
s.ServeHTTP(rec, httptest.NewRequestWithContext(
t.Context(), http.MethodGet, target, nil))
t.Logf("GET %s: %d, Content-Type %s, body %s",
target, rec.Code, rec.Header().Get("Content-Type"), rec.Body.String())
if rec.Code != http.StatusUnsupportedMediaType {
t.Errorf("GET %s status = %d, want %d",
target, rec.Code, http.StatusUnsupportedMediaType)
}
}
}
+113
View File
@@ -0,0 +1,113 @@
package signature_test
import (
"errors"
"testing"
"time"
"sneak.berlin/go/pixa/internal/signature"
)
// TestSigner_Verify_ColonCannotMovePathQueryBoundary checks that a URL
// signed for path "/a:b" with no query does not verify for path "/a" with
// query "b:", a different resource on the same host.
func TestSigner_Verify_ColonCannotMovePathQueryBoundary(t *testing.T) {
t.Parallel()
signer := signature.New(goldenSigningKey)
signed := &signature.Request{
SourceHost: testHost,
SourcePath: "/a:b",
Width: 800,
Height: 600,
Format: testFormatWebP,
Quality: 85,
FitMode: testFitCover,
Expires: time.Now().Add(1 * time.Hour),
}
signed.Signature = signer.Sign(signed)
replayed := *signed
replayed.SourcePath = "/a"
replayed.SourceQuery = "b:"
t.Logf("signature for path %q query %q: %s",
signed.SourcePath, signed.SourceQuery, signed.Signature)
t.Logf("signature for path %q query %q: %s",
replayed.SourcePath, replayed.SourceQuery, signer.Sign(&replayed))
err := signer.Verify(&replayed)
if !errors.Is(err, signature.ErrInvalid) {
t.Errorf("Verify() of path %q query %q with the signature for "+
"path %q = %v, want %v",
replayed.SourcePath, replayed.SourceQuery, signed.SourcePath,
err, signature.ErrInvalid)
}
}
// TestSigner_EscapedGoldenVectors pins the signature of requests whose path
// or query holds ":" or "%", which are signed with "%" written as "%25" and
// ":" as "%3A". The expected values were computed once, outside pixa, from
// the signed data in each comment.
func TestSigner_EscapedGoldenVectors(t *testing.T) {
t.Parallel()
signer := signature.New(goldenSigningKey)
vectors := []struct {
name string
path string
query string
wantSignature string
}{
{
name: "colon in path",
path: "/photos/a:b.jpg",
// Signed data:
// "cdn.example.com:/photos/a%3Ab.jpg::800:600:webp:1704067200:85:cover"
wantSignature: "WnEpZ7bQp1Zvw8WDatlj2vu-4wYKWE3BFBizV8m__6k=",
},
{
name: "colon in query",
path: testPath,
query: "t=1:2",
// Signed data:
// "cdn.example.com:/photos/cat.jpg:t=1%3A2:800:600:webp:1704067200:85:cover"
wantSignature: "85kRoN8hhr4cCUkEUjNvUsvVYKjYYfWdeffx5TjJjqA=",
},
{
name: "percent in path and query",
path: "/photos/a%20b.jpg",
query: "q=50%",
// Signed data:
// "cdn.example.com:/photos/a%2520b.jpg:q=50%25:800:600:webp:1704067200:85:cover"
wantSignature: "MjDiiRdgXMrsjqPMsENtD4aG1hp1e-a6zy67vKTX4CQ=",
},
}
for _, tt := range vectors {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
req := &signature.Request{
SourceHost: testHost,
SourcePath: tt.path,
SourceQuery: tt.query,
Width: 800,
Height: 600,
Format: testFormatWebP,
Quality: 85,
FitMode: testFitCover,
Expires: time.Unix(goldenExpiresUnix, 0),
}
gotSignature := signer.Sign(req)
t.Logf("Sign() of path %q query %q = %s", tt.path, tt.query, gotSignature)
if gotSignature != tt.wantSignature {
t.Errorf("Sign() = %q, want %q", gotSignature, tt.wantSignature)
}
})
}
}
+17 -6
View File
@@ -10,6 +10,7 @@ import (
"fmt" "fmt"
"net/url" "net/url"
"strconv" "strconv"
"strings"
"time" "time"
) )
@@ -154,15 +155,17 @@ func (s *Signer) GenerateSignedURL(
// buildSignatureData creates the string to be signed. // buildSignatureData creates the string to be signed.
// Format: "host:path:query:width:height:format:expiration:quality:fit" // Format: "host:path:query:width:height:format:expiration:quality:fit"
// All components are used verbatim (exact match). No normalization, // The path and query go through escapePercentAndColon, so that a ":" in
// suffix matching, or wildcard expansion is performed. Quality and fit // either cannot be taken for the one between them; every other component
// are the effective transform values, so replaying a signed URL with a // is used verbatim. No normalization, suffix matching, or wildcard
// different quality or fit mode fails verification. // expansion is performed. Quality and fit are the effective transform
// values, so replaying a signed URL with a different quality or fit mode
// fails verification.
func (s *Signer) buildSignatureData(req *Request) string { func (s *Signer) buildSignatureData(req *Request) string {
return fmt.Sprintf("%s:%s:%s:%d:%d:%s:%d:%d:%s", return fmt.Sprintf("%s:%s:%s:%d:%d:%s:%d:%d:%s",
req.SourceHost, req.SourceHost,
req.SourcePath, escapePercentAndColon(req.SourcePath),
req.SourceQuery, escapePercentAndColon(req.SourceQuery),
req.Width, req.Width,
req.Height, req.Height,
req.Format, req.Format,
@@ -172,6 +175,14 @@ func (s *Signer) buildSignatureData(req *Request) string {
) )
} }
// escapePercentAndColon returns s with each "%" written as "%25" and each
// ":" as "%3A", so that the result holds no ":".
func escapePercentAndColon(s string) string {
s = strings.ReplaceAll(s, "%", "%25")
return strings.ReplaceAll(s, ":", "%3A")
}
// ParseParams extracts signature and expiration from query parameters. // ParseParams extracts signature and expiration from query parameters.
func ParseParams(sig, expStr string) (string, time.Time, error) { func ParseParams(sig, expStr string) (string, time.Time, error) {
if expStr == "" { if expStr == "" {