Tell upaas operators to set the cache size limit (closes #251)

"Running under upaas" listed PIXA_CACHE_MAX_BYTES as optional. Under
upaas the volume is a directory on the host's filesystem. The default,
75% of the sum of that filesystem's free space and what the cache
holds, and at least 500 MiB, lets one pixa take three quarters or more
of the space the host and every other app have left, and all of it
when that is 500 MiB or less. The section now says to set it and why.
The default is unchanged.

Model: opus-5-5

README.md

@@ -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
@@ -124,8 +123,12 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for pixa needs:
       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;
-      default 75% of (free space + what the cache holds)
+    - `PIXA_CACHE_MAX_BYTES`: disk cache limit in bytes; `0` disables it. Set
+      it: the volume is a directory on the host's filesystem, shared with the
+      host and every other app. The default is 75% of the sum of that
+      filesystem's free space and what the cache holds, and at least 500 MiB, so
+      the cache can grow to three quarters or more of the space they all have
+      left, and to all of it when that is 500 MiB or less.
     - the rest are in the table under Configuration below
 - **Health check:** the image's `HEALTHCHECK` requests
   `/.well-known/healthcheck.json`. upaas reads the container's health 60 seconds
@@ -185,10 +188,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 +356,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 +423,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`

TODO.md

@@ -30,18 +30,21 @@ 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 "Running under upaas" in `README.md` tells the operator to set
+  `PIXA_CACHE_MAX_BYTES` (closes #251): it listed the variable as optional, but
+  under upaas the volume is a directory on the host's filesystem. The default,
+  75% of the sum of that filesystem's free space and what the cache holds, and
+  at least 500 MiB, let one pixa take three quarters or more of the space the
+  host and every other app had left, and all of it when that was 500 MiB or
+  less. The section now says so. The default is unchanged.
+- 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
@@ -50,6 +53,15 @@ P2: security: per-origin rate limiting
   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

internal/db/migrations/001_schema.sql

@@ -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
-);

internal/handlers/auth.go

@@ -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)
 	}
 }

internal/handlers/auth_session_internal_test.go

@@ -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.

internal/handlers/handlers.go

@@ -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
 	}

internal/httpfetcher/fetch_internal_test.go

@@ -9,6 +9,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"slices"
+	"strconv"
 	"strings"
 	"sync"
 	"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) {
 	t.Parallel()
 

internal/httpfetcher/httpfetcher.go

@@ -660,14 +660,33 @@ func dialSSRFSafe(
 	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 {
 	reader    io.Reader
 	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) {
-	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
 	}
 

internal/imgcache/storage.go

@@ -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

internal/imgcache/storage_internal_test.go

@@ -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) {

internal/server/logout_internal_test.go

@@ -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")
-	}
-}

internal/session/session.go

@@ -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
-}

internal/session/session_logout_test.go

@@ -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")
-	}
-}

internal/signature/escaping_test.go

@@ -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)
+			}
+		})
+	}
+}

internal/signature/signature.go

@@ -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 == "" {