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
19 changed files with 354 additions and 536 deletions
+28 -20
View File
@@ -71,10 +71,9 @@ The state directory (`state_dir`, `/var/lib/pixa` in the container) holds the
database and the disk cache:
- It needs a persistent volume: without one, every restart starts with an empty
cache and forgets the latest logout, so the login sessions that logout ended
work again (see Encrypted URLs). In the container, the startup script gives
the directory to the user pixa runs as (uid 65532) and sets its mode to `750`;
outside it, that user must be able to write the directory.
cache. In the container, the startup script gives the directory to the user
pixa runs as (uid 65532) and sets its mode to `750`; outside it, that user
must be able to write the directory.
- `cache_max_bytes` limits the source and transformed images together. The
database, the metadata files, the `.meta` file beside each transformed image
and files still being written come on top, and eviction runs in the
@@ -127,6 +126,17 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for pixa needs:
- `PIXA_CACHE_MAX_BYTES`: disk cache limit in bytes; `0` disables it;
default 75% of (free space + what the cache holds)
- 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
`/.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
@@ -185,10 +195,7 @@ path under `/v1/` answers 200, in maintenance mode too.
session it answers 303 to `/`. Answers: 200 with the page showing the URL; 400
with the page naming a field that is not valid; 500 when the URL cannot be
made.
- `GET /logout` — end every login session, in every browser (see Encrypted
URLs), and remove the session cookie. Needs: a login session; without one it
only removes the cookie. Answers: 303 to `/`; 500 when the logout cannot be
saved in the database.
- `GET /logout` — end the login session. Needs: nothing. Answers: 303 to `/`.
- `GET` or `HEAD` `/v1/image/<host>/<path>/<size>.<format>`, or
`/v1/image/<host>/<path>/<size>` with no format — an image, fetched, resized
and converted (below). Needs: a signature, unless the host is allowlisted (see
@@ -356,11 +363,7 @@ nor change what it asks for.
1. Open `/` in a browser over HTTPS (or over plain HTTP while `debug` is on, see
Routes) and log in with the signing key (`signing_key`). The login session
lasts 30 days, or until someone logs out at `/logout`. Every login is made
with the one signing key, so logging out ends every login session, in every
browser: pixa saves the time of the latest logout in its database and refuses
a login session made before it, a copy of the session cookie included, also
after a restart that keeps the state directory (see Deployment).
lasts 30 days, or until `/logout`.
2. On the generator page, give the source image's URL, the width and height, the
format, quality and fit, and how long the URL lasts, then submit the form
(`POST /generate`). The format is JPEG XL unless another is chosen; a form
@@ -427,8 +430,11 @@ HMAC-SHA256(secret, "host:path:query:width:height:format:expiration:quality:fit"
Where:
- `host` — source origin hostname (e.g. `cdn.example.com`)
- `path` — source path (e.g. `/photos/cat.jpg`)
- `query` — source query string, empty string if none
- `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
- `height` — requested height in pixels, `0` for original
- `format` — output format, one of those listed under Routes, with `original`
@@ -520,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_UPSTREAM_CONNECTIONS_PER_HOST` | `upstream_connections_per_host` | Concurrent connections per upstream host; default `20` |
| `PIXA_UPSTREAM_CONNECTIONS` | `upstream_connections` | Concurrent connections to all upstream hosts together; default `64` |
| `PIXA_MAX_CONCURRENT_PROCESSING` | `max_concurrent_processing` | Images processed at once; default the number of CPUs |
| `PIXA_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_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 |
@@ -606,10 +612,12 @@ Key settings in more detail:
does, and `downstream_timeout` has not ended first, the request is answered
503 with the error `server busy, try again later`
- `max_concurrent_processing` — the most images decoded and encoded at once;
default the number of CPUs pixa can use (`GOMAXPROCS`), which follows a
container's CPU limit. A request that finds all of them in use waits up to 10
seconds for one to free up; if none does, and `downstream_timeout` has not
ended first, it is answered 503 the same way
default half the CPUs pixa can use (`GOMAXPROCS`, which follows a container's
CPU limit), rounded down, and at least 1. A value that is not a whole number
of at least 1 aborts startup. A request that finds all of them in use waits up
to 10 seconds for one to free up; if none does, and `downstream_timeout` has
not ended first, it is answered 503 the same way. 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 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.
+18 -12
View File
@@ -30,18 +30,24 @@ P2: security: per-origin rate limiting
# Completed Steps
- 2026-10-15 logging out ends every login session (closes #256): a login session
was a cookie with nothing kept on the server, so `/logout` removed it from one
browser and a copy of it kept working for its 30 days. Every login is made
with the one signing key, so `/logout` now ends them all: the session manager
keeps the time of the latest logout, saves it in the new `logout` table so
that it outlasts a restart, and refuses a session made before it. A restart
without a persistent volume for the state directory forgets it. A `/logout`
without a valid login session, such as one another site makes a browser send
without the `SameSite=Strict` cookie, ends none and only removes the cookie. A
logout that cannot be saved answers 500. `session.NewManager`, which the tests
use, keeps logouts in memory only; pixad uses
`session.NewManagerWithDatabase`.
- 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
+5 -4
View File
@@ -108,10 +108,11 @@ upstream_connections_per_host: 20
# downstream_timeout has ended first.
upstream_connections: 64
# Maximum number of images decoded and encoded at once (default: the
# number of CPUs pixa can use, which follows a container's CPU limit). A
# request that finds none free waits up to 10 seconds for one, and if none
# frees up it is answered 503, unless downstream_timeout has ended first.
# Maximum number of images decoded and encoded at once (default: half the
# number of CPUs pixa can use, at least 1; the number of CPUs follows a
# container's CPU limit). A request that finds none free waits up to 10
# seconds for one, and if none frees up it is answered 503, unless
# downstream_timeout has ended first.
# max_concurrent_processing: 4
# How many requests to the image routes (/v1/image/ and /v1/e/) one client
@@ -1,6 +1,7 @@
package config
import (
"fmt"
"runtime"
"testing"
)
@@ -12,8 +13,8 @@ const (
)
// TestOmittedConcurrencyLimitsUseDefaults checks that an omitted
// max_concurrent_processing is the number of CPUs Go uses and an omitted
// upstream_connections is 64.
// max_concurrent_processing is half the CPUs Go uses, and at least 1, and
// an omitted upstream_connections is 64.
func TestOmittedConcurrencyLimitsUseDefaults(t *testing.T) {
t.Parallel()
@@ -22,9 +23,9 @@ func TestOmittedConcurrencyLimitsUseDefaults(t *testing.T) {
t.Fatalf("minimal config should be valid, got error: %v", err)
}
if c.MaxConcurrentProcessing != runtime.GOMAXPROCS(0) {
t.Errorf("MaxConcurrentProcessing = %d, want %d, one per CPU",
c.MaxConcurrentProcessing, runtime.GOMAXPROCS(0))
if c.MaxConcurrentProcessing != max(runtime.GOMAXPROCS(0)/2, 1) {
t.Errorf("MaxConcurrentProcessing = %d, want %d, half the CPUs",
c.MaxConcurrentProcessing, max(runtime.GOMAXPROCS(0)/2, 1))
}
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
// two limits are used as given.
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"
"path/filepath"
"regexp"
"runtime"
"sort"
"strconv"
"strings"
@@ -21,6 +20,7 @@ import (
"git.eeqj.de/sneak/smartconfig"
"go.uber.org/fx"
"sneak.berlin/go/pixa/internal/imageprocessor"
"sneak.berlin/go/pixa/internal/logger"
)
@@ -297,10 +297,8 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
keyUpstreamConnectionsPerHost, DefaultUpstreamConnectionsPerHost),
UpstreamConnections: loader.intVal(
keyUpstreamConnections, DefaultUpstreamConnections),
// Decoding and encoding are CPU-bound, so the default is one image
// per CPU Go uses, which follows a container's CPU limit.
MaxConcurrentProcessing: loader.intVal(
keyMaxConcurrentProcessing, runtime.GOMAXPROCS(0)),
MaxConcurrentProcessing: loader.intVal(keyMaxConcurrentProcessing,
imageprocessor.DefaultMaxConcurrentProcessing()),
UpstreamFetchTimeout: loader.durationVal(
keyUpstreamFetchTimeout, DefaultUpstreamFetchTimeout),
UpstreamMaxResponseSize: loader.int64Val(
-7
View File
@@ -176,10 +176,3 @@ CREATE TABLE IF NOT EXISTS cache_stats (
last_updated_at DATETIME DEFAULT CURRENT_TIMESTAMP
);
INSERT OR IGNORE INTO cache_stats (id) VALUES (1);
-- Time of the latest logout. A login session made before it is refused,
-- so logging out ends every login session. No row: nobody has logged out.
CREATE TABLE IF NOT EXISTS logout (
id INTEGER PRIMARY KEY CHECK (id = 1),
logged_out_at DATETIME NOT NULL
);
+2 -10
View File
@@ -89,18 +89,10 @@ func (s *Handlers) handleLoginPost(w http.ResponseWriter, r *http.Request) {
http.Redirect(w, r, "/", http.StatusSeeOther)
}
// HandleLogout logs out, which ends every login session (see
// session.Manager.Logout), and redirects to login.
// HandleLogout clears the session and redirects to login.
func (s *Handlers) HandleLogout() http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
err := s.sessMgr.Logout(w, r)
if err != nil {
s.log.Error("failed to log out", "error", err)
http.Error(w, "Failed to log out", http.StatusInternalServerError)
return
}
s.sessMgr.ClearSession(w)
http.Redirect(w, r, "/", http.StatusSeeOther)
}
}
@@ -185,98 +185,6 @@ func TestHandleLogout_ClearsSessionCookie(t *testing.T) {
}
}
// TestHandleLogout_EndsCopiedSession verifies that after GET /logout a copy
// of the session cookie taken before it shows the login page, not the
// generator page.
func TestHandleLogout_EndsCopiedSession(t *testing.T) {
t.Parallel()
h, srv := newCSRFTestRouter(t)
copied := newSessionCookie(t, h)
req := httptest.NewRequestWithContext(
t.Context(), http.MethodGet, "/logout", nil)
req.AddCookie(copied)
rec := httptest.NewRecorder()
h.HandleLogout().ServeHTTP(rec, req)
if rec.Code != http.StatusSeeOther {
t.Fatalf("GET /logout status = %d, want %d",
rec.Code, http.StatusSeeOther)
}
req = httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
req.AddCookie(copied)
rec = httptest.NewRecorder()
srv.ServeHTTP(rec, req)
body := rec.Body.String()
t.Logf("GET / with the copied cookie: status %d, login page %v",
rec.Code, strings.Contains(body, loginForm))
if !strings.Contains(body, loginForm) || strings.Contains(body, generatorForm) {
t.Error("a cookie copied before /logout still opens the generator page")
}
}
// TestHandleLogout_SaveFails_Answers500 verifies that when the logout cannot
// be saved in the database, GET /logout answers 500, leaves the session
// cookie in place and ends no session.
func TestHandleLogout_SaveFails_Answers500(t *testing.T) {
t.Parallel()
h, srv := newCSRFTestRouter(t)
db := setupTestDB(t)
sessMgr, err := session.NewManagerWithDatabase(
t.Context(), testSigningKey, db)
if err != nil {
t.Fatalf("session.NewManagerWithDatabase() error = %v", err)
}
h.sessMgr = sessMgr
cookie := newSessionCookie(t, h)
// Saving the logout fails on a closed database.
err = db.Close()
if err != nil {
t.Fatalf("closing the database: %v", err)
}
req := httptest.NewRequestWithContext(
t.Context(), http.MethodGet, "/logout", nil)
req.AddCookie(cookie)
rec := httptest.NewRecorder()
h.HandleLogout().ServeHTTP(rec, req)
t.Logf("GET /logout: status %d, Set-Cookie %q",
rec.Code, rec.Header().Get("Set-Cookie"))
if rec.Code != http.StatusInternalServerError {
t.Errorf("GET /logout status = %d, want %d",
rec.Code, http.StatusInternalServerError)
}
if c := findSessionCookie(rec); c != nil {
t.Errorf("a logout that failed set the session cookie: %s", c)
}
req = httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
req.AddCookie(cookie)
rec = httptest.NewRecorder()
srv.ServeHTTP(rec, req)
if !strings.Contains(rec.Body.String(), generatorForm) {
t.Error("a logout that failed ended the session")
}
}
// TestGeneratePost_NoSession_RedirectsToLogin verifies that POST /generate
// with a valid CSRF token but no login session answers 303 to / and makes no
// URL.
+1 -3
View File
@@ -163,9 +163,7 @@ func (s *Handlers) initImageService() error {
// Initialize session manager (signing key is validated at config load
// time). Session cookies are always Secure/HttpOnly/SameSite=Strict.
// Logouts are saved in the database, so they outlast a restart.
sessMgr, err := session.NewManagerWithDatabase(
context.Background(), s.config.SigningKey, s.db.DB())
sessMgr, err := session.NewManager(s.config.SigningKey)
if err != nil {
return err
}
+16 -7
View File
@@ -20,11 +20,11 @@ import (
var vipsOnce sync.Once
// initVips initializes libvips with quiet logging, one worker thread per
// image and no operation cache. Process already works on one image per CPU
// by default, so more threads per image would only compete for the CPUs.
// Each request decodes different source bytes, so the operation cache
// would rarely be hit and would hold memory outside MaxConcurrentProcessing;
// repeated requests are served from pixa's disk cache instead.
// image and no operation cache. With one thread each, the images processed
// at once use no more CPUs than MaxConcurrentProcessing. Each request
// decodes different source bytes, so the operation cache would rarely be
// hit and would hold memory outside MaxConcurrentProcessing; repeated
// requests are served from pixa's disk cache instead.
func initVips() {
vipsOnce.Do(func() {
vips.LoggingSettings(nil, vips.LogLevelError)
@@ -179,10 +179,19 @@ type Params struct {
// If <= 0, DefaultMaxInputBytes is used.
MaxInputBytes int64
// 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
}
// 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.
// A zero-value Params{} uses sensible defaults.
func New(params Params) *ImageProcessor {
@@ -195,7 +204,7 @@ func New(params Params) *ImageProcessor {
maxConcurrentProcessing := params.MaxConcurrentProcessing
if maxConcurrentProcessing <= 0 {
maxConcurrentProcessing = runtime.GOMAXPROCS(0)
maxConcurrentProcessing = DefaultMaxConcurrentProcessing()
}
return &ImageProcessor{
@@ -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()
for _, limit := range []int{0, -1} {
proc := New(Params{MaxConcurrentProcessing: limit})
if got := cap(proc.processingSemaphore); got != runtime.GOMAXPROCS(0) {
t.Errorf("MaxConcurrentProcessing %d: %d slots, want %d, one per CPU",
limit, got, runtime.GOMAXPROCS(0))
if got := cap(proc.processingSemaphore); got != max(runtime.GOMAXPROCS(0)/2, 1) {
t.Errorf("MaxConcurrentProcessing %d: %d slots, want %d, half the CPUs",
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
// than MaxConcurrentProcessing allows and holds the first ones inside
// 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 []string
// 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
// Logger for logging
Logger *slog.Logger
+13 -2
View File
@@ -10,6 +10,7 @@ import (
"log/slog"
"os"
"path/filepath"
"strings"
"time"
)
@@ -371,11 +372,13 @@ func HashPath(path string) PathHash {
// CacheKey generates a unique key for a request variant.
// 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 {
data := fmt.Sprintf("%s:%s:%s:%d:%d:%s:%d:%s",
req.SourceHost,
req.SourcePath,
req.SourceQuery,
escapePercentAndColon(req.SourcePath),
escapePercentAndColon(req.SourceQuery),
req.Size.Width,
req.Size.Height,
req.Format,
@@ -387,6 +390,14 @@ func CacheKey(req *ImageRequest) VariantKey {
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.
// Files are stored at: <basedir>/<ab>/<cd>/<cache_key>
// 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
// that fails is logged at warn and the store still succeeds.
func TestVariantStorage_StoreLogsFailedMetaWrite(t *testing.T) {
-112
View File
@@ -1,112 +0,0 @@
package server
import (
"net/http"
"net/http/httptest"
"strings"
"testing"
"go.uber.org/fx"
"go.uber.org/fx/fxtest"
"sneak.berlin/go/pixa/internal/config"
"sneak.berlin/go/pixa/internal/database"
"sneak.berlin/go/pixa/internal/globals"
"sneak.berlin/go/pixa/internal/handlers"
"sneak.berlin/go/pixa/internal/healthcheck"
"sneak.berlin/go/pixa/internal/logger"
"sneak.berlin/go/pixa/internal/middleware"
"sneak.berlin/go/pixa/internal/session"
)
// startPixa starts the components pixad's fx app builds, from cfg, and
// returns the server with its routes, as newTestServer does.
func startPixa(t *testing.T, cfg *config.Config) *Server {
t.Helper()
var (
h *handlers.Handlers
mw *middleware.Middleware
)
app := fxtest.New(t,
fx.Supply(cfg),
fx.Provide(
globals.New,
logger.New,
database.New,
healthcheck.New,
handlers.New,
middleware.New,
),
fx.Populate(&h, &mw),
)
app.RequireStart()
t.Cleanup(app.RequireStop)
s := &Server{config: cfg, mw: mw, h: h}
s.SetupRoutes()
return s
}
// opensGeneratorPage reports whether GET / with cookie shows s's generator
// page, which only a valid login session opens.
func opensGeneratorPage(t *testing.T, s *Server, cookie *http.Cookie) bool {
t.Helper()
req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
req.AddCookie(cookie)
rec := httptest.NewRecorder()
s.ServeHTTP(rec, req)
return rec.Code == http.StatusOK &&
strings.Contains(rec.Body.String(), `action="/generate"`)
}
// TestLogoutOutlastsRestart verifies that a copy of the session cookie taken
// before /logout is still refused by a pixa started again on the same state
// directory, as pixad is after a restart.
func TestLogoutOutlastsRestart(t *testing.T) {
t.Parallel()
s := newTestServer(t)
login := postLogin(t, s, firstClient, "", testSigningKey)
var copied *http.Cookie
for _, c := range login.Result().Cookies() {
if c.Name == session.CookieName {
copied = c
}
}
if copied == nil || !opensGeneratorPage(t, s, copied) {
t.Fatal("logging in gave no session cookie that opens the generator page")
}
req := httptest.NewRequestWithContext(
t.Context(), http.MethodGet, "/logout", nil)
req.AddCookie(copied)
logout := httptest.NewRecorder()
s.ServeHTTP(logout, req)
if logout.Code != http.StatusSeeOther {
t.Fatalf("GET /logout status = %d, want %d",
logout.Code, http.StatusSeeOther)
}
restarted := startPixa(t, s.config)
opens := opensGeneratorPage(t, restarted, copied)
t.Logf("after the restart, the copied cookie opens the generator page: %v",
opens)
if opens {
t.Error("after a restart, a cookie copied before /logout " +
"still opens the generator page")
}
}
+1 -97
View File
@@ -1,15 +1,9 @@
// Package session provides encrypted session cookie management, and keeps the
// time of the latest logout in the database so that a logout ends every
// session made before it.
// Package session provides encrypted session cookie management.
package session
import (
"context"
"database/sql"
"errors"
"fmt"
"net/http"
"sync"
"time"
"github.com/gorilla/securecookie"
@@ -41,25 +35,11 @@ type Data struct {
}
// Manager handles session creation and validation using encrypted cookies.
//
// Every login is made with the one signing key, so a logout ends every
// session made before it, in every browser. The Manager keeps the time of
// the latest logout in memory, so checking a session reads no database; a
// Manager from NewManagerWithDatabase also saves it in the database, so that
// it outlasts a restart.
type Manager struct {
sc *securecookie.SecureCookie
db *sql.DB
// mu guards latestLogout, and is held while a logout is saved, so that
// the database and latestLogout agree.
mu sync.Mutex
latestLogout time.Time
}
// NewManager creates a session manager with keys derived from the signing key.
// It keeps logouts in memory only, so a restart forgets them;
// NewManagerWithDatabase keeps them in the database.
//
// Session cookies always carry the Secure, HttpOnly, and SameSite=Strict
// attributes; this cannot be configured. Browsers treat http://localhost as a
@@ -87,29 +67,6 @@ func NewManager(signingKey string) (*Manager, error) {
}, nil
}
// NewManagerWithDatabase creates a session manager like NewManager that saves
// each logout in db, which must have pixa's schema, and starts from the
// latest logout saved there.
func NewManagerWithDatabase(
ctx context.Context, signingKey string, db *sql.DB,
) (*Manager, error) {
m, err := NewManager(signingKey)
if err != nil {
return nil, err
}
err = db.QueryRowContext(ctx,
"SELECT logged_out_at FROM logout WHERE id = 1",
).Scan(&m.latestLogout)
if err != nil && !errors.Is(err, sql.ErrNoRows) {
return nil, fmt.Errorf("failed to read the latest logout: %w", err)
}
m.db = db
return m, nil
}
// CreateSession creates a new authenticated session and sets the cookie.
func (m *Manager) CreateSession(w http.ResponseWriter) error {
now := time.Now()
@@ -165,31 +122,9 @@ func (m *Manager) ValidateSession(r *http.Request) (*Data, error) {
return nil, ErrInvalidSession
}
// A session made before the latest logout has ended.
if !data.CreatedAt.After(m.latestLogoutTime()) {
return nil, ErrInvalidSession
}
return &data, nil
}
// Logout removes the session cookie and, when r has a valid session, ends
// every session made until now. A request without one ends no session:
// another site can make a browser request /logout, but the browser leaves
// the SameSite=Strict session cookie out of that request.
func (m *Manager) Logout(w http.ResponseWriter, r *http.Request) error {
if m.IsAuthenticated(r) {
err := m.saveLogout(r.Context())
if err != nil {
return err
}
}
m.ClearSession(w)
return nil
}
// ClearSession removes the session cookie.
func (m *Manager) ClearSession(w http.ResponseWriter) {
http.SetCookie(w, &http.Cookie{
@@ -210,34 +145,3 @@ func (m *Manager) IsAuthenticated(r *http.Request) bool {
return err == nil && data != nil && data.Authenticated
}
// saveLogout makes now the time of the latest logout, saving it in the
// database first when the Manager has one.
func (m *Manager) saveLogout(ctx context.Context) error {
m.mu.Lock()
defer m.mu.Unlock()
now := time.Now().UTC()
if m.db != nil {
_, err := m.db.ExecContext(ctx,
"INSERT OR REPLACE INTO logout (id, logged_out_at) VALUES (1, ?)",
now)
if err != nil {
return fmt.Errorf("failed to save the logout: %w", err)
}
}
m.latestLogout = now
return nil
}
// latestLogoutTime returns the time of the latest logout, or the zero time
// when there has been none.
func (m *Manager) latestLogoutTime() time.Time {
m.mu.Lock()
defer m.mu.Unlock()
return m.latestLogout
}
-149
View File
@@ -1,149 +0,0 @@
package session_test
import (
"database/sql"
"errors"
"net/http"
"net/http/httptest"
"testing"
"sneak.berlin/go/pixa/internal/database"
"sneak.berlin/go/pixa/internal/session"
)
const logoutTestSigningKey = "test-signing-key-12345"
// newTestDatabase opens an empty in-memory database with pixa's schema. It
// has one connection, as pixad's has: each connection to ":memory:" would
// open a database of its own.
func newTestDatabase(t *testing.T) *sql.DB {
t.Helper()
db, err := sql.Open("sqlite", ":memory:")
if err != nil {
t.Fatalf("sql.Open() error = %v", err)
}
db.SetMaxOpenConns(1)
t.Cleanup(func() { _ = db.Close() })
err = database.ApplyMigrations(t.Context(), db, nil)
if err != nil {
t.Fatalf("ApplyMigrations() error = %v", err)
}
return db
}
// createSessionCookie logs in with mgr and returns the session cookie it
// sets.
func createSessionCookie(t *testing.T, mgr *session.Manager) *http.Cookie {
t.Helper()
rec := httptest.NewRecorder()
err := mgr.CreateSession(rec)
if err != nil {
t.Fatalf("CreateSession() error = %v", err)
}
for _, c := range rec.Result().Cookies() {
if c.Name == session.CookieName {
return c
}
}
t.Fatalf("CreateSession() did not set a %q cookie", session.CookieName)
return nil
}
// requestWithCookie returns a GET request carrying cookie, or no cookie when
// cookie is nil.
func requestWithCookie(t *testing.T, cookie *http.Cookie) *http.Request {
t.Helper()
req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
if cookie != nil {
req.AddCookie(cookie)
}
return req
}
// TestLogout_EndsEverySessionMadeBeforeIt verifies that after a logout a
// copy of the session cookie taken before it is refused and a login after it
// works, and that both still hold for a new Manager on the same database, as
// after a restart.
func TestLogout_EndsEverySessionMadeBeforeIt(t *testing.T) {
t.Parallel()
db := newTestDatabase(t)
mgr, err := session.NewManagerWithDatabase(
t.Context(), logoutTestSigningKey, db)
if err != nil {
t.Fatalf("NewManagerWithDatabase() error = %v", err)
}
copied := createSessionCookie(t, mgr)
err = mgr.Logout(httptest.NewRecorder(), requestWithCookie(t, copied))
if err != nil {
t.Fatalf("Logout() error = %v", err)
}
_, err = mgr.ValidateSession(requestWithCookie(t, copied))
t.Logf("cookie copied before the logout: ValidateSession() error = %v", err)
if !errors.Is(err, session.ErrInvalidSession) {
t.Errorf("cookie copied before the logout: error = %v, want %v",
err, session.ErrInvalidSession)
}
loginAfter := createSessionCookie(t, mgr)
_, err = mgr.ValidateSession(requestWithCookie(t, loginAfter))
if err != nil {
t.Errorf("login after the logout: ValidateSession() error = %v", err)
}
restarted, err := session.NewManagerWithDatabase(
t.Context(), logoutTestSigningKey, db)
if err != nil {
t.Fatalf("NewManagerWithDatabase() after restart error = %v", err)
}
if restarted.IsAuthenticated(requestWithCookie(t, copied)) {
t.Error("after a restart, a cookie copied before the logout is accepted")
}
if !restarted.IsAuthenticated(requestWithCookie(t, loginAfter)) {
t.Error("after a restart, a login made after the logout is refused")
}
}
// TestLogout_WithoutSession_EndsNoSession verifies that a logout from a
// request without a session cookie ends no session. Another site can make a
// browser request /logout, but the browser leaves the SameSite=Strict
// session cookie out of that request.
func TestLogout_WithoutSession_EndsNoSession(t *testing.T) {
t.Parallel()
mgr, err := session.NewManagerWithDatabase(
t.Context(), logoutTestSigningKey, newTestDatabase(t))
if err != nil {
t.Fatalf("NewManagerWithDatabase() error = %v", err)
}
cookie := createSessionCookie(t, mgr)
err = mgr.Logout(httptest.NewRecorder(), requestWithCookie(t, nil))
if err != nil {
t.Fatalf("Logout() error = %v", err)
}
if !mgr.IsAuthenticated(requestWithCookie(t, cookie)) {
t.Error("a logout without a session cookie ended a session")
}
}
+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"
"net/url"
"strconv"
"strings"
"time"
)
@@ -154,15 +155,17 @@ func (s *Signer) GenerateSignedURL(
// buildSignatureData creates the string to be signed.
// Format: "host:path:query:width:height:format:expiration:quality:fit"
// All components are used verbatim (exact match). No normalization,
// suffix matching, or wildcard 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.
// The path and query go through escapePercentAndColon, so that a ":" in
// either cannot be taken for the one between them; every other component
// is used verbatim. No normalization, suffix matching, or wildcard
// 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 {
return fmt.Sprintf("%s:%s:%s:%d:%d:%s:%d:%d:%s",
req.SourceHost,
req.SourcePath,
req.SourceQuery,
escapePercentAndColon(req.SourcePath),
escapePercentAndColon(req.SourceQuery),
req.Width,
req.Height,
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.
func ParseParams(sig, expStr string) (string, time.Time, error) {
if expStr == "" {