Say that losing the state directory forgets the latest logout (part of #256)

The state directory bullet in README.md and the session package doc now
say that the time of the latest logout is kept in the database, which a
restart without a persistent volume loses.

Model: opus-5-5

README.md

@@ -71,9 +71,10 @@ 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. 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 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_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
@@ -184,7 +185,10 @@ 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 the login session. Needs: nothing. Answers: 303 to `/`.
+- `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` 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
@@ -200,10 +204,9 @@ path under `/v1/` answers 200, in maintenance mode too.
   `image_requests_per_minute` (below); 415 when the upstream answers with a
   `Content-Type` that is not one of the source formats (below), or with an image
   whose first bytes libvips reads as another format; 502 when the upstream
-  answered with an error status, and for 5 minutes after that for an image of
-  the same source URL that pixa can neither serve from its cache nor make from
-  the source image still in its cache; 503 when pixa is busy or in maintenance
-  mode; 500 for any other failure.
+  answered with an error status, and for 5 minutes after that for the same
+  source URL; 503 when pixa is busy or in maintenance mode; 500 for any other
+  failure.
 - `GET` or `HEAD` `/v1/e/<token>/<name>` — an image through an encrypted URL
   (see Encrypted URLs). Needs: nothing but the URL. Answers: 200; 304 when
   `If-None-Match` matches the image's `ETag`; 400 for a token that does not
@@ -353,7 +356,11 @@ 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 `/logout`.
+   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).
 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
@@ -419,14 +426,9 @@ HMAC-SHA256(secret, "host:path:query:width:height:format:expiration:quality:fit"
 
 Where:
 
-- `host` — source host as the URL writes it up to the first `/`, in lower case,
-  keeping an IPv6 address's brackets and any `:` and port after it (e.g.
-  `cdn.example.com`, `cdn.example.com:8443`, `[2001:db8::1]:8443`)
-- `path` — source path (e.g. `/photos/cat.jpg`), with each `%` written as `%25`
-  and each `:` as `%3A`, so `/photos/a:b.jpg` is signed as `/photos/a%3Ab.jpg`
-- `query` — source query string, empty string if none, with `%` and `:` written
-  as in `path`; neither then holds a `:`, so path `/a:b` with no query and path
-  `/a` with query `b:` are signed differently
+- `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
 - `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`
@@ -476,12 +478,6 @@ entry that is neither a host name (letters, digits, hyphens, underscores and
 dots, with at most one leading dot) nor an IP address, such as one with a port
 or a `*.` wildcard, aborts startup.
 
-pixa lower-cases the host of a `/v1/image/` URL before anything else uses it, so
-`CDN.Example.com` is matched, fetched, cached and counted against
-`upstream_connections_per_host` as `cdn.example.com`. Such a URL whose host has
-a port, as in `cdn.example.com:8443`, or a `:` with no port, needs a signature
-even when the host is allowlisted. A host holding `@` is refused with 400.
-
 ### Configuration
 
 Every setting can be given as an environment variable, in a YAML config file

TODO.md

@@ -30,35 +30,18 @@ P2: security: per-origin rate limiting
 
 # Completed Steps
 
-- 2026-10-15 a cached variant is served while its source URL is in the negative
-  cache (closes #245): `Get` checked the negative cache before the variant
-  cache, so once a fetch of the source failed, as one does when another variant
-  misses after the source was evicted and the upstream is down, every variant of
-  that source answered 502 for 5 minutes, also one on disk or one that could be
-  made from the cached source. The negative cache is now checked only just
-  before a fetch from the upstream, so a cached variant is served and a variant
-  not in the cache is made from the cached source when there is one. `README.md`
-  says so under `/v1/image/`. A refusal from the negative cache now counts as a
-  miss, as any other request that misses the variant cache does.
-- 2026-10-15 the host of a `/v1/image/` URL is lower-cased, and one with a port
-  needs a signature (closes #246): with `cdn.example.com` allowlisted,
-  `CDN.Example.com` and `cdn.example.com:8443` passed the allowlist too, and
-  each spelling was fetched and cached on its own and had connections of its own
-  under `upstream_connections_per_host`. The image URL parser now lower-cases
-  the host and refuses one holding `@`, which a URL reads as a user name and
-  password, with 400. A host with a port, or with a `:` and no port, needs a
-  signature even when it is allowlisted. That is checked in the image service,
-  not in `allowlist.IsAllowed`, which also matches `referer_blocklist`, where a
-  `Referer` is blocked whatever its port. A URL is signed over its host as it
-  writes it, in lower case, with any IPv6 brackets, `:` and port kept.
-- 2026-10-15 a `:` in the source path or query no longer moves where the path
-  ends and the query begins (closes #247): the signed data and the cache key
-  joined host, path and query with `:` and no escaping, so a URL signed for
-  `https://h/a:b` verified for `https://h/a?b:`; the two also shared a cache
-  key, and whichever was fetched first was served for both. In both, the path
-  and the query now have each `%` written as `%25` and each `:` as `%3A`; a path
-  and query holding neither are signed and keyed as before. Signature
-  Specification in `README.md` says so.
+- 2026-10-15 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 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
@@ -67,15 +50,6 @@ 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,3 +176,10 @@ 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,10 +89,18 @@ func (s *Handlers) handleLoginPost(w http.ResponseWriter, r *http.Request) {
 	http.Redirect(w, r, "/", http.StatusSeeOther)
 }
 
-// HandleLogout clears the session and redirects to login.
+// HandleLogout logs out, which ends every login session (see
+// session.Manager.Logout), and redirects to login.
 func (s *Handlers) HandleLogout() http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
-		s.sessMgr.ClearSession(w)
+		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
+		}
+
 		http.Redirect(w, r, "/", http.StatusSeeOther)
 	}
 }

internal/handlers/auth_session_internal_test.go

@@ -185,6 +185,98 @@ 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,7 +163,9 @@ func (s *Handlers) initImageService() error {
 
 	// Initialize session manager (signing key is validated at config load
 	// time). Session cookies are always Secure/HttpOnly/SameSite=Strict.
-	sessMgr, err := session.NewManager(s.config.SigningKey)
+	// Logouts are saved in the database, so they outlast a restart.
+	sessMgr, err := session.NewManagerWithDatabase(
+		context.Background(), s.config.SigningKey, s.db.DB())
 	if err != nil {
 		return err
 	}

internal/httpfetcher/fetch_internal_test.go

@@ -9,7 +9,6 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"slices"
-	"strconv"
 	"strings"
 	"sync"
 	"testing"
@@ -298,139 +297,6 @@ 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,33 +660,14 @@ func dialSSRFSafe(
 	return nil, fmt.Errorf("%w to %s", errConnectFailed, host)
 }
 
-// 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.
+// limitedReader wraps a reader and limits the number of bytes read.
 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 {
-		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
-		}
-
+	if r.remaining <= 0 {
 		return 0, ErrResponseTooLarge
 	}
 

internal/imgcache/negative_cache_internal_test.go

@@ -94,71 +94,6 @@ func TestNegativeCache_Expired(t *testing.T) {
 	}
 }
 
-// TestService_Get_ServesCachedVariantOfNegativeCachedURL caches a variant of
-// the test photo, then keeps a failed fetch of the photo's URL in the negative
-// cache, as a failed fetch for another variant does once the source has been
-// evicted. The cached variant is still served, and a variant not in the cache
-// is made from the cached source. Once the source is evicted, a variant not in
-// the cache needs a fetch and is refused.
-func TestService_Get_ServesCachedVariantOfNegativeCachedURL(t *testing.T) {
-	t.Parallel()
-
-	svc, fixtures := SetupTestService(t)
-
-	resp, err := svc.Get(t.Context(), photoVariant(fixtures, 85, FitCover))
-	if err != nil {
-		t.Fatalf("Get() first request error = %v", err)
-	}
-
-	_ = resp.Content.Close()
-
-	err = svc.cache.StoreNegative(t.Context(), photoVariant(fixtures, 85, FitCover),
-		httpStatusBadGateway, "upstream answered 503")
-	if err != nil {
-		t.Fatalf("StoreNegative() error = %v", err)
-	}
-
-	resp, err = svc.Get(t.Context(), photoVariant(fixtures, 85, FitCover))
-	if err != nil {
-		t.Fatalf("Get() of the cached variant error = %v", err)
-	}
-
-	_ = resp.Content.Close()
-
-	if resp.CacheStatus != CacheHit {
-		t.Errorf("cached variant CacheStatus = %q, want %q",
-			resp.CacheStatus, CacheHit)
-	}
-
-	resp, err = svc.Get(t.Context(), photoVariant(fixtures, 40, FitCover))
-	t.Logf("Get() of a variant made from the cached source: error = %v", err)
-
-	if err != nil {
-		t.Fatalf("Get() of a variant made from the cached source error = %v", err)
-	}
-
-	_ = resp.Content.Close()
-
-	contentHash, _, err := svc.cache.LookupSource(t.Context(),
-		photoVariant(fixtures, 85, FitCover))
-	if err != nil || contentHash == "" {
-		t.Fatalf("LookupSource() = %q, %v, want the cached source", contentHash, err)
-	}
-
-	err = svc.cache.evictSourceBlob(t.Context(), contentHash)
-	if err != nil {
-		t.Fatalf("evictSourceBlob() error = %v", err)
-	}
-
-	_, err = svc.Get(t.Context(), photoVariant(fixtures, 60, FitCover))
-	t.Logf("Get() of a variant whose source was evicted: error = %v", err)
-
-	if !errors.Is(err, ErrNegativeCached) {
-		t.Errorf("Get() of a variant whose source was evicted error = %v, want %v",
-			err, ErrNegativeCached)
-	}
-}
-
 func TestService_Get_ReturnsErrorForNegativeCachedURL(t *testing.T) {
 	t.Parallel()
 

internal/imgcache/service.go

@@ -9,7 +9,6 @@ import (
 	"log/slog"
 	"net/url"
 	"runtime/debug"
-	"strings"
 	"time"
 
 	"github.com/dustin/go-humanize"
@@ -130,13 +129,25 @@ func NewService(cfg *ServiceConfig) (*Service, error) {
 var ErrNegativeCached = errors.New("request is in negative cache (recently failed)")
 
 // Get retrieves a processed image, fetching and processing if necessary.
-// While its source URL is in the negative cache, a cached variant is still
-// served and a variant not in the cache is still made from a cached source;
-// only a request that needs the source fetched is refused.
 func (s *Service) Get(ctx context.Context, req *ImageRequest) (*ImageResponse, error) {
 	// Propagate AllowHTTP setting to the request
 	req.AllowHTTP = s.allowHTTP
 
+	// Check negative cache first - skip fetching for recently-failed URLs
+	negHit, err := s.cache.checkNegativeCache(ctx, req)
+	if err != nil {
+		s.log.Warn("negative cache check failed", "error", err)
+	}
+
+	if negHit {
+		s.log.Debug("negative cache hit",
+			"host", req.SourceHost,
+			"path", req.SourcePath,
+		)
+
+		return nil, fmt.Errorf("%w: %w", httpfetcher.ErrUpstreamError, ErrNegativeCached)
+	}
+
 	// Check variant cache first (disk only, no DB)
 	result, err := s.cache.Lookup(ctx, req)
 	if err != nil {
@@ -207,16 +218,7 @@ func (s *Service) ValidateRequest(req *ImageRequest) error {
 		return fmt.Errorf("invalid source URL: %w", err)
 	}
 
-	// An allowlist entry names no port, so a host followed by ":", with or
-	// without a port, needs a signature. The port is read from the parsed
-	// URL, as the fetcher reads it: in an IPv6 address written without
-	// brackets, such as 2001:db8::1:8443, it is what follows the last ":".
-	// This is checked here and not in allowlist.IsAllowed, which also matches
-	// referer_blocklist, where a Referer must be blocked whatever its port.
-	hostHasPort := parsedURL.Port() != "" ||
-		strings.HasSuffix(parsedURL.Host, ":")
-
-	if !hostHasPort && s.allowlist.IsAllowed(parsedURL) {
+	if s.allowlist.IsAllowed(parsedURL) {
 		return nil
 	}
 
@@ -421,28 +423,12 @@ func (s *Service) processFromSourceOrFetch(
 
 // fetchAndProcess fetches from upstream, processes, and caches the result.
 // It counts the fetch with the bytes read from upstream, including when
-// reading the response or a later step fails. It fetches nothing while the
-// source URL is in the negative cache.
+// reading the response or a later step fails.
 func (s *Service) fetchAndProcess(
 	ctx context.Context,
 	req *ImageRequest,
 	cacheKey VariantKey,
 ) (*processedVariant, error) {
-	// Check negative cache - skip fetching for recently-failed URLs
-	negHit, err := s.cache.checkNegativeCache(ctx, req)
-	if err != nil {
-		s.log.Warn("negative cache check failed", "error", err)
-	}
-
-	if negHit {
-		s.log.Debug("negative cache hit",
-			"host", req.SourceHost,
-			"path", req.SourcePath,
-		)
-
-		return nil, fmt.Errorf("%w: %w", httpfetcher.ErrUpstreamError, ErrNegativeCached)
-	}
-
 	// Fetch from upstream
 	sourceURL := req.SourceURL()
 

internal/imgcache/service_internal_test.go

@@ -250,97 +250,6 @@ func TestService_ValidateRequest_SignatureExactHostMatch(t *testing.T) {
 	}
 }
 
-// TestService_ValidateRequest_HostWithPortNeedsSignature verifies that a host
-// followed by a port, or by a colon with no port, matches no allowlist entry,
-// so that a request for it without a signature is refused, while the same
-// host with no port is allowlisted.
-func TestService_ValidateRequest_HostWithPortNeedsSignature(t *testing.T) {
-	t.Parallel()
-
-	svc, _ := SetupTestService(t,
-		WithSigningKey(testSigningKey),
-		WithAllowlist(testHostCDN, "203.0.113.5", "2001:db8::1"),
-	)
-
-	tests := []struct {
-		host        string
-		allowlisted bool
-	}{
-		{testHostCDN, true},
-		{"cdn.example.com:8443", false},
-		{"cdn.example.com:443", false},
-		{"cdn.example.com:", false},
-		{"203.0.113.5", true},
-		{"203.0.113.5:8080", false},
-		{"[2001:db8::1]", true},
-		{"[2001:db8::1]:8443", false},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.host, func(t *testing.T) {
-			t.Parallel()
-
-			req := &ImageRequest{
-				SourceHost: tt.host,
-				SourcePath: testPathCat,
-				Size:       Size{Width: 50, Height: 50},
-				Format:     FormatJPEG,
-				Quality:    85,
-				FitMode:    FitCover,
-			}
-
-			err := svc.ValidateRequest(req)
-			t.Logf("ValidateRequest() for host %q with no signature: %v",
-				tt.host, err)
-
-			if allowlisted := err == nil; allowlisted != tt.allowlisted {
-				t.Errorf("host %q served without a signature = %v, want %v",
-					tt.host, allowlisted, tt.allowlisted)
-			}
-		})
-	}
-}
-
-// TestService_ValidateRequest_UnbracketedIPv6PortNeedsSignature verifies that
-// an allowlisted IPv6 address written without brackets and followed by a
-// port, or by a colon with no port, needs a signature. Go's URL parser, which
-// the fetcher uses, reads 2001:db8::1:8443 as the host 2001:db8::1 with the
-// port 8443.
-func TestService_ValidateRequest_UnbracketedIPv6PortNeedsSignature(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	svc, _ := SetupTestService(t,
-		WithSigningKey(testSigningKey),
-		WithAllowlist("2001:db8::1"),
-	)
-
-	for _, host := range []string{"2001:db8::1:8443", "2001:db8::1:"} {
-		t.Run(host, func(t *testing.T) {
-			t.Parallel()
-
-			req := &ImageRequest{
-				SourceHost: host,
-				SourcePath: testPathCat,
-				Size:       Size{Width: 50, Height: 50},
-				Format:     FormatJPEG,
-				Quality:    85,
-				FitMode:    FitCover,
-			}
-
-			err := svc.ValidateRequest(req)
-			t.Logf("ValidateRequest() for host %q with no signature: %v",
-				host, err)
-
-			if err == nil {
-				t.Errorf("host %q served without a signature, want refused",
-					host)
-			}
-		})
-	}
-}
-
 func TestService_Get_InvalidFile(t *testing.T) {
 	t.Parallel()
 

internal/imgcache/storage.go

@@ -10,7 +10,6 @@ import (
 	"log/slog"
 	"os"
 	"path/filepath"
-	"strings"
 	"time"
 )
 
@@ -372,13 +371,11 @@ 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,
-		escapePercentAndColon(req.SourcePath),
-		escapePercentAndColon(req.SourceQuery),
+		req.SourcePath,
+		req.SourceQuery,
 		req.Size.Width,
 		req.Size.Height,
 		req.Format,
@@ -390,14 +387,6 @@ 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,62 +407,6 @@ 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/imgcache/urlparser.go

@@ -15,7 +15,6 @@ import (
 var (
 	ErrInvalidPath       = errors.New("invalid image path")
 	ErrMissingHost       = errors.New("missing source host")
-	ErrInvalidHost       = errors.New("invalid source host")
 	ErrMissingSize       = errors.New("missing size specification")
 	ErrInvalidSize       = errors.New("invalid size format")
 	ErrInvalidFormat     = errors.New("invalid or unsupported format")
@@ -144,17 +143,6 @@ func parseImageComponents(remainder string) (*ParsedURL, error) {
 		return nil, ErrMissingHost
 	}
 
-	// A URL reads what comes before "@" as a user name and password, and the
-	// rest as the host
-	if strings.Contains(host, "@") {
-		return nil, fmt.Errorf("%w: %q holds @", ErrInvalidHost, host)
-	}
-
-	// Host names do not depend on letter case, so the allowlist, the upstream
-	// connection limits, the cache and the signature see one spelling of each
-	// host
-	host = strings.ToLower(host)
-
 	// Extract query string if present in path
 	if qIndex := strings.Index(path, "?"); qIndex != -1 {
 		query = path[qIndex+1:]

internal/imgcache/urlparser_internal_test.go

@@ -234,49 +234,6 @@ func TestParseImagePath(t *testing.T) {
 	}
 }
 
-// TestParseImagePath_LowerCasesHost verifies that the host is lower-cased, so
-// that every spelling of its letter case is the same host to the allowlist,
-// the upstream connection limits and the cache, and that the path keeps its
-// case.
-func TestParseImagePath_LowerCasesHost(t *testing.T) {
-	t.Parallel()
-
-	got, err := ParseImagePath("CDN.Example.COM/Photos/Cat.jpg/800x600.webp")
-	if err != nil {
-		t.Fatalf("ParseImagePath() error = %v", err)
-	}
-
-	t.Logf("parsed host %q, path %q", got.Host, got.Path)
-
-	if got.Host != testHostCDN {
-		t.Errorf("Host = %q, want %q", got.Host, testHostCDN)
-	}
-
-	if got.Path != "/Photos/Cat.jpg" {
-		t.Errorf("Path = %q, want %q", got.Path, "/Photos/Cat.jpg")
-	}
-}
-
-// TestParseImagePath_RefusesUserinfo verifies that a host holding "@" is
-// refused: a URL reads what comes before the "@" as a user name and password
-// and the rest as the host.
-func TestParseImagePath_RefusesUserinfo(t *testing.T) {
-	t.Parallel()
-
-	for _, input := range []string{
-		"user@cdn.example.com/photos/cat.jpg/800x600.webp",
-		"user:password@cdn.example.com/photos/cat.jpg/800x600.webp",
-	} {
-		got, err := ParseImagePath(input)
-		t.Logf("ParseImagePath(%q) = %+v, error = %v", input, got, err)
-
-		if !errors.Is(err, ErrInvalidHost) {
-			t.Errorf("ParseImagePath(%q) error = %v, want %v",
-				input, err, ErrInvalidHost)
-		}
-	}
-}
-
 func TestParsedURL_ToImageRequest(t *testing.T) {
 	t.Parallel()
 

internal/server/logout_internal_test.go

@@ -0,0 +1,112 @@
+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,9 +1,15 @@
-// Package session provides encrypted session cookie management.
+// 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
 
 import (
+	"context"
+	"database/sql"
 	"errors"
+	"fmt"
 	"net/http"
+	"sync"
 	"time"
 
 	"github.com/gorilla/securecookie"
@@ -35,11 +41,25 @@ 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
@@ -67,6 +87,29 @@ 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()
@@ -122,9 +165,31 @@ 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{
@@ -145,3 +210,34 @@ 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

@@ -0,0 +1,149 @@
+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

@@ -1,113 +0,0 @@
-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,7 +10,6 @@ import (
 	"fmt"
 	"net/url"
 	"strconv"
-	"strings"
 	"time"
 )
 
@@ -155,17 +154,15 @@ func (s *Signer) GenerateSignedURL(
 
 // buildSignatureData creates the string to be signed.
 // Format: "host:path:query:width:height:format:expiration:quality:fit"
-// 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.
+// 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.
 func (s *Signer) buildSignatureData(req *Request) string {
 	return fmt.Sprintf("%s:%s:%s:%d:%d:%s:%d:%d:%s",
 		req.SourceHost,
-		escapePercentAndColon(req.SourcePath),
-		escapePercentAndColon(req.SourceQuery),
+		req.SourcePath,
+		req.SourceQuery,
 		req.Width,
 		req.Height,
 		req.Format,
@@ -175,14 +172,6 @@ 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 == "" {