Author SHA1 Message Date
sneak d6602d5bf8 Default max_concurrent_processing to half the CPUs pixa sees (closes #242)
check / check (push) Waiting to run
The default was one image per CPU Go uses, and a container with no CPU
limit sees every CPU of the host. It is now half of runtime.GOMAXPROCS(0),
rounded down and at least 1, from imageprocessor.DefaultMaxConcurrentProcessing,
which both the config and imageprocessor.New use. The two existing tests of
the default expect the new one, as the owner allowed on the issue.

"Running under upaas" gives the worst-case memory, with a per-image peak of
about 1.6 GiB measured on pixad for an 8192x8192 source in each output
format, and tells the operator to set PIXA_MAX_CONCURRENT_PROCESSING and
PIXA_UPSTREAM_CONNECTIONS to fit the container's memory.

Model: opus-5-5
2026-10-15 09:51:17 +00:00
sneak d0377ae3da Test that max_concurrent_processing defaults to half the CPUs
An omitted max_concurrent_processing, and New given none, must be half
the CPUs Go uses, rounded down, and at least 1. The tests set GOMAXPROCS
to 1, 2, 3 and 8 and fail until the default changes
(#242).

Model: opus-5-5
2026-10-15 09:51:04 +00:00
sneak fc5ffc91f1 Test that an invalid PIXA_MAX_CONCURRENT_PROCESSING aborts startup
An empty, fractional, negative or non-numeric value must abort startup
naming the variable and the key, never fall back to the default. The
config loader already refuses these; this pins it before the default
changes (#242).

Model: opus-5-5
2026-10-15 09:50:51 +00: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
22 changed files with 997 additions and 66 deletions
+39 -9
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,10 +120,23 @@ 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
- **Memory:** pixa can use up to `max_concurrent_processing` times 1.6 GiB for
the images it processes at once, plus `upstream_connections` times
`upstream_max_response_size` for the source images it has fetched that wait
for processing. 1.6 GiB is the most pixad's resident memory was measured to
grow while it processed one 8192x8192 source, the largest it accepts, into
each output format in turn, one at a time; it grew most for a 40 MB JPEG of
random pixels saved as AVIF. At the defaults that is half the CPUs pixa sees
times 1.6 GiB, plus 64 times 50 MiB: on a host where pixa sees 48 CPUs, 24
times 1.6 GiB plus 3.1 GiB, about 42 GiB. Set `PIXA_MAX_CONCURRENT_PROCESSING`
and `PIXA_UPSTREAM_CONNECTIONS` so that this fits the memory the container may
use.
- **Health check:** the image's `HEALTHCHECK` requests - **Health check:** the image's `HEALTHCHECK` requests
`/.well-known/healthcheck.json`. upaas reads the container's health 60 seconds `/.well-known/healthcheck.json`. upaas reads the container's health 60 seconds
after a deploy and marks the deploy failed unless it is `healthy`. The probe after a deploy and marks the deploy failed unless it is `healthy`. The probe
@@ -263,6 +281,12 @@ 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.
A size with one side `0` has that side scaled to keep the source's proportions,
and is refused with 400 when that side would come out above 8192, the most
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 The source image may be JPEG, PNG, GIF, WebP, AVIF or JPEG XL. The upstream's
`Content-Type` must name its format, and the image's first bytes must match it. `Content-Type` must name its format, and the image's first bytes must match it.
An upstream answering with any other `Content-Type`, SVG included, is refused An upstream answering with any other `Content-Type`, SVG included, is refused
@@ -345,7 +369,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
@@ -405,8 +430,11 @@ 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 origin hostname (e.g. `cdn.example.com`)
- `path` — source path (e.g. `/photos/cat.jpg`) - `path` — source path (e.g. `/photos/cat.jpg`), with each `%` written as `%25`
- `query` — source query string, empty string if none 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`
@@ -498,7 +526,7 @@ tell. With no file, pixa uses the environment and the defaults.
| `PIXA_ALLOW_HTTP` | `allow_http` | Allow plain-HTTP upstreams, for testing only; default `false` | | `PIXA_ALLOW_HTTP` | `allow_http` | Allow plain-HTTP upstreams, for testing only; default `false` |
| `PIXA_UPSTREAM_CONNECTIONS_PER_HOST` | `upstream_connections_per_host` | Concurrent connections per upstream host; default `20` | | `PIXA_UPSTREAM_CONNECTIONS_PER_HOST` | `upstream_connections_per_host` | Concurrent connections per upstream host; default `20` |
| `PIXA_UPSTREAM_CONNECTIONS` | `upstream_connections` | Concurrent connections to all upstream hosts together; default `64` | | `PIXA_UPSTREAM_CONNECTIONS` | `upstream_connections` | Concurrent connections to all upstream hosts together; default `64` |
| `PIXA_MAX_CONCURRENT_PROCESSING` | `max_concurrent_processing` | Images processed at once; default the number of CPUs | | `PIXA_MAX_CONCURRENT_PROCESSING` | `max_concurrent_processing` | Images processed at once; default half the CPUs, at least 1 |
| `PIXA_IMAGE_REQUESTS_PER_MINUTE` | `image_requests_per_minute` | Image requests per client per minute; `0` turns it off; default `600` | | `PIXA_IMAGE_REQUESTS_PER_MINUTE` | `image_requests_per_minute` | Image requests per client per minute; `0` turns it off; default `600` |
| `PIXA_UPSTREAM_FETCH_TIMEOUT` | `upstream_fetch_timeout` | Time allowed for one fetch from an upstream host; default `30s` | | `PIXA_UPSTREAM_FETCH_TIMEOUT` | `upstream_fetch_timeout` | Time allowed for one fetch from an upstream host; default `30s` |
| `PIXA_UPSTREAM_MAX_RESPONSE_SIZE` | `upstream_max_response_size` | Largest upstream response accepted, in bytes; default 50 MiB | | `PIXA_UPSTREAM_MAX_RESPONSE_SIZE` | `upstream_max_response_size` | Largest upstream response accepted, in bytes; default 50 MiB |
@@ -584,10 +612,12 @@ Key settings in more detail:
does, and `downstream_timeout` has not ended first, the request is answered does, and `downstream_timeout` has not ended first, the request is answered
503 with the error `server busy, try again later` 503 with the error `server busy, try again later`
- `max_concurrent_processing` — the most images decoded and encoded at once; - `max_concurrent_processing` — the most images decoded and encoded at once;
default the number of CPUs pixa can use (`GOMAXPROCS`), which follows a default half the CPUs pixa can use (`GOMAXPROCS`, which follows a container's
container's CPU limit. A request that finds all of them in use waits up to 10 CPU limit), rounded down, and at least 1. A value that is not a whole number
seconds for one to free up; if none does, and `downstream_timeout` has not of at least 1 aborts startup. A request that finds all of them in use waits up
ended first, it is answered 503 the same way to 10 seconds for one to free up; if none does, and `downstream_timeout` has
not ended first, it is answered 503 the same way. Each one can take a lot of
memory: see "Running under upaas"
- `image_requests_per_minute` — how many requests one client may send to the - `image_requests_per_minute` — how many requests one client may send to the
image routes per minute (see the limit on them under Routes); default `600`. image routes per minute (see the limit on them under Routes); default `600`.
`0` turns the limit off, and a negative or non-integer value aborts startup. `0` turns the limit off, and a negative or non-integer value aborts startup.
+51
View File
@@ -30,6 +30,57 @@ P2: security: per-origin rate limiting
# Completed Steps # Completed Steps
- 2026-10-15 `max_concurrent_processing` defaults to half the CPUs pixa sees,
rounded down, and at least 1 (closes #242): it defaulted to one image per CPU,
and a container with no CPU limit sees every CPU of the host, so on a 48-CPU
host pixa processed 48 images at once, each of which can take about 1.6 GiB.
`imageprocessor.DefaultMaxConcurrentProcessing` gives the default to the
config and to `imageprocessor.New`; `PIXA_MAX_CONCURRENT_PROCESSING` still
overrides it. "Running under upaas" in `README.md` gives the worst-case
memory, measured, and tells the operator to set
`PIXA_MAX_CONCURRENT_PROCESSING` and `PIXA_UPSTREAM_CONNECTIONS` to fit the
container's memory.
- 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): - 2026-10-13 the unused cache lifetime is removed (closes #69):
`CacheConfig.CacheTTL`, which pixad set to `DefaultCacheTTL` (24 hours), was `CacheConfig.CacheTTL`, which pixad set to `DefaultCacheTTL` (24 hours), was
never read, so cached images never expired. The field and the constant are never read, so cached images never expired. The field and the constant are
+10 -10
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
@@ -109,10 +108,11 @@ upstream_connections_per_host: 20
# downstream_timeout has ended first. # downstream_timeout has ended first.
upstream_connections: 64 upstream_connections: 64
# Maximum number of images decoded and encoded at once (default: the # Maximum number of images decoded and encoded at once (default: half the
# number of CPUs pixa can use, which follows a container's CPU limit). A # number of CPUs pixa can use, at least 1; the number of CPUs follows a
# request that finds none free waits up to 10 seconds for one, and if none # container's CPU limit). A request that finds none free waits up to 10
# frees up it is answered 503, unless downstream_timeout has ended first. # seconds for one, and if none frees up it is answered 503, unless
# downstream_timeout has ended first.
# max_concurrent_processing: 4 # max_concurrent_processing: 4
# How many requests to the image routes (/v1/image/ and /v1/e/) one client # How many requests to the image routes (/v1/image/ and /v1/e/) one client
@@ -1,6 +1,7 @@
package config package config
import ( import (
"fmt"
"runtime" "runtime"
"testing" "testing"
) )
@@ -12,8 +13,8 @@ const (
) )
// TestOmittedConcurrencyLimitsUseDefaults checks that an omitted // TestOmittedConcurrencyLimitsUseDefaults checks that an omitted
// max_concurrent_processing is the number of CPUs Go uses and an omitted // max_concurrent_processing is half the CPUs Go uses, and at least 1, and
// upstream_connections is 64. // an omitted upstream_connections is 64.
func TestOmittedConcurrencyLimitsUseDefaults(t *testing.T) { func TestOmittedConcurrencyLimitsUseDefaults(t *testing.T) {
t.Parallel() t.Parallel()
@@ -22,9 +23,9 @@ func TestOmittedConcurrencyLimitsUseDefaults(t *testing.T) {
t.Fatalf("minimal config should be valid, got error: %v", err) t.Fatalf("minimal config should be valid, got error: %v", err)
} }
if c.MaxConcurrentProcessing != runtime.GOMAXPROCS(0) { if c.MaxConcurrentProcessing != max(runtime.GOMAXPROCS(0)/2, 1) {
t.Errorf("MaxConcurrentProcessing = %d, want %d, one per CPU", t.Errorf("MaxConcurrentProcessing = %d, want %d, half the CPUs",
c.MaxConcurrentProcessing, runtime.GOMAXPROCS(0)) c.MaxConcurrentProcessing, max(runtime.GOMAXPROCS(0)/2, 1))
} }
if c.UpstreamConnections != 64 { if c.UpstreamConnections != 64 {
@@ -32,6 +33,36 @@ func TestOmittedConcurrencyLimitsUseDefaults(t *testing.T) {
} }
} }
// TestOmittedMaxConcurrentProcessingIsHalfTheCPUsAndAtLeastOne checks
// that an omitted max_concurrent_processing is half the CPUs Go uses,
// rounded down, and 1 when Go uses one CPU.
//
//nolint:paralleltest // it sets GOMAXPROCS, which other tests read
func TestOmittedMaxConcurrentProcessingIsHalfTheCPUsAndAtLeastOne(t *testing.T) {
cpusBefore := runtime.GOMAXPROCS(0)
t.Cleanup(func() { runtime.GOMAXPROCS(cpusBefore) })
for _, tc := range []struct{ cpus, want int }{
{1, 1}, {2, 1}, {3, 1}, {8, 4},
} {
runtime.GOMAXPROCS(tc.cpus)
c, err := configFromYAML(t, signingKeyLine)
if err != nil {
t.Fatalf("minimal config should be valid, got error: %v", err)
}
t.Logf("GOMAXPROCS %d: MaxConcurrentProcessing %d",
tc.cpus, c.MaxConcurrentProcessing)
if c.MaxConcurrentProcessing != tc.want {
t.Errorf("GOMAXPROCS %d: MaxConcurrentProcessing = %d, want %d",
tc.cpus, c.MaxConcurrentProcessing, tc.want)
}
}
}
// TestExplicitConcurrencyLimitsAreUsed checks that valid values for the // TestExplicitConcurrencyLimitsAreUsed checks that valid values for the
// two limits are used as given. // two limits are used as given.
func TestExplicitConcurrencyLimitsAreUsed(t *testing.T) { func TestExplicitConcurrencyLimitsAreUsed(t *testing.T) {
@@ -160,3 +191,19 @@ func TestConcurrencyLimitsFromEnvironment(t *testing.T) {
}) })
} }
} }
// TestInvalidMaxConcurrentProcessingFromEnvironmentAbortsStartup checks
// that a PIXA_MAX_CONCURRENT_PROCESSING that is not a whole number of at
// least 1, an empty one included, aborts startup naming the variable and
// the key instead of taking the default.
func TestInvalidMaxConcurrentProcessingFromEnvironmentAbortsStartup(t *testing.T) {
for _, value := range []string{"", " ", "0.5", "half", "-1", "4 images"} {
t.Run(fmt.Sprintf("value %q", value), func(t *testing.T) {
t.Setenv(testMaxConcurrentProcessingVar, value)
_, err := configFromYAML(t, signingKeyLine)
wantStartupError(t, err,
testMaxConcurrentProcessingVar, keyMaxConcurrentProcessing)
})
}
}
+3 -5
View File
@@ -12,7 +12,6 @@ import (
"os" "os"
"path/filepath" "path/filepath"
"regexp" "regexp"
"runtime"
"sort" "sort"
"strconv" "strconv"
"strings" "strings"
@@ -21,6 +20,7 @@ import (
"git.eeqj.de/sneak/smartconfig" "git.eeqj.de/sneak/smartconfig"
"go.uber.org/fx" "go.uber.org/fx"
"sneak.berlin/go/pixa/internal/imageprocessor"
"sneak.berlin/go/pixa/internal/logger" "sneak.berlin/go/pixa/internal/logger"
) )
@@ -297,10 +297,8 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
keyUpstreamConnectionsPerHost, DefaultUpstreamConnectionsPerHost), keyUpstreamConnectionsPerHost, DefaultUpstreamConnectionsPerHost),
UpstreamConnections: loader.intVal( UpstreamConnections: loader.intVal(
keyUpstreamConnections, DefaultUpstreamConnections), keyUpstreamConnections, DefaultUpstreamConnections),
// Decoding and encoding are CPU-bound, so the default is one image MaxConcurrentProcessing: loader.intVal(keyMaxConcurrentProcessing,
// per CPU Go uses, which follows a container's CPU limit. imageprocessor.DefaultMaxConcurrentProcessing()),
MaxConcurrentProcessing: loader.intVal(
keyMaxConcurrentProcessing, runtime.GOMAXPROCS(0)),
UpstreamFetchTimeout: loader.durationVal( UpstreamFetchTimeout: loader.durationVal(
keyUpstreamFetchTimeout, DefaultUpstreamFetchTimeout), keyUpstreamFetchTimeout, DefaultUpstreamFetchTimeout),
UpstreamMaxResponseSize: loader.int64Val( UpstreamMaxResponseSize: loader.int64Val(
+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)
+8
View File
@@ -255,6 +255,14 @@ func (s *Handlers) respondImageError(
return 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)
} }
+4
View File
@@ -165,6 +165,10 @@ func (s *Handlers) handleImageError(w http.ResponseWriter, err error) {
errors.Is(err, imageprocessor.ErrUnsupportedInputFormat): errors.Is(err, imageprocessor.ErrUnsupportedInputFormat):
s.respondError(w, "unsupported source format", s.respondError(w, "unsupported source format",
http.StatusUnsupportedMediaType) 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)
+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 -2
View File
@@ -660,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)
}
})
}
}
+58 -20
View File
@@ -20,11 +20,11 @@ import (
var vipsOnce sync.Once var vipsOnce sync.Once
// initVips initializes libvips with quiet logging, one worker thread per // initVips initializes libvips with quiet logging, one worker thread per
// image and no operation cache. Process already works on one image per CPU // image and no operation cache. With one thread each, the images processed
// by default, so more threads per image would only compete for the CPUs. // at once use no more CPUs than MaxConcurrentProcessing. Each request
// Each request decodes different source bytes, so the operation cache // decodes different source bytes, so the operation cache would rarely be
// would rarely be hit and would hold memory outside MaxConcurrentProcessing; // hit and would hold memory outside MaxConcurrentProcessing; repeated
// repeated requests are served from pixa's disk cache instead. // requests are served from pixa's disk cache instead.
func initVips() { func initVips() {
vipsOnce.Do(func() { vipsOnce.Do(func() {
vips.LoggingSettings(nil, vips.LogLevelError) vips.LoggingSettings(nil, vips.LogLevelError)
@@ -133,6 +133,16 @@ 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")
@@ -169,10 +179,19 @@ type Params struct {
// If <= 0, DefaultMaxInputBytes is used. // If <= 0, DefaultMaxInputBytes is used.
MaxInputBytes int64 MaxInputBytes int64
// MaxConcurrentProcessing is the most images processed at once. // MaxConcurrentProcessing is the most images processed at once.
// If <= 0, the number of CPUs Go uses (runtime.GOMAXPROCS(0)) is used. // If <= 0, DefaultMaxConcurrentProcessing() is used.
MaxConcurrentProcessing int MaxConcurrentProcessing int
} }
// DefaultMaxConcurrentProcessing returns the most images processed at once
// when none is set: half the CPUs Go uses (runtime.GOMAXPROCS(0), which
// follows a container's CPU limit), rounded down, and at least 1.
func DefaultMaxConcurrentProcessing() int {
const cpusPerImage = 2
return max(runtime.GOMAXPROCS(0)/cpusPerImage, 1)
}
// New creates a new image processor with the given parameters. // New creates a new image processor with the given parameters.
// A zero-value Params{} uses sensible defaults. // A zero-value Params{} uses sensible defaults.
func New(params Params) *ImageProcessor { func New(params Params) *ImageProcessor {
@@ -185,7 +204,7 @@ func New(params Params) *ImageProcessor {
maxConcurrentProcessing := params.MaxConcurrentProcessing maxConcurrentProcessing := params.MaxConcurrentProcessing
if maxConcurrentProcessing <= 0 { if maxConcurrentProcessing <= 0 {
maxConcurrentProcessing = runtime.GOMAXPROCS(0) maxConcurrentProcessing = DefaultMaxConcurrentProcessing()
} }
return &ImageProcessor{ return &ImageProcessor{
@@ -259,8 +278,11 @@ func (p *ImageProcessor) Process(
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 {
@@ -296,21 +318,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.
@@ -441,7 +471,9 @@ func (p *ImageProcessor) detectFormat(imageType vips.ImageType) 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 {
@@ -489,6 +521,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:
@@ -115,14 +115,14 @@ func waitForEntries(t *testing.T, entered <-chan struct{}, count int) {
} }
} }
func TestNewDefaultsMaxConcurrentProcessingToCPUs(t *testing.T) { func TestNewDefaultsMaxConcurrentProcessingToHalfTheCPUs(t *testing.T) {
t.Parallel() t.Parallel()
for _, limit := range []int{0, -1} { for _, limit := range []int{0, -1} {
proc := New(Params{MaxConcurrentProcessing: limit}) proc := New(Params{MaxConcurrentProcessing: limit})
if got := cap(proc.processingSemaphore); got != runtime.GOMAXPROCS(0) { if got := cap(proc.processingSemaphore); got != max(runtime.GOMAXPROCS(0)/2, 1) {
t.Errorf("MaxConcurrentProcessing %d: %d slots, want %d, one per CPU", t.Errorf("MaxConcurrentProcessing %d: %d slots, want %d, half the CPUs",
limit, got, runtime.GOMAXPROCS(0)) limit, got, max(runtime.GOMAXPROCS(0)/2, 1))
} }
} }
@@ -132,6 +132,30 @@ func TestNewDefaultsMaxConcurrentProcessingToCPUs(t *testing.T) {
} }
} }
// TestNewDefaultIsHalfTheCPUsAndAtLeastOne checks that New, given no
// MaxConcurrentProcessing, makes one slot for every two CPUs Go uses,
// rounded down, and one slot when Go uses one CPU.
//
//nolint:paralleltest // it sets GOMAXPROCS, which other tests read
func TestNewDefaultIsHalfTheCPUsAndAtLeastOne(t *testing.T) {
cpusBefore := runtime.GOMAXPROCS(0)
t.Cleanup(func() { runtime.GOMAXPROCS(cpusBefore) })
for _, tc := range []struct{ cpus, want int }{
{1, 1}, {2, 1}, {3, 1}, {8, 4},
} {
runtime.GOMAXPROCS(tc.cpus)
got := cap(New(Params{}).processingSemaphore)
t.Logf("GOMAXPROCS %d: %d slots", tc.cpus, got)
if got != tc.want {
t.Errorf("GOMAXPROCS %d: %d slots, want %d", tc.cpus, got, tc.want)
}
}
}
// TestProcessNeverExceedsMaxConcurrentProcessing starts more Process calls // TestProcessNeverExceedsMaxConcurrentProcessing starts more Process calls
// than MaxConcurrentProcessing allows and holds the first ones inside // than MaxConcurrentProcessing allows and holds the first ones inside
// Process until the test lets them go. No more than the limit may be // Process until the test lets them go. No more than the limit may be
+1 -1
View File
@@ -52,7 +52,7 @@ type ServiceConfig struct {
// Allowlist is the list of hosts that don't require signatures // Allowlist is the list of hosts that don't require signatures
Allowlist []string Allowlist []string
// MaxConcurrentProcessing is the most images processed at once; zero // MaxConcurrentProcessing is the most images processed at once; zero
// uses the image processor's default, one per CPU // uses the image processor's default, half the CPUs
MaxConcurrentProcessing int MaxConcurrentProcessing int
// Logger for logging // Logger for logging
Logger *slog.Logger Logger *slog.Logger
+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) {
+5 -2
View File
@@ -7,6 +7,8 @@ import (
"regexp" "regexp"
"strconv" "strconv"
"strings" "strings"
"sneak.berlin/go/pixa/internal/imageprocessor"
) )
// URL parsing errors. // URL parsing errors.
@@ -20,8 +22,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.
@@ -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)
}
}
}
}
+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 == "" {